feat(crypto): use the ziren keccak precompile and implement ziren keccak state #32816 #32996#1991
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This pull request integrates the Ziren zkvm keccak precompile and implements build-tag-based conditional compilation to support both standard and Ziren-optimized keccak hashing implementations.
Changes:
- Added Ziren zkvm_runtime dependency for zk-optimized Keccak256 implementation
- Refactored keccak implementations into separate build-tag-controlled files (ziren vs non-ziren)
- Removed Keccak512 function (unused in the codebase)
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Added zkvm_runtime dependency as an indirect requirement |
| go.sum | Added checksums for the new zkvm_runtime dependency |
| crypto/keccak.go | Standard (non-ziren) implementation of Keccak functions using golang.org/x/crypto/sha3 |
| crypto/keccak_ziren.go | Ziren-optimized implementation using zkvm_runtime precompile |
| crypto/crypto.go | Removed Keccak function implementations and unused imports (moved to build-tag-specific files) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b2691bf to
cf9b93c
Compare
Uses the go module's `replace` directive to delegate keccak computation to precompiles. This is still in draft because it needs more testing. Also, it relies on a PR that I created, that hasn't been merged yet. _Note that this PR doesn't implement the stateful keccak state structure, and it reverts to the current behavior. This is a bit silly since this is what is used in the tree root computation. The runtime doesn't currently export the sponge. I will see if I can fix that in a further PR, but it is going to take more time. In the meantime, this is a useful first step_
The ethereum#32816 was only using the keccak precompile for some minor task. This PR implements a keccak state, which is what is used for hashing the tree.
cf9b93c to
3708d2b
Compare
Proposed changes
Ref:
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that