Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

fix: StorageProof key should be U256 #2699

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

blckngm
Copy link
Contributor

@blckngm blckngm commented Dec 16, 2023

Motivation

According to API sepcs, storage proof keys may not be full 32-byte long:

https://github.com/ethereum/execution-apis/blob/a0d03086564ab1838b462befbc083f873dcf0c0f/src/schemas/state.yaml#L50C29-L50C29

https://github.com/ethereum/execution-apis/blob/a0d03086564ab1838b462befbc083f873dcf0c0f/tests/eth_getProof/get-account-proof-with-storage.io#L2

Solution

Change the key to U256.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes
    This might be a breaking change. Fixing this without a breaking change will be cumbersome, by e.g. manually implementing Deserialize.

@mattsse
Copy link
Collaborator

mattsse commented Dec 16, 2023

@Rjected is this fixed in geth?

@blckngm
Copy link
Contributor Author

blckngm commented Dec 16, 2023

We should still be compatible with geth since U256 can deserialize keys with or without leading zeros.

Related issue:

ethereum/go-ethereum#27306

@Rjected
Copy link
Contributor

Rjected commented Dec 18, 2023

We should still be compatible with geth since U256 can deserialize keys with or without leading zeros.

yes, so deserialization should still work. serializing as a U256 should also work

@DaniPopes DaniPopes requested a review from mattsse February 12, 2024 21:12
@mattsse mattsse merged commit 40eaa41 into gakonst:master Feb 12, 2024
18 of 19 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants