feat(ev-deployer): part 2 - add Hyperlane contract support#181
Open
feat(ev-deployer): part 2 - add Hyperlane contract support#181
Conversation
…e patching Add Hyperlane monorepo as git submodule pinned to @hyperlane-xyz/core@11.0.3. Implement bytecode patching utility for Solidity immutable variables and MerkleTreeHook genesis alloc generation with mailbox/localDomain/deployedBlock patching. This is the foundation for embedding Hyperlane contracts at genesis.
📝 WalkthroughWalkthroughThis PR extends the ev-deployer tool to support deploying five additional Hyperlane contracts (FeeVault, MerkleTreeHook, Mailbox, NoopIsm, ProtocolFee) by adding configuration structures, bytecode builders with genesis storage encoding, validation logic, and e2e tests. It also integrates the hyperlane-monorepo as a Git submodule and updates CI to install soldeer dependencies. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Claude finished @randygrok's task —— View job Code Review CompleteI've completed a thorough review of this PR that adds Hyperlane contract support to ev-deployer. Here's my analysis: Todo List:
Overall AssessmentThis is a high-quality implementation that carefully handles bytecode patching for Solidity immutable variables. The code demonstrates good engineering practices with proper testing, documentation, and validation. ✅ Strengths1. Security & Correctness
2. Architecture & Design
3. Testing & Verification
|
randygrok
changed the title
# Conflicts: # Cargo.lock
…deployer-merkle-tree-hook
The MerkleTreeHook bytecode test needs OpenZeppelin dependencies from the Hyperlane monorepo, which are managed by soldeer.
Verify bytecode, storage slots, and patched immutables (mailbox, localDomain, deployedBlock) for the MerkleTreeHook contract via RPC.
Rustdoc interprets [644], [578], and [32] as intra-doc links, causing the docs CI job to fail with -D warnings.
…racts Add three Hyperlane core contracts to ev-deployer so the full messaging stack can be embedded at genesis without post-deploy transactions. - Mailbox: core messaging hub with localDomain/deployedBlock immutables - NoopIsm: stateless ISM that accepts all messages (for devnet) - ProtocolFee: post-dispatch hook with MAX_PROTOCOL_FEE immutable
…lean ci build The embedded bytecodes were compiled with --extra-output storageLayout which subtly altered the output. Regenerated from a clean ci profile build to match what forge produces without extra flags.
- Expand CI workflow path triggers to include Cargo.toml, Cargo.lock,
and the workflow file itself
- Fix README merge behavior description (not in-place)
- Validate contract addresses are unique in config
- Use trim_start_matches("0x") instead of strip_prefix().unwrap() in
bytecode verification tests
- Add curl timeouts to e2e test RPC calls to prevent CI hangs
…s validation Merge ev-deployer-part1-core into ev-deployer-merkle-tree-hook, resolving config.rs conflicts by keeping both the MerkleTreeHook/ProtocolFee validations and the new duplicate-address check between AdminProxy and FeeVault.
FeeVault will be added in a separate PR to keep this one focused on AdminProxy only.
Keep only the CLI framework (config, genesis merge, manifest output) without any contract implementations. Contracts will be added in subsequent PRs.
This reverts commit 089ef22.
Default to an empty ContractsConfig when the section is omitted, so a minimal config only needs [chain].
Generates a TOML config template with all supported contracts commented out and documented.
…ence Move Init subcommand first in help output and remove leftover fee_vault mention from --contract flag doc.
Canonicalize address keys (lowercase, strip 0x) before comparing during merge, so collisions are detected regardless of case or prefix in the existing genesis file.
…deployer-merkle-tree-hook
Resolve conflicts keeping all 6 contract additions (fee_vault, merkle_tree_hook, mailbox, noop_ism, protocol_fee, admin_proxy) across config, genesis, output, e2e tests, CI workflow, README, init template, and devnet example. Fix rustfmt trailing blank lines.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
bin/ev-deployer/tests/e2e_genesis.sh (1)
132-240: Extend the node-boot test to cover the other new builders.
devnet.tomlnow provisionsmailbox,noop_ism, andprotocol_fee, but this script only validatesFeeVaultandMerkleTreeHookafterAdminProxy. A broken alloc or immutable patch in those three contracts would still pass the end-to-end suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/ev-deployer/tests/e2e_genesis.sh` around lines 132 - 240, Add E2E checks for the other builders (mailbox, noop_ism, protocol_fee) similar to the existing FeeVault and MerkleTreeHook blocks: for each of MAILBOX (address constant/mailbox_word), NOOP_ISM, and PROTOCOL_FEE, call eth_getCode and assert non-empty bytecode, compare the node bytecode to the genesis alloc entry (like expected_mth_code logic), and validate any contract-specific storage slots or immutables (e.g., owner slots, mailbox address patching, domain or fee constants) using the existing check_immutable and eth_getStorageAt patterns; reference the same helper symbols used in the script (rpc_call, check_immutable, pass, fail) and mirror the pass/fail messages for consistency.bin/ev-deployer/src/config.rs (1)
292-294: Minor: Stray blank line between#[test]and function.The empty line between the
#[test]attribute (line 292) andfn admin_proxy_only()(line 294) is unusual. While valid Rust, this may be an editing artifact.✂️ Remove stray blank line
#[test] - fn admin_proxy_only() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/ev-deployer/src/config.rs` around lines 292 - 294, Remove the stray blank line between the attribute and the test function: move the #[test] attribute directly above fn admin_proxy_only() so the attribute applies without an intervening empty line; update the test declaration in the admin_proxy_only function to have #[test] immediately preceding it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitmodules:
- Around line 4-6: The .gitmodules entry for the submodule
"contracts/lib/hyperlane-monorepo" is present but the repository lacks the
corresponding gitlink (submodule pointer), so CI won't check out the submodule;
add and commit the submodule gitlink pinned to the intended commit: either run
git submodule add <url> contracts/lib/hyperlane-monorepo (or update the
submodule to the target commit and run git add on the submodule directory) so
the index contains the 160000 gitlink entry, commit that change alongside the
.gitmodules update, and push the commit so workflows (ev_deployer.yml) can
recurse and find the solidity code.
In `@bin/ev-deployer/README.md`:
- Line 17: Remove the duplicated sentence "See
[`examples/devnet.toml`](examples/devnet.toml) for a complete example with all
contracts configured." from the README so it only appears once; keep the
original occurrence and delete the redundant repeat to avoid repetition in the
configuration section.
In `@bin/ev-deployer/src/config.rs`:
- Around line 186-191: The current validation only compares
contracts.admin_proxy and contracts.fee_vault, leaving other contract addresses
unchecked and allowing silent overwrites when build_alloc()/insert_contract()
merges entries; update the validation in the Config (where self.contracts is
checked) to collect all present contract addresses from each optional contract
field (e.g., admin_proxy, fee_vault and the other four contract variants) into a
map or vector, detect duplicates (same address appearing for more than one
contract), and return an error via eyre::ensure! (or similar) listing the
conflicting contract names; reference self.contracts and
build_alloc()/insert_contract() to locate the relevant logic.
In `@bin/ev-deployer/src/contracts/mailbox.rs`:
- Around line 86-119: The code currently writes _initialized = 1 (B256::ZERO
slot) and directly inserts default_ism/default_hook/required_hook into storage
before checking invariants; instead, validate MailboxConfig.owner is non-zero
(or require caller to provide a valid owner) and run the runtime
contract-address checks for config.default_ism, config.default_hook, and
config.required_hook first (the same checks that initialize() would enforce),
only writing their B256 entries into storage after those validations succeed,
and only then set the _initialized flag; in short, move the storage.insert for
B256::ZERO (the _initialized marker) to the end of this initialization routine
and perform all owner and contract-address validations prior to any
storage.insert of default_ism/default_hook/required_hook so initialize() is not
made permanently unreachable.
In `@bin/ev-deployer/src/contracts/merkle_tree_hook.rs`:
- Around line 88-101: The builder currently only inserts the owner slot when
config.owner is non-zero, which silently combines a zero owner with the
unconditional _initialized=1; instead, always write the owner storage slot (slot
51) from MerkleTreeHookConfig.owner even when it is zero so a zero owner is an
explicit renounce; update the code around the storage.insert for slot 51 (the
block referencing MerkleTreeHookConfig.owner and
B256::from(U256::from_be_bytes(...))) to unconditionally insert that entry
rather than skipping when owner.is_zero().
In `@bin/ev-deployer/src/genesis.rs`:
- Around line 25-33: The code currently casts config.chain.chain_id to u32 with
"as u32" when constructing local_domain for contracts::merkle_tree_hook::build
and contracts::mailbox::build, which can silently truncate values; replace the
lossy cast with a checked conversion (e.g., u32::try_from(config.chain.chain_id)
or a validated helper) and propagate or panic on out-of-range values so the
function fails fast and does not insert a truncated local_domain via
insert_contract; update the local_domain construction sites and add a clear
error message on failure.
- Around line 20-45: The code currently calls insert_contract(&mut alloc,
&contract) multiple times which silently overwrites existing entries if
addresses collide; modify the flow to detect and reject duplicate contract
addresses: either (A) add a uniqueness check during configuration validation
that scans all optional contract configs for duplicate addresses before calling
build/insert, or (B) change insert_contract to return Result<(), Error> (or
Option) and have the callers (in build_alloc/genesis code) check the return
value and propagate/return an error when a collision is detected; reference the
functions/variables insert_contract, build (e.g., contracts::fee_vault::build,
contracts::mailbox::build, etc.), and the alloc map so the fix ensures any
duplicate address yields a clear error instead of silently replacing the earlier
contract.
---
Nitpick comments:
In `@bin/ev-deployer/src/config.rs`:
- Around line 292-294: Remove the stray blank line between the attribute and the
test function: move the #[test] attribute directly above fn admin_proxy_only()
so the attribute applies without an intervening empty line; update the test
declaration in the admin_proxy_only function to have #[test] immediately
preceding it.
In `@bin/ev-deployer/tests/e2e_genesis.sh`:
- Around line 132-240: Add E2E checks for the other builders (mailbox, noop_ism,
protocol_fee) similar to the existing FeeVault and MerkleTreeHook blocks: for
each of MAILBOX (address constant/mailbox_word), NOOP_ISM, and PROTOCOL_FEE,
call eth_getCode and assert non-empty bytecode, compare the node bytecode to the
genesis alloc entry (like expected_mth_code logic), and validate any
contract-specific storage slots or immutables (e.g., owner slots, mailbox
address patching, domain or fee constants) using the existing check_immutable
and eth_getStorageAt patterns; reference the same helper symbols used in the
script (rpc_call, check_immutable, pass, fail) and mirror the pass/fail messages
for consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f4281f7-81ee-4854-a617-4d0ab5b2c20e
📒 Files selected for processing (18)
.github/workflows/ev_deployer.yml.gitmodulesbin/ev-deployer/README.mdbin/ev-deployer/examples/devnet.tomlbin/ev-deployer/src/config.rsbin/ev-deployer/src/contracts/fee_vault.rsbin/ev-deployer/src/contracts/immutables.rsbin/ev-deployer/src/contracts/mailbox.rsbin/ev-deployer/src/contracts/merkle_tree_hook.rsbin/ev-deployer/src/contracts/mod.rsbin/ev-deployer/src/contracts/noop_ism.rsbin/ev-deployer/src/contracts/protocol_fee.rsbin/ev-deployer/src/genesis.rsbin/ev-deployer/src/init_template.tomlbin/ev-deployer/src/main.rsbin/ev-deployer/src/output.rsbin/ev-deployer/tests/e2e_genesis.shcontracts/lib/hyperlane-monorepo
Comment on lines
+4
to
+6
| [submodule "contracts/lib/hyperlane-monorepo"] | ||
| path = contracts/lib/hyperlane-monorepo | ||
| url = https://github.com/hyperlane-xyz/hyperlane-monorepo.git |
There was a problem hiding this comment.
Submodule declaration appears incomplete without committed gitlink.
If only .gitmodules is added but the submodule gitlink is not committed, actions/checkout with recursive submodules will not materialize contracts/lib/hyperlane-monorepo, and the workflow step at Line 44 in .github/workflows/ev_deployer.yml will fail on cd .../solidity.
Please ensure the submodule pointer (gitlink) is included in the PR, pinned to the intended commit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitmodules around lines 4 - 6, The .gitmodules entry for the submodule
"contracts/lib/hyperlane-monorepo" is present but the repository lacks the
corresponding gitlink (submodule pointer), so CI won't check out the submodule;
add and commit the submodule gitlink pinned to the intended commit: either run
git submodule add <url> contracts/lib/hyperlane-monorepo (or update the
submodule to the target commit and run git add on the submodule directory) so
the index contains the 160000 gitlink entry, commit that change alongside the
.gitmodules update, and push the commit so workflows (ev_deployer.yml) can
recurse and find the solidity code.
| address = "0x000000000000000000000000000000000000Ad00" | ||
| owner = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" | ||
| ``` | ||
| See [`examples/devnet.toml`](examples/devnet.toml) for a complete example with all contracts configured. |
There was a problem hiding this comment.
Remove duplicated sentence in configuration section.
Line 17 repeats the same examples/devnet.toml guidance already stated at Line 15.
Proposed docs cleanup
-See [`examples/devnet.toml`](examples/devnet.toml) for a complete example with all contracts configured.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| See [`examples/devnet.toml`](examples/devnet.toml) for a complete example with all contracts configured. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/ev-deployer/README.md` at line 17, Remove the duplicated sentence "See
[`examples/devnet.toml`](examples/devnet.toml) for a complete example with all
contracts configured." from the README so it only appears once; keep the
original occurrence and delete the redundant repeat to avoid repetition in the
configuration section.
Comment on lines
+186
to
+191
| if let (Some(ap), Some(fv)) = (&self.contracts.admin_proxy, &self.contracts.fee_vault) { | ||
| eyre::ensure!( | ||
| ap.address != fv.address, | ||
| "contracts.admin_proxy.address and contracts.fee_vault.address must be distinct" | ||
| ); | ||
| } |
There was a problem hiding this comment.
Address collision validation is incomplete—silent overwrites possible.
Only admin_proxy vs fee_vault is checked, but there are 6 contract types now. If any two contracts share the same address, build_alloc() will silently overwrite one entry with another (per the insert_contract() pattern in genesis.rs), leading to a corrupted genesis state.
Consider collecting all configured addresses and validating they are distinct:
🛠️ Suggested approach to validate all addresses are distinct
+ // Collect all configured contract addresses and ensure they are distinct.
+ let mut addresses: Vec<(&str, Address)> = Vec::new();
+ if let Some(ref ap) = self.contracts.admin_proxy {
+ addresses.push(("admin_proxy", ap.address));
+ }
+ if let Some(ref fv) = self.contracts.fee_vault {
+ addresses.push(("fee_vault", fv.address));
+ }
+ if let Some(ref mth) = self.contracts.merkle_tree_hook {
+ addresses.push(("merkle_tree_hook", mth.address));
+ }
+ if let Some(ref mb) = self.contracts.mailbox {
+ addresses.push(("mailbox", mb.address));
+ }
+ if let Some(ref ni) = self.contracts.noop_ism {
+ addresses.push(("noop_ism", ni.address));
+ }
+ if let Some(ref pf) = self.contracts.protocol_fee {
+ addresses.push(("protocol_fee", pf.address));
+ }
+ for i in 0..addresses.len() {
+ for j in (i + 1)..addresses.len() {
+ eyre::ensure!(
+ addresses[i].1 != addresses[j].1,
+ "contracts.{}.address and contracts.{}.address must be distinct",
+ addresses[i].0,
+ addresses[j].0
+ );
+ }
+ }
+
- if let (Some(ap), Some(fv)) = (&self.contracts.admin_proxy, &self.contracts.fee_vault) {
- eyre::ensure!(
- ap.address != fv.address,
- "contracts.admin_proxy.address and contracts.fee_vault.address must be distinct"
- );
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if let (Some(ap), Some(fv)) = (&self.contracts.admin_proxy, &self.contracts.fee_vault) { | |
| eyre::ensure!( | |
| ap.address != fv.address, | |
| "contracts.admin_proxy.address and contracts.fee_vault.address must be distinct" | |
| ); | |
| } | |
| // Collect all configured contract addresses and ensure they are distinct. | |
| let mut addresses: Vec<(&str, Address)> = Vec::new(); | |
| if let Some(ref ap) = self.contracts.admin_proxy { | |
| addresses.push(("admin_proxy", ap.address)); | |
| } | |
| if let Some(ref fv) = self.contracts.fee_vault { | |
| addresses.push(("fee_vault", fv.address)); | |
| } | |
| if let Some(ref mth) = self.contracts.merkle_tree_hook { | |
| addresses.push(("merkle_tree_hook", mth.address)); | |
| } | |
| if let Some(ref mb) = self.contracts.mailbox { | |
| addresses.push(("mailbox", mb.address)); | |
| } | |
| if let Some(ref ni) = self.contracts.noop_ism { | |
| addresses.push(("noop_ism", ni.address)); | |
| } | |
| if let Some(ref pf) = self.contracts.protocol_fee { | |
| addresses.push(("protocol_fee", pf.address)); | |
| } | |
| for i in 0..addresses.len() { | |
| for j in (i + 1)..addresses.len() { | |
| eyre::ensure!( | |
| addresses[i].1 != addresses[j].1, | |
| "contracts.{}.address and contracts.{}.address must be distinct", | |
| addresses[i].0, | |
| addresses[j].0 | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/ev-deployer/src/config.rs` around lines 186 - 191, The current validation
only compares contracts.admin_proxy and contracts.fee_vault, leaving other
contract addresses unchecked and allowing silent overwrites when
build_alloc()/insert_contract() merges entries; update the validation in the
Config (where self.contracts is checked) to collect all present contract
addresses from each optional contract field (e.g., admin_proxy, fee_vault and
the other four contract variants) into a map or vector, detect duplicates (same
address appearing for more than one contract), and return an error via
eyre::ensure! (or similar) listing the conflicting contract names; reference
self.contracts and build_alloc()/insert_contract() to locate the relevant logic.
Comment on lines
+86
to
+119
| // Slot 0: _initialized = 1 (OZ v4 Initializable), _initializing = false | ||
| storage.insert(B256::ZERO, B256::from(U256::from(1u8))); | ||
|
|
||
| // Slot 51: _owner | ||
| if !config.owner.is_zero() { | ||
| storage.insert( | ||
| B256::from(U256::from(51u64)), | ||
| B256::from(U256::from_be_bytes(config.owner.into_word().0)), | ||
| ); | ||
| } | ||
|
|
||
| // Slot 103: defaultIsm | ||
| if !config.default_ism.is_zero() { | ||
| storage.insert( | ||
| B256::from(U256::from(103u64)), | ||
| B256::from(U256::from_be_bytes(config.default_ism.into_word().0)), | ||
| ); | ||
| } | ||
|
|
||
| // Slot 104: defaultHook | ||
| if !config.default_hook.is_zero() { | ||
| storage.insert( | ||
| B256::from(U256::from(104u64)), | ||
| B256::from(U256::from_be_bytes(config.default_hook.into_word().0)), | ||
| ); | ||
| } | ||
|
|
||
| // Slot 105: requiredHook | ||
| if !config.required_hook.is_zero() { | ||
| storage.insert( | ||
| B256::from(U256::from(105u64)), | ||
| B256::from(U256::from_be_bytes(config.required_hook.into_word().0)), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Don't mark the mailbox initialized before enforcing the initializer's invariants.
MailboxConfig.owner defaults to zero, but this builder always stores _initialized = 1. That makes initialize() permanently unreachable, so an omitted owner silently creates an unadministerable mailbox. The direct writes to default_ism / default_hook / required_hook also bypass the runtime contract-address checks, so those preconditions need to be enforced before this state is committed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/ev-deployer/src/contracts/mailbox.rs` around lines 86 - 119, The code
currently writes _initialized = 1 (B256::ZERO slot) and directly inserts
default_ism/default_hook/required_hook into storage before checking invariants;
instead, validate MailboxConfig.owner is non-zero (or require caller to provide
a valid owner) and run the runtime contract-address checks for
config.default_ism, config.default_hook, and config.required_hook first (the
same checks that initialize() would enforce), only writing their B256 entries
into storage after those validations succeed, and only then set the _initialized
flag; in short, move the storage.insert for B256::ZERO (the _initialized marker)
to the end of this initialization routine and perform all owner and
contract-address validations prior to any storage.insert of
default_ism/default_hook/required_hook so initialize() is not made permanently
unreachable.
Comment on lines
+88
to
+101
| let mut storage = BTreeMap::new(); | ||
|
|
||
| // Slot 0: _initialized = 1 (OZ v4 Initializable), _initializing = false | ||
| // byte layout: [_initialized (1 byte)] [_initializing (1 byte)] [... 30 zero bytes] | ||
| // Packed at slot 0: value = 0x01 (just _initialized = 1) | ||
| storage.insert(B256::ZERO, B256::from(U256::from(1u8))); | ||
|
|
||
| // Slot 51: _owner — set the owner so the contract can be administered post-genesis | ||
| if !config.owner.is_zero() { | ||
| storage.insert( | ||
| B256::from(U256::from(51u64)), | ||
| B256::from(U256::from_be_bytes(config.owner.into_word().0)), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Treat owner = 0 here as an explicit renounce, not a silent default.
This builder sets _initialized = 1 unconditionally. If MerkleTreeHookConfig.owner is left at its default zero value, there is no path to recover ownership later because initialize() is permanently disabled and transferOwnership() requires the current owner.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/ev-deployer/src/contracts/merkle_tree_hook.rs` around lines 88 - 101, The
builder currently only inserts the owner slot when config.owner is non-zero,
which silently combines a zero owner with the unconditional _initialized=1;
instead, always write the owner storage slot (slot 51) from
MerkleTreeHookConfig.owner even when it is zero so a zero owner is an explicit
renounce; update the code around the storage.insert for slot 51 (the block
referencing MerkleTreeHookConfig.owner and B256::from(U256::from_be_bytes(...)))
to unconditionally insert that entry rather than skipping when owner.is_zero().
Comment on lines
+20
to
+45
| if let Some(ref fv_config) = config.contracts.fee_vault { | ||
| let contract = contracts::fee_vault::build(fv_config); | ||
| insert_contract(&mut alloc, &contract); | ||
| } | ||
|
|
||
| if let Some(ref mth_config) = config.contracts.merkle_tree_hook { | ||
| let local_domain = config.chain.chain_id as u32; | ||
| let contract = contracts::merkle_tree_hook::build(mth_config, local_domain); | ||
| insert_contract(&mut alloc, &contract); | ||
| } | ||
|
|
||
| if let Some(ref mb_config) = config.contracts.mailbox { | ||
| let local_domain = config.chain.chain_id as u32; | ||
| let contract = contracts::mailbox::build(mb_config, local_domain); | ||
| insert_contract(&mut alloc, &contract); | ||
| } | ||
|
|
||
| if let Some(ref ni_config) = config.contracts.noop_ism { | ||
| let contract = contracts::noop_ism::build(ni_config); | ||
| insert_contract(&mut alloc, &contract); | ||
| } | ||
|
|
||
| if let Some(ref pf_config) = config.contracts.protocol_fee { | ||
| let contract = contracts::protocol_fee::build(pf_config); | ||
| insert_contract(&mut alloc, &contract); | ||
| } |
There was a problem hiding this comment.
New contract inserts can silently overwrite each other on duplicate addresses.
build_alloc inserts multiple optional contracts into the same map; if two configured contracts share an address, later inserts replace earlier entries without error. With the newly added contracts, this risk increased materially.
Please enforce uniqueness across all configured contract addresses (preferably during config validation) or make insert_contract reject collisions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/ev-deployer/src/genesis.rs` around lines 20 - 45, The code currently
calls insert_contract(&mut alloc, &contract) multiple times which silently
overwrites existing entries if addresses collide; modify the flow to detect and
reject duplicate contract addresses: either (A) add a uniqueness check during
configuration validation that scans all optional contract configs for duplicate
addresses before calling build/insert, or (B) change insert_contract to return
Result<(), Error> (or Option) and have the callers (in build_alloc/genesis code)
check the return value and propagate/return an error when a collision is
detected; reference the functions/variables insert_contract, build (e.g.,
contracts::fee_vault::build, contracts::mailbox::build, etc.), and the alloc map
so the fix ensures any duplicate address yields a clear error instead of
silently replacing the earlier contract.
Comment on lines
+25
to
+33
| if let Some(ref mth_config) = config.contracts.merkle_tree_hook { | ||
| let local_domain = config.chain.chain_id as u32; | ||
| let contract = contracts::merkle_tree_hook::build(mth_config, local_domain); | ||
| insert_contract(&mut alloc, &contract); | ||
| } | ||
|
|
||
| if let Some(ref mb_config) = config.contracts.mailbox { | ||
| let local_domain = config.chain.chain_id as u32; | ||
| let contract = contracts::mailbox::build(mb_config, local_domain); |
There was a problem hiding this comment.
Avoid lossy u64 -> u32 cast for Hyperlane local_domain.
At Line 26 and Line 32, config.chain.chain_id as u32 silently truncates when chain_id > u32::MAX, which can patch incorrect immutable values into Mailbox/MerkleTreeHook bytecode.
Please enforce a checked conversion (or config validation) and fail fast on out-of-range values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/ev-deployer/src/genesis.rs` around lines 25 - 33, The code currently
casts config.chain.chain_id to u32 with "as u32" when constructing local_domain
for contracts::merkle_tree_hook::build and contracts::mailbox::build, which can
silently truncate values; replace the lossy cast with a checked conversion
(e.g., u32::try_from(config.chain.chain_id) or a validated helper) and propagate
or panic on out-of-range values so the function fails fast and does not insert a
truncated local_domain via insert_contract; update the local_domain construction
sites and add a clear error message on failure.
damiannolan
reviewed
Mar 26, 2026
Contributor
damiannolan
left a comment
damiannolan
left a comment
There was a problem hiding this comment.
I left some initial questions. I'll need to get a better understanding of what the main goal of the hyperlane deployer is?
| other_recipient = "0x0000000000000000000000000000000000000000" | ||
| hyp_native_minter = "0x0000000000000000000000000000000000000000" | ||
|
|
||
| [contracts.mailbox] |
Contributor
There was a problem hiding this comment.
Should hyperlane contain its own namespace under contracts in order to logically group components. For example, mailbox, isms, hooks contracts are directly parts of hyperlane protocol, where as admin proxy / fee vault are not.
Suggested change
| [contracts.mailbox] | |
| [contracts.hyperlane.mailbox] |
Contributor
Author
There was a problem hiding this comment.
makes total sense
Comment on lines
+6
to
+9
| pub(crate) mod mailbox; | ||
| pub(crate) mod merkle_tree_hook; | ||
| pub(crate) mod noop_ism; | ||
| pub(crate) mod protocol_fee; |
Contributor
There was a problem hiding this comment.
These 4 hyperlane contracts are a limited scope of the entire protocol suite.
Is the intention for this to be a first pass iteration for getting a bare minimum setup working?
For mainnet deployments, the IGP hook (interchain gas paymaster) is normally used in favour of protocol fee hook, however it is configured and overridable.
The mailbox mandates a required hook (usually the merkle tree hook) and a default hook (usually IGP in mainnets, less configuration involved in protocol fee which means that is easier for testnets). The default hook is overridable by users. For example, a user submitting a warp token transfer can provide a customHookId which would take precedence over the default mailbox hook.
The noop/testIsm provides zero security. It should also be used in testnets. Mainnet deployments usually use a MerkleRootMultisigIsm with a configured validator set. This also requires a ValidatorAnnounce contract in order to allow validators to post storage locations for checkpoint signatures.
Contributor
Author
There was a problem hiding this comment.
do you see a benefit on using two flavours of hyperledger for the deployer (testnet, production) or something like this?
Contributor
There was a problem hiding this comment.
Is the intension here to essentially add precompiles to the genesis file with preallocated storage etc. ?
tac0turtle
reviewed
Mar 26, 2026
|
|
||
| /// `FeeVault` runtime bytecode compiled with solc 0.8.33 (`cbor_metadata=false`). | ||
| /// Regenerate with: `cd contracts && forge inspect FeeVault deployedBytecode` | ||
| const FEE_VAULT_BYTECODE: &[u8] = &hex!("608060405260043610610101575f3560e01c80636cb53e1611610094578063bb0c829811610063578063bb0c8298146102dc578063c3f909d414610306578063eeb4a9c814610337578063f2fde38b1461035f578063f63188b71461038757610108565b80636cb53e16146102565780637d57d97a1461027e5780638da5cb5b1461028857806390321e1a146102b257610108565b806339bb1c5b116100d057806339bb1c5b146101ae5780634cebdc49146101d85780635aff5999146102025780635c4a6d841461022c57610108565b80631636b3681461010c57806326465826146101345780632858c55a1461015c5780632c2d80891461018657610108565b3661010857005b5f5ffd5b348015610117575f5ffd5b50610132600480360381019061012d919061117f565b6103af565b005b34801561013f575f5ffd5b5061015a600480360381019061015591906111dd565b610526565b005b348015610167575f5ffd5b506101706105f6565b60405161017d9190611226565b60405180910390f35b348015610191575f5ffd5b506101ac60048036038101906101a7919061129c565b61060c565b005b3480156101b9575f5ffd5b506101c2610700565b6040516101cf9190611335565b60405180910390f35b3480156101e3575f5ffd5b506101ec610724565b6040516101f9919061135d565b60405180910390f35b34801561020d575f5ffd5b50610216610749565b6040516102239190611385565b60405180910390f35b348015610237575f5ffd5b5061024061074f565b60405161024d91906113ad565b60405180910390f35b348015610261575f5ffd5b5061027c6004803603810190610277919061117f565b610755565b005b6102866108cb565b005b348015610293575f5ffd5b5061029c610caa565b6040516102a9919061135d565b60405180910390f35b3480156102bd575f5ffd5b506102c6610ccf565b6040516102d391906113ad565b60405180910390f35b3480156102e7575f5ffd5b506102f0610cd5565b6040516102fd91906113ad565b60405180910390f35b348015610311575f5ffd5b5061031a610cdb565b60405161032e9897969594939291906113c6565b60405180910390f35b348015610342575f5ffd5b5061035d600480360381019061035891906111dd565b610d81565b005b34801561036a575f5ffd5b506103856004803603810190610380919061117f565b610e51565b005b348015610392575f5ffd5b506103ad60048036038101906103a891906111dd565b61100c565b005b60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff161461043e576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610435906114c2565b60405180910390fd5b5f73ffffffffffffffffffffffffffffffffffffffff168173ffffffffffffffffffffffffffffffffffffffff16036104ac576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016104a39061152a565b60405180910390fd5b8060055f6101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055507fa50c88d04012de3892b47d81943c983dc2690cfb81f0428eaa7d382f95683e4a8160405161051b919061135d565b60405180910390a150565b60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff16146105b5576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016105ac906114c2565b60405180910390fd5b806004819055507f63a8f7442c91b7117b3f235d24793c034fd752a01266bef3ef1d051efb56ca3d816040516105eb91906113ad565b60405180910390a150565b600160149054906101000a900463ffffffff1681565b60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff161461069b576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610692906114c2565b60405180910390fd5b81600160146101000a81548163ffffffff021916908363ffffffff160217905550806002819055507fcac2c3add78f132121267d80a684a62d521a9799fd8434bd0da1a27c491b044982826040516106f4929190611548565b60405180910390a15050565b5f5f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1681565b60055f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1681565b60025481565b60065481565b60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff16146107e4576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016107db906114c2565b60405180910390fd5b5f73ffffffffffffffffffffffffffffffffffffffff168173ffffffffffffffffffffffffffffffffffffffff1603610852576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016108499061152a565b60405180910390fd5b805f5f6101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055507f6eedba6e0a60268e3d78633f8822cea5dc75430d531f96fb46a29333834665c6816040516108c0919061135d565b60405180910390a150565b5f73ffffffffffffffffffffffffffffffffffffffff165f5f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1603610959576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610950906115b9565b60405180910390fd5b60045434101561099e576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161099590611621565b60405180910390fd5b5f4790505f612710600654836109b4919061166c565b6109be91906116da565b90505f81836109cd919061170a565b9050600354821015610a14576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610a0b90611787565b60405180910390fd5b7f50ecfcc47f2c5b2a26f91422abf650476ec7f701c48b1cf6d1d6d4d51a872ed6838383604051610a47939291906117a5565b60405180910390a15f811115610bb1575f73ffffffffffffffffffffffffffffffffffffffff1660055f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1603610ae6576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610add9061184a565b60405180910390fd5b5f60055f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1682604051610b2c90611895565b5f6040518083038185875af1925050503d805f8114610b66576040519150601f19603f3d011682016040523d82523d5f602084013e610b6b565b606091505b5050905080610baf576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610ba6906118f3565b60405180910390fd5b505b5f5f5f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff166381b4e8b484600160149054906101000a900463ffffffff16600254876040518563ffffffff1660e01b8152600401610c2493929190611911565b60206040518083038185885af1158015610c40573d5f5f3e3d5ffd5b50505050506040513d601f19601f82011682018060405250810190610c65919061195a565b90507f301fb78c068680a9fb5daa4ebadf5914ddc3a317f1fdc2c97f32740374d61e748360025483604051610c9c93929190611985565b60405180910390a150505050565b60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1681565b60045481565b60035481565b5f5f5f5f5f5f5f5f60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff16600160149054906101000a900463ffffffff1660025460035460045460065460055f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff165f5f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff16975097509750975097509750975097509091929394959697565b60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff1614610e10576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610e07906114c2565b60405180910390fd5b806003819055507f6ea576632a91ef2f8d4ee43600561b386f3c0254692977f0d33e17742bc5355881604051610e4691906113ad565b60405180910390a150565b60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff1614610ee0576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610ed7906114c2565b60405180910390fd5b5f73ffffffffffffffffffffffffffffffffffffffff168173ffffffffffffffffffffffffffffffffffffffff1603610f4e576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401610f4590611a2a565b60405180910390fd5b8073ffffffffffffffffffffffffffffffffffffffff1660015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff167f8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e060405160405180910390a38060015f6101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff16021790555050565b60015f9054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff161461109b576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401611092906114c2565b60405180910390fd5b6127108111156110e0576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016110d790611a92565b60405180910390fd5b806006819055507fa8da92ecf88f6d9f058e5f86d614520d5f20a3ecf87914deb605f649bd63de878160405161111691906113ad565b60405180910390a150565b5f5ffd5b5f73ffffffffffffffffffffffffffffffffffffffff82169050919050565b5f61114e82611125565b9050919050565b61115e81611144565b8114611168575f5ffd5b50565b5f8135905061117981611155565b92915050565b5f6020828403121561119457611193611121565b5b5f6111a18482850161116b565b91505092915050565b5f819050919050565b6111bc816111aa565b81146111c6575f5ffd5b50565b5f813590506111d7816111b3565b92915050565b5f602082840312156111f2576111f1611121565b5b5f6111ff848285016111c9565b91505092915050565b5f63ffffffff82169050919050565b61122081611208565b82525050565b5f6020820190506112395f830184611217565b92915050565b61124881611208565b8114611252575f5ffd5b50565b5f813590506112638161123f565b92915050565b5f819050919050565b61127b81611269565b8114611285575f5ffd5b50565b5f8135905061129681611272565b92915050565b5f5f604083850312156112b2576112b1611121565b5b5f6112bf85828601611255565b92505060206112d085828601611288565b9150509250929050565b5f819050919050565b5f6112fd6112f86112f384611125565b6112da565b611125565b9050919050565b5f61130e826112e3565b9050919050565b5f61131f82611304565b9050919050565b61132f81611315565b82525050565b5f6020820190506113485f830184611326565b92915050565b61135781611144565b82525050565b5f6020820190506113705f83018461134e565b92915050565b61137f81611269565b82525050565b5f6020820190506113985f830184611376565b92915050565b6113a7816111aa565b82525050565b5f6020820190506113c05f83018461139e565b92915050565b5f610100820190506113da5f83018b61134e565b6113e7602083018a611217565b6113f46040830189611376565b611401606083018861139e565b61140e608083018761139e565b61141b60a083018661139e565b61142860c083018561134e565b61143560e083018461134e565b9998505050505050505050565b5f82825260208201905092915050565b7f4665655661756c743a2063616c6c6572206973206e6f7420746865206f776e655f8201527f7200000000000000000000000000000000000000000000000000000000000000602082015250565b5f6114ac602183611442565b91506114b782611452565b604082019050919050565b5f6020820190508181035f8301526114d9816114a0565b9050919050565b7f4665655661756c743a207a65726f2061646472657373000000000000000000005f82015250565b5f611514601683611442565b915061151f826114e0565b602082019050919050565b5f6020820190508181035f83015261154181611508565b9050919050565b5f60408201905061155b5f830185611217565b6115686020830184611376565b9392505050565b7f4665655661756c743a206d696e746572206e6f742073657400000000000000005f82015250565b5f6115a3601883611442565b91506115ae8261156f565b602082019050919050565b5f6020820190508181035f8301526115d081611597565b9050919050565b7f4665655661756c743a20696e73756666696369656e74206665650000000000005f82015250565b5f61160b601a83611442565b9150611616826115d7565b602082019050919050565b5f6020820190508181035f830152611638816115ff565b9050919050565b7f4e487b71000000000000000000000000000000000000000000000000000000005f52601160045260245ffd5b5f611676826111aa565b9150611681836111aa565b925082820261168f816111aa565b915082820484148315176116a6576116a561163f565b5b5092915050565b7f4e487b71000000000000000000000000000000000000000000000000000000005f52601260045260245ffd5b5f6116e4826111aa565b91506116ef836111aa565b9250826116ff576116fe6116ad565b5b828204905092915050565b5f611714826111aa565b915061171f836111aa565b92508282039050818111156117375761173661163f565b5b92915050565b7f4665655661756c743a206d696e696d756d20616d6f756e74206e6f74206d65745f82015250565b5f611771602083611442565b915061177c8261173d565b602082019050919050565b5f6020820190508181035f83015261179e81611765565b9050919050565b5f6060820190506117b85f83018661139e565b6117c5602083018561139e565b6117d2604083018461139e565b949350505050565b7f4665655661756c743a206f7468657220726563697069656e74206e6f742073655f8201527f7400000000000000000000000000000000000000000000000000000000000000602082015250565b5f611834602183611442565b915061183f826117da565b604082019050919050565b5f6020820190508181035f83015261186181611828565b9050919050565b5f81905092915050565b50565b5f6118805f83611868565b915061188b82611872565b5f82019050919050565b5f61189f82611875565b9150819050919050565b7f4665655661756c743a207472616e73666572206661696c6564000000000000005f82015250565b5f6118dd601983611442565b91506118e8826118a9565b602082019050919050565b5f6020820190508181035f83015261190a816118d1565b9050919050565b5f6060820190506119245f830186611217565b6119316020830185611376565b61193e604083018461139e565b949350505050565b5f8151905061195481611272565b92915050565b5f6020828403121561196f5761196e611121565b5b5f61197c84828501611946565b91505092915050565b5f6060820190506119985f83018661139e565b6119a56020830185611376565b6119b26040830184611376565b949350505050565b7f4665655661756c743a206e6577206f776e657220697320746865207a65726f205f8201527f6164647265737300000000000000000000000000000000000000000000000000602082015250565b5f611a14602783611442565b9150611a1f826119ba565b604082019050919050565b5f6020820190508181035f830152611a4181611a08565b9050919050565b7f4665655661756c743a20696e76616c69642062707300000000000000000000005f82015250565b5f611a7c601583611442565b9150611a8782611a48565b602082019050919050565b5f6020820190508181035f830152611aa981611a70565b905091905056"); |
Contributor
There was a problem hiding this comment.
how do we make sure this stays in sync with the contracts if they change?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #119
Summary
@hyperlane-xyz/core@11.0.3immutables.rs) for Solidity immutable variables embedded in runtime bytecodeMerkleTreeHookgenesis alloc generation withmailbox,localDomain, anddeployedBlockimmutable patchingMerkleTreeHookConfiginto config parsing, genesis builder, address manifest, and compute-address CLIDetails
Immutable patching: Solidity
immutablevalues are compiled into bytecode, not storage. The newimmutables.rsmodule patches zero-placeholder bytecodes at known byte offsets with actual values from the deploy config at genesis time.MerkleTreeHook (Hyperlane required hook): Maintains an incremental Merkle tree of dispatched message IDs. Storage is minimal at genesis (slot 0:
_initialized=1, slot 51:_owner). Three immutables patched in bytecode:mailbox(address, 2 refs),localDomain(uint32),deployedBlock(uint256, set to 0).Bytecode source: Compiled with Hyperlane v11.0.3, solc 0.8.22, Foundry
ciprofile (cbor_metadata=false,bytecode_hash="none").Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Chores