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

eth_getProof does not support short storage keys #2046

Closed
1 task done
Tracked by #1579
Rjected opened this issue Mar 30, 2023 · 4 comments · Fixed by #2067
Closed
1 task done
Tracked by #1579

eth_getProof does not support short storage keys #2046

Rjected opened this issue Mar 30, 2023 · 4 comments · Fixed by #2067
Labels
A-rpc Related to the RPC implementation C-bug An unexpected or incorrect behavior

Comments

@Rjected
Copy link
Member

Rjected commented Mar 30, 2023

Describe the bug

The rpc-compat test eth_getProof/get-account-proof-with-storage fails with the following error:

>>  {"jsonrpc":"2.0","id":1,"method":"eth_getProof","params":["0xaa00000000000000000000000000000000000000",["0x01"],"0x3"]}
<<  {"jsonrpc":"2.0","error":{"code":-32602,"message":"invalid length 2, expected a (both 0x-prefixed or not) hex string with length of 64 at line 1 column 7"},"id":1}
response differs from expected:
{
-  "error": {
-    "code": -32602,
-    "message": "invalid length 2, expected a (both 0x-prefixed or not) hex string with length of 64 at line 1 column 7"
-  },
   "id": 1,
   "jsonrpc": "2.0"
+  "result": {
+    "accountProof": [
+      : "0xf8718080808080a0a2bd2175aed7ed88ed854c914fab94115c092ffb3c3c2ef647b70b7e73e3345880a0457ae8d978cd387f5332f978f5653226588b6cc76a355fc5977cd4325ffcff78a0c4bdbdbb240f8343b7f84bc83d4b7426e803a914138792d1e369907be8098b2d8080808080808080",
+      : "0xf869a0335649db80be637d281db0cc5896b0ff9869d08379a80fdc38dd073bba633949b846f8440101a08afc95b7d18a226944b9c2070b6bda1c3a36afcc3730429d47579c94b9fe5850a0ce92c756baff35fa740c3557c1a971fd24d2d35b7c8e067880d50cd86bb0bc99"
+    ],
+    "address": "0xaa00000000000000000000000000000000000000",
+    "balance": "0x1",
+    "codeHash": "0xce92c756baff35fa740c3557c1a971fd24d2d35b7c8e067880d50cd86bb0bc99",
+    "nonce": "0x1",
+    "storageHash": "0x8afc95b7d18a226944b9c2070b6bda1c3a36afcc3730429d47579c94b9fe5850",
+    "storageProof": [
+      : {
+        "key": "0x01",
+        "proof": [
+          : "0xf871808080a0ce028e108cf5c832b0a9afd3ed101183857b3b9ddaea5670c4f09b62e4d38d05a0de3ecad66628a5743ed089e3a35ebeedc25a922fb0ac346304613403911c18e0a0128c5f7abac505794cda09bde44d143e4736b50b1c42d6807a989c10af51e8d18080808080808080808080"
+        ],
+        "value": "0x0"
+      }
+    ]
+  }
}

This is because our keys are H256:

/// Returns the account and storage values of the specified account including the Merkle-proof.
/// This call can be used to verify that the data you are pulling from is not tampered with.
#[method(name = "eth_getProof")]
async fn get_proof(
&self,
address: Address,
keys: Vec<H256>,
block_number: Option<BlockId>,
) -> Result<EIP1186AccountProofResponse>;

The input storage key is 0x01, which I guess should represent the following entry from the test's genesis.json:
https://github.com/ethereum/execution-apis/blob/3e381a7b32c105f1aa62086b9bdcab57d8e3427c/tests/genesis.json#L38

Maybe the storage keys should be U256, if its deserialization trims trailing zeros. We would then need to convert back into a H256 for the state provider proof method:

/// Get account and storage proofs.
fn proof(&self, address: Address, keys: &[H256])
-> Result<(Vec<Bytes>, H256, Vec<Vec<Bytes>>)>;

We would also need to make sure the key shows up as "key": "0x01" in the response.

Steps to reproduce

Run hive:

./hive --client reth --sim ethereum/rpc-compat --sim.limit "/eth_getProof"

Node logs

No response

Platform(s)

No response

What version/commit are you on?

No response

Code of Conduct

  • I agree to follow the Code of Conduct
@Rjected Rjected added C-bug An unexpected or incorrect behavior A-rpc Related to the RPC implementation labels Mar 30, 2023
@mattsse
Copy link
Collaborator

mattsse commented Mar 31, 2023

Maybe the storage keys should be U256, if its deserialization trims trailing zeros.

yeh seems like the consensus is that they are quantity however the spec does not mention that:
https://ethereum.github.io/execution-apis/api-documentation/

I remember that this was incredibly annoying in anvil as well, and we eventually settled on H256, but can't remember the details.

how does geth deserialize the keys?

@mattsse
Copy link
Collaborator

mattsse commented Mar 31, 2023

I see, we need a compatible deserialize function for this then

@Rjected
Copy link
Member Author

Rjected commented Mar 31, 2023

Hmmm, given this:

+        "key": "0x01",

The "key" value in the response is not quantity, because it has a leading zero

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-bug An unexpected or incorrect behavior
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants