4
\$\begingroup\$

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
     *
     * @param args none
     */
    public static void main(String[] args) {
        MessageQueue messageQueue = new MessageQueue();
        OPReturnListener OPReturnListener = new OPReturnListener(messageQueue);

        try {
            PeerListener peerListener = new PeerListener(OPReturnListener);
            peerListener.start();
            log.info("Listening for OP_Return messages...");
            System.out.println("Additional information is available at: " + System.getProperty("user.dir") + "/logs/error.log");

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

            while (true) {
                try {
                    String message = messageQueue.takeMessage();
                    System.out.format(leftAlignFormat, message, hexToUTF8(message));
                } catch (DecoderException e) {
                    log.error("Unable to decode the message: " + e);
                } catch (InterruptedException e) {
                    log.error(e.getStackTrace());
                }
            }
        } catch (BlockStoreException e) {
            log.error("Failed to initialize the blockchain: " + e);
            System.exit(-1);
        }

    }

    /**
     * Converts hex to a UTF-8 string
     *
     * @param hex the string to decode
     * @return the decoded string
     * @throws DecoderException
     */
    public static String hexToUTF8(String hex) throws DecoderException {
        return new String(Hex.decodeHex(hex.toCharArray()));
    }

}

MessageQueue.java

package com.gmail.lifeofreilly.ostendo;

import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;

/**
 * A simple blocking message queue
 */
class MessageQueue {
    private final BlockingQueue<String> messageQueue = new LinkedBlockingQueue<String>();

    /**
     * Add a message to the queue to be processed.
     *
     * @param message the message.
     */
    public void addMessage(final String message) {
        messageQueue.add(message);
    }

    /**
     * Removes and returns the head message in the queue, waiting if necessary until an element becomes available.
     *
     * @return the message.
     */
    public String takeMessage() throws InterruptedException {
        return messageQueue.take();
    }


}

PeerListener.java

package com.gmail.lifeofreilly.ostendo;

import org.bitcoinj.core.BlockChain;
import org.bitcoinj.core.NetworkParameters;
import org.bitcoinj.core.PeerEventListener;
import org.bitcoinj.core.PeerGroup;
import org.bitcoinj.net.discovery.DnsDiscovery;
import org.bitcoinj.params.MainNetParams;
import org.bitcoinj.store.BlockStoreException;
import org.bitcoinj.store.MemoryBlockStore;

/**
 * A bitcoin network peer configured to use a custom listener
 * This leverages the production network. To use testnet please see:
 * https://bitcoinj.github.io/javadoc/0.12/org/bitcoinj/core/NetworkParameters.html
 */
class PeerListener {
    private static final NetworkParameters NET_PARAMS = MainNetParams.get();
    private final PeerEventListener peerEventListener;

    /**
     * Sole constructor for PeerListener
     *
     * @param peerEventListener the custom peer event listener you would like to use
     */
    public PeerListener(PeerEventListener peerEventListener) {
        this.peerEventListener = peerEventListener;
    }

    /**
     * Starts a bitcoin network peer and begins to listen
     *
     * @throws BlockStoreException
     */
    public void start() throws BlockStoreException {
        BlockChain blockChain = new BlockChain(NET_PARAMS, new MemoryBlockStore(NET_PARAMS));
        PeerGroup peerGroup = new PeerGroup(NET_PARAMS, blockChain);
        peerGroup.addPeerDiscovery(new DnsDiscovery(NET_PARAMS));
        peerGroup.addEventListener(peerEventListener);
        peerGroup.startAsync();
    }
}

OPReturnListener.java

package com.gmail.lifeofreilly.ostendo;

import org.apache.log4j.Logger;
import org.bitcoinj.core.Block;
import org.bitcoinj.core.FilteredBlock;
import org.bitcoinj.core.GetDataMessage;
import org.bitcoinj.core.TransactionOutput;
import org.bitcoinj.core.Message;
import org.bitcoinj.core.Peer;
import org.bitcoinj.core.PeerAddress;
import org.bitcoinj.core.PeerEventListener;
import org.bitcoinj.core.Transaction;

import javax.annotation.Nullable;
import java.util.List;
import java.util.Set;

/**
 * A custom PeerEventListener that adds all OP_RETURN messages to a message queue
 */
class OPReturnListener implements PeerEventListener {
    private final MessageQueue messageQueue;
    private static final Logger log = Logger.getLogger(OPReturnListener.class);

    /**
     * Sole constructor for OPReturnListener
     *
     * @param messageQueue new messages will be added to this queue
     */
    public OPReturnListener(final MessageQueue messageQueue) {
        this.messageQueue = messageQueue;
    }

    @Override
    public void onTransaction(Peer peer, Transaction transaction) {
        for (TransactionOutput transactionOutput : transaction.getOutputs()) {
            if (transactionOutput.toString().contains("RETURN PUSHDATA")) {
                String message = extractMessage(transactionOutput.toString());
                messageQueue.addMessage(message);
                log.info("Found a message in Transaction: " + transaction.toString());
            }
        }
    }

    @Override
    public void onPeersDiscovered(Set<PeerAddress> set) {

    }

    @Override
    public void onBlocksDownloaded(Peer peer, Block block, FilteredBlock filteredBlock, int i) {

    }

    @Override
    public void onChainDownloadStarted(Peer peer, int i) {

    }

    @Override
    public void onPeerConnected(Peer peer, int i) {

    }

    @Override
    public void onPeerDisconnected(Peer peer, int i) {

    }

    @Override
    public Message onPreMessageReceived(Peer peer, Message message) {
        return null;
    }

    @Nullable
    @Override
    public List<Message> getData(Peer peer, GetDataMessage getDataMessage) {
        return null;
    }

    public static String extractMessage(String transactionOutput) {
        return transactionOutput.substring(transactionOutput.indexOf("[") + 1, transactionOutput.indexOf("]"));
    }

}
\$\endgroup\$

1 Answer 1

3
+50
\$\begingroup\$

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...

  1. leftAlignFormat should be at the very least final, at best a private static constant
  2. 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

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the detailed review. I incorporated all feedback. This indeed helped to simply the code quite a bit. Updates are available at github.com/lifeofreilly/ostendo Cheers. \$\endgroup\$
    – weare138
    Sep 27, 2015 at 20:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.