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

BaseProvider getStorageAt position is not spec-compliant #2982

Open
tkporter opened this issue May 13, 2022 · 14 comments
Open

BaseProvider getStorageAt position is not spec-compliant #2982

tkporter opened this issue May 13, 2022 · 14 comments
Assignees
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@tkporter
Copy link

tkporter commented May 13, 2022

Ethers Version

@ethersproject/providers 5.6.5

Search Terms

BaseProvider,getStorageAt

Describe the Problem

I came across this while trying to use provider.getStorageAt(someAddress, 0); against a fresh install of Hardhat. The fresh install evidently used Hardhat v2.9.5 that includes a change to make Hardhat network's eth_getStorageAt spec compliant (Hardhat PR here, references spec found here). Apparently the spec requires the storage position passed into eth_getStorageAt to be a 32 byte hex string, which BaseProvider doesn't do.

Calling getStorageAt would give this Hardhat error: InvalidArgumentsError: Errors encountered in param 1: Storage slot argument must have a length of 66 ("0x" + 32 bytes), but '0x0' has a length of 3. Eventually I tracked it down to this line: https://github.com/ethers-io/ethers.js/blob/master/packages/providers/src.ts/base-provider.ts#L1482

I was able to fix it locally by changing the hexValue(p) to hexZeroPad(p, 32). I'm happy to open a PR but I'm not sure if that's the best place to make the change / if it totally preserves backward compatibility for other use cases.

For any passerby, downgrading Hardhat version to <= v2.9.4 seems to be a sufficient workaround for now

Code Snippet

provider.getStorageAt(someAddress, 0);
// or even
provider.getStorageAt(someAddress, '0x0000000000000000000000000000000000000000000000000000000000000000');

Contract ABI

No response

Errors

No response

Environment

node.js (v12 or newer), Hardhat

Environment (Other)

No response

@tkporter tkporter added the investigate Under investigation and may be a bug. label May 13, 2022
@ricmoo
Copy link
Member

ricmoo commented May 13, 2022

The change should probably go into JsonRpcProvider for going over the JSON-RPC wire protocol, since I don't want to change the behaviour of the the class BaseProvider which has other sub-classes.

Thanks! I'll fix this today. :)

(Note: in v6, the formatter has a storageSlot method, which is used to safely transform this value based on the network)

@ricmoo ricmoo added bug Verified to be an issue. on-deck This Enhancement or Bug is currently being worked on. and removed investigate Under investigation and may be a bug. labels May 13, 2022
@tkporter
Copy link
Author

Thanks so much! Makes sense to me :)

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels May 21, 2022
@ricmoo
Copy link
Member

ricmoo commented May 21, 2022

This should be fixed in 5.6.7. Try it out and let me know if you still have any problems.

Thanks! :)

@alcuadrado
Copy link

Sorry for the inconvenience guys. Fixing this was a tough call, as we accidentally deviated from the "standard" previously.

@ricmoo
Copy link
Member

ricmoo commented May 26, 2022

No worries; I'm a huge fan of things being standards compliant. :)

I've already made changes in v6 that are aligned with this too.

@ricmoo ricmoo closed this as completed May 26, 2022
@3commascapital
Copy link

just tried this and am seeing the same error

@ricmoo
Copy link
Member

ricmoo commented May 26, 2022

Did you update to the latest version? 5.6.8?

@3commascapital
Copy link

i did! went there first actually. trying 5.6.7 now

@ricmoo
Copy link
Member

ricmoo commented May 26, 2022

5.6.7 should work too. I’ll re-open and look at it tomorrow.

@ricmoo ricmoo reopened this May 26, 2022
@ricmoo ricmoo added the on-deck This Enhancement or Bug is currently being worked on. label May 26, 2022
@3commascapital
Copy link

i am seeing this most readily when i do a uniswap v3 swap then try to trace the transaction using hardhat-tracer if that is of any help

@blgaman
Copy link

blgaman commented Jun 10, 2022

both 5.6.7 and 5.6.8 are not working now.

> await ethers.provider.getStorageAt("0xaE80289290853f78219f2AfD3C69e0A3105fF96C", "0x0000000000000000000000000000000000000000000000000000000000000000");
Uncaught:
ProviderError: Errors encountered in param 1: Storage slot argument must have a length of 66 ("0x" + 32 bytes), but '0x0' has a length of 3
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at GanacheGasMultiplierProvider.request (/Users/sgnoo/Workspace/xociety-monorepo/packages/contracts/node_modules/hardhat/src/internal/core/providers/gas-providers.ts:312:34)
    at HttpProvider.request (/Users/sgnoo/Workspace/xociety-monorepo/packages/contracts/node_modules/hardhat/src/internal/core/providers/http.ts:78:19)

@zemse
Copy link
Collaborator

zemse commented Jun 10, 2022

Can you once try removing lock files and reinstalling on your end if it solves the problem?

@ricmoo
Copy link
Member

ricmoo commented Jun 17, 2022

@3commascapital @0xdagarn Have you tried @zemse suggestion of removing any lock files and re-installing? Can you make sure the version of the @ethersproject/providers package in the node_modules/@ethersproject/providers/package.json is 5.6.8?

I've just tried to reproduce this against the latest version and it still works for me.

@Skyge
Copy link

Skyge commented Mar 2, 2023

Same error when I use 6.0.8, but if I use 5.6.8, it works well.

Woodpile37 pushed a commit to Woodpile37/ethers.js that referenced this issue Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

7 participants