-
Notifications
You must be signed in to change notification settings - Fork 521
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 RLP marshal byte array and enhancements to stake manager #1461
Conversation
107d819
to
73ff8c5
Compare
1d8676b
to
22a5539
Compare
9195cc3
to
3179304
Compare
c76923c
to
931771e
Compare
931771e
to
cf1cd8f
Compare
07ce3ac
to
b162e55
Compare
8b48874
to
a46117b
Compare
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.
I left some comments, I think should be addressed.
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 💯
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.
There are a few other places that uses arena.NewBytes
in the Edge codebase. Should we change all of these to use NewCopyBytes
instead?
e525f9c
to
758c396
Compare
c780fad
to
e7bead7
Compare
Description
A bug is discovered in the
fastrlp
library in theNewBytes
method. We should useNewCopyBytes
wherever we have a non-transient slice/array, sinceNewBytes
was not resetting memory properly. In a meanwhile,fastrlp
library was fixed by wrappingNewCopyBytes
function within theNewBytes
(umbracle/fastrlp#14), meaning that these two functions are interchangeable, so this PR updates the version offastrlp
library as well.The stake manager is reverted to the behavior before the workaround for the bug in
fastrlp
is applied.Changes include
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it
Checklist
Testing