HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Listens for custom messages on the production Bitcoin blockchain

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theproductioncustommessagesforlistensblockchainbitcoin

Problem

The application listens for OP_RETURN messages on the Bitcoin blockchain and prints them to standard output. OP_RETURN is a custom transaction locking script that can store data, but nothing beyond that. It's used for experimental use cases such as proofofexistence.com and coinprism.com.

I would like some critical feedback, specifically code correctness, code smells, and my usage of the bitcoinj library. It's currently compatible with Java 6, but I would be interested in recommendations on Java 8 specific features that could streamline the use case.

Sample Invocation:


java -jar target/ostendo-1.0-SNAPSHOT-jar-with-dependencies.jar

Output:

+--------------------------------------------+---------------------+
| Hex Value | UTF-8 Value |
+--------------------------------------------+---------------------+
| 69643b6a65666662657a6f732e6964 | id;jeffbezos.id |
| 69643b6c61727279706167652e6964 | id;larrypage.id |
| 69643b6a6f686e656c746f6e2e6964 | id;johnelton.id |
| 69643b6a6f6e726f6d65726f2e6964 | id;jonromero.id |
| 6a134153435249424553504f4f4c30315049454345 | ASCRIBESPOOL01PIECE |


The full transaction is output to a log file for reference.

If you would like to build and execute the application, it's available at GitHub.

Ostendo.java

`package com.gmail.lifeofreilly.ostendo;

import org.apache.commons.codec.DecoderException;
import org.apache.commons.codec.binary.Hex;
import org.apache.log4j.Logger;
import org.bitcoinj.store.BlockStoreException;

class Ostendo {
private static final Logger log = Logger.getLogger(Ostendo.class);

/**
* A command line application for listening to OP_RETURN output messages on the Bitcoin blockchain.
* This application leverages the production network. To use the test network, please see:
* https://bitcoinj.github.io/javadoc/0.12/org/bitcoinj/core/NetworkParameters.html
*

Solution

The first thing I want to talk about is:

while (true) {
            try {
                String message = messageQueue.takeMessage();


This loop smells. Instead of using your custom class delegating to BlockingQueue, your messages can be handled by standard java library classes.

Also I'd expect a thread interruption to cancel the currently running thread instead of just being logged as error and then carrying on like before.

As such you might prefer to handle the InterruptedException in the outer try-catch, just like BlockStoreException by terminating the program.

If you switch to an actual BlockingQueue the MessageQueue class becomes completely obsolete.

* This leverages the production network. To use testnet please see:
 * https://bitcoinj.github.io/javadoc/0.12/org/bitcoinj/core/NetworkParameters.html


I'd want this JavaDoc to link to the relevant class javadoc instead of an external link like follows:

* This leverages the production network. To use testnet please see {@link NetworkParameters}


This makes it available for offline inspection, if you have the relevant javadoc on the classpath (as you should have anyways...)

String leftAlignFormat = "| %-60s | %-30s |%n";
        System.out.format("+--------------------------------------------------------------+--------------------------+%n");
        System.out.printf("| Hex Value                                                    | UTF-8 Value              |%n");
        System.out.format("+--------------------------------------------------------------+--------------------------+%n");


many things wrong here...

  • leftAlignFormat should be at the very least final, at best a private static constant



  • Your calls to printf and format are nonsensical and rely on a certian console width to be presented correctly.



Instead use System.out.println to present this as independent as possible of your console.

Additionally you may want to encapsulate your printing logic into a separate class, which takes a PrintStream as input.

Consider the following:

TablePrinter printer = new TablePrinter(System.out);
printer.printHeader();
// loop header and then
    printer.print(messageQueue.take());


It may be interesting to simplify reading your OPReturnListener by having it extend a NoOpListener that overrides all methods with an empty declaration. This would get rid of the clutter from being required to Override methods you do not use.

Some more nitpicks:

  • You use some IMO unnecessary intermediate variables. You can get rid of message in OPReturnListener and in main



-
variable names should be camelCased, ergo:

OPReturnListener opReturnListener = new OPReturnListener(messageQueue);


-
the if-condition with contains("RETURN DATA") might be better off as a separate method (may be overkill), but at least the string should be a constant

Code Snippets

while (true) {
            try {
                String message = messageQueue.takeMessage();
* This leverages the production network. To use testnet please see:
 * https://bitcoinj.github.io/javadoc/0.12/org/bitcoinj/core/NetworkParameters.html
* This leverages the production network. To use testnet please see {@link NetworkParameters}
String leftAlignFormat = "| %-60s | %-30s |%n";
        System.out.format("+--------------------------------------------------------------+--------------------------+%n");
        System.out.printf("| Hex Value                                                    | UTF-8 Value              |%n");
        System.out.format("+--------------------------------------------------------------+--------------------------+%n");
TablePrinter printer = new TablePrinter(System.out);
printer.printHeader();
// loop header and then
    printer.print(messageQueue.take());

Context

StackExchange Code Review Q#104252, answer score: 3

Revisions (0)

No revisions yet.