-
Notifications
You must be signed in to change notification settings - Fork 677
[NIT-4075] Extract keccak for ZKVM precompile substitution #4001
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
base: master
Are you sure you want to change the base?
Conversation
This reverts commit d3b6472.
❌ 3 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In these set of PRs we introduce keccak for ZKVM precompile but only start using in some another PR, the ones mentioned in the description? Or am I missing something? And for my own sanity, the introduced keccak for replay.wasm uses the same implementation since wasm counterpart calls into arbkeccak::keccak256 as well?
This set of PRs introduce just one thing: whenever we compile nitro into wasm (e.g. The other PRs mentioned in the description are just about ensuring that there are no other keccak hashing across the codebase that would still use the old way.
after this PR, the |
Native nitro
This PR doesn't change anything for natively compiled nitro.
Wasm nitro
For wasm target compilation any Keccak hash computation that was done with
crypto.Keccak256()orcrypto.Keccak256Hash()will be now realized by an external call - the resultingwasmbinary/library expects an externalarbkeccakmodule exposingkeccak256function (see OffchainLabs/go-ethereum#578).Almost all keccaking is done with these two function. However, there are still some places where we unnecessarily use stateful keccak. This might be problematic, since as a result, our hash delegation will not be applied there. Specifically:
nitroexcludinggo-ethereumwe have 1 such usage; [NIT-4123] Use stateless keccak where possible #4025 has been filed to fix itgo-ethereumthere are ~20 such usages; I opened an upstream PR for this: all: use stateless keccak ethereum/go-ethereum#33222; if it doesn't get merged, we have a backup PR for our fork: arbitrum: Unify keccaking go-ethereum#580Running
replay.wasmwith JIT
we inject a Rust binding to the
tiny_keccaklibrary, which is no-std friendly and we already did use it inarbutil:with arbitrator
we build a new wasm library
arbkeccakproviding expected functionality:companion PR: OffchainLabs/go-ethereum#578
part of NIT-4075