Fix issues with RLPExceptions #5140

Gabriel-Trintinalia · 2023-02-27T02:37:55Z

PR description

This PR fixes two issues with RLP encoding:

1- BlockBody

For the BlockBody RLP serialisation, when an empty body [] is sent, a breach of protocol is triggered and Besu disconnects the peer. That causes Besu to disconnect peers with a good reputation. This PR adds an option in the readFrom method to allow an empty BlockBody whenever a [] is found. The message will be verified later and marked as invalid anyway, but besu will not disconnect the peer.

2 -NewPooledTransactionHashesMessage

NewPooledTransactionHashesMessage on Eth/68 has an issue with the array of types (byte):

[[type_0: B_1, type_1: B_1, ...], [size_0: B_4, size_1: B_4, ...], [hash_0: B_32, hash_1: B_32, ...]]

The current implementation serializes it as:

[
      ["0x01","0x02"]
      ["0x00000001","0x00000002"],
      ["0x0000000000000000000000000000000000000000000000000000000000000001",
       "0x0000000000000000000000000000000000000000000000000000000000000002"]
]

The other clients send the first array of this message as one element instead of an array:

[
      ["0x0102"]
      ["0x00000002","0x00000003"],
      ["0x0000000000000000000000000000000000000000000000000000000000000002",
       "0x0000000000000000000000000000000000000000000000000000000000000003"]
]

This PR fixes the encoding to match the other client messages.

see #5056

Documentation

I thought about documentation and added the doc-change-required label to this PR if
updates are required.

Acceptance Tests (Non Mainnet)

I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.

Changelog

I thought about the changelog and included a changelog update if required.

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Gabriel-Trintinalia · 2023-02-28T05:15:44Z

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

+    if (input.isEndOfCurrentList() && allowEmptyBody) {
+      // empty block [] -> Return empty body.
+      input.leaveList();
+      return empty();
+    }


Handle empty block body

Gabriel-Trintinalia · 2023-02-28T05:16:25Z

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

+    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));
+    }


Fix decoding

Gabriel-Trintinalia · 2023-02-28T05:16:57Z

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

    out.startList();
-    out.writeList(
-        types, (h, w) -> w.writeByte(h == TransactionType.FRONTIER ? 0x00 : h.getSerializedType()));
+    out.writeBytes(Bytes.wrap((types)));


Fix encoding

Gabriel-Trintinalia · 2023-02-28T05:17:28Z

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

        ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
    return new BytesValueRLPInput(data, false)
-        .readList(rlp -> BlockBody.readFrom(rlp, blockHeaderFunctions));
+        .readList(rlp -> BlockBody.readFrom(rlp, blockHeaderFunctions, true));


Allow empty block bodies here

pinges

LGTM

pinges · 2023-02-28T06:39:01Z

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

-          hashes.add(transaction.getHash());
-        });
+
+    for (int i = 0; i < transactions.size(); i++) {


I do prefer the forEach ... But the for loop should work as well :-)

pinges · 2023-02-28T06:43:47Z

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

      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++) {


forEach would work here as well :-)

...perledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java

Signed-off-by: Gabriel-Trintinalia <[email protected]>

macfarla

LGTm.
is there a related hive test for the eth/68 encoding issue?

macfarla · 2023-03-02T05:10:30Z

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

+  public void shouldThrowRLPExceptionIfNotAllowedEmptyBody() {
+    final Bytes bytes = Bytes.fromHexString("0xc0");
+    final BlockHeaderFunctions blockHeaderFunctions =
+        ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);


is the blockHeaderFunctions used?

it's not created for the other test shouldNotThrowRLPExceptionIfAllowedEmptyBody

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Gabriel-Trintinalia added 8 commits February 27, 2023 13:35

Log message data when RLPException

a47afef

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Fix wrong Eth/68 RLP Encoding/decoding

15d4298

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Fix wrong Eth/68 RLP size Encoding

47ba092

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Ignore empty blocks for BlockBody

5ce791b

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Remove this condition as it is checked later

6406e1d

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Fix unit test

29c5f90

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Remove extra logging

308fa94

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Add trace logging to trace the malformad message

c3b0d86

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Gabriel-Trintinalia changed the title ~~WIP - Log message data when RLPException~~ WIP - Fix issues with RLPExceptions Feb 28, 2023

Add a flag allowEmpty to the readFrom method

88cc399

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Gabriel-Trintinalia commented Feb 28, 2023

View reviewed changes

pinges approved these changes Feb 28, 2023

View reviewed changes

pinges reviewed Feb 28, 2023

View reviewed changes

...perledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java Outdated Show resolved Hide resolved

Gabriel-Trintinalia marked this pull request as ready for review February 28, 2023 08:10

Gabriel-Trintinalia changed the title ~~WIP - Fix issues with RLPExceptions~~ Fix issues with RLPExceptions Feb 28, 2023

Gabriel-Trintinalia added 3 commits March 1, 2023 09:14

Fix comment

2b9ce4b

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Merge branch 'main' into test-peering-log-rpc-exception-data

46c1dbe

Merge branch 'main' into test-peering-log-rpc-exception-data

bf503bf

macfarla approved these changes Mar 2, 2023

View reviewed changes

Gabriel-Trintinalia enabled auto-merge (squash) March 2, 2023 23:59

Gabriel-Trintinalia disabled auto-merge March 2, 2023 23:59

Merge branch 'main' into test-peering-log-rpc-exception-data

fd25eb4

Gabriel-Trintinalia enabled auto-merge (squash) March 3, 2023 00:11

Gabriel-Trintinalia merged commit 19eef23 into hyperledger:main Mar 3, 2023

duncancmt mentioned this pull request Mar 8, 2023

Archive sync intermittently freezes with "No sync target" #5056

Closed

Gabriel-Trintinalia deleted the test-peering-log-rpc-exception-data branch March 9, 2023 01:27

macfarla added the mainnet label May 3, 2023

macfarla added the peering label May 3, 2023

elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023

Fix issues with RLPExceptions (hyperledger#5140)

6485a5b

Signed-off-by: Gabriel-Trintinalia <[email protected]>

eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023

Fix issues with RLPExceptions (hyperledger#5140)

5e64dbf

Signed-off-by: Gabriel-Trintinalia <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix issues with RLPExceptions #5140

Fix issues with RLPExceptions #5140

Uh oh!

Gabriel-Trintinalia commented Feb 27, 2023 •

edited

Loading

Uh oh!

Gabriel-Trintinalia Feb 28, 2023 •

edited

Loading

Uh oh!

Gabriel-Trintinalia Feb 28, 2023

Uh oh!

Gabriel-Trintinalia Feb 28, 2023

Uh oh!

Gabriel-Trintinalia Feb 28, 2023

Uh oh!

pinges left a comment

Uh oh!

pinges Feb 28, 2023

Uh oh!

pinges Feb 28, 2023

Uh oh!

Uh oh!

macfarla left a comment

Uh oh!

macfarla Mar 2, 2023

Uh oh!

macfarla Mar 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix issues with RLPExceptions #5140

Fix issues with RLPExceptions #5140

Uh oh!

Conversation

Gabriel-Trintinalia commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description

1- BlockBody

2 -NewPooledTransactionHashesMessage

Documentation

Acceptance Tests (Non Mainnet)

Changelog

Uh oh!

Gabriel-Trintinalia Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gabriel-Trintinalia Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

Gabriel-Trintinalia Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

Gabriel-Trintinalia Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

pinges left a comment

Choose a reason for hiding this comment

Uh oh!

pinges Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

pinges Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

macfarla left a comment

Choose a reason for hiding this comment

Uh oh!

macfarla Mar 2, 2023

Choose a reason for hiding this comment

Uh oh!

macfarla Mar 2, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Gabriel-Trintinalia commented Feb 27, 2023 •

edited

Loading

Gabriel-Trintinalia Feb 28, 2023 •

edited

Loading