fix(withdrawal): support legacy + nullifier finalize selectors#350
fix(withdrawal): support legacy + nullifier finalize selectors#350cryptocake wants to merge 1 commit intovianetwork:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for multiple L2 withdrawal message function signatures to facilitate the Bridgehub/L1Nullifier migration. It replaces the single WITHDRAW_FUNC_SIG constant with a list of supported signatures and updates the parsing logic to validate against any of these selectors. Feedback focuses on maintaining backward compatibility for the removed constant and optimizing the selector retrieval process to avoid redundant hashing and memory allocations.
| pub const WITHDRAW_FUNC_SIG_LEGACY: &str = | ||
| "finalizeEthWithdrawal(uint256,uint256,uint16,bytes,bytes32[])"; |
There was a problem hiding this comment.
The constant WITHDRAW_FUNC_SIG has been removed and replaced by WITHDRAW_FUNC_SIG_LEGACY. If this constant is used by other crates in the workspace or by external consumers, this change will break their compilation. Consider keeping WITHDRAW_FUNC_SIG as an alias (possibly marked with #[deprecated]) to maintain backward compatibility during the migration period.
| fn _get_supported_withdraw_function_selectors() -> Vec<Vec<u8>> { | ||
| WITHDRAW_FUNC_SIGS | ||
| .iter() | ||
| .map(|sig| { | ||
| let hash = keccak256(sig.as_bytes()); | ||
| hash[0..4].to_vec() | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
The function _get_supported_withdraw_function_selectors is called on every invocation of parse_l2_withdrawal_message. It performs multiple keccak256 hashes and several Vec allocations each time. Since the function signatures are static constants, these 4-byte selectors should be computed once or defined as constants to improve performance, especially when processing large batches of withdrawals. Consider using std::sync::OnceLock (available in Rust 1.70+) or a similar lazy initialization pattern to cache the results.
|
Closing per author request. |
1 participant
Summary
Add withdrawal selector compatibility for the deprecation migration by accepting both legacy and nullifier finalize selectors when parsing L2->L1 withdrawal messages.
Changes
via_verifier/lib/via_da_client/src/types.rsWITHDRAW_FUNC_SIG_LEGACYWITHDRAW_FUNC_SIG_NULLIFIERWITHDRAW_FUNC_SIGSallowlistvia_verifier/lib/via_withdrawal_client/src/withdraw.rs_get_supported_withdraw_function_selectors()test_parse_l2_withdrawal_message_accepts_nullifier_selectorWhy
Issue #347 tracks zkSync Mailbox deprecations. This change keeps withdrawal message parsing compatible during migration windows where both legacy and replacement finalize selectors may appear.
Validation
cargo check -qinvia_verifier/lib/via_da_client✅cargo test -qinvia_verifier/lib/via_withdrawal_clientis blocked in this environment by missingprotoc(dependency build forcelestia-proto) ❌ environment/tooling issueLinked issues