Skip to content

fix page zktrie: change the domain of nodeKey hashing to 512 instead of 256 #282

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

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

jimjimvalkema
Copy link
Contributor

Closing issues

closes #281

Description

The docs states the domain used in creating the nodeKey is 256 but its actually 512.

In the zktrie repo nodekeys are hashed with ToSecureKey() in type/util.go.
ToSecureKey() uses Hash() in types/bytes32.go which uses HASH_DOMAIN_BYTE32 as domain which is 256 *2 (=512).

ToSecureKey: https://github.com/scroll-tech/zktrie/blob/23181f209e94137f74337b150179aeb80c72e7c8/types/util.go#L107
HASH_DOMAIN_BYTES32: https://github.com/scroll-tech/zktrie/blob/23181f209e94137f74337b150179aeb80c72e7c8/types/hash.go#L16

Am also making a storage prover and here is where i did my test in noir :D (account leaf is lower in same file).
https://github.com/jimjimvalkema/scrollZkStorageProofs/blob/bcdd922405e046e99787b8b47f374befac170779/ScrollStorageProver/src/main.nr#L168

...

Changes

Changed domain value for the nodeKey mentioned in account leaf and storage leaf section.
Changed the domain from 256 to 512

Am currently making a storage prover in noir and found this error

In the zktrie repo nodekeys are hashed with ToSecureKey() in type/util.go (afaik)
https://github.com/scroll-tech/zktrie/blob/23181f209e94137f74337b150179aeb80c72e7c8/types/util.go#L107

ToSecureKey() uses Hash()  in types/bytes32.go which uses HASH_DOMAIN_BYTE32 as domain which is 256 *2 (512)
see here: https://github.com/scroll-tech/zktrie/blob/23181f209e94137f74337b150179aeb80c72e7c8/types/hash.go#L16

and here is where i did my test in noir :D (account leaf is lower in same file)
https://github.com/jimjimvalkema/scrollZkStorageProofs/blob/bcdd922405e046e99787b8b47f374befac170779/ScrollStorageProver/src/main.nr#L168
Copy link

netlify bot commented Jun 25, 2024

Deploy Preview for scroll-documentation ready!

Name Link
🔨 Latest commit 3930a4c
🔍 Latest deploy log https://app.netlify.com/sites/scroll-documentation/deploys/667b391ec8a1450008577b61
😎 Deploy Preview https://deploy-preview-282.docs.scroll.xyz
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@dghelm dghelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. Thanks for the close assessment here!

@dghelm dghelm merged commit 8e81ea8 into scroll-tech:develop Jun 27, 2024
4 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.

[BUG] nodeKey domain is incorrect on the zktrie page
2 participants