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_getStorageAt uint256 handling and QUANTITY handling in general #3164

Closed
aathan opened this issue Sep 11, 2022 · 10 comments · Fixed by #3177
Closed

eth_getStorageAt uint256 handling and QUANTITY handling in general #3164

aathan opened this issue Sep 11, 2022 · 10 comments · Fixed by #3177
Assignees

Comments

@aathan
Copy link

aathan commented Sep 11, 2022

The merge of #2581 is causing problems when using the Foundry project's scripting features (see for example: gakonst/ethers-rs#1687). Meanwhile, the workaround of using an earlier hardhat version (e.g. 2.9.4) also doesn't work due to possibly related problems:

eth_feeHistory

  Remote node did not answer to eth_feeHistory correctly

eth_feeHistory

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

I say "possibly related" because the discussion in #2581 and #2230 seem to reference how hardhat handles quantity values, and whether leading zeros etc should be allowed. The root cause of the gakonst/ethers-rs#1687 incompatibility appears to be an incorrect spec referenced in #2230 (closed) that indicated a 64 digit hext string was required. I've provided details in #2581 as to why hardhat should probably accept variable length hex strings, as supported by the current open rpc spec (which says {0,64} ), and the go-ethereum implementation (which does not force a length either).

I'm opening this new issue because the discussion in #2581 is on an already merged PR and #2230 is already closed. I've also reported to the ethereum open rpc spec project that {0,64} should probably be {1,64} in their spec. Based on the above further error with eth_feeHistory called from foundry's scripting feature, I'm concerned that QUANTITY handling in hardhat or foundry and/or in the specs may also be an issue.

@github-actions
Copy link
Contributor

This issue is also being tracked on Linear.

We use Linear to manage our development process, but we keep the conversations on Github.

LINEAR-ID: bff9fb21-1558-43d8-913a-8d8892e47dee

@aathan
Copy link
Author

aathan commented Sep 12, 2022

If the hardhat project feels it's important to maintain strict format checks, I might suggest adding a flag or configuration parameter that changes how strictly hardhat enforces parameter formatting issues. Whenever there is a mismatch, users end up in a potentially difficult scenario that involves finding some set of versions of all the tools they want to use, that work properly together, and have the same opinion about those formats. Unless there is an "industry wide" accepted scenario for this, you end up unable to use a tool, even though it's emitting fundamentally correct requests.

@alcuadrado
Copy link
Member

As commented here, you are right. Thanks for investigating this and pointing us in the right direction. We'll fix it asap.

If the hardhat project feels it's important to maintain strict format checks, I might suggest adding a flag or configuration parameter that changes how strictly hardhat enforces parameter formatting issues. Whenever there is a mismatch, users end up in a potentially difficult scenario that involves finding some set of versions of all the tools they want to use, that work properly together, and have the same opinion about those formats. Unless there is an "industry wide" accepted scenario for this, you end up unable to use a tool, even though it's emitting fundamentally correct requests.

Also, I like this idea!

@alcuadrado
Copy link
Member

Now, about the fix: should we do ^(0x)?[0-9a-f]{0,64}$? It's more restrictive than Geth, but get's behavior is somewhat crazy in this case.

I'll also investigate what's going on with those eth_feeHistory calls. As far as I remember that's also a weird method.

@aathan
Copy link
Author

aathan commented Sep 12, 2022

Thanks. I'm currently blocked by the combination of behaviors between foundry and hardhat, in both the latest version of hardhat, and older versions of hardhat (e.g., hardhat 2.9.4). Is there a relatively straight forward way to disable the formatting checks in hardhat? I'd like to patch my copy so I can make progress. I'm guessing it's just reversing 13b5cd2 but if you have any other hints, I'm all ears.

@fvictorio
Copy link
Member

I opened #3177 to fix this. The only difference with what was discussed here is that empty strings are rejected (but 0x is valid). Sending an empty string seems to me to be an unintentional mistake, so I think is better to reject the argument in that case.

@aathan we are aiming for releasing this today, I'll let you know here when the release is out.

@aathan
Copy link
Author

aathan commented Sep 14, 2022

Ok. Please be aware of ethereum/execution-apis#303 and associated PR ethereum/execution-apis#305

In particular:

RPC endpoints SHOULD accept hex encoded byte values of length < 66 bytes. I apologize for the error in the specification.

@alcuadrado
Copy link
Member

Thanks @aathan :)

I'll follow those and we may update Hardhat once again after it gets resolved, but we'll release this smaller change now to unblock you, as you were very helpful.

@fvictorio
Copy link
Member

Released in hardhat@2.11.2.

@aathan
Copy link
Author

aathan commented Sep 15, 2022

Thanks everyone!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2022
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 a pull request may close this issue.

3 participants