Fix rlp validate to handle lists ending at the start of another list #8143
Conversation
and handle empty lists in AbstractRLPInput Signed-off-by: Matilda Clerke <[email protected]>
Signed-off-by: Matilda Clerke <[email protected]>
Signed-off-by: Matilda Clerke <[email protected]>
| @@ -168,6 +168,14 @@ public void calculateSize_overflowMaxRLPStringLength() { | |||
| .hasMessageContaining("RLP item exceeds max supported size of 2147483647: 2147483648"); | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
Is this testing the case where you have multiple lists end? If not, could you please add another test that does test that!
There was a problem hiding this comment.
Yes, it is. This is testing the exact transaction that I have been investigating
| @@ -168,6 +168,14 @@ public void calculateSize_overflowMaxRLPStringLength() { | |||
| .hasMessageContaining("RLP item exceeds max supported size of 2147483647: 2147483648"); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testValidateWithEmptyListIncluded() { | |||
There was a problem hiding this comment.
nit: Sounds like the issue is more specific than just an empty list? Would be nice to make the test name and RLP example precisely highlight the issue.
There was a problem hiding this comment.
Yep, I wrote the test back when I still thought the empty list was the problem
Signed-off-by: Matilda Clerke <[email protected]>
Signed-off-by: Matilda Clerke <[email protected]>
| // ["0x02"] | ||
| // ] | ||
| Bytes validRlp = Bytes.fromHexString("c4c101c102"); | ||
| RLP.validate(validRlp); |
There was a problem hiding this comment.
| RLP.validate(validRlp); | |
| assertThatCode(() -> RLP.validate(bytes)).doesNotThrowAnyException(); |
|
This looks a pretty critical part of the code, that is running fine for years. Would you mind testing a sync from scratch to see if your change doesn't have any regression ? |
|
Fair point @ahamlat I'll be running full syncs for Matilda-Clerke:7582-avoid-unnecessary-rlp-encoding-during-sync with this change included |
PR description
RLP.validate attempts to traverse the supplied RLP, but when the end of a list is at the start of a new list it fails to correctly leave the previous list. In particular, this was noticed to occur when validating RLP representing an access list transaction in which both the access list entry and its list of storage keys ended on the start of the next access list entry.