Skip to content
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 Uint256 deserialization #2786

Merged
merged 8 commits into from
Nov 10, 2021
Merged

Fix Uint256 deserialization #2786

merged 8 commits into from
Nov 10, 2021

Conversation

pawanjay176
Copy link
Member

Issue Addressed

N/A

Proposed Changes

Builds upon @realbigsean 's merge spec test fixes from #2781. Add a quoted_u256 module in eth2_serde_utils to correctly decode Uint256 values from string.

Additional Info

Had to add the ethereum-types dependency to serde_utils and use the local crate instead of the published version.
We could also move the quoted_u256 module to types instead if we want to use the published version.

@paulhauner paulhauner mentioned this pull request Nov 5, 2021
@paulhauner paulhauner added kintsugi 🍵 work-in-progress PR is a work-in-progress labels Nov 5, 2021
@paulhauner
Copy link
Member

FYI clippy is unhappy and I've made some suggestions to Sean's PR which you might cause conflicts (I don't think they will).

I'll give this a review once Sean's PR goes into Kintsugi and the diff is clean 🚀

@pawanjay176 pawanjay176 added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Nov 10, 2021
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice and clean!

I wrote a quick test to double-check my understand over at paulhauner@032ffd2. It might be nice to cherry-pick that on this branch. Up to you.

Feel free to squash-merge into kintsugi when you're happy :)

@paulhauner paulhauner merged commit f817784 into sigp:kintsugi Nov 10, 2021
paulhauner added a commit that referenced this pull request Nov 11, 2021
* Change base_fee_per_gas to Uint256

* Add custom (de)serialization to ExecutionPayload

* Fix errors

* Add a quoted_u256 module

* Remove unused function

* lint

* Add test

* Remove extra line

Co-authored-by: Paul Hauner <[email protected]>
paulhauner added a commit that referenced this pull request Nov 28, 2021
* Change base_fee_per_gas to Uint256

* Add custom (de)serialization to ExecutionPayload

* Fix errors

* Add a quoted_u256 module

* Remove unused function

* lint

* Add test

* Remove extra line

Co-authored-by: Paul Hauner <[email protected]>
paulhauner added a commit that referenced this pull request Nov 28, 2021
* Change base_fee_per_gas to Uint256

* Add custom (de)serialization to ExecutionPayload

* Fix errors

* Add a quoted_u256 module

* Remove unused function

* lint

* Add test

* Remove extra line

Co-authored-by: Paul Hauner <[email protected]>
paulhauner added a commit that referenced this pull request Dec 2, 2021
* Change base_fee_per_gas to Uint256

* Add custom (de)serialization to ExecutionPayload

* Fix errors

* Add a quoted_u256 module

* Remove unused function

* lint

* Add test

* Remove extra line

Co-authored-by: Paul Hauner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kintsugi 🍵 ready-for-review The code is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants