Skip to content

storage proof key - use 0x0 for zero #8499

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

Merged
merged 13 commits into from
Apr 4, 2025
Merged

Conversation

macfarla
Copy link
Contributor

@macfarla macfarla commented Apr 1, 2025

PR description

return 0x0 for zero key rather than 0x

Added a test for visibility

Fixed Issue(s)

related to #5377
related to rpc-compat hive tests

this fixes 1 hive test:
eth_getProof/get-account-proof-with-storage

Refs this comment 5cb6c26#r2017642440
This was discussed on RPC standards call and since odd-length hex strings are widely used we will stick with that. Quantity.create handles the zero edge case already.

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

macfarla added 2 commits April 1, 2025 10:25
Signed-off-by: Sally MacFarlane <[email protected]>
@macfarla macfarla added RPC hive relating to hive tests labels Apr 1, 2025
@macfarla
Copy link
Contributor Author

macfarla commented Apr 1, 2025

Note I had to update several eth_getProof json spec tests to make the key consistent with this PR (remove leading zero from non-zero key). Using Quantity.create makes the format consistent with other places in the output - same as for nonce, the eth_getProof value and other places too.

I don't think this would be a breaking change - and we'll be now consistent with geth - but flagging for reviewers.

Signed-off-by: Sally MacFarlane <[email protected]>
Copy link
Member

@lu-pinto lu-pinto left a comment

Choose a reason for hiding this comment

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

LGTM

macfarla added 5 commits April 4, 2025 12:20
Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: Sally MacFarlane <[email protected]>
@macfarla
Copy link
Contributor Author

macfarla commented Apr 4, 2025

@lu-pinto based on your feedback and further chat w Gary, adopted your suggestion to keep the leading zeros where we had them before.

@macfarla macfarla changed the title use Quantity.create for storage proof key to avoid 0x for zero storage proof key - use 0x0 for zero Apr 4, 2025
@macfarla macfarla merged commit 2afb161 into hyperledger:main Apr 4, 2025
43 checks passed
@macfarla macfarla deleted the key-zero branch April 8, 2025 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hive relating to hive tests RPC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants