VersionMessage, AddressMessage: Fix address messages embedded in version messages cannot have time.

core/src/main/java/org/bitcoinj/core/PeerAddress.java

@@ -73,7 +73,7 @@ public class PeerAddress extends ChildMessage {
         this.port = port;
         this.protocolVersion = protocolVersion;
         this.services = services;
-        length = protocolVersion > 31402 ? MESSAGE_SIZE : MESSAGE_SIZE - 4;
+        length = isSerializeTime() ? MESSAGE_SIZE : MESSAGE_SIZE - 4;
     }
 
     /**
@@ -117,7 +117,7 @@ public class PeerAddress extends ChildMessage {
 
     @Override
     protected void bitcoinSerializeToStream(OutputStream stream) throws IOException {
-        if (protocolVersion >= 31402) {
+        if (isSerializeTime()) {
             //TODO this appears to be dynamic because the client only ever sends out it's own address
             //so assumes itself to be up.  For a fuller implementation this needs to be dynamic only if
             //the address refers to this client.
@@ -139,6 +139,10 @@ public class PeerAddress extends ChildMessage {
         Utils.uint16ToByteStreamBE(port, stream);
     }
 
+    private boolean isSerializeTime() {
+        return protocolVersion >= 31402 && !(parent instanceof VersionMessage);
+    }
+
     @Override
     protected void parse() throws ProtocolException {
         // Format of a serialized address:
@@ -146,7 +150,7 @@ public class PeerAddress extends ChildMessage {
         //   uint64 services   (flags determining what the node can do)
         //   16 bytes ip address
         //   2 bytes port num
-        if (protocolVersion > 31402)
+        if (isSerializeTime())
             time = readUint32();
         else
             time = -1;
@@ -160,7 +164,7 @@ public class PeerAddress extends ChildMessage {
         port = Utils.readUint16BE(payload, cursor);
         cursor += 2;
         // The 4 byte difference is the uint32 timestamp that was introduced in version 31402 
-        length = protocolVersion > 31402 ? MESSAGE_SIZE : MESSAGE_SIZE - 4;
+        length = isSerializeTime() ? MESSAGE_SIZE : MESSAGE_SIZE - 4;
     }
 
     public String getHostname() {

core/src/main/java/org/bitcoinj/core/VersionMessage.java

@@ -105,16 +105,16 @@ public class VersionMessage extends Message {
         // Note that the Bitcoin Core doesn't do anything with these, and finding out your own external IP address
         // is kind of tricky anyway, so we just put nonsense here for now.
         InetAddress localhost = InetAddresses.forString("127.0.0.1");
-        receivingAddr = new PeerAddress(params, localhost, params.getPort(), 0, BigInteger.ZERO);
+        receivingAddr = new PeerAddress(params, localhost, params.getPort(), clientVersion, BigInteger.ZERO);
-        fromAddr = new PeerAddress(params, localhost, params.getPort(), 0, BigInteger.ZERO);
+        receivingAddr.setParent(this);
+        fromAddr = new PeerAddress(params, localhost, params.getPort(), clientVersion, BigInteger.ZERO);
+        fromAddr.setParent(this);
         subVer = LIBRARY_SUBVER;
         bestHeight = newBestHeight;
         relayTxesBeforeFilter = true;
 
-        length = 85;
+        length = 4 + 8 + 8 + receivingAddr.getMessageSize() + fromAddr.getMessageSize() + 8
-        if (protocolVersion > 31402)
+                + VarInt.sizeOf(subVer.length()) + subVer.length() + 4 + 1;
-            length += 8;
-        length += VarInt.sizeOf(subVer.length()) + subVer.length();
     }
 
     @Override
@@ -122,33 +122,33 @@ public class VersionMessage extends Message {
         clientVersion = (int) readUint32();
         localServices = readUint64().longValue();
         time = readUint64().longValue();
-        receivingAddr = new PeerAddress(params, payload, cursor, 0);
+        receivingAddr = new PeerAddress(params, payload, cursor, 0, this, serializer);
         cursor += receivingAddr.getMessageSize();
-        fromAddr = new PeerAddress(params, payload, cursor, 0);
+        if (clientVersion >= 106) {
-        cursor += fromAddr.getMessageSize();
+            fromAddr = new PeerAddress(params, payload, cursor, 0, this, serializer);
-        // uint64 localHostNonce  (random data)
+            cursor += fromAddr.getMessageSize();
-        // We don't care about the localhost nonce. It's used to detect connecting back to yourself in cases where
+            // uint64 localHostNonce (random data)
-        // there are NATs and proxies in the way. However we don't listen for inbound connections so it's irrelevant.
+            // We don't care about the localhost nonce. It's used to detect connecting back to yourself in cases where
-        readUint64();
+            // there are NATs and proxies in the way. However we don't listen for inbound connections so it's
-        try {
+            // irrelevant.
-            // Initialize default values for flags which may not be sent by old nodes
+            readUint64();
+            // string subVer (currently "")
+            subVer = readStr();
+            // int bestHeight (size of known block chain).
+            bestHeight = readUint32();
+            if (clientVersion >= params.getProtocolVersionNum(NetworkParameters.ProtocolVersion.BLOOM_FILTER)) {
+                relayTxesBeforeFilter = readBytes(1)[0] != 0;
+            } else {
+                relayTxesBeforeFilter = true;
+            }
+        } else {
+            // Default values for flags which may not be sent by old nodes
+            fromAddr = null;
             subVer = "";
             bestHeight = 0;
             relayTxesBeforeFilter = true;
-            if (!hasMoreBytes())
-                return;
-            //   string subVer  (currently "")
-            subVer = readStr();
-            if (!hasMoreBytes())
-                return;
-            //   int bestHeight (size of known block chain).
-            bestHeight = readUint32();
-            if (!hasMoreBytes())
-                return;
-            relayTxesBeforeFilter = readBytes(1)[0] != 0;
-        } finally {
-            length = cursor - offset;
         }
+        length = cursor - offset;
     }
 
     @Override
@@ -158,24 +158,24 @@ public class VersionMessage extends Message {
         Utils.uint32ToByteStreamLE(localServices >> 32, buf);
         Utils.uint32ToByteStreamLE(time, buf);
         Utils.uint32ToByteStreamLE(time >> 32, buf);
-        try {
+        receivingAddr.bitcoinSerializeToStream(buf);
-            receivingAddr.bitcoinSerialize(buf);
+        if (clientVersion >= 106) {
-            fromAddr.bitcoinSerialize(buf);
+            fromAddr.bitcoinSerializeToStream(buf);
-        } catch (IOException e) {
+            // Next up is the "local host nonce", this is to detect the case of connecting
-            throw new RuntimeException(e);  // Can't happen.
+            // back to yourself. We don't care about this as we won't be accepting inbound
+            // connections.
+            Utils.uint32ToByteStreamLE(0, buf);
+            Utils.uint32ToByteStreamLE(0, buf);
+            // Now comes subVer.
+            byte[] subVerBytes = subVer.getBytes(StandardCharsets.UTF_8);
+            buf.write(new VarInt(subVerBytes.length).encode());
+            buf.write(subVerBytes);
+            // Size of known block chain.
+            Utils.uint32ToByteStreamLE(bestHeight, buf);
+            if (clientVersion >= params.getProtocolVersionNum(NetworkParameters.ProtocolVersion.BLOOM_FILTER)) {
+                buf.write(relayTxesBeforeFilter ? 1 : 0);
+            }
         }
-        // Next up is the "local host nonce", this is to detect the case of connecting
-        // back to yourself. We don't care about this as we won't be accepting inbound 
-        // connections.
-        Utils.uint32ToByteStreamLE(0, buf);
-        Utils.uint32ToByteStreamLE(0, buf);
-        // Now comes subVer.
-        byte[] subVerBytes = subVer.getBytes(StandardCharsets.UTF_8);
-        buf.write(new VarInt(subVerBytes.length).encode());
-        buf.write(subVerBytes);
-        // Size of known block chain.
-        Utils.uint32ToByteStreamLE(bestHeight, buf);
-        buf.write(relayTxesBeforeFilter ? 1 : 0);
     }
 
     /**

core/src/test/java/org/bitcoinj/core/VersionMessageTest.java

@@ -17,38 +17,138 @@
 
 package org.bitcoinj.core;
 
-import org.bitcoinj.params.UnitTestParams;
-import org.junit.Test;
-
 import static org.bitcoinj.core.Utils.HEX;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.net.InetAddress;
+
+import org.bitcoinj.params.UnitTestParams;
+import org.junit.Test;
+
 public class VersionMessageTest {
     private static final NetworkParameters UNITTEST = UnitTestParams.get();
 
     @Test
-    // Test that we can decode version messages which miss data which some old nodes may not include
+    public void decode_noRelay_bestHeight_subVer() throws Exception {
-    public void testDecode() throws Exception {
+        // Test that we can decode version messages which miss data which some old nodes may not include
-        VersionMessage ver = new VersionMessage(UNITTEST, HEX.decode("7111010000000000000000003334a85500000000000000000000000000000000000000000000ffff7f000001479d000000000000000000000000000000000000ffff7f000001479d00000000000000000f2f626974636f696e6a3a302e31332f0004000000"));
+        String hex = "7111010000000000000000003334a85500000000000000000000000000000000000000000000ffff7f000001479d000000000000000000000000000000000000ffff7f000001479d00000000000000000f2f626974636f696e6a3a302e31332f0004000000";
+        VersionMessage ver = new VersionMessage(UNITTEST, HEX.decode(hex));
         assertFalse(ver.relayTxesBeforeFilter);
         assertEquals(1024, ver.bestHeight);
         assertEquals("/bitcoinj:0.13/", ver.subVer);
+    }
 
-        ver = new VersionMessage(UNITTEST, HEX.decode("711101000000000000000000a634a85500000000000000000000000000000000000000000000ffff7f000001479d000000000000000000000000000000000000ffff7f000001479d00000000000000000f2f626974636f696e6a3a302e31332f0004000001"));
+    @Test
+    public void decode_relay_bestHeight_subVer() throws Exception {
+        String hex = "711101000000000000000000a634a85500000000000000000000000000000000000000000000ffff7f000001479d000000000000000000000000000000000000ffff7f000001479d00000000000000000f2f626974636f696e6a3a302e31332f0004000001";
+        VersionMessage ver = new VersionMessage(UNITTEST, HEX.decode(hex));
         assertTrue(ver.relayTxesBeforeFilter);
         assertEquals(1024, ver.bestHeight);
         assertEquals("/bitcoinj:0.13/", ver.subVer);
+    }
 
-        ver = new VersionMessage(UNITTEST, HEX.decode("711101000000000000000000c334a85500000000000000000000000000000000000000000000ffff7f000001479d000000000000000000000000000000000000ffff7f000001479d00000000000000000f2f626974636f696e6a3a302e31332f0000000001"));
+    @Test
+    public void decode_relay_noBestHeight_subVer() throws Exception {
+        String hex = "711101000000000000000000c334a85500000000000000000000000000000000000000000000ffff7f000001479d000000000000000000000000000000000000ffff7f000001479d00000000000000000f2f626974636f696e6a3a302e31332f0000000001";
+        VersionMessage ver = new VersionMessage(UNITTEST, HEX.decode(hex));
         assertTrue(ver.relayTxesBeforeFilter);
         assertEquals(0, ver.bestHeight);
         assertEquals("/bitcoinj:0.13/", ver.subVer);
+    }
 
-        ver = new VersionMessage(UNITTEST, HEX.decode("71110100000000000000000048e5e95000000000000000000000000000000000000000000000ffff7f000001479d000000000000000000000000000000000000ffff7f000001479d0000000000000000"));
+    @Test
+    public void decode_relay_noBestHeight_noSubVer() throws Exception {
+        String hex = "00000000000000000000000048e5e95000000000000000000000000000000000000000000000ffff7f000001479d000000000000000000000000000000000000ffff7f000001479d0000000000000000";
+        VersionMessage ver = new VersionMessage(UNITTEST, HEX.decode(hex));
         assertTrue(ver.relayTxesBeforeFilter);
         assertEquals(0, ver.bestHeight);
         assertEquals("", ver.subVer);
     }
+
+    @Test
+    public void roundTrip_ipv4_currentProtocolVersion() throws Exception {
+        VersionMessage ver = new VersionMessage(UNITTEST, 1234);
+        ver.time = 23456;
+        ver.subVer = "/bitcoinj/";
+        ver.clientVersion = NetworkParameters.ProtocolVersion.CURRENT.getBitcoinProtocolVersion();
+        ver.localServices = 1;
+        ver.fromAddr = new PeerAddress(UNITTEST, InetAddress.getByName("1.2.3.4"), 3888);
+        ver.fromAddr.setParent(ver);
+        ver.receivingAddr = new PeerAddress(UNITTEST, InetAddress.getByName("4.3.2.1"), 8333);
+        ver.receivingAddr.setParent(ver);
+        byte[] serialized = ver.bitcoinSerialize();
+        VersionMessage ver2 = new VersionMessage(UNITTEST, serialized);
+        assertEquals(1234, ver2.bestHeight);
+        assertEquals(23456, ver2.time);
+        assertEquals("/bitcoinj/", ver2.subVer);
+        assertEquals(NetworkParameters.ProtocolVersion.CURRENT.getBitcoinProtocolVersion(), ver2.clientVersion);
+        assertEquals(1, ver2.localServices);
+        assertEquals("1.2.3.4", ver2.fromAddr.getAddr().getHostAddress());
+        assertEquals(3888, ver2.fromAddr.getPort());
+        assertEquals("4.3.2.1", ver2.receivingAddr.getAddr().getHostAddress());
+        assertEquals(8333, ver2.receivingAddr.getPort());
+    }
+
+    @Test
+    public void roundTrip_ipv4_ancientProtocolVersion() throws Exception {
+        VersionMessage ver = new VersionMessage(UNITTEST, 0);
+        ver.time = 23456;
+        ver.clientVersion = 0;
+        ver.localServices = 1;
+        ver.receivingAddr = new PeerAddress(UNITTEST, InetAddress.getByName("4.3.2.1"), 8333);
+        ver.receivingAddr.setParent(ver);
+        byte[] serialized = ver.bitcoinSerialize();
+        VersionMessage ver2 = new VersionMessage(UNITTEST, serialized);
+        assertEquals(23456, ver2.time);
+        assertEquals(0, ver2.clientVersion);
+        assertEquals(1, ver2.localServices);
+        assertEquals("4.3.2.1", ver2.receivingAddr.getAddr().getHostAddress());
+        assertEquals(8333, ver2.receivingAddr.getPort());
+    }
+
+    @Test
+    public void roundTrip_ipv6_currentProtocolVersion() throws Exception {
+        VersionMessage ver = new VersionMessage(UNITTEST, 1234);
+        ver.time = 23456;
+        ver.subVer = "/bitcoinj/";
+        ver.clientVersion = NetworkParameters.ProtocolVersion.CURRENT.getBitcoinProtocolVersion();
+        ver.localServices = 1;
+        ver.fromAddr = new PeerAddress(UNITTEST, InetAddress.getByName("2001:db8:85a3:0:0:8a2e:370:7334"), 3888);
+        ver.fromAddr.setParent(ver);
+        ver.receivingAddr = new PeerAddress(UNITTEST, InetAddress.getByName("2002:db8:85a3:0:0:8a2e:370:7335"), 8333);
+        ver.receivingAddr.setParent(ver);
+        byte[] serialized = ver.bitcoinSerialize();
+        VersionMessage ver2 = new VersionMessage(UNITTEST, serialized);
+        assertEquals(1234, ver2.bestHeight);
+        assertEquals(23456, ver2.time);
+        assertEquals("/bitcoinj/", ver2.subVer);
+        assertEquals(NetworkParameters.ProtocolVersion.CURRENT.getBitcoinProtocolVersion(), ver2.clientVersion);
+        assertEquals(1, ver2.localServices);
+        assertEquals("2001:db8:85a3:0:0:8a2e:370:7334", ver2.fromAddr.getAddr().getHostAddress());
+        assertEquals(3888, ver2.fromAddr.getPort());
+        assertEquals("2002:db8:85a3:0:0:8a2e:370:7335", ver2.receivingAddr.getAddr().getHostAddress());
+        assertEquals(8333, ver2.receivingAddr.getPort());
+    }
+
+    @Test
+    public void roundTrip_ipv6_ancientProtocolVersion() throws Exception {
+        VersionMessage ver = new VersionMessage(UNITTEST, 1234);
+        ver.time = 23456;
+        ver.subVer = "/bitcoinj/";
+        ver.clientVersion = 0;
+        ver.localServices = 1;
+        ver.fromAddr = new PeerAddress(UNITTEST, InetAddress.getByName("2001:db8:85a3:0:0:8a2e:370:7334"), 3888);
+        ver.fromAddr.setParent(ver);
+        ver.receivingAddr = new PeerAddress(UNITTEST, InetAddress.getByName("2002:db8:85a3:0:0:8a2e:370:7335"), 8333);
+        ver.receivingAddr.setParent(ver);
+        byte[] serialized = ver.bitcoinSerialize();
+        VersionMessage ver2 = new VersionMessage(UNITTEST, serialized);
+        assertEquals(23456, ver2.time);
+        assertEquals(0, ver2.clientVersion);
+        assertEquals(1, ver2.localServices);
+        assertEquals("2002:db8:85a3:0:0:8a2e:370:7335", ver2.receivingAddr.getAddr().getHostAddress());
+        assertEquals(8333, ver2.receivingAddr.getPort());
+    }
 }