feat: delayed p2p message decoding via rlp.RawList#179
Open
Conversation
* fix: finalize listIterator on parse error to prevent non-advancing loops (ethereum#33245)
The list iterator previously returned true on parse errors without
advancing the input, which could lead to non-advancing infinite loops
for callers that do not check Err() inside the loop; to make iteration
safe while preserving error visibility, Next() now marks the iterator as
finished when readKind fails, returning true for the error step so
existing users that check Err() can handle it, and then false on
subsequent calls, and the function comment was updated to document this
behavior and the need to check Err().
* feat: add RawList for working with un-decoded lists (ethereum#33755)
This adds a new type wrapper that decodes as a list, but does not
actually decode the contents of the list. The type parameter exists as a
marker, and enables decoding the elements lazily. RawList can also be
used for building a list incrementally.
* fix: return Iterator as non-pointer (ethereum#33818)
Most uses of the iterator are like this:
it, _ := rlp.NewListIterator(data)
for it.Next() {
do(it.Value())
}
This doesn't require the iterator to be a pointer and it's better to
have it stack-allocated. AFAIK the compiler cannot prove it is OK to
stack-allocate when it is returned as a pointer because the methods of
`Iterator` use pointer receiver and also mutate the object.
The iterator type was not exported until very recently, so I think it is
still OK to change this API.
* feat: add AppendRaw method to RawList (ethereum#33834)
This is helpful when building a list from already-encoded elements.
* feat: validate and cache element count in RawList (ethereum#33840)
This changes `RawList` to ensure the count of items is always valid.
Lists with invalid structure, i.e. ones where an element exceeds the
size of the container, are now detected during decoding of the `RawList`
and thus cannot exist.
Also remove `RawList.Empty` since it is now fully redundant, and
`Iterator.Count` since it returns incorrect results in the presence of
invalid input. There are no callers of these methods (yet).
* fix: add back Iterator.Count, with fixes (ethereum#33841)
I removed `Iterator.Count` in ethereum#33840, because it appeared to be unused
and did not provide the documented invariant: the returned count should
always be an upper bound on the number of iterations allowed by `Next`.
In order to make `Count` work, the semantics of `CountValues` has to
change to return the number of items up and including the invalid one. I
have reviewed all callsites of `CountValues` to assess if changing this
is safe. There aren't that many, and the only call that doesn't check
the error and return is in the trie node parser,
`trie.decodeNodeUnsafe`. There, we distinguish the node type based on
the number of items, and it previously returned an error for item count
zero. In order to avoid any potential issue that could result from this
change, I'm adding an error check in that function, though it isn't
necessary.
* feat: delayed p2p message decoding (ethereum#33835)
This changes the p2p protocol handlers to delay message decoding. For
responses, we delay the decoding until it is confirmed that the response
matches an active request and does not exceed its limits.
In order to make this work, all messages have been changed to use
rlp.RawList instead of a slice of the decoded item type. For block
bodies specifically, the decoding has been delayed all the way until
after verification of the response hash.
The role of p2p/tracker.Tracker changes significantly in this PR. The
Tracker's original purpose was to maintain metrics about requests and
responses in the peer-to-peer protocols. Each protocol maintained a
single global Tracker instance. As of this change, the Tracker is now
always active (regardless of metrics collection), and there is a
separate instance of it for each peer. Whenever a response arrives, it
is first verified that a request exists for it in the tracker. The
tracker is also the place where limits are kept.
Adapted for go-wbft: ETH68 only, no ETH69 support.
* fix: add missing error check
* fix: incorrect comment
* fix: crash in clean when tracker is stopped (ethereum#33940)
---------
Co-Authored-By: radik878 <radikpadik76@gmail.com>
Co-Authored-By: Felix Lange <fjl@twurst.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces delayed RLP decoding for P2P Ethereum protocol messages, porting upstream go-ethereum #33835 to go-wbft. Instead of decoding incoming wire messages immediately on receipt, message payloads are stored as
rlp.RawList[T]and decoded only when actually consumed. This reduces unnecessary CPU work and memory allocations when received data is ultimately discarded (e.g., due to hash mismatch or request cancellation).Upstream References
Changes
rlp/— RawListIntroduces
rlp.RawList[T], a generic list type that stores raw RLP bytes and decodes elements on demand via.Items(). Wire encoding is identical to[]T, so no protocol compatibility changes are needed. Several iterator improvements accompany this type (Count(), non-pointer return, parse-error finalization).p2p/tracker/— Per-peer request trackerRewrites
tracker.Trackerfrom a global singleton to a per-peer instance. The sharedtracker.goshims ineth/andsnap/are removed.tracker.Fulfil(id)is added to signal request completion.eth/protocols/eth/— Core delayed decodingTransactionsPacket,BlockHeadersPacket,BlockBodiesPacket,ReceiptsPacket,BlockBody) converted to userlp.RawListfields.hashBodyParts()+derivableRawList: compute MPT hashes directly from raw bytes without full deserialization.maxTransactionAnnouncements = 5000: DoS defense against oversized transaction messages.markTransactionexported asMarkTransaction; transaction marking moved to the application layer (eth/handler_eth.go).eth/protocols/snap/andeth/upper layerssnap.Peermigrated to per-peer tracker (same pattern as eth).downloader,fetcher, andhandler_eth.goupdated to work with the new packet type fields.Security
maxTransactionAnnouncements = 5000prevents memory exhaustion from oversized transaction lists.hashBodyParts()validates block body hashes before full decode.