-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: check payload length and consumed buf for pooled tx #5153
Conversation
Codecov Report
... and 220 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
nice!
only naming nits
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.
rlp 👍
there's another thing we should do (separately):
currently, we're always consuming the &mut &
however ideally we only advance this when decoding is successful.
see Decode
derive macro template for example
good to merge @Rjected ? |
needs rebase @Rjected |
c9227f9
to
b041ef1
Compare
ruint PR should be merged first
b041ef1
to
5bffcc6
Compare
This is one of the bugs found by #5125, we were previously allowing decoding of legacy transactions that were shorter than their payload length, as well as legacy transactions that were longer than their payload length. We will need to similarly check header lengths in other manual decodings.
Tests are added that contain RLP strings generated by the fuzzer.