Add shouldRejectTrailingBytes test for messages

src/main/java/org/ethereum/beacon/discovery/message/FindNodeMessage.java

@@ -40,7 +40,6 @@ public static FindNodeMessage fromBytes(Bytes bytes) throws DecodeException {
         reader -> {
           final Bytes requestId = checkMaxSize(reader.readValue(), MAX_REQUEST_ID_SIZE);
           List<Integer> distances = reader.readListContents(RLPReader::readInt);
-          RlpUtil.checkComplete(reader);
           return new FindNodeMessage(requestId, distances);
         });
   }

src/main/java/org/ethereum/beacon/discovery/message/NodesMessage.java

@@ -39,7 +39,6 @@ public static NodesMessage fromBytes(Bytes messageBytes, NodeRecordFactory nodeR
           final Bytes requestId = checkMaxSize(reader.readValue(), MAX_REQUEST_ID_SIZE);
           final int total = reader.readInt();
           final List<NodeRecord> nodeRecords = reader.readListContents(nodeRecordFactory::fromRlp);
-          RlpUtil.checkComplete(reader);
           return new NodesMessage(requestId, total, nodeRecords);
         });
   }

src/main/java/org/ethereum/beacon/discovery/message/PingMessage.java

@@ -32,7 +32,6 @@ public static PingMessage fromBytes(Bytes bytes) {
         reader -> {
           final Bytes requestId = checkMaxSize(reader.readValue(), MAX_REQUEST_ID_SIZE);
           final UInt64 enrSeq = UInt64.valueOf(reader.readBigInteger());
-          RlpUtil.checkComplete(reader);
           return new PingMessage(requestId, enrSeq);
         });
   }

src/main/java/org/ethereum/beacon/discovery/message/PongMessage.java

@@ -39,7 +39,6 @@ public static PongMessage fromBytes(Bytes bytes) {
           final UInt64 enrSeq = UInt64.valueOf(reader.readBigInteger());
           final Bytes recipientIp = checkSizeEither(reader.readValue(), 4, 16);
           final int recipientPort = reader.readInt();
-          RlpUtil.checkComplete(reader);
           return new PongMessage(requestId, enrSeq, recipientIp, recipientPort);
         });
   }

src/main/java/org/ethereum/beacon/discovery/message/TalkReqMessage.java

@@ -37,7 +37,6 @@ public static TalkReqMessage fromBytes(Bytes bytes) {
           final Bytes requestId = checkMaxSize(reader.readValue(), MAX_REQUEST_ID_SIZE);
           final Bytes protocol = reader.readValue();
           final Bytes request = reader.readValue();
-          RlpUtil.checkComplete(reader);
           return new TalkReqMessage(requestId, protocol, request);
         });
   }

src/main/java/org/ethereum/beacon/discovery/message/TalkRespMessage.java

@@ -30,7 +30,6 @@ public static TalkRespMessage fromBytes(Bytes bytes) {
         reader -> {
           final Bytes requestId = checkMaxSize(reader.readValue(), MAX_REQUEST_ID_SIZE);
           final Bytes response = reader.readValue();
-          RlpUtil.checkComplete(reader);
           return new TalkRespMessage(requestId, response);
         });
   }

src/main/java/org/ethereum/beacon/discovery/packet/impl/HeaderImpl.java

@@ -20,8 +20,8 @@
 import org.ethereum.beacon.discovery.util.DecodeException;
 import org.ethereum.beacon.discovery.util.DecryptException;
 
-public class HeaderImpl<TAUthData extends AuthData> extends AbstractBytes
-    implements Header<TAUthData> {
+public class HeaderImpl<TAuthData extends AuthData> extends AbstractBytes
+    implements Header<TAuthData> {
 
   public static Header<?> decrypt(Bytes data, Bytes16 iv, Bytes16 destNodeId)
       throws DecodeException {
@@ -62,9 +62,9 @@ private static AuthData decodeAuthData(StaticHeader header, Bytes authDataBytes)
   }
 
   private final StaticHeader staticHeader;
-  private final TAUthData authData;
+  private final TAuthData authData;
 
-  public HeaderImpl(StaticHeader staticHeader, TAUthData authData) {
+  public HeaderImpl(StaticHeader staticHeader, TAuthData authData) {
     super(Bytes.wrap(staticHeader.getBytes(), authData.getBytes()));
     checkArgument(
         authData.getBytes().size() == staticHeader.getAuthDataSize(),
@@ -84,7 +84,7 @@ public StaticHeader getStaticHeader() {
   }
 
   @Override
-  public TAUthData getAuthData() {
+  public TAuthData getAuthData() {
     return authData;
   }
 

src/test/java/org/ethereum/beacon/discovery/TestUtil.java

@@ -4,6 +4,9 @@
 
 package org.ethereum.beacon.discovery;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
 import java.net.InetSocketAddress;
 import java.util.BitSet;
 import java.util.Random;
@@ -13,6 +16,8 @@
 import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.crypto.SECP256K1.KeyPair;
 import org.apache.tuweni.crypto.SECP256K1.SecretKey;
+import org.ethereum.beacon.discovery.message.DiscoveryV5MessageDecoder;
+import org.ethereum.beacon.discovery.message.V5Message;
 import org.ethereum.beacon.discovery.mock.IdentitySchemaV4InterpreterMock;
 import org.ethereum.beacon.discovery.packet.HandshakeMessagePacket.HandshakeAuthData;
 import org.ethereum.beacon.discovery.packet.StaticHeader;
@@ -22,6 +27,7 @@
 import org.ethereum.beacon.discovery.schema.NodeRecordBuilder;
 import org.ethereum.beacon.discovery.schema.NodeRecordFactory;
 import org.ethereum.beacon.discovery.util.Functions;
+import org.ethereum.beacon.discovery.util.RlpDecodeException;
 
 public class TestUtil {
   public static final NodeRecordFactory NODE_RECORD_FACTORY =
@@ -173,4 +179,19 @@ public interface ThrowingRunnable {
 
     void run() throws Exception;
   }
+
+  @SuppressWarnings("unchecked")
+  public static <M extends V5Message> void assertRoundTrip(
+      final DiscoveryV5MessageDecoder decoder, final M message) {
+    final Bytes rlp = message.getBytes();
+    final M result = (M) decoder.decode(rlp);
+    assertThat(result).isEqualTo(message);
+    assertThat(result.getBytes()).isEqualTo(rlp);
+  }
+
+  public static <M extends V5Message> void assertRejectTrailingBytes(
+      final DiscoveryV5MessageDecoder decoder, final M message) {
+    final Bytes rlp = Bytes.concatenate(message.getBytes(), Bytes.fromHexString("0x1234"));
+    assertThatThrownBy(() -> decoder.decode(rlp)).isInstanceOf(RlpDecodeException.class);
+  }
 }

src/test/java/org/ethereum/beacon/discovery/integration/DiscoveryIntegrationTest.java

@@ -246,7 +246,7 @@ public void shouldRetrieveNewEnrFromPeerWhenPingReportsItChanged() throws Except
     assertThat(findNodeRecordByNodeId(remoteNode, originalLocalNodeRecord.getNodeId()))
         .contains(originalLocalNodeRecord);
 
-    // Remote node pings us which should trigger us updating it's ENR.
+    // Remote node pings us which should trigger us updating its ENR.
     remoteNode.updateCustomFieldValue("eth2", Bytes.fromHexString("0x5555"));
     final NodeRecord updatedRemoteNodeRecord = remoteNode.getLocalNodeRecord();
     waitFor(remoteNode.ping(localNode.getLocalNodeRecord()));

src/test/java/org/ethereum/beacon/discovery/message/FindNodeMessageTest.java

@@ -4,35 +4,28 @@
 
 package org.ethereum.beacon.discovery.message;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.ethereum.beacon.discovery.TestUtil.assertRejectTrailingBytes;
+import static org.ethereum.beacon.discovery.TestUtil.assertRoundTrip;
 
 import java.util.List;
 import org.apache.tuweni.bytes.Bytes;
 import org.ethereum.beacon.discovery.SimpleIdentitySchemaInterpreter;
 import org.ethereum.beacon.discovery.schema.NodeRecordFactory;
-import org.ethereum.beacon.discovery.util.RlpDecodeException;
 import org.junit.jupiter.api.Test;
 
 class FindNodeMessageTest {
-
-  private final DiscoveryV5MessageDecoder decoder =
+  private static final DiscoveryV5MessageDecoder DECODER =
       new DiscoveryV5MessageDecoder(new NodeRecordFactory(new SimpleIdentitySchemaInterpreter()));
+  private static final FindNodeMessage MESSAGE =
+      new FindNodeMessage(Bytes.fromHexString("0x134488556699"), List.of(1, 2, 3, 4, 6, 9, 10));
 
   @Test
-  void shouldRoundTripViaRlp() {
-    final FindNodeMessage message =
-        new FindNodeMessage(Bytes.fromHexString("0x134488556699"), List.of(1, 2, 3, 4, 6, 9, 10));
-    final Bytes rlp = message.getBytes();
-    final FindNodeMessage result = (FindNodeMessage) decoder.decode(rlp);
-    assertThat(result).isEqualTo(message);
+  void shouldRoundTrip() {
+    assertRoundTrip(DECODER, MESSAGE);
   }
 
   @Test
   void shouldRejectTrailingBytes() {
-    final FindNodeMessage message =
-        new FindNodeMessage(Bytes.fromHexString("0x134488556699"), List.of(1, 2, 3, 4, 6, 9, 10));
-    final Bytes rlp = Bytes.concatenate(message.getBytes(), Bytes.fromHexString("0x1234"));
-    assertThatThrownBy(() -> decoder.decode(rlp)).isInstanceOf(RlpDecodeException.class);
+    assertRejectTrailingBytes(DECODER, MESSAGE);
   }
 }

src/test/java/org/ethereum/beacon/discovery/message/NodesMessageTest.java

@@ -4,32 +4,34 @@
 
 package org.ethereum.beacon.discovery.message;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.ethereum.beacon.discovery.TestUtil.assertRejectTrailingBytes;
+import static org.ethereum.beacon.discovery.TestUtil.assertRoundTrip;
 
 import java.util.List;
 import org.apache.tuweni.bytes.Bytes;
 import org.ethereum.beacon.discovery.schema.NodeRecordFactory;
 import org.junit.jupiter.api.Test;
 
 class NodesMessageTest {
-  private final DiscoveryV5MessageDecoder decoder =
+  private static final DiscoveryV5MessageDecoder DECODER =
       new DiscoveryV5MessageDecoder(NodeRecordFactory.DEFAULT);
+  private static final NodesMessage MESSAGE =
+      new NodesMessage(
+          Bytes.fromHexString("0x85482293"),
+          8,
+          List.of(
+              NodeRecordFactory.DEFAULT.fromEnr(
+                  "enr:-KK4QH0RsNJmIG0EX9LSnVxMvg-CAOr3ZFF92hunU63uE7wcYBjG1cFbUTvEa5G_4nDJkRhUq9q2ck9xY-VX1RtBsruBtIRldGgykIL0pysBABAg__________-CaWSCdjSCaXCEEnXQ0YlzZWNwMjU2azGhA1grTzOdMgBvjNrk-vqWtTZsYQIi0QawrhoZrsn5Hd56g3RjcIIjKIN1ZHCCIyg"),
+              NodeRecordFactory.DEFAULT.fromEnr(
+                  "enr:-LK4QH1xnjotgXwg25IDPjrqRGFnH1ScgNHA3dv1Z8xHCp4uP3N3Jjl_aYv_WIxQRdwZvSukzbwspXZ7JjpldyeVDzMCh2F0dG5ldHOIAAAAAAAAAACEZXRoMpB53wQoAAAQIP__________gmlkgnY0gmlwhIe1te-Jc2VjcDI1NmsxoQOkcGXqbCJYbcClZ3z5f6NWhX_1YPFRYRRWQpJjwSHpVIN0Y3CCIyiDdWRwgiMo")));
 
   @Test
   void shouldRoundTrip() {
-    final NodesMessage original =
-        new NodesMessage(
-            Bytes.fromHexString("0x85482293"),
-            8,
-            List.of(
-                NodeRecordFactory.DEFAULT.fromEnr(
-                    "enr:-KK4QH0RsNJmIG0EX9LSnVxMvg-CAOr3ZFF92hunU63uE7wcYBjG1cFbUTvEa5G_4nDJkRhUq9q2ck9xY-VX1RtBsruBtIRldGgykIL0pysBABAg__________-CaWSCdjSCaXCEEnXQ0YlzZWNwMjU2azGhA1grTzOdMgBvjNrk-vqWtTZsYQIi0QawrhoZrsn5Hd56g3RjcIIjKIN1ZHCCIyg"),
-                NodeRecordFactory.DEFAULT.fromEnr(
-                    "enr:-LK4QH1xnjotgXwg25IDPjrqRGFnH1ScgNHA3dv1Z8xHCp4uP3N3Jjl_aYv_WIxQRdwZvSukzbwspXZ7JjpldyeVDzMCh2F0dG5ldHOIAAAAAAAAAACEZXRoMpB53wQoAAAQIP__________gmlkgnY0gmlwhIe1te-Jc2VjcDI1NmsxoQOkcGXqbCJYbcClZ3z5f6NWhX_1YPFRYRRWQpJjwSHpVIN0Y3CCIyiDdWRwgiMo")));
+    assertRoundTrip(DECODER, MESSAGE);
+  }
 
-    final Bytes rlp = original.getBytes();
-    final Message result = decoder.decode(rlp);
-    assertThat(result).isEqualTo(original);
-    assertThat(result.getBytes()).isEqualTo(rlp);
+  @Test
+  void shouldRejectTrailingBytes() {
+    assertRejectTrailingBytes(DECODER, MESSAGE);
   }
 }

src/test/java/org/ethereum/beacon/discovery/message/PingMessageTest.java

@@ -4,26 +4,27 @@
 
 package org.ethereum.beacon.discovery.message;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.ethereum.beacon.discovery.TestUtil.assertRejectTrailingBytes;
+import static org.ethereum.beacon.discovery.TestUtil.assertRoundTrip;
 
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.units.bigints.UInt64;
 import org.ethereum.beacon.discovery.schema.NodeRecordFactory;
 import org.junit.jupiter.api.Test;
 
 class PingMessageTest {
-
-  private final DiscoveryV5MessageDecoder decoder =
+  private static final DiscoveryV5MessageDecoder DECODER =
       new DiscoveryV5MessageDecoder(NodeRecordFactory.DEFAULT);
+  private static final PingMessage MESSAGE =
+      new PingMessage(Bytes.fromHexString("0x85482293"), UInt64.MAX_VALUE);
 
   @Test
   void shouldRoundTrip() {
-    final PingMessage original =
-        new PingMessage(Bytes.fromHexString("0x85482293"), UInt64.MAX_VALUE);
+    assertRoundTrip(DECODER, MESSAGE);
+  }
 
-    final Bytes rlp = original.getBytes();
-    final Message result = decoder.decode(rlp);
-    assertThat(result).isEqualTo(original);
-    assertThat(result.getBytes()).isEqualTo(rlp);
+  @Test
+  void shouldRejectTrailingBytes() {
+    assertRejectTrailingBytes(DECODER, MESSAGE);
   }
 }

src/test/java/org/ethereum/beacon/discovery/message/PongMessageTest.java

@@ -4,8 +4,9 @@
 
 package org.ethereum.beacon.discovery.message;
 
-import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.ethereum.beacon.discovery.TestUtil.assertRejectTrailingBytes;
+import static org.ethereum.beacon.discovery.TestUtil.assertRoundTrip;
 
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.units.bigints.UInt64;
@@ -14,38 +15,31 @@
 import org.junit.jupiter.api.Test;
 
 class PongMessageTest {
-
-  private final DiscoveryV5MessageDecoder decoder =
+  private static final DiscoveryV5MessageDecoder DECODER =
       new DiscoveryV5MessageDecoder(NodeRecordFactory.DEFAULT);
+  private static final PongMessage MESSAGE_IPV4 =
+      new PongMessage(
+          Bytes.fromHexString("0x85482293"),
+          UInt64.MAX_VALUE.subtract(1),
+          Bytes.fromHexString("0x12121212"),
+          48);
+  private static final PongMessage MESSAGE_IPV6 =
+      new PongMessage(
+          Bytes.fromHexString("0x85482293"),
+          UInt64.MAX_VALUE.subtract(1),
+          Bytes.fromHexString("0x12121212121212121212121212121212"),
+          48);
 
   @Test
-  void shouldRoundTripIpv4() {
-    final PongMessage original =
-        new PongMessage(
-            Bytes.fromHexString("0x85482293"),
-            UInt64.MAX_VALUE.subtract(1),
-            Bytes.fromHexString("0x12121212"),
-            48);
-
-    final Bytes rlp = original.getBytes();
-    final Message result = decoder.decode(rlp);
-    assertThat(result).isEqualTo(original);
-    assertThat(result.getBytes()).isEqualTo(rlp);
+  void shouldRoundTrip() {
+    assertRoundTrip(DECODER, MESSAGE_IPV4);
+    assertRoundTrip(DECODER, MESSAGE_IPV6);
   }
 
   @Test
-  void shouldRoundTripIpv6() {
-    final PongMessage original =
-        new PongMessage(
-            Bytes.fromHexString("0x85482293"),
-            UInt64.MAX_VALUE.subtract(1),
-            Bytes.fromHexString("0x12121212121212121212121212121212"),
-            48);
-
-    final Bytes rlp = original.getBytes();
-    final Message result = decoder.decode(rlp);
-    assertThat(result).isEqualTo(original);
-    assertThat(result.getBytes()).isEqualTo(rlp);
+  void shouldRejectTrailingBytes() {
+    assertRejectTrailingBytes(DECODER, MESSAGE_IPV4);
+    assertRejectTrailingBytes(DECODER, MESSAGE_IPV6);
   }
 
   @Test
@@ -58,6 +52,6 @@ void shouldFailDecodingWhenIpHasIncorrectNumberOfBytes() {
             48);
 
     final Bytes rlp = original.getBytes();
-    assertThatThrownBy(() -> decoder.decode(rlp)).isInstanceOf(RlpDecodeException.class);
+    assertThatThrownBy(() -> DECODER.decode(rlp)).isInstanceOf(RlpDecodeException.class);
   }
 }

src/test/java/org/ethereum/beacon/discovery/message/TalkReqMessageTest.java

@@ -4,27 +4,29 @@
 
 package org.ethereum.beacon.discovery.message;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.ethereum.beacon.discovery.TestUtil.assertRejectTrailingBytes;
+import static org.ethereum.beacon.discovery.TestUtil.assertRoundTrip;
 
 import org.apache.tuweni.bytes.Bytes;
 import org.ethereum.beacon.discovery.schema.NodeRecordFactory;
 import org.junit.jupiter.api.Test;
 
 class TalkReqMessageTest {
-  private final DiscoveryV5MessageDecoder decoder =
+  private static final DiscoveryV5MessageDecoder DECODER =
       new DiscoveryV5MessageDecoder(NodeRecordFactory.DEFAULT);
+  private static final TalkReqMessage MESSAGE =
+      new TalkReqMessage(
+          Bytes.fromHexString("0x85482293"),
+          Bytes.fromHexString("0x12345678"),
+          Bytes.fromHexString("0x9876543231"));
 
   @Test
   void shouldRoundTrip() {
-    final TalkReqMessage original =
-        new TalkReqMessage(
-            Bytes.fromHexString("0x85482293"),
-            Bytes.fromHexString("0x12345678"),
-            Bytes.fromHexString("0x9876543231"));
+    assertRoundTrip(DECODER, MESSAGE);
+  }
 
-    final Bytes rlp = original.getBytes();
-    final Message result = decoder.decode(rlp);
-    assertThat(result).isEqualTo(original);
-    assertThat(result.getBytes()).isEqualTo(rlp);
+  @Test
+  void shouldRejectTrailingBytes() {
+    assertRejectTrailingBytes(DECODER, MESSAGE);
   }
 }