eth66 #2365

RatanRSur · 2021-06-02T20:56:12Z

PR description

https://eips.ethereum.org/EIPS/eip-2481

Review suggestion: hide whitespace changes

Fixed Issue(s)

fixes #2210

Changelog

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

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

RatanRSur · 2021-08-11T15:47:53Z

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocol.java

+  private static final List<Integer> eth63Messages =
+      Stream.concat(
+              eth62Messages.stream(),
+              Stream.of(
+                  EthPV63.GET_NODE_DATA, EthPV63.NODE_DATA, EthPV63.GET_RECEIPTS, EthPV63.RECEIPTS))
+          .collect(toUnmodifiableList());

-  static {
-    eth63Messages.addAll(
-        Arrays.asList(
-            EthPV63.GET_NODE_DATA, EthPV63.NODE_DATA, EthPV63.GET_RECEIPTS, EthPV63.RECEIPTS));
-  }
+  private static final List<Integer> eth65Messages =
+      Stream.concat(
+              eth62Messages.stream(),
+              Stream.of(
+                  EthPV65.NEW_POOLED_TRANSACTION_HASHES,
+                  EthPV65.GET_POOLED_TRANSACTIONS,
+                  EthPV65.POOLED_TRANSACTIONS))
+          .collect(toUnmodifiableList());


RatanRSur · 2021-08-11T15:49:10Z

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java

+  private final Set<Hash> knownBlocks =
+      Collections.newSetFromMap(
+          Collections.synchronizedMap(
+              new LinkedHashMap<>(16, 0.75f, true) {
+                @Override
+                protected boolean removeEldestEntry(final Map.Entry<Hash, Boolean> eldest) {
+                  return size() > maxTrackedSeenBlocks;
+                }
+              }));


cosmetic, moved from constructor

RatanRSur · 2021-08-11T15:52:13Z

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthServerTest.java

-  @Test
-  public void shouldRespondToNodeDataRequests() throws Exception {
-    when(worldStateArchive.getNodeData(HASH1)).thenReturn(Optional.of(VALUE1));
-    when(worldStateArchive.getNodeData(HASH2)).thenReturn(Optional.of(VALUE2));
-    ethMessages.dispatch(new EthMessage(ethPeer, GetNodeDataMessage.create(asList(HASH1, HASH2))));
-
-    verify(ethPeer).send(NodeDataMessage.create(asList(VALUE1, VALUE2)));
-  }
-


ethMessages is no longer responsible for sending. Also, this behavior is already covered by EthProtocolManagerTest

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

garyschulte

Looks good, I have a few questions to clarify my understanding and some minor nitpicky stuff

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocol.java

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/RequestId.java

garyschulte · 2021-08-20T19:44:07Z

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/RequestId.java

+  }
+
+  static Map.Entry<BigInteger, MessageData> unwrapRequestId(final MessageData messageData) {
+    final RLPInput messageDataRLP = RLP.input(messageData.getData());


do we need to handle MalformedRLPInputException here or would it get appropriately handled further up the call stack?

good catch, will fix

did you determine we do not need to handle this here?

It's handled here now
https://github.com/RatanRSur/besu/blob/36d61b87c3a192d1b8bbd049230ba5a1ee00de08/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java#L292

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java

garyschulte · 2021-08-20T21:25:19Z

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/RequestManager.java

+          .ifPresentOrElse(
+              responseStream -> responseStream.processMessage(requestIdAndEthMessage.getValue()),
+              // disconnect on incorrect requestIds
+              () -> peer.disconnect(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL));


How does this happen in practice? We have a response, but no open stream to receive the response, so we disconnect the target peer? This seems likely to occur on our side rather than it being the fault of the remote ?

We should have all the request ids for the outstanding requests we sent so if we receive a request id we're not tracking, that means the remote is breaching protocol somehow.

isn't this our response to a request rather than a response to our request? Seems to me if we have formulated a response but can't find the request id, that is a problem on our end and we shouldn't disconnect from the peer. thoughts?

This is handling their response to our request.

This is how we generate responses to remote requests:
https://github.com/RatanRSur/besu/blob/36d61b87c3a192d1b8bbd049230ba5a1ee00de08/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java#L278-L305

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RequestManagerTest.java

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

garyschulte

🚢 🥳

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

https://eips.ethereum.org/EIPS/eip-2481 Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

RatanRSur force-pushed the eth66 branch 2 times, most recently from 33ca7f2 to b87e7b0 Compare June 28, 2021 16:13

RatanRSur added 28 commits July 1, 2021 11:26

refactor response constructor and sending

07f882b

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

nullable fix

db59e70

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

restore original name

a021144

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

remove early return

1725d62

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

add eth66 version code and cleanup

5db16eb

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

dry

97d46c6

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

move method

3d9ed89

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

add requestId tracking

3717fbd

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

make RequestManager test work

89b0cad

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

tests successfully fail

8b6cb8d

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

write rlp raw instead of as opaque bytes

dfd8557

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

use unwrapping from production code

bd09170

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

wip: json parsing stuff

a46dc83

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

todo

9af245e

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

change json parsers to factory methods

a815aa3

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

random json stuff

edd3c6b

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

get transaction parsing working

3babbc1

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

fix block header

712540e

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

some more tests

90cae83

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

Receipts

fd1813a

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

finish unit tests

f202eae

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

disconnect if they send us a bad requestId

7f4e117

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

cleanup

d5a4b9e

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

remove unused method

5cb5ad5

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

add test

e57300a

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

spotless

2eb03be

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

remove unused var

c8f018a

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

clean up constructor

22c9a42

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

RatanRSur added 2 commits August 11, 2021 11:42

use big integer

5c28e93

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

spotless

1eee08b

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

RatanRSur commented Aug 11, 2021

View reviewed changes

RatanRSur added 5 commits August 16, 2021 10:37

Merge branch 'main' of github.com:hyperledger/besu into eth66

98f9c31

robust to future protocols

332778a

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

streamline codespace

0b26535

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

make big integer scalar encoding consistent with long scalar

ce32e90

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

Merge branch 'main' of github.com:hyperledger/besu into eth66

abaffc4

RatanRSur marked this pull request as ready for review August 19, 2021 19:28

garyschulte reviewed Aug 20, 2021

View reviewed changes

RatanRSur added 9 commits August 23, 2021 11:43

Merge branch 'main' of github.com:hyperledger/besu into eth66

45cad8c

rename

54e86f8

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

handle malformed rlp exception

261da8f

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

spotless

c3e67db

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

ethPeer

faca857

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

minimize try/catch block scope

36d61b8

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

remove unused logger

9841b70

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

Merge branch 'main' of github.com:hyperledger/besu into eth66

c15c5b7

spotless

f181530

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

RatanRSur requested a review from garyschulte August 24, 2021 14:57

Merge branch 'main' into eth66

3922a9d

garyschulte approved these changes Aug 25, 2021

View reviewed changes

RatanRSur added 3 commits August 25, 2021 13:05

changelog

d0d6c73

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

Merge branch 'main' of github.com:hyperledger/besu into eth66

a013b8c

Merge branch 'eth66' of github.com:RatanRSur/besu into eth66

731ad55

RatanRSur enabled auto-merge (squash) August 25, 2021 17:06

RatanRSur merged commit f888983 into hyperledger:main Aug 25, 2021

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

eth66 (hyperledger#2365)

2fd59b3

https://eips.ethereum.org/EIPS/eip-2481 Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eth66 #2365

eth66 #2365

RatanRSur commented Jun 2, 2021 •

edited

Loading

RatanRSur Aug 11, 2021

RatanRSur Aug 11, 2021

RatanRSur Aug 11, 2021

garyschulte left a comment

garyschulte Aug 20, 2021

RatanRSur Aug 23, 2021

garyschulte Aug 24, 2021

RatanRSur Aug 24, 2021

garyschulte Aug 20, 2021

RatanRSur Aug 23, 2021

garyschulte Aug 24, 2021

RatanRSur Aug 24, 2021

garyschulte left a comment

eth66 #2365

eth66 #2365

Conversation

RatanRSur commented Jun 2, 2021 • edited Loading

PR description

Fixed Issue(s)

Changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garyschulte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garyschulte left a comment

Choose a reason for hiding this comment

RatanRSur commented Jun 2, 2021 •

edited

Loading