Fix issues with RLPExceptions

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockBody.java

@@ -97,7 +97,19 @@ public void writeTo(final RLPOutput output) {
 
   public static BlockBody readFrom(
       final RLPInput input, final BlockHeaderFunctions blockHeaderFunctions) {
+    return readFrom(input, blockHeaderFunctions, false);
+  }
+
+  public static BlockBody readFrom(
+      final RLPInput input,
+      final BlockHeaderFunctions blockHeaderFunctions,
+      final boolean allowEmptyBody) {
     input.enterList();
+    if (input.isEndOfCurrentList() && allowEmptyBody) {
+      // empty block [] -> Return empty body.
+      input.leaveList();
+      return empty();
+    }
     // TODO: Support multiple hard fork transaction formats.
     final BlockBody body =
         new BlockBody(
@@ -125,6 +137,10 @@ public int hashCode() {
     return Objects.hash(transactions, ommers, withdrawals);
   }
 
+  public boolean isEmpty() {
+    return transactions.isEmpty() && ommers.isEmpty() && withdrawals.isEmpty();
+  }
+
   @Override
   public String toString() {
     return "BlockBody{"

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/encoding/TransactionAnnouncementDecoder.java

@@ -22,6 +22,7 @@
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
 import org.hyperledger.besu.plugin.data.TransactionType;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -69,13 +70,14 @@ private static List<TransactionAnnouncement> decodeForEth66(final RLPInput input
    */
   private static List<TransactionAnnouncement> decodeForEth68(final RLPInput input) {
     input.enterList();
-    final List<TransactionType> types =
-        input.readList(
-            rlp -> {
-              final int type = rlp.readByte() & 0xff;
-              return type == 0 ? TransactionType.FRONTIER : TransactionType.of(type);
-            });
-    final List<Long> sizes = input.readList(RLPInput::readUnsignedInt);
+
+    final List<TransactionType> types = new ArrayList<>();
+    final byte[] bytes = input.readBytes().toArray();
+    for (final byte b : bytes) {
+      types.add(b == 0 ? TransactionType.FRONTIER : TransactionType.of(b));
+    }
+
+    final List<Long> sizes = input.readList(in -> (long) in.readBytes().toInt());
     final List<Hash> hashes = input.readList(rlp -> Hash.wrap(rlp.readBytes32()));
     input.leaveList();
     if (!(types.size() == hashes.size() && hashes.size() == sizes.size())) {

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/encoding/TransactionAnnouncementEncoder.java

@@ -76,30 +76,42 @@ private static Bytes encodeForEth66(final List<Transaction> transactions) {
    */
   private static Bytes encodeForEth68(final List<Transaction> transactions) {
     final List<Integer> sizes = new ArrayList<>(transactions.size());
-    final List<TransactionType> types = new ArrayList<>(transactions.size());
+    final byte[] types = new byte[transactions.size()];
     final List<Hash> hashes = new ArrayList<>(transactions.size());
-    transactions.forEach(
-        transaction -> {
-          types.add(transaction.getType());
-          sizes.add(transaction.getSize());
-          hashes.add(transaction.getHash());
-        });
+
+    for (int i = 0; i < transactions.size(); i++) {
+      final TransactionType type = transactions.get(i).getType();
+      types[i] = type == TransactionType.FRONTIER ? 0x00 : type.getSerializedType();
+      sizes.add(transactions.get(i).getSize());
+      hashes.add(transactions.get(i).getHash());
+    }
 
     return encodeForEth68(types, sizes, hashes);
   }
 
   @VisibleForTesting
   public static Bytes encodeForEth68(
       final List<TransactionType> types, final List<Integer> sizes, final List<Hash> hashes) {
+
+    final byte[] byteTypes = new byte[types.size()];
+    for (int i = 0; i < types.size(); i++) {
+      final TransactionType type = types.get(i);
+      byteTypes[i] = type == TransactionType.FRONTIER ? 0x00 : type.getSerializedType();
+    }
+    return encodeForEth68(byteTypes, sizes, hashes);
+  }
+
+  @VisibleForTesting
+  public static Bytes encodeForEth68(
+      final byte[] types, final List<Integer> sizes, final List<Hash> hashes) {
     final BytesValueRLPOutput out = new BytesValueRLPOutput();
     // Check if lists have the same size
-    if (!(types.size() == hashes.size() && hashes.size() == sizes.size())) {
+    if (!(types.length == hashes.size() && hashes.size() == sizes.size())) {
       throw new IllegalArgumentException(
           "Hashes, sizes and types must have the same number of elements");
     }
     out.startList();
-    out.writeList(
-        types, (h, w) -> w.writeByte(h == TransactionType.FRONTIER ? 0x00 : h.getSerializedType()));
+    out.writeBytes(Bytes.wrap((types)));
     out.writeList(sizes, (h, w) -> w.writeInt(h));
     out.writeList(hashes, (h, w) -> w.writeBytes(h));
     out.endList();

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java

@@ -109,6 +109,7 @@ private void handleMessage(
     } catch (final RLPException e) {
       // Peer sent us malformed data - disconnect
       LOG.debug("Disconnecting with BREACH_OF_PROTOCOL due to malformed message: {}", peer, e);
+      LOG.trace("Message data: {}", message.getData());
       peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
       promise.completeExceptionally(new PeerBreachedProtocolException());
     }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/messages/BlockBodiesMessage.java

@@ -71,6 +71,6 @@ public List<BlockBody> bodies(final ProtocolSchedule protocolSchedule) {
     final BlockHeaderFunctions blockHeaderFunctions =
         ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
     return new BytesValueRLPInput(data, false)
-        .readList(rlp -> BlockBody.readFrom(rlp, blockHeaderFunctions));
+        .readList(rlp -> BlockBody.readFrom(rlp, blockHeaderFunctions, true));
   }
 }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java

@@ -138,6 +138,7 @@ private void processNewPooledTransactionHashesMessage(
             "Malformed pooled transaction hashes message received (BREACH_OF_PROTOCOL), disconnecting: {}",
             peer,
             ex);
+        LOG.trace("Message data: {}", transactionsMessage.getData());
         peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
       }
     }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/BlockBodiesMessageTest.java

@@ -14,16 +14,22 @@
  */
 package org.hyperledger.besu.ethereum.eth.messages;
 
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
 import org.hyperledger.besu.config.GenesisConfigFile;
 import org.hyperledger.besu.ethereum.core.BlockBody;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.difficulty.fixed.FixedDifficultyProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
+import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
+import org.hyperledger.besu.ethereum.mainnet.ScheduleBasedBlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.RawMessage;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput;
 import org.hyperledger.besu.ethereum.rlp.RLP;
+import org.hyperledger.besu.ethereum.rlp.RLPException;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 
@@ -36,11 +42,21 @@
 import com.google.common.io.Resources;
 import org.apache.tuweni.bytes.Bytes;
 import org.assertj.core.api.Assertions;
+import org.junit.Before;
 import org.junit.Test;
 
 /** Tests for {@link BlockBodiesMessage}. */
 public final class BlockBodiesMessageTest {
 
+  private ProtocolSchedule protocolSchedule;
+
+  @Before
+  public void setup() {
+    protocolSchedule =
+        FixedDifficultyProtocolSchedule.create(
+            GenesisConfigFile.development().getConfigOptions(), false, EvmConfiguration.DEFAULT);
+  }
+
   @Test
   public void blockBodiesRoundTrip() throws IOException {
     final List<BlockBody> bodies = new ArrayList<>();
@@ -65,16 +81,37 @@ public void blockBodiesRoundTrip() throws IOException {
     final MessageData initialMessage = BlockBodiesMessage.create(bodies);
     final MessageData raw = new RawMessage(EthPV62.BLOCK_BODIES, initialMessage.getData());
     final BlockBodiesMessage message = BlockBodiesMessage.readFrom(raw);
-    final Iterator<BlockBody> readBodies =
-        message
-            .bodies(
-                FixedDifficultyProtocolSchedule.create(
-                    GenesisConfigFile.development().getConfigOptions(),
-                    false,
-                    EvmConfiguration.DEFAULT))
-            .iterator();
+    final Iterator<BlockBody> readBodies = message.bodies(protocolSchedule).iterator();
     for (int i = 0; i < 50; ++i) {
       Assertions.assertThat(readBodies.next()).isEqualTo(bodies.get(i));
     }
   }
+
+  @Test
+  public void shouldEncodeEmptyBlocksInBlockBodiesMessage() {
+    final Bytes bytes = Bytes.fromHexString("0xc2c0c0");
+    final MessageData raw = new RawMessage(EthPV62.BLOCK_BODIES, bytes);
+    final BlockBodiesMessage message = BlockBodiesMessage.readFrom(raw);
+    final List<BlockBody> bodies = message.bodies(protocolSchedule);
+    bodies.forEach(blockBody -> Assertions.assertThat(blockBody.isEmpty()).isTrue());
+  }
+
+  @Test
+  public void shouldNotThrowRLPExceptionIfAllowedEmptyBody() {
+    final Bytes bytes = Bytes.fromHexString("0xc0");
+    final BlockBody empty = BlockBody.readFrom(RLP.input(bytes), null, true);
+    Assertions.assertThat(empty.isEmpty()).isTrue();
+  }
+
+  @Test
+  public void shouldThrowRLPExceptionIfNotAllowedEmptyBody() {
+    final Bytes bytes = Bytes.fromHexString("0xc0");
+    final BlockHeaderFunctions blockHeaderFunctions =
+        ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
+    assertThrows(
+        RLPException.class,
+        () -> {
+          BlockBody.readFrom(RLP.input(bytes), blockHeaderFunctions, false);
+        });
+  }
 }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java

@@ -212,7 +212,7 @@ public void shouldNotScheduleGetPooledTransactionsTaskTwice() {
   public void shouldCreateAndDecodeForEth66() {
 
     final List<TransactionAnnouncement> expectedAnnouncementList =
-        transactionList.stream().map(TransactionAnnouncement::new).collect(Collectors.toList());
+        transactionList.stream().map(TransactionAnnouncement::new).toList();
 
     final NewPooledTransactionHashesMessage message =
         NewPooledTransactionHashesMessage.create(transactionList, EthProtocol.ETH66);
@@ -268,7 +268,7 @@ public void shouldThrowRLPExceptionIfIncorrectVersion() {
   public void shouldEncodeTransactionsCorrectly_Eth68() {
 
     final String expected =
-        "0xf879c3000102cf840000000184000000028400000003f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003";
+        "0xf87983000102cf840000000184000000028400000003f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003";
     final List<Hash> hashes =
         List.of(
             Hash.fromHexString(
@@ -289,7 +289,7 @@ public void shouldEncodeTransactionsCorrectly_Eth68() {
   public void shouldDecodeBytesCorrectly_Eth68() {
     /*
      * [
-     * ["0x00","0x01","0x02"]
+     * "0x00102"]
      * ["0x00000001","0x00000002","0x00000003"],
      * ["0x0000000000000000000000000000000000000000000000000000000000000001",
      *  "0x0000000000000000000000000000000000000000000000000000000000000002",
@@ -299,7 +299,7 @@ public void shouldDecodeBytesCorrectly_Eth68() {
 
     final Bytes bytes =
         Bytes.fromHexString(
-            "0xf879c3000102cf840000000184000000028400000003f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003");
+            "0xf87983000102cf840000000184000000028400000003f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003");
 
     final List<TransactionAnnouncement> announcementList =
         getDecoder(EthProtocol.ETH68).decode(RLP.input(bytes));
@@ -375,9 +375,8 @@ public void shouldThrowInvalidArgumentExceptionWhenCreatingListsWithDifferentSiz
     final Exception exception =
         assertThrows(
             IllegalArgumentException.class,
-            () -> {
-              TransactionAnnouncement.create(new ArrayList<>(), List.of(1L), new ArrayList<>());
-            });
+            () ->
+                TransactionAnnouncement.create(new ArrayList<>(), List.of(1L), new ArrayList<>()));
     final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
     final String actualMessage = exception.getMessage();
     assertThat(actualMessage).isEqualTo(expectedMessage);
@@ -388,10 +387,9 @@ public void shouldThrowInvalidArgumentExceptionWhenEncodingListsWithDifferentSiz
     final Exception exception =
         assertThrows(
             IllegalArgumentException.class,
-            () -> {
-              TransactionAnnouncementEncoder.encodeForEth68(
-                  new ArrayList<>(), List.of(1), new ArrayList<>());
-            });
+            () ->
+                TransactionAnnouncementEncoder.encodeForEth68(
+                    new ArrayList<>(), List.of(1), new ArrayList<>()));
     final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
     final String actualMessage = exception.getMessage();
     assertThat(actualMessage).isEqualTo(expectedMessage);
@@ -401,18 +399,17 @@ public void shouldThrowInvalidArgumentExceptionWhenEncodingListsWithDifferentSiz
   @SuppressWarnings("UnusedVariable")
   public void shouldThrowRLPExceptionWhenDecodingListsWithDifferentSizes() {
 
-    // [[],[],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
+    // ["0x000102",[],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
     final Bytes invalidMessageBytes =
         Bytes.fromHexString(
-            "0xe4c0c0e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
+            "0xe783000102c0e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
 
     final Exception exception =
         assertThrows(
             RLPException.class,
-            () -> {
-              TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
-                  .decode(RLP.input(invalidMessageBytes));
-            });
+            () ->
+                TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
+                    .decode(RLP.input(invalidMessageBytes)));
 
     final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
     final String actualMessage = exception.getMessage();
@@ -423,38 +420,36 @@ public void shouldThrowRLPExceptionWhenDecodingListsWithDifferentSizes() {
   public void shouldThrowRLPExceptionWhenTypeIsInvalid() {
     final Bytes invalidMessageBytes =
         Bytes.fromHexString(
-            // [["0x09"],["0x00000002"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
-            "0xeac109c58400000002e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
+            // ["0x07",["0x00000002"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
+            "0xe907c58400000002e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
 
     final Exception exception =
         assertThrows(
-            RLPException.class,
-            () -> {
-              TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
-                  .decode(RLP.input(invalidMessageBytes));
-            });
+            IllegalArgumentException.class,
+            () ->
+                TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
+                    .decode(RLP.input(invalidMessageBytes)));
 
     final String expectedMessage = "Unsupported transaction type";
-    final String actualMessage = exception.getCause().getMessage();
+    final String actualMessage = exception.getMessage();
     assertThat(actualMessage).contains(expectedMessage);
   }
 
   @Test
   public void shouldThrowRLPExceptionWhenSizeSizeGreaterThanFourBytes() {
     final Bytes invalidMessageBytes =
         Bytes.fromHexString(
-            // [["0x02"],["0xffffffff01"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
-            "0xebc102c685ffffffff00e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
+            // ["0x02",["0xffffffff01"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
+            "0xea02c685ffffffff00e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
 
     final Exception exception =
         assertThrows(
             RLPException.class,
-            () -> {
-              TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
-                  .decode(RLP.input(invalidMessageBytes));
-            });
+            () ->
+                TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
+                    .decode(RLP.input(invalidMessageBytes)));
 
-    final String expectedMessage = "Cannot read a 4-byte int";
+    final String expectedMessage = "Value of size 5 has more than 4 bytes";
     final String actualMessage = exception.getCause().getMessage();
     assertThat(actualMessage).contains(expectedMessage);
   }