From f2d9a6162dc30cfcc50140b5f97421aa0c213a56 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 13 Aug 2012 23:30:01 +0200 Subject: [PATCH] Fix a number of issues with message length caching. --- .../java/com/google/bitcoin/core/Block.java | 5 ++- .../com/google/bitcoin/core/ChildMessage.java | 11 ++++--- .../java/com/google/bitcoin/core/Message.java | 20 ++++++++--- .../com/google/bitcoin/core/Transaction.java | 8 ++--- .../com/google/bitcoin/core/BlockTest.java | 33 +++++++++++++++++++ 5 files changed, 63 insertions(+), 14 deletions(-) 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 78b24391b..bcb662399 100644 --- a/core/src/main/java/com/google/bitcoin/core/Block.java +++ b/core/src/main/java/com/google/bitcoin/core/Block.java @@ -739,7 +739,7 @@ public class Block extends Message { else if (transactions.size() > 0 && t.isCoinBase()) throw new RuntimeException("Attempted to add a coinbase transaction when there already is one: " + t); transactions.add(t); - adjustLength(t.length); + adjustLength(transactions.size(), t.length); // Force a recalculation next time the values are needed. merkleRoot = null; hash = null; @@ -842,6 +842,9 @@ public class Block extends Message { coinbase.addInput(new TransactionInput(params, coinbase, new byte[]{(byte) txCounter++})); coinbase.addOutput(new TransactionOutput(params, coinbase, Script.createOutputScript(pubKeyTo))); transactions.add(coinbase); + coinbase.setParent(this); + coinbase.length = coinbase.bitcoinSerialize().length; + adjustLength(transactions.size(), coinbase.length); } static final byte[] EMPTY_BYTES = new byte[32]; diff --git a/core/src/main/java/com/google/bitcoin/core/ChildMessage.java b/core/src/main/java/com/google/bitcoin/core/ChildMessage.java index 41d0ebf8f..22749e475 100644 --- a/core/src/main/java/com/google/bitcoin/core/ChildMessage.java +++ b/core/src/main/java/com/google/bitcoin/core/ChildMessage.java @@ -72,12 +72,15 @@ public abstract class ChildMessage extends Message { if (parent != null) parent.unCache(); } - + protected void adjustLength(int adjustment) { - if (length != UNKNOWN_LENGTH) - length += adjustment; + adjustLength(0, adjustment); + } + + protected void adjustLength(int newArraySize, int adjustment) { + super.adjustLength(newArraySize, adjustment); if (parent != null) - parent.adjustLength(adjustment); + parent.adjustLength(newArraySize, adjustment); } } diff --git a/core/src/main/java/com/google/bitcoin/core/Message.java b/core/src/main/java/com/google/bitcoin/core/Message.java index 40c16e0f4..cc906b627 100644 --- a/core/src/main/java/com/google/bitcoin/core/Message.java +++ b/core/src/main/java/com/google/bitcoin/core/Message.java @@ -36,7 +36,7 @@ public abstract class Message implements Serializable { public static final int MAX_SIZE = 0x02000000; - public static final int UNKNOWN_LENGTH = -1; + public static final int UNKNOWN_LENGTH = Integer.MIN_VALUE; // Useful to ensure serialize/deserialize are consistent with each other. private static final boolean SELF_CHECK = false; @@ -221,10 +221,20 @@ public abstract class Message implements Serializable { recached = false; } - protected void adjustLength(int adjustment) { - if (length != UNKNOWN_LENGTH) - // Our own length is now unknown if we have an unknown length adjustment. - length = adjustment == UNKNOWN_LENGTH ? UNKNOWN_LENGTH : length + adjustment; + protected void adjustLength(int newArraySize, int adjustment) { + if (length == UNKNOWN_LENGTH) + return; + // Our own length is now unknown if we have an unknown length adjustment. + if (adjustment == UNKNOWN_LENGTH) { + length = UNKNOWN_LENGTH; + return; + } + length += adjustment; + // Check if we will need more bytes to encode the length prefix. + if (newArraySize == 1) + length++; // The assumption here is we never call adjustLength with the same arraySize as before. + else if (newArraySize != 0) + length += VarInt.sizeOf(newArraySize) - VarInt.sizeOf(newArraySize - 1); } /** diff --git a/core/src/main/java/com/google/bitcoin/core/Transaction.java b/core/src/main/java/com/google/bitcoin/core/Transaction.java index 489d076fd..4e6647b5d 100644 --- a/core/src/main/java/com/google/bitcoin/core/Transaction.java +++ b/core/src/main/java/com/google/bitcoin/core/Transaction.java @@ -83,7 +83,7 @@ public class Transaction extends ChildMessage implements Serializable { inputs = new ArrayList(); outputs = new ArrayList(); // We don't initialize appearsIn deliberately as it's only useful for transactions stored in the wallet. - length = 10; // 8 for std fields + 1 for each 0 varint + length = 8; // 8 for std fields } public Transaction(NetworkParameters params, int version, Sha256Hash hash) { @@ -93,7 +93,7 @@ public class Transaction extends ChildMessage implements Serializable { outputs = new ArrayList(); this.hash = hash; // We don't initialize appearsIn deliberately as it's only useful for transactions stored in the wallet. - length = 10; //8 for std fields + 1 for each 0 varint + length = 8; //8 for std fields } /** @@ -609,7 +609,7 @@ public class Transaction extends ChildMessage implements Serializable { unCache(); input.setParent(this); inputs.add(input); - adjustLength(input.length); + adjustLength(inputs.size(), input.length); } /** @@ -619,7 +619,7 @@ public class Transaction extends ChildMessage implements Serializable { unCache(); to.setParent(this); outputs.add(to); - adjustLength(to.length); + adjustLength(outputs.size(), to.length); } /** diff --git a/core/src/test/java/com/google/bitcoin/core/BlockTest.java b/core/src/test/java/com/google/bitcoin/core/BlockTest.java index ae32b75ae..1655e4deb 100644 --- a/core/src/test/java/com/google/bitcoin/core/BlockTest.java +++ b/core/src/test/java/com/google/bitcoin/core/BlockTest.java @@ -149,4 +149,37 @@ public class BlockTest { // of this test is to ensure no errors occur during the Java serialization/deserialization process. assertEquals(tx, tx2); } + + @Test + public void testUpdateLength() { + NetworkParameters params = NetworkParameters.unitTests(); + Block block = params.genesisBlock.createNextBlockWithCoinbase(new ECKey().getPubKey()); + assertEquals(block.bitcoinSerialize().length, block.length); + final int origBlockLen = block.length; + Transaction tx = new Transaction(params); + // this is broken until the transaction has > 1 input + output (which is required anyway...) + //assertTrue(tx.length == tx.bitcoinSerialize().length && tx.length == 8); + byte[] outputScript = new byte[10]; + Arrays.fill(outputScript, (byte)Script.OP_FALSE); + tx.addOutput(new TransactionOutput(params, null, BigInteger.valueOf(1), outputScript)); + tx.addInput(new TransactionInput(params, null, new byte[] {(byte)Script.OP_FALSE}, + new TransactionOutPoint(params, 0, Sha256Hash.create(new byte[] {1})))); + int origTxLength = 8 + 2 + 8 + 1 + 10 + 40 + 1 + 1; + assertEquals(tx.bitcoinSerialize().length, tx.length); + assertEquals(origTxLength, tx.length); + block.addTransaction(tx); + assertEquals(block.bitcoinSerialize().length, block.length); + assertEquals(origBlockLen + tx.length, block.length); + block.getTransactions().get(1).getInputs().get(0).setScriptBytes(new byte[] {(byte)Script.OP_FALSE, (byte)Script.OP_FALSE}); + assertEquals(block.length, origBlockLen + tx.length); + assertEquals(tx.length, origTxLength + 1); + block.getTransactions().get(1).getInputs().get(0).setScriptBytes(new byte[] {}); + assertEquals(block.length, block.bitcoinSerialize().length); + assertEquals(block.length, origBlockLen + tx.length); + assertEquals(tx.length, origTxLength - 1); + block.getTransactions().get(1).addInput(new TransactionInput(params, null, new byte[] {(byte)Script.OP_FALSE}, + new TransactionOutPoint(params, 0, Sha256Hash.create(new byte[] {1})))); + assertEquals(block.length, origBlockLen + tx.length); + assertEquals(tx.length, origTxLength + 41); // - 1 + 40 + 1 + 1 + } }