Ensure all node records are valid length (#185)

* Refactor NodeRecord to ensure all created NodeRecords are no larger than 300 bytes

* Create new map entry instead of trying to modify the existing one

* Test that exception is thrown when enr is too long

* spotless

* Use Bytes.repeat to clean up new test

* Ensure field list contains no duplicate entries before collecting to map

* spotless

.github/workflows/gradle-wrapper-validation.yml

@@ -6,5 +6,5 @@ jobs:
     name: "Validation"
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: gradle/wrapper-validation-action@v1
+      - uses: actions/checkout@v4
+      - uses: gradle/actions/wrapper-validation@v4

src/main/java/org/ethereum/beacon/discovery/schema/NodeRecord.java

@@ -47,36 +47,48 @@ public class NodeRecord {
   private final IdentitySchemaInterpreter identitySchemaInterpreter;
 
   private NodeRecord(
-      IdentitySchemaInterpreter identitySchemaInterpreter, UInt64 seq, Bytes signature) {
+      IdentitySchemaInterpreter identitySchemaInterpreter,
+      UInt64 seq,
+      Bytes signature,
+      Map<String, Object> fields) {
     this.seq = seq;
     this.signature = signature;
     this.identitySchemaInterpreter = identitySchemaInterpreter;
-  }
-
-  private NodeRecord(IdentitySchemaInterpreter identitySchemaInterpreter, UInt64 seq) {
-    this.seq = seq;
-    this.signature = MutableBytes.create(96);
-    this.identitySchemaInterpreter = identitySchemaInterpreter;
+    this.fields.putAll(fields);
+    // serialise to check size
+    Bytes serializedNodeRecord = serialize();
+    checkArgument(
+        serializedNodeRecord.size() <= MAX_ENCODED_SIZE,
+        "Node record exceeds maximum encoded size");
   }
 
   public static NodeRecord fromValues(
       IdentitySchemaInterpreter identitySchemaInterpreter,
       UInt64 seq,
       List<EnrField> fieldKeyPairs) {
-    NodeRecord nodeRecord = new NodeRecord(identitySchemaInterpreter, seq);
-    fieldKeyPairs.forEach(objects -> nodeRecord.set(objects.getName(), objects.getValue()));
-    return nodeRecord;
+    return new NodeRecord(
+        identitySchemaInterpreter,
+        seq,
+        MutableBytes.create(96),
+        fieldKeyPairs.stream()
+            .distinct()
+            .collect(Collectors.toMap(EnrField::getName, EnrField::getValue)));
   }
 
   public static NodeRecord fromRawFields(
       IdentitySchemaInterpreter identitySchemaInterpreter,
       UInt64 seq,
       Bytes signature,
       Map<String, Object> rawFields) {
-    NodeRecord nodeRecord = new NodeRecord(identitySchemaInterpreter, seq, signature);
-    rawFields.forEach(
-        (key, value) -> nodeRecord.set(key, ENR_FIELD_INTERPRETER.decode(key, value)));
-    return nodeRecord;
+    return new NodeRecord(
+        identitySchemaInterpreter,
+        seq,
+        signature,
+        rawFields.entrySet().stream()
+            .map(
+                (e) ->
+                    Map.entry(e.getKey(), ENR_FIELD_INTERPRETER.decode(e.getKey(), e.getValue())))
+            .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
   }
 
   public String asBase64() {
@@ -179,14 +191,6 @@ void writeRlp(
         });
   }
 
-  public Bytes asRlp() {
-    return asRlpImpl(true);
-  }
-
-  public Bytes asRlpNoSignature() {
-    return asRlpImpl(false);
-  }
-
   private Bytes asRlpImpl(boolean withSignature) {
     return RLP.encode(writer -> writeRlp(writer, withSignature));
   }
@@ -200,9 +204,7 @@ public Bytes serializeNoSignature() {
   }
 
   private Bytes serializeImpl(boolean withSignature) {
-    Bytes bytes = withSignature ? asRlp() : asRlpNoSignature();
-    checkArgument(bytes.size() <= MAX_ENCODED_SIZE, "Node record exceeds maximum encoded size");
-    return bytes;
+    return asRlpImpl(withSignature);
   }
 
   public Bytes getNodeId() {

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

@@ -4,7 +4,6 @@
 
 package org.ethereum.beacon.discovery;
 
-import static org.assertj.core.api.Assertions.assertThat;
 import static org.ethereum.beacon.discovery.TestUtil.SEED;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -23,11 +22,13 @@
 import org.apache.tuweni.units.bigints.UInt64;
 import org.ethereum.beacon.discovery.schema.EnrField;
 import org.ethereum.beacon.discovery.schema.IdentitySchema;
+import org.ethereum.beacon.discovery.schema.IdentitySchemaInterpreter;
 import org.ethereum.beacon.discovery.schema.IdentitySchemaV4Interpreter;
 import org.ethereum.beacon.discovery.schema.NodeRecord;
 import org.ethereum.beacon.discovery.schema.NodeRecordBuilder;
 import org.ethereum.beacon.discovery.schema.NodeRecordFactory;
 import org.ethereum.beacon.discovery.util.Functions;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -101,7 +102,7 @@ public void testEnrWithCustomFieldsDecodes() {
 
     final String serialized = record.asBase64();
     final NodeRecord result = NODE_RECORD_FACTORY.fromBase64(serialized);
-    assertThat(result).isEqualTo(record);
+    assertEquals(record, result);
   }
 
   @Test
@@ -230,23 +231,33 @@ public void testEnrWithEthFieldDecodes() {
 
     final String serialized = record.asBase64();
     final NodeRecord result = NODE_RECORD_FACTORY.fromBase64(serialized);
-    assertThat(result).isEqualTo(record);
-    assertThat(result.asEnr())
-        .isEqualTo(
-            "enr:-JC4QBF-k6ezIoeB4BTMx9sLpFmcTZT4nBGUZLJ0JeYlT_rtfpVIa02sdfJwjUcXCb1h_HwGUtgPwwz2cbJiyAP4wT4Bg2V0aMfGhHpznvGAgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN0Y3CCdl8");
+    assertEquals(record, result);
+    String expectedEnr =
+        "enr:-JC4QBF-k6ezIoeB4BTMx9sLpFmcTZT4nBGUZLJ0JeYlT_rtfpVIa02sdfJwjUcXCb1h_HwGUtgPwwz2cbJiyAP4wT4Bg2V0aMfGhHpznvGAgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN0Y3CCdl8";
+    assertEquals(expectedEnr, result.asEnr());
     final NodeRecord nodeRecord = NODE_RECORD_FACTORY.fromEnr(result.asEnr());
-    assertThat(nodeRecord.isValid()).isTrue();
-    assertThat(nodeRecord.get("eth")).isEqualTo(Collections.singletonList(forkIdList));
-    assertThat(nodeRecord).isEqualTo(result);
+    assertTrue(nodeRecord.isValid());
+    assertEquals(Collections.singletonList(forkIdList), nodeRecord.get("eth"));
+    assertEquals(result, nodeRecord);
   }
 
   @Test
   void testEnrWithValidEthFieldDecodes() {
     final String enr =
         "enr:-KK4QH0RsNJmIG0EX9LSnVxMvg-CAOr3ZFF92hunU63uE7wcYBjG1cFbUTvEa5G_4nDJkRhUq9q2ck9xY-VX1RtBsruBtIRldGgykIL0pysBABAg__________-CaWSCdjSCaXCEEnXQ0YlzZWNwMjU2azGhA1grTzOdMgBvjNrk-vqWtTZsYQIi0QawrhoZrsn5Hd56g3RjcIIjKIN1ZHCCIyg";
     final NodeRecord nodeRecord = NODE_RECORD_FACTORY.fromEnr(enr);
-    assertThat(nodeRecord.asEnr()).isEqualTo(enr);
-    assertThat(nodeRecord.get("eth2"))
-        .isEqualTo(Bytes.fromHexString("0x82f4a72b01001020ffffffffffffffff"));
+    assertEquals(enr, nodeRecord.asEnr());
+    assertEquals(Bytes.fromHexString("0x82f4a72b01001020ffffffffffffffff"), nodeRecord.get("eth2"));
+  }
+
+  @Test
+  public void testNodeRecordConstructorFailsToConstructOver300Bytes() {
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            NodeRecord.fromValues(
+                IdentitySchemaInterpreter.V4,
+                UInt64.ONE,
+                List.of(new EnrField("test", Bytes.repeat((byte) 0xFF, 300)))));
   }
 }