From 3e5f7964075ea19b80ad0e9e1a5b9b6d03acd5bf Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Mon, 2 Apr 2012 13:40:20 +0200 Subject: [PATCH] Add a dependency on Guava base libraries and replace a few asserts with Preconditions, which means they will always run including in production code. Fix a bug revealed by this (IntelliJ does not run unit tests with assertions enabled!) --- core/pom.xml | 5 ++++ .../java/com/google/bitcoin/core/Block.java | 14 +++++---- .../java/com/google/bitcoin/core/Peer.java | 30 +++++++++---------- .../bitcoin/core/BitcoinSerializerTest.java | 1 - 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 0b395c84..93d585aa 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -218,6 +218,11 @@ com.google.protobuf protobuf-java + + com.google.guava + guava-base + r03 + \ No newline at end of file diff --git a/core/src/main/java/com/google/bitcoin/core/Block.java b/core/src/main/java/com/google/bitcoin/core/Block.java index e906cd42..6677d070 100644 --- a/core/src/main/java/com/google/bitcoin/core/Block.java +++ b/core/src/main/java/com/google/bitcoin/core/Block.java @@ -16,6 +16,7 @@ package com.google.bitcoin.core; +import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -87,9 +88,9 @@ public class Block extends Message { length = 80; } - /** Constructs a block object from the BitCoin wire format. */ + /** Constructs a block object from the Bitcoin wire format. */ public Block(NetworkParameters params, byte[] payloadBytes) throws ProtocolException { - super(params, payloadBytes, 0); + super(params, payloadBytes, 0, false, false, payloadBytes.length); } /** @@ -170,8 +171,9 @@ public class Block extends Message { // Ignore the header since it has fixed length. If length is not provided we will have to // invoke a light parse of transactions to calculate the length. if (length == UNKNOWN_LENGTH) { - assert !parseLazy : "Performing lite parse of block transaction as block was initialised from byte array " + - "without providing length. This should never need to happen. parseLazy: " + parseLazy; + Preconditions.checkState(parseLazy, + "Performing lite parse of block transaction as block was initialised from byte array " + + "without providing length. This should never need to happen."); parseTransactions(); length = cursor - offset; } else { @@ -334,7 +336,7 @@ public class Block extends Message { // we have completely cached byte array. if (headerBytesValid && transactionBytesValid) { - assert bytes != null : "Bytes should never be null if headerBytesValid && transactionBytesValid"; + Preconditions.checkNotNull(bytes, "Bytes should never be null if headerBytesValid && transactionBytesValid"); if (length == bytes.length) { return bytes; } else { @@ -674,7 +676,7 @@ public class Block extends Message { // an invalid block, but if we didn't validate this then an untrusted man-in-the-middle could obtain the next // valid block from the network and simply replace the transactions in it with their own fictional // transactions that reference spent or non-existant inputs. - assert transactions.size() > 0; + Preconditions.checkState(!transactions.isEmpty()); maybeParseTransactions(); checkTransactions(); checkMerkleRoot(); diff --git a/core/src/main/java/com/google/bitcoin/core/Peer.java b/core/src/main/java/com/google/bitcoin/core/Peer.java index c9a55b0f..3e815e6d 100644 --- a/core/src/main/java/com/google/bitcoin/core/Peer.java +++ b/core/src/main/java/com/google/bitcoin/core/Peer.java @@ -19,6 +19,7 @@ package com.google.bitcoin.core; import com.google.bitcoin.store.BlockStore; import com.google.bitcoin.store.BlockStoreException; import com.google.bitcoin.utils.EventListenerInvoker; +import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -255,11 +256,12 @@ public class Peer { private void processHeaders(HeadersMessage m) throws IOException, ProtocolException { // Runs in network loop thread for this peer. // - // This can happen if a peer just randomly sends us a "headers" message (should never happen), or more likely - // when we've requested them as part of chain download using fast catchup. We need to add each block to the - // chain if it pre-dates the fast catchup time. If we go past it, we can stop processing the headers and request - // the full blocks from that point on instead. - assert !downloadBlockBodies; + // This method can run if a peer just randomly sends us a "headers" message (should never happen), or more + // likely when we've requested them as part of chain download using fast catchup. We need to add each block to + // the chain if it pre-dates the fast catchup time. If we go past it, we can stop processing the headers and + // request the full blocks from that point on instead. + Preconditions.checkState(!downloadBlockBodies); + try { for (int i = 0; i < m.getBlockHeaders().size(); i++) { Block header = m.getBlockHeaders().get(i); @@ -446,7 +448,7 @@ public class Peer { synchronized (announcedTransactionHashes) { if (announcedTransactionHashes.containsKey(tx.getHash())) { Transaction storedTx = announcedTransactionHashes.get(tx.getHash()); - assert storedTx == tx || storedTx == null : "single Transaction instance"; + Preconditions.checkState(storedTx == tx || storedTx == null, "single Transaction instance"); log.debug("Provided with a downloaded transaction we have seen before: {}", tx.getHash()); tx.getConfidence().markBroadcastBy(address); } else { @@ -553,15 +555,13 @@ public class Peer { public T get() throws InterruptedException, ExecutionException { latch.await(); - assert result != null; - return result; + return Preconditions.checkNotNull(result); } public T get(long l, TimeUnit timeUnit) throws InterruptedException, ExecutionException, TimeoutException { if (!latch.await(l, timeUnit)) throw new TimeoutException(); - assert result != null; - return result; + return Preconditions.checkNotNull(result); } InventoryItem getItem() { @@ -686,12 +686,10 @@ public class Peer { public int getPeerBlockHeightDifference() { // Chain will overflow signed int blocks in ~41,000 years. int chainHeight = (int) conn.getVersionMessage().bestHeight; - if (chainHeight <= 0) { - // This should not happen because we shouldn't have given the user a Peer that is to another client-mode - // node, nor should it be unconnected. If that happens it means the user overrode us somewhere or there is - // a bug in the peer management code. - throw new RuntimeException("Connected to peer advertising zero/negative chain height."); - } + // chainHeight should not be zero/negative because we shouldn't have given the user a Peer that is to another + // client-mode node, nor should it be unconnected. If that happens it means the user overrode us somewhere or + // there is a bug in the peer management code. + Preconditions.checkState(chainHeight > 0, "Connected to peer with zero/negative chain height", chainHeight); return chainHeight - blockChain.getChainHead().getHeight(); } diff --git a/core/src/test/java/com/google/bitcoin/core/BitcoinSerializerTest.java b/core/src/test/java/com/google/bitcoin/core/BitcoinSerializerTest.java index fc4c70c9..22336c67 100644 --- a/core/src/test/java/com/google/bitcoin/core/BitcoinSerializerTest.java +++ b/core/src/test/java/com/google/bitcoin/core/BitcoinSerializerTest.java @@ -203,7 +203,6 @@ public class BitcoinSerializerTest { */ @Test public void testHeaders1() throws Exception { - BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), true, null);