-
Notifications
You must be signed in to change notification settings - Fork 83
fix: decode the fork ID as a list #3492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the eth fork ID decoding to use a proper RLP list decoder and tidies up the RLP decoder struct.
- Swap single-byte-slice decode for
Decoder::new
+decode_optional_field
inNodeRecord
’s eth branch - Add blank-line spacing before
get_encoded_item
in RLP decoder for readability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
crates/networking/p2p/types.rs | Replaced manual slice decode of fork ID with RLP Decoder logic |
crates/common/rlp/structs.rs | Inserted spacing before get_encoded_item method for consistent layout |
Comments suppressed due to low confidence (2)
crates/networking/p2p/types.rs:270
- [nitpick] Add a brief comment explaining that this decodes the outer RLP list and extracts an optional fork ID field, replacing the previous single-byte assumption.
let Ok(decoder) = Decoder::new(&value) else {
crates/common/rlp/structs.rs:69
- Add unit tests for
get_encoded_item
to verify it returns the correct byte slice and handles error cases as expected.
/// Returns the next field without decoding it, i.e. the payload bytes including its prefix.
decoder.finish_unchecked(); | ||
decoded_pairs.eth = fork_id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using decoder.finish()
(which returns a Result
) instead of finish_unchecked()
to catch any leftover bytes or malformed input.
decoder.finish_unchecked(); | |
decoded_pairs.eth = fork_id; | |
match decoder.finish() { | |
Ok(_) => decoded_pairs.eth = fork_id, | |
Err(_) => continue, | |
} |
Copilot uses AI. Check for mistakes.
Lines of code reportTotal lines added: Detailed view
|
This PR decodes the fork ID of a node record's "eth" entry as a list. Previously, we were assuming the list was small enough to have a length of a single byte.