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: serialize slot as h256 66 byte string #1687

Closed

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Sep 11, 2022

Motivation

Ref foundry-rs/foundry#2687

hardhat recently enforced 66byte slot argument for eth_getStorageAt NomicFoundation/hardhat#2581 because:

According to the spec, the storage slot in eth_getStorageAt should have a length of 66 (0x + 32 bytes).

this is a bit confusing to me because this spec https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_getstorageat says it's a QUANTITY...

this is not very clear to me

resolved here NomicFoundation/hardhat#2581 (comment)

we should merge this to be on par with hardhat

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@gakonst
Copy link
Owner

gakonst commented Sep 11, 2022

Suggest we hold off until NomicFoundation/hardhat#2581 (comment) or ethereum/execution-apis#303 gets some reply? Not obvious to me what's right here.

@mattsse
Copy link
Collaborator Author

mattsse commented Sep 11, 2022

sgtm,

reasonable

@aathan
Copy link

aathan commented Sep 11, 2022

Yes, I would suggest holding off. I just added a comment to NomicFoundation/hardhat#2581 (comment) where I looked into the geth source and it does not enforce a size. I'm pretty sure all of this stems from an incorrect spec at NomicFoundation/hardhat#2230 ... which says {64} instead of {0,64}.

I posted a bug to the spec project also suggesting this should say {1,64} anyway.

@aathan
Copy link

aathan commented Sep 11, 2022

Perhaps the etheres-rs implementation SHOULD retry the request with 0 padding if the unpadded request fails. I believe older versions of hardhat contained an issue that would reject values with leading zeros where QUANTITY is required (as mentioned in NomicFoundation/hardhat#2230). Perhaps foundry could tag each RPC URL provider as either "leading zeros required" or "leading zeros rejected" and act accordingly... this would perhaps allow it to be compatible in all cases, given there seems to be ambiguity around this aspect of the spec.

EDIT: I somehow just realized this PR is on ethers-rs not foundry-rs. Sorry about any confusion from that.

@aathan
Copy link

aathan commented Sep 12, 2022

FYI, I tried running foundry's script features vs hardhat 2.9.4 (pre-dating their "fix") and, the deployment seemed to work, but I then got this error:

eth_feeHistory

  Remote node did not answer to eth_feeHistory correctly

eth_feeHistory

  Errors encountered in param 0: Invalid value 10 supplied to : QUANTITY

Not sure what about the parameter was considered invalid (formatting, or something about "10" ... I'm pretty certain the chain was longer than 10 blocks given my hardhat config is forking a public chain).

EDIT: Posted here because I'm assuming these calls are actually done by foundry via ethers-rs

@aathan
Copy link

aathan commented Sep 14, 2022

Just making sure you saw: NomicFoundation/hardhat#3177 which closes NomicFoundation/hardhat#3164 .

@mattsse
Copy link
Collaborator Author

mattsse commented Sep 24, 2022

this appears to be solved, thx @aathan

@mattsse mattsse closed this Sep 24, 2022
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.

None yet

3 participants