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

v0.2.0: Revamp data types #109

Merged
merged 1 commit into from
Dec 12, 2023
Merged

v0.2.0: Revamp data types #109

merged 1 commit into from
Dec 12, 2023

Conversation

jangko
Copy link
Contributor

@jangko jangko commented Dec 8, 2023

  • Clear distinction between engine_api_types and eth_api_types
  • Add convenience functions to handle V1, V2, and V3 of engine_api_types
  • Add more tests

This PR is the first step toward using nim-json-serialization in nim-json-rpc.
More PR will come.

@arnetheduck
Copy link
Member

Similar to how we have ssz tests, it would be good to have test cases with json examples of the objects that the serializer verifies can be read / written - we've had multiple cases of where the encoding inadvertedly was broken because of small changes to the codebase.

Ideally, @zah would also add support for strict mode for json-ser so that random serializers don't get included in the overload set for the ones that the standard encodings, as we've also had multiple cases of when nim-json-serialization uses the wrong encoding depending on what's imported or in which order.

@jangko
Copy link
Contributor Author

jangko commented Dec 8, 2023

I will add test from https://github.com/ethereum/execution-apis/tree/main/tests for eth_api, and also test cases for engine_api when we migrate to json-serialization.

@@ -0,0 +1,237 @@
# nim-web3
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 👍 on the naming on eth_api_types.nim and engine_api_types.nim

@jangko jangko force-pushed the revamp-types branch 5 times, most recently from 90904b5 to f7aa387 Compare December 12, 2023 02:54
@jangko jangko marked this pull request as ready for review December 12, 2023 08:19
@jangko jangko merged commit dcabb8f into master Dec 12, 2023
12 checks passed
@jangko jangko deleted the revamp-types branch December 12, 2023 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants