From 1c8ddaad363a76499f12e380547b0a9d234abbba Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Sat, 9 Feb 2013 12:35:58 +0100 Subject: [PATCH] Delete long-dead code related to the previous protocol version that didn't use checksumming until post-handshake. --- .../bitcoin/core/BitcoinSerializer.java | 145 +++++------------- .../bitcoin/core/TCPNetworkConnection.java | 2 +- .../bitcoin/store/FullPrunedBlockStore.java | 11 +- .../bitcoin/core/BitcoinSerializerTest.java | 39 +---- .../bitcoin/core/LazyParseByteCacheTest.java | 10 +- .../com/google/bitcoin/core/TestUtils.java | 2 +- 6 files changed, 56 insertions(+), 153 deletions(-) diff --git a/core/src/main/java/com/google/bitcoin/core/BitcoinSerializer.java b/core/src/main/java/com/google/bitcoin/core/BitcoinSerializer.java index dc17dc88..47940e09 100644 --- a/core/src/main/java/com/google/bitcoin/core/BitcoinSerializer.java +++ b/core/src/main/java/com/google/bitcoin/core/BitcoinSerializer.java @@ -24,7 +24,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.UnsupportedEncodingException; -import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -47,7 +46,6 @@ public class BitcoinSerializer { private static final int COMMAND_LEN = 12; private NetworkParameters params; - private boolean usesChecksumming; private boolean parseLazy = false; private boolean parseRetain = false; @@ -76,46 +74,24 @@ public class BitcoinSerializer { * Constructs a BitcoinSerializer with the given behavior. * * @param params networkParams used to create Messages instances and termining packetMagic - * @param usesChecksumming set to true if checkums should be included and expected in headers */ - public BitcoinSerializer(NetworkParameters params, boolean usesChecksumming) { - this(params, usesChecksumming, false, false); + public BitcoinSerializer(NetworkParameters params) { + this(params, false, false); } /** * Constructs a BitcoinSerializer with the given behavior. * * @param params networkParams used to create Messages instances and termining packetMagic - * @param usesChecksumming set to true if checkums should be included and expected in headers * @param parseLazy deserialize messages in lazy mode. * @param parseRetain retain the backing byte array of a message for fast reserialization. */ - public BitcoinSerializer(NetworkParameters params, boolean usesChecksumming, - boolean parseLazy, boolean parseRetain) { + public BitcoinSerializer(NetworkParameters params, boolean parseLazy, boolean parseRetain) { this.params = params; - this.usesChecksumming = usesChecksumming; this.parseLazy = parseLazy; this.parseRetain = parseRetain; } - // TODO: Remove this dead code. - - public void setUseChecksumming(boolean usesChecksumming) { - this.usesChecksumming = usesChecksumming; - } - - public boolean getUseChecksumming() { - return usesChecksumming; - } - - /** - * Provides the expected header length, which varies depending on whether checksumming is used. - * Header length includes 4 byte magic number. - */ - public int getHeaderLength() { - return 4 + COMMAND_LEN + 4 + (usesChecksumming ? 4 : 0); - } - /** * Writes message to to the output stream. */ @@ -125,8 +101,7 @@ public class BitcoinSerializer { throw new Error("BitcoinSerializer doesn't currently know how to serialize " + message.getClass()); } - byte[] header = new byte[4 + COMMAND_LEN + 4 + (usesChecksumming ? 4 : 0)]; - + byte[] header = new byte[4 + COMMAND_LEN + 4 + 4 /* checksum */]; uint32ToByteArrayBE(params.packetMagic, header, 0); // The header array is initialized to zero by Java so we don't have to worry about @@ -139,32 +114,27 @@ public class BitcoinSerializer { Utils.uint32ToByteArrayLE(payload.length, header, 4 + COMMAND_LEN); - if (usesChecksumming) { - byte[] checksum = message.getChecksum(); - if (checksum == null) { - Sha256Hash msgHash = message.getHash(); - if (msgHash != null && message instanceof Transaction) { - // if the message happens to have a precalculated hash use - // it. - // reverse copying 4 bytes is about 1600 times faster than - // calculating a new hash - // this is only possible for transactions as block hashes - // are hashes of the header only - byte[] hash = msgHash.getBytes(); - int start = 4 + COMMAND_LEN + 4; - for (int i = start; i < start + 4; i++) - header[i] = hash[31 - i + start]; + byte[] checksum = message.getChecksum(); + if (checksum == null) { + Sha256Hash msgHash = message.getHash(); + if (msgHash != null && message instanceof Transaction) { + // if the message happens to have a precalculated hash use + // it. + // reverse copying 4 bytes is about 1600 times faster than + // calculating a new hash + // this is only possible for transactions as block hashes + // are hashes of the header only + byte[] hash = msgHash.getBytes(); + int start = 4 + COMMAND_LEN + 4; + for (int i = start; i < start + 4; i++) + header[i] = hash[31 - i + start]; - } else { - byte[] hash = doubleDigest(payload); - System.arraycopy(hash, 0, header, 4 + COMMAND_LEN + 4, 4); - } } else { - assert Arrays.equals(checksum, Utils.copyOf(doubleDigest(payload), 4)) - : "Checksum match failure on serialization. Cached: " + Arrays.toString(checksum) - + " Calculated: " + Arrays.toString(Utils.copyOf(doubleDigest(payload), 4)); - System.arraycopy(checksum, 0, header, 4 + COMMAND_LEN + 4, 4); + byte[] hash = doubleDigest(payload); + System.arraycopy(hash, 0, header, 4 + COMMAND_LEN + 4, 4); } + } else { + System.arraycopy(checksum, 0, header, 4 + COMMAND_LEN + 4, 4); } out.write(header); @@ -193,7 +163,7 @@ public class BitcoinSerializer { // Satoshi's implementation ignores garbage before the magic header bytes. We have to do the same because // sometimes it sends us stuff that isn't part of any message. seekPastMagicBytes(in); - BitcoinPacketHeader header = new BitcoinPacketHeader(usesChecksumming, in); + BitcoinPacketHeader header = new BitcoinPacketHeader(in); // Now try to read the whole message. return deserializePayload(header, in); } @@ -203,7 +173,7 @@ public class BitcoinSerializer { * the payload. This method assumes you have already called seekPastMagicBytes() */ public BitcoinPacketHeader deserializeHeader(InputStream in) throws ProtocolException, IOException { - return new BitcoinPacketHeader(usesChecksumming, in); + return new BitcoinPacketHeader(in); } /** @@ -223,14 +193,12 @@ public class BitcoinSerializer { // Verify the checksum. byte[] hash = null; - if (usesChecksumming) { - hash = doubleDigest(payloadBytes); - if (header.checksum[0] != hash[0] || header.checksum[1] != hash[1] || - header.checksum[2] != hash[2] || header.checksum[3] != hash[3]) { - throw new ProtocolException("Checksum failed to verify, actual " + - bytesToHexString(hash) + - " vs " + bytesToHexString(header.checksum)); - } + hash = doubleDigest(payloadBytes); + if (header.checksum[0] != hash[0] || header.checksum[1] != hash[1] || + header.checksum[2] != hash[2] || header.checksum[3] != hash[3]) { + throw new ProtocolException("Checksum failed to verify, actual " + + bytesToHexString(hash) + + " vs " + bytesToHexString(header.checksum)); } if (log.isDebugEnabled()) { @@ -332,13 +300,13 @@ public class BitcoinSerializer { public class BitcoinPacketHeader { - final byte[] header; - final String command; - final int size; - final byte[] checksum; + public final byte[] header; + public final String command; + public final int size; + public final byte[] checksum; - BitcoinPacketHeader(boolean usesCheckSumminng, InputStream in) throws ProtocolException, IOException { - header = new byte[COMMAND_LEN + 4 + (usesChecksumming ? 4 : 0)]; + public BitcoinPacketHeader(InputStream in) throws ProtocolException, IOException { + header = new byte[COMMAND_LEN + 4 + 4]; int readCursor = 0; while (readCursor < header.length) { int bytesRead = in.read(header, readCursor, header.length - readCursor); @@ -372,44 +340,9 @@ public class BitcoinSerializer { // Old clients don't send the checksum. checksum = new byte[4]; - if (usesChecksumming) { - // Note that the size read above includes the checksum bytes. - System.arraycopy(header, cursor, checksum, 0, 4); - cursor += 4; - } + // Note that the size read above includes the checksum bytes. + System.arraycopy(header, cursor, checksum, 0, 4); + cursor += 4; } - - public boolean hasCheckSum() { - return checksum != null; - } - - /** - * @return the header - */ - public byte[] getHeader() { - return header; - } - - /** - * @return the command - */ - public String getCommand() { - return command; - } - - /** - * @return the size - */ - public int getPayloadSize() { - return size; - } - - /** - * @return the checksum - */ - public byte[] getChecksum() { - return checksum; - } - } } diff --git a/core/src/main/java/com/google/bitcoin/core/TCPNetworkConnection.java b/core/src/main/java/com/google/bitcoin/core/TCPNetworkConnection.java index ee497378..ed918d6d 100644 --- a/core/src/main/java/com/google/bitcoin/core/TCPNetworkConnection.java +++ b/core/src/main/java/com/google/bitcoin/core/TCPNetworkConnection.java @@ -77,7 +77,7 @@ public class TCPNetworkConnection implements NetworkConnection { public TCPNetworkConnection(NetworkParameters params, VersionMessage ver) { this.params = params; this.myVersionMessage = ver; - this.serializer = new BitcoinSerializer(this.params, true); + this.serializer = new BitcoinSerializer(this.params); this.handler = new NetworkHandler(); } diff --git a/core/src/main/java/com/google/bitcoin/store/FullPrunedBlockStore.java b/core/src/main/java/com/google/bitcoin/store/FullPrunedBlockStore.java index 5211abc5..e6c69210 100644 --- a/core/src/main/java/com/google/bitcoin/store/FullPrunedBlockStore.java +++ b/core/src/main/java/com/google/bitcoin/store/FullPrunedBlockStore.java @@ -61,16 +61,15 @@ public interface FullPrunedBlockStore extends BlockStore { void put(StoredBlock storedBlock, StoredUndoableBlock undoableBlock) throws BlockStoreException; /** - * Returns the StoredBlock that was added as a StoredUndoableBlock given a hash. The returned values block.getHash() method will be equal to the - * parameter. If no such block is found, returns null. + * Returns the StoredBlock that was added as a StoredUndoableBlock given a hash. The returned values block.getHash() + * method will be equal to the parameter. If no such block is found, returns null. */ StoredBlock getOnceUndoableStoredBlock(Sha256Hash hash) throws BlockStoreException; /** - * Returns a {@link StoredUndoableBlock} who's block.getHash() method will be equal to the - * parameter. If no such block is found, returns null. - * Note that this may return null more often than get(Sha256Hash hash) as not all {@link StoredBlock}s have a - * {@link StoredUndoableBlock} copy stored as well. + * Returns a {@link StoredUndoableBlock} whose block.getHash() method will be equal to the parameter. If no such + * block is found, returns null. Note that this may return null more often than get(Sha256Hash hash) as not all + * {@link StoredBlock}s have a {@link StoredUndoableBlock} copy stored as well. */ StoredUndoableBlock getUndoBlock(Sha256Hash hash) throws BlockStoreException; 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 bf7db5d7..6e1e6e0e 100644 --- a/core/src/test/java/com/google/bitcoin/core/BitcoinSerializerTest.java +++ b/core/src/test/java/com/google/bitcoin/core/BitcoinSerializerTest.java @@ -51,38 +51,9 @@ public class BitcoinSerializerTest { "0E AB 5B EA 43 6A 04 84 CF AB 12 48 5E FD A0 B7" + "8B 4E CC 52 88 AC 00 00 00 00"); - @Test - public void testVersion() throws Exception { - BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), false); - // the actual data from https://en.bitcoin.it/wiki/Protocol_specification#version - ByteArrayInputStream bais = new ByteArrayInputStream(Hex.decode("f9beb4d976657273696f6e0000000000560000009" + - "c7c00000100000000000000e615104d00000000010000000000000000000000000000000000ffff0a000001daf6010000" + - "000000000000000000000000000000ffff0a000002208ddd9d202c3ab45713005581010000")); - VersionMessage vm = (VersionMessage)bs.deserialize(bais); - assertEquals(31900, vm.clientVersion); - assertEquals(1292899814L, vm.time); - assertEquals(98645L, vm.bestHeight); - - // Standard version messsages don't use strings. Create one and round-trip here to check that works OK. - vm.subVer = "test string"; - byte[] bits = vm.bitcoinSerialize(); - VersionMessage vm2 = new VersionMessage(NetworkParameters.prodNet(), bits); - assertEquals(vm, vm2); - } - - - @Test - public void testVerack() throws Exception { - BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), false); - // the actual data from https://en.bitcoin.it/wiki/Protocol_specification#verack - ByteArrayInputStream bais = new ByteArrayInputStream(Hex.decode("f9beb4d976657261636b00000000000000000000")); - VersionAck va = (VersionAck)bs.deserialize(bais); - assertNotNull(va); - } - @Test public void testAddr() throws Exception { - BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), true); + BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet()); // the actual data from https://en.bitcoin.it/wiki/Protocol_specification#addr ByteArrayInputStream bais = new ByteArrayInputStream(addrMessage); AddressMessage a = (AddressMessage)bs.deserialize(bais); @@ -99,7 +70,7 @@ public class BitcoinSerializerTest { @Test public void testLazyParsing() throws Exception { - BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), true, true, false); + BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), true, false); ByteArrayInputStream bais = new ByteArrayInputStream(txMessage); Transaction tx = (Transaction)bs.deserialize(bais); @@ -124,7 +95,7 @@ public class BitcoinSerializerTest { } private void testCachedParsing(boolean lazy) throws Exception { - BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), true, lazy, true); + BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), lazy, true); //first try writing to a fields to ensure uncaching and children are not affected ByteArrayInputStream bais = new ByteArrayInputStream(txMessage); @@ -191,7 +162,7 @@ public class BitcoinSerializerTest { */ @Test public void testHeaders1() throws Exception { - BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), true); + BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet()); ByteArrayInputStream bais = new ByteArrayInputStream(Hex.decode("f9beb4d9686561" + "646572730000000000520000005d4fab8101010000006fe28c0ab6f1b372c1a6a246ae6" + @@ -218,7 +189,7 @@ public class BitcoinSerializerTest { * Get 6 headers of blocks 1-6 in the chain */ public void testHeaders2() throws Exception { - BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet(), true); + BitcoinSerializer bs = new BitcoinSerializer(NetworkParameters.prodNet()); ByteArrayInputStream bais = new ByteArrayInputStream(Hex.decode("f9beb4d96865616465" + "72730000000000e701000085acd4ea06010000006fe28c0ab6f1b372c1a6a246ae63f74f931e" + diff --git a/core/src/test/java/com/google/bitcoin/core/LazyParseByteCacheTest.java b/core/src/test/java/com/google/bitcoin/core/LazyParseByteCacheTest.java index 45d703b2..61f5c41f 100644 --- a/core/src/test/java/com/google/bitcoin/core/LazyParseByteCacheTest.java +++ b/core/src/test/java/com/google/bitcoin/core/LazyParseByteCacheTest.java @@ -99,7 +99,7 @@ public class LazyParseByteCacheTest { Block b1 = createFakeBlock(blockStore, tx1, tx2).block; - BitcoinSerializer bs = new BitcoinSerializer(unitTestParams, true); + BitcoinSerializer bs = new BitcoinSerializer(unitTestParams); ByteArrayOutputStream bos = new ByteArrayOutputStream(); bs.serialize(tx1, bos); @@ -171,10 +171,10 @@ public class LazyParseByteCacheTest { public void testBlock(byte[] blockBytes, boolean isChild, boolean lazy, boolean retain) throws Exception { //reference serializer to produce comparison serialization output after changes to //message structure. - BitcoinSerializer bsRef = new BitcoinSerializer(unitTestParams, true, false, false); + BitcoinSerializer bsRef = new BitcoinSerializer(unitTestParams, false, false); ByteArrayOutputStream bos = new ByteArrayOutputStream(); - BitcoinSerializer bs = new BitcoinSerializer(unitTestParams, true, lazy, retain); + BitcoinSerializer bs = new BitcoinSerializer(unitTestParams, lazy, retain); Block b1; Block bRef; b1 = (Block) bs.deserialize(new ByteArrayInputStream(blockBytes)); @@ -407,10 +407,10 @@ public class LazyParseByteCacheTest { //reference serializer to produce comparison serialization output after changes to //message structure. - BitcoinSerializer bsRef = new BitcoinSerializer(params, true, false, false); + BitcoinSerializer bsRef = new BitcoinSerializer(params, false, false); ByteArrayOutputStream bos = new ByteArrayOutputStream(); - BitcoinSerializer bs = new BitcoinSerializer(params, true, lazy, retain); + BitcoinSerializer bs = new BitcoinSerializer(params, lazy, retain); Transaction t1; Transaction tRef; t1 = (Transaction) bs.deserialize(new ByteArrayInputStream(txBytes)); diff --git a/core/src/test/java/com/google/bitcoin/core/TestUtils.java b/core/src/test/java/com/google/bitcoin/core/TestUtils.java index bb6f9466..08ff6f2c 100644 --- a/core/src/test/java/com/google/bitcoin/core/TestUtils.java +++ b/core/src/test/java/com/google/bitcoin/core/TestUtils.java @@ -99,7 +99,7 @@ public class TestUtils { * Roundtrip a transaction so that it appears as if it has just come from the wire */ public static Transaction roundTripTransaction(NetworkParameters params, Transaction tx) throws IOException, ProtocolException { - BitcoinSerializer bs = new BitcoinSerializer(params, true); + BitcoinSerializer bs = new BitcoinSerializer(params); ByteArrayOutputStream bos = new ByteArrayOutputStream(); bs.serialize(tx, bos); return (Transaction) bs.deserialize(new ByteArrayInputStream(bos.toByteArray()));