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 a bug where some addresses would be unverified due to padding #64

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

rdvorkin
Copy link
Contributor

@rdvorkin rdvorkin commented Aug 10, 2023

balanceSlot is padded to '0x' + 64 chars with leading zeros, however contractProof.storageProof[0]?.key isn't. This means that for some addresses (and for some tokens, if more are added) the balance slot will have leading zeros and proof key will not, failing the equality check.

Example:

const balanceSlot = web3.utils.keccak256(defaultAbiCoder.encode(["address", "uint"], ["0xA4b06Da12C746666D08e1163CD18Ab57bA544291", 2]));
const contractProof = await web3.eth.getProof(
        "0x6b175474e89094c44da98b954eedeac495271d0f",
        [balanceSlot],
        17884555);
key=0x9dd74d0da16d86e947a553ada92b0d3f6cee1cfb3c43e4a74617566fbd4c5be
balanceSlot=0x09dd74d0da16d86e947a553ada92b0d3f6cee1cfb3c43e4a74617566fbd4c5be

Or just input 0xA4b06Da12C746666D08e1163CD18Ab57bA544291 at the demo https://lodestar-lightclient.chainsafe.io

balanceSlot is padded to '0x' + 64 chars with leading zeros, however contractProof.storageProof[0]?.key isn't. 
This means that for some addresses (and for some tokens, if more are added) the balance slot will have leading zeros and proof key will not, failing the equality check.

Example:
const balanceSlot = web3.utils.keccak256(defaultAbiCoder.encode(["address", "uint"], ["0xA4b06Da12C746666D08e1163CD18Ab57bA544291", 2]));
const contractProof = await web3.eth.getProof(
        "0x6b175474e89094c44da98b954eedeac495271d0f",
        [balanceSlot],
        17884555);

key=0x9dd74d0da16d86e947a553ada92b0d3f6cee1cfb3c43e4a74617566fbd4c5be
balanceSlot=0x09dd74d0da16d86e947a553ada92b0d3f6cee1cfb3c43e4a74617566fbd4c5be

Or just input 0xA4b06Da12C746666D08e1163CD18Ab57bA544291 at the demo https://lodestar-lightclient.chainsafe.io
@rdvorkin rdvorkin requested a review from a team as a code owner August 10, 2023 13:17
@CLAassistant
Copy link

CLAassistant commented Aug 10, 2023

CLA assistant check
All committers have signed the CLA.

@jeluard
Copy link
Contributor

jeluard commented Mar 4, 2024

@rdvorkin Your change makes sense, now I still have an error with your test case even after applying it:

Error: Invalid proof provided: account is not empty (storageHash does not equal KECCAK256_RLP)
    at DefaultStateManager.verifyProof (http://localhost:3000/static/js/bundle.js:18994:15)
    at async fetchAndVerifyAddressBalances (http://localhost:3000/static/js/bundle.js:1052:151)
    at async fetchAndVerifyAccount (http://localhost:3000/static/js/bundle.js:509:31)

Does that ring a bell?

@jeluard jeluard merged commit 80057b7 into ChainSafe:master Mar 8, 2024
2 checks passed
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.

4 participants