RLP zero encoding fix. (#4388)

* RLP zero encoding fix.

Signed-off-by: mark-terry <mark.terry@consensys.net>

CHANGELOG.md

@@ -7,6 +7,7 @@
   * For the EC encryptor, the encoded public key length is 91
 
 ### Additions and Improvements
+- Improved RLP processing of zero-length string as 0x80 [#4283](https://github.com/hyperledger/besu/pull/4283) [#4388](https://github.com/hyperledger/besu/issues/4388)
 
 ### Bug Fixes
 - Corrects emission of blockadded events when rewinding during a re-org. Fix for [#4495](https://github.com/hyperledger/besu/issues/4495)

ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/Capability.java

@@ -52,7 +52,7 @@ public class Capability {
   public void writeTo(final RLPOutput out) {
     out.startList();
     out.writeBytes(wrap(getName().getBytes(StandardCharsets.US_ASCII)));
-    out.writeUnsignedByte(getVersion());
+    out.writeUnsignedInt(getVersion());
     out.endList();
   }
 

ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/WireMessagesSedesTest.java

@@ -34,7 +34,7 @@ public class WireMessagesSedesTest {
                 + "b35761b0fe3f0de19cb96092be29a0d0c033a1629d3cf270345586679aba8bbda61069532e3ac7551fc3a9"
                 + "7766c30037184a5bed48a821861");
 
-    assertSedesWorks(rlp, true);
+    assertSedesWorks(rlp);
 
     rlp =
         decodeHexDump(
@@ -43,21 +43,18 @@ public class WireMessagesSedesTest {
                 + "dd83a6a6580b2559c3c2d87527b83ea8f232ddeed2fff3263949105761ab5d0fe3733046e0e75aaa83cada"
                 + "3b1e5d41");
 
-    assertSedesWorks(rlp, true);
+    assertSedesWorks(rlp);
 
     rlp =
         decodeHexDump(
             "f8a305b74e65746865726d696e642f76312e31332e342d302d3365353937326332342d32303232303831312f5836342d4c696e75782f362e302e36e4c5836574683ec5836574683fc58365746840c58365746841c58365746842c5837769748082765db84005c95b2618ba1ca53f0f019d1750d12769267705e46b4dbfb77f73998b21d30973161542a2090bcaa5876e6aed99436009f3f646029bb723b8dff75feec27374");
-    // This test conatains a hello message from Nethermind. The Capability version of the wit
+    // This test contains a hello message from Nethermind. The Capability version of the wit
     // capability is encoded as 0x80, which is an empty string
-    assertSedesWorks(rlp, false);
+    assertSedesWorks(rlp);
   }
 
-  private static void assertSedesWorks(final byte[] data, final boolean encEqualsInput) {
-    // TODO: the boolean was added because we do encode the version number 0 as 0x00, not as 0x80
-    // Ticket #4284 to address the broader issue of encoding/decoding zero
+  private static void assertSedesWorks(final byte[] data) {
     final Bytes input = Bytes.wrap(data);
-
     final PeerInfo peerInfo = PeerInfo.readFrom(RLP.input(input));
 
     assertThat(peerInfo.getClientId()).isNotBlank();
@@ -69,8 +66,6 @@ public class WireMessagesSedesTest {
     // Re-serialize and check that data matches
     final BytesValueRLPOutput out = new BytesValueRLPOutput();
     peerInfo.writeTo(out);
-    if (encEqualsInput) {
-      assertThat(out.encoded()).isEqualTo(input);
-    }
+    assertThat(out.encoded()).isEqualTo(input);
   }
 }

ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/RLPOutput.java

@@ -17,6 +17,7 @@ package org.hyperledger.besu.ethereum.rlp;
 import java.math.BigInteger;
 import java.net.InetAddress;
 import java.util.function.BiConsumer;
+import java.util.function.Consumer;
 
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.MutableBytes;
@@ -187,7 +188,7 @@ public interface RLPOutput {
    *     {@code b < 0} or {@code b > 0xFF}.
    */
   default void writeUnsignedByte(final int b) {
-    writeBytes(Bytes.of(b));
+    processZeroByte(Long.valueOf(b), a -> writeBytes(Bytes.of(b)));
   }
 
   /**
@@ -198,7 +199,7 @@ public interface RLPOutput {
    *     if either {@code s < 0} or {@code s > 0xFFFF}.
    */
   default void writeUnsignedShort(final int s) {
-    writeBytes(Bytes.ofUnsignedShort(s));
+    processZeroByte(Long.valueOf(s), a -> writeBytes(Bytes.ofUnsignedShort(s).trimLeadingZeros()));
   }
 
   /**
@@ -209,7 +210,7 @@ public interface RLPOutput {
    *     either {@code i < 0} or {@code i > 0xFFFFFFFFL}.
    */
   default void writeUnsignedInt(final long i) {
-    writeBytes(Bytes.ofUnsignedInt(i));
+    processZeroByte(i, a -> writeBytes(Bytes.ofUnsignedInt(i).trimLeadingZeros()));
   }
 
   /**
@@ -290,4 +291,19 @@ public interface RLPOutput {
    * @param bytes An already RLP encoded value to write as next item of this output.
    */
   void writeRaw(Bytes bytes);
+
+  /**
+   * Check if the incoming value is 0 and writes it as 0x80, per the spec.
+   *
+   * @param input The value to check
+   * @param writer The consumer to write the non-zero output
+   */
+  private void processZeroByte(final Long input, final Consumer<Long> writer) {
+    // If input == 0, encode 0 value as 0x80
+    if (input == 0) {
+      writeRaw(Bytes.of(0x80));
+    } else {
+      writer.accept(input);
+    }
+  }
 }