feat: added integration test with actual roots + update of OFT deployment [SV-1468]

[supervaulter]

…ment [SV-1468]

---

Note

Medium Risk
Updates LayerZero ULN confirmation parameters for HyperEVM pathways and adds scripts to apply them on-chain; incorrect values or execution could disrupt cross-chain message delivery. New fork-based integration tests depend on live RPC/Odos quoting, which may increase CI flakiness/timeouts.

Overview
Aligns HyperEVM LayerZero ULN confirmation settings by splitting send vs receive configs in DeployUpOFT.s.sol and updating the HyperEVM↔ETH/Base pathways (e.g., HL→ETH/HL→Base send confirmations to 1, Base→HL send to 10) so each side’s send confirmations match the other side’s receive confirmations.

Adds UpdateHyperEVMLZConfig.s.sol plus a runnable shell wrapper to apply these ULN confirmation updates on HyperEVM via endpoint.setConfig.

Introduces new SuperBank integration coverage: a pure SuperBankMerkleTreeValidation test that checks production merkle roots/proofs, and a fork-based SuperBankSwapIntegration test that executes hooks against production deployments (live Odos swap quoting, production merkle roots/proofs, and an ETH→Base Across bridge + swap flow using Pigeon relay).

^{Written by Cursor Bugbot for commit b1e914c. This will update automatically on new commits. Configure here.}

[linear]

SV-1468 Integration test with executeHook on SB

[claude]

PR Review — feat: SuperBank integration tests + HyperEVM LZ config updates [SV-1468]

Overall this PR is well-structured. The LZ config corrections address a real bug (receive configs were using the wrong ULN objects), and the integration tests provide meaningful end-to-end coverage. A few issues worth flagging before merge.

---

Critical / Correctness

Shell script name vs. function called — likely operator error waiting to happen

script/run/update_hyperevm_lz_config_base.sh is named to suggest it handles the Base<>HL path, but it calls runEth() and its console banner reads "Update HyperEVM LZ Config - ETH<>HL Path". This is a confusing mismatch. If an operator runs the script expecting it to update the Base path, they will silently apply only the ETH path change. Either:

• Rename the file to update_hyperevm_lz_config_eth.sh, or
• Change the script to call runBase() (and update the banner/comments accordingly)

Also notable: there is no corresponding shell wrapper for runBase() or run() (the full update). If Base-side changes still need to be applied on-chain, a companion script is missing.

---

Minor Issues

Unused constant in UpdateHyperEVMLZConfig.s.sol

uint32 constant EXECUTOR_CONFIG_TYPE = 1;  // defined but never referenced

Can be removed, or used if executor configs will ever need updating via this script.

No post-execution config verification

The script applies endpoint.setConfig but never reads back the config to confirm the change landed. For an on-chain correction script, a trailing getConfig + console2.log sanity check would help operators confirm success and is standard practice in Superform scripts.

assert() vs Forge test assertions in SuperBankMerkleTreeValidation.t.sol

The _verifyLeaf helper uses raw Solidity assert():

assert(computedLeaf == expectedLeafHash);
assert(MerkleProof.verify(proof, root, computedLeaf));

Failures from assert() produce no context (no "expected X, got Y" output). Prefer Forge's assertEq / assertTrue with a message parameter so failures point directly to the failing leaf. The existing negative tests at the bottom of the file already use assert(!MerkleProof.verify(...))) — same issue.

Slippage calculation is opaque

In two places:

decoded.tokenInfo.outputMin - decoded.tokenInfo.outputMin * 1e4 / 1e5  // extra slippage buffer

This applies a 10% extra haircut on top of whatever Odos already computed for outputMin. The intent is to avoid test failures on volatile forks, but without a comment explaining the formula, it looks like an arithmetic mistake. A short inline comment clarifying "subtract 10% to widen slippage tolerance" would help reviewers and future editors.

Live API dependency not skippable in CI

SuperBankSwapIntegration calls the Odos API live via surlCallQuoteV2 / surlCallAssemble. The PR description acknowledges flakiness risk. Consider wrapping these tests with a skip guard:

function setUp() public {
    string memory baseRpc = vm.envOr("BASE_RPC_URL", string(""));
    if (bytes(baseRpc).length == 0) {
        vm.skip(true);
        return;
    }
    ...
}

This lets developers run the tests intentionally while keeping CI green when the RPC is unavailable.

---

Positive Notes

• The comment block in DeployUpOFT.s.sol explaining the LZ symmetric confirmation requirement is excellent — it directly documents why the bug existed and what the correct invariant is.
• Splitting the config objects into distinctly-named send/receive structs (e.g., ulnHyperEVMReceiveFromEth vs ulnHyperEVMToEth) eliminates the category of bug being fixed here and makes the intent self-documenting.
• The SuperBankMerkleTreeValidation test is a clean, dependency-free approach to validating production merkle trees. Testing both first and last leaves of large trees is a good heuristic.
• The _forceGrantRole helper clearly documents the OZ 5.x storage layout it targets, which will help future maintainers if the slot scheme ever changes.
• The multi-fork Pigeon relay test in Phase 2/3 provides real cross-chain coverage that's hard to achieve otherwise.

[vikramarun]

lgtm

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[claude]

PR Review: feat/integration-test-superbank-sv-1468

Overall this is a well-structured PR that fixes a real LZ ULN configuration bug and adds solid integration test coverage. A few issues worth addressing below.

---

script/DeployUpOFT.s.sol

Positive:

• The send/receive split is correct LZ V2 behavior. The comment block mapping each pathway to its confirmation counts is excellent documentation.
• Confirmation values are internally consistent (e.g. HL→ETH send=1 matches ETH receive=1, ETH→HL send=15 matches HL receive=15).

Issues:

1. PR description mentions UpdateHyperEVMLZConfig.s.sol but this script isn't in the changeset. The auto-generated summary references it, but the file list only shows DeployUpOFT.s.sol. If this script was intentionally omitted, the description should be corrected to avoid confusion.

2. configureLibrariesOnHyperEVM only configures the ETH pathway (dstEid/srcEid = ETH_EID). For a complete fix, the HyperEVM↔Base pathway also needs its libraries configured with the new receive configs — configureLibrariesOnHyperEVMForBase handles that separately, but it's worth confirming both are run as part of the on-chain migration.

---

test/integration/SuperBank/SuperBankMerkleTreeValidation.t.sol

Positive:

• Pure tests (no fork) provide fast, deterministic proof validation.
• Testing both first and last leaf for large trees is good boundary coverage.
• Negative tests (test_invalidLeaf_failsMerkleProof, test_wrongRoot_failsMerkleProof) are well-designed.

Issues:

3. _verifyLeaf uses raw assert() instead of Forge assertions. On failure, assert() just reverts with no message, making debugging harder. Prefer:

   assertTrue(computedLeaf == expectedLeafHash, "Leaf hash mismatch");
   assertTrue(MerkleProof.verify(proof, root, computedLeaf), "Merkle proof verification failed");

4. Stale merkle tree for Base SwapOdosV2Hook. The validation test validates a 3-leaf tree with root 0x1f04c759..., but SuperBankSwapIntegration uses a 4-leaf production root 0xa5317c91... (with an additional executor 0xbF44De8f... not present in the validation tree). The validation test is testing an outdated version of the tree. It should be updated to match the production root used in the integration test, or a comment should clearly explain why they diverge.

---

test/integration/SuperBank/SuperBankSwapIntegration.t.sol

Positive:

• Excellent end-to-end coverage: single-leaf tree, production tree, and multi-chain bridge+swap scenarios.
• Separating the bridge/relay/swap phases into _bridgeUsdcFromEthToBase, _relayAcrossBridge, and _swapUsdcToUpOnBase keeps the test readable.
• Dynamically selecting the merkle proof based on decoded.executor is the right approach given Odos can return different executors per quote.

Issues:

5. _getBaseSwapOdosExecutorProof will silently fail CI if Odos returns an unknown executor. The revert message "Unknown Odos executor - not in SwapOdosV2Hook tree" is correct behavior, but if Odos deploys a new executor this test will produce a confusing failure. Consider adding a console2.log of decoded.executor before the proof selection so it's immediately visible in the failure output.

6. Magic number in slippage buffer (lines 137, 219, 380): outputMin - outputMin * 1e4 / 1e5 applies an additional 10% buffer but is non-obvious. A named constant or comment would improve readability:

   // Apply 10% additional buffer on top of Odos's slippage tolerance
   decoded.tokenInfo.outputMin * 9 / 10

7. _forceGrantRole hardcodes OZ 5.x AccessControl storage layout. The comment explaining the slot derivation is good, but if the contract is upgraded or the OZ version changes, this will break silently (role granted to wrong slot). Consider adding a post-condition check that hasRole returns true before proceeding — this is already done via assertTrue(superGovernor.hasRole(...)) in setUp(), so this is mostly fine, but the ETH fork branch in _bridgeUsdcFromEthToBase doesn't have those sanity checks after _forceGrantRole.

8. Minor: double blank line between SWAP_ODOS_V2_HOOK and // Tokens constants (line 40–41).

---

Test Coverage Summary

Scenario	Covered
Merkle proof correctness (pure, no fork)	✅
USDC→UP swap with single-leaf tree	✅
USDC→UP swap with production merkle tree	✅
ETH→Base bridge + swap (multi-fork)	✅
Invalid merkle leaf	✅
Wrong merkle root	✅
Slippage failure (outputMin not satisfied)	❌ Not covered

The live Odos API dependency (points 1/5 in the PR description risk section) is the main CI concern — tests 2 and 3 will flake if the API is unavailable or returns an unexpected executor. These should ideally be gated behind a separate CI job or tagged to only run on-demand.

---

Summary: The LZ config fix in DeployUpOFT.s.sol is correct and well-documented. The test infrastructure is solid. The main actionable items are: (1) update SuperBankMerkleTreeValidation to match the production 4-leaf Base SwapOdosV2 tree used in the integration test, (2) switch assert() to Forge assertions in _verifyLeaf, and (3) add console2.log of the executor before proof selection for easier debugging.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[claude]

Code Review — PR #299

Overview: Good PR that fixes an important LZ ULN send/receive config bug and adds high-value integration tests. A few reliability and quality concerns worth addressing.

---

script/DeployUpOFT.s.sol

[Fix — Correct] Splitting send vs. receive ULN configs is the right approach. The original code was passing the send config struct to the receive config setter for all HyperEVM pathways, which would have misconfigured confirmations on the receiving side. The new structure is clearly documented and correct.

[Info] The Base→HL send confirmations changed from 15 → 10. This is a substantive security parameter reduction. The inline comments explain the intended symmetry (Base send=10, HL receive=10), but a brief note on why 10 specifically for Base (vs the previous 15) would help auditors. Is this based on Base's block finality SLA?

[Info] The PR description mentions a new UpdateHyperEVMLZConfig.s.sol script and a shell wrapper, but neither appears in the diff. If they exist in a separate PR, this should be noted; if they're still pending, a follow-up PR link would be useful.

---

test/integration/SuperBank/SuperBankMerkleTreeValidation.t.sol

[Low — Style] _verifyLeaf uses raw assert() for both the leaf hash check and the merkle proof check. In Foundry, assert() fails silently with only a revert — no expected/actual values. Prefer:

assertEq(computedLeaf, expectedLeafHash, "leaf hash mismatch");
assertTrue(MerkleProof.verify(proof, root, computedLeaf), "merkle proof invalid");

This significantly improves debug output on failures.

[Good] The positive/negative test split, the pure annotation (no fork needed), and covering first/last leaves of each production tree are all solid choices.

---

test/integration/SuperBank/SuperBankSwapIntegration.t.sol

[Medium — Reliability] Several tests call live Odos API endpoints via surlCallQuoteV2 / surlCallAssemble at test-time. This creates multiple failure modes in CI:

• Odos API rate limits or downtime → test failure
• Odos routing changes (e.g. new executor address) → revert("Unknown Odos executor - not in SwapOdosV2Hook tree")
• Quote price drift between fetch and execution on the forked state

Consider gating these behind an env variable (e.g. RUN_LIVE_ODOS_TESTS) and skipping with vm.skip(true) when not set, so CI remains stable unless explicitly opted-in.

[Medium — Hard Failures vs. Graceful Skips] _getBaseSwapOdosExecutorProof, _getApproveAndSwapWethToUsdcProof, and _getApproveAndSwapWbtcToUsdcProof all end with revert("Unknown Odos executor") when an unseen executor/inputReceiver combination is returned. This makes the failure mode opaque — CI shows an unexpected revert rather than a clear message that the tree needs updating. A vm.skip(true) with a descriptive message would be more actionable:

} else {
    // New executor from Odos not yet in the merkle tree — add it and regenerate proofs
    vm.skip(true);
}

[Low — Slippage] The expression decoded.tokenInfo.outputMin - decoded.tokenInfo.outputMin * 1e4 / 1e5 subtracts an additional 10% from Odos' already-computed minimum output. This is on top of whatever slippage tolerance Odos already baked in, so the effective accepted slippage could be significant. A comment clarifying the intent (e.g. "10% buffer on top of Odos min to prevent fork-state drift failures") would prevent future readers from misreading this as the primary slippage setting.

[Low — Maintainability] _getApproveAndSwapWethToUsdcProof and _getApproveAndSwapWbtcToUsdcProof use long if-else chains keyed on executor/inputReceiver address pairs. As the production merkle tree grows this will become unwieldy. A mapping from bytes32 key → bytes32[] proof populated in a helper initializer would scale better long term.

[Low — Duplicate coverage] test_executeHooks_swapUSDCtoUP and test_executeHooks_swapUSDCtoUP_onBase appear to test the same Base USDC→UP flow (one with a single-leaf ad-hoc root, one with the production root). If the intent of the first is only to validate the dynamic-root flow without caring about the specific root, that distinction should be noted in the NatSpec. Otherwise consolidating to the production-root test is preferable.

[Info — Setup] _forceGrantRole uses direct storage writes to bypass access control on production contracts in the fork. This is a reasonable fork-test pattern but is fragile if the storage layout of SuperGovernor changes. Worth noting in the function's NatSpec that the slot offset must be kept in sync with the deployed contract.

---

Summary

Area	Severity	Item
DeployUpOFT	Info	Confirm why Base→HL confirmations changed from 15→10
DeployUpOFT	Info	UpdateHyperEVMLZConfig.s.sol not in diff
MerkleValidation	Low	Use assertEq/assertTrue instead of assert()
SwapIntegration	Medium	Live Odos API calls make CI flaky — consider env-var gating
SwapIntegration	Medium	Proof lookup reverts hard-fail CI; use vm.skip instead
SwapIntegration	Low	Slippage calculation warrants an explanatory comment
SwapIntegration	Low	Long if-else proof tables will need refactoring as tree grows
SwapIntegration	Low	Consolidate or clarify duplicate USDC→UP test variants

Overall the production ULN fix is correct and well-documented, and the integration test coverage is a meaningful addition. The main concern is the CI reliability of the live-API-dependent tests — strongly recommend env-var gating or vm.skip guards before merging to avoid blocking the main branch on Odos API availability.

[claude]

Follow-up Review: latest commit (2026-03-13) — V2 ApproveAndSwap hooks

Prior reviews covered the LZ ULN config fix and initial test suite. This is a focused update on what is new in the latest commit and issues that remain open.

---

Items from prior reviews still not addressed

1. assert() in _verifyLeaf (SuperBankMerkleTreeValidation.t.sol:252-257)

Raw Solidity assert() produces no diagnostic output on failure. Please replace with Forge assertions:

• assertEq(computedLeaf, expectedLeafHash, "leaf hash mismatch");
• assertTrue(MerkleProof.verify(proof, root, computedLeaf), "merkle proof invalid");

2. Live Odos API calls without skip guards (SuperBankSwapIntegration.t.sol:651)

setUp() calls vm.envString("BASE_RPC_URL") which hard-fails if unset, and all tests call surlCallQuoteV2/surlCallAssemble with no availability guard. When the Odos API or RPC is unavailable in CI, the failure message is cryptic. The standard Foundry pattern is vm.envOr + vm.skip(true).

3. Slippage formula still uncommented (appears 8+ times)

outputMin - outputMin * 1e4 / 1e5 has no inline explanation. A comment like "10% extra buffer on top of Odos slippage to absorb fork-state price drift" would prevent this from being read as an arithmetic mistake.

---

New issues in the latest commit

4. V2 hook address constants placed mid-contract

V2_APPROVE_AND_SWAP_ODOS_V2_HOOK and BASE_V2_APPROVE_AND_SWAP_ODOS_V2_HOOK are declared at the bottom of the contract body, not in the "PRODUCTION ADDRESSES" block at the top with the other constants. Move them up for consistency.

5. Proof helpers still revert on unknown executor

_getV2ApproveAndSwapExecutorProof and _getBaseV2ApproveAndSwapExecutorProof end with hard reverts for unrecognised executors, same as the _getBaseSwapOdosExecutorProof pattern flagged in prior reviews. When Odos returns a new executor the CI failure appears as an unexpected revert with no context. Prefer vm.skip(true) with a message like "Odos returned new executor not yet in merkle tree — regenerate proofs".

6. Missing post-condition checks in fork-switch setup helpers

setUp() for the Base fork includes assertTrue(superGovernor.hasRole(...)) sanity checks after _forceGrantRole. The new _setupEthForkForV2ApproveAndSwap() and _setupBaseForkForV2ApproveAndSwap() helpers call _forceGrantRole but omit those checks. If the OZ 5.x storage slot ever changes, the fork tests would silently pass role setup and fail later with a confusing access-control revert instead of a clear assertion.

7. New V2 hook trees not validated in SuperBankMerkleTreeValidation.t.sol

The V2 ApproveAndSwapOdosV2Hook deployments (ETH 0x067696e1..., Base 0x3E10d410...) have new merkle roots but the pure validation test file was not updated. The executor-only leaf structure is simpler than the existing trees, so adding first/last leaf tests is straightforward and keeps the file as the canonical proof-correctness reference.

---

Summary

#	File	Severity	Status
1	SuperBankMerkleTreeValidation	Low	Still open
2	SuperBankSwapIntegration	Medium	Still open
3	SuperBankSwapIntegration	Low	Still open
4	SuperBankSwapIntegration	Low	New in latest commit
5	SuperBankSwapIntegration	Medium	New in latest commit
6	SuperBankSwapIntegration	Low	New in latest commit
7	SuperBankMerkleTreeValidation	Low	New in latest commit

The LZ ULN config fix in DeployUpOFT.s.sol remains correct and well-documented. Items 2 and 5 (live API CI reliability + hard reverts on unknown executors) are the highest-value fixes before merge.

[claude]

PR Review — SuperBank Integration Tests + HyperEVM LZ Config [SV-1468]

I'll focus on issues not already covered by the existing reviews.

---

Correctness

Merkle tree mismatch between validation and integration tests

SuperBankMerkleTreeValidation.t.sol validates the Base SwapOdosV2Hook tree with root 0x1f04c759..., which contains executors:

• 0x6131B5fa..., 0xd4F48096..., 0x365084B0..., 0xCf5540fF...

But SuperBankSwapIntegration.t.sol sets root 0xa5317c91... in _setMerkleRoot(SWAP_ODOS_V2_HOOK, ...) and _getBaseSwapOdosExecutorProof branches on executors:

• 0x19cEeAd7..., 0xbF44De8f..., 0x6131B5fa..., 0xd4F48096...

These are two different generations of the production tree. Executors 0x365084B0 and 0xCf5540fF appear in the validation test but not in the integration test tree; 0x19cEeAd7 (Odos Router itself) and 0xbF44De8f appear in the integration test but not in the validation test. If the validation test is intended to document and verify the currently active production tree, it's now testing a stale snapshot. Consider aligning both tests to the same tree or clearly commenting that the validation test covers the previous deployment.

_getBaseSwapOdosExecutorProof can silently fail for old executor 0x365084B0...

If Odos quotes return executor 0x365084B05Fa7d5028346bD21D842eD0601bAB5b8 (which is in the validation tree but absent from _getBaseSwapOdosExecutorProof's switch), the function hits the revert("Unknown Odos executor") path, causing the test to fail with a non-obvious message. This isn't a production issue but could confuse developers debugging a test failure.

---

Minor Issues

Cumulative vm.warp in multi-step cross-chain test

test_executeHooks_bridgeAndSwapUSDCtoUP internally calls _setMerkleRoot four times (two on ETH fork, two on Base fork), each warping forward 7 days + 1 second. Total time advance: ~28 days. This is harmless today but if any contract (e.g. a rate-limiting hook or a price oracle using block.timestamp) is sensitive to wall-clock time, this test can produce misleading results. A single vm.warp call before the first propose/execute cycle would be cleaner and more explicit.

optionalDVNCount: type(uint8).max lacks a clarifying comment

In every new UlnConfig struct:

optionalDVNCount: type(uint8).max,
optionalDVNThreshold: 0,
optionalDVNs: new address[](0)

type(uint8).max (255) is the LayerZero V2 sentinel meaning "use the default / no optional DVN constraint". Without a comment, it reads as if 255 optional DVNs are expected, which would alarm a reviewer who's unfamiliar with the LZ ULN encoding. A one-line comment (// sentinel: use default optional DVN settings) would make the intent self-documenting, consistent with the excellent send/receive comment block already added above.

Stale SwapOdosV2Hook merkle root in test_executeHooks_swapUSDCtoUP_withProductionMerkleTree

The comment says:

SwapOdosV2Hook  (0x074F9973...): root 0x1f04c759..., leaf 0  (executor=Odos Router)

But the root actually set in code is 0xa5317c91.... The comment refers to the old tree. Update the comment to match.

---

Positive Notes

• The send/receive config split in DeployUpOFT.s.sol is the right fix; the asymmetry between the old ulnHyperEVMToEth being used for receive was a real correctness bug.
• The pathway confirmation table comment is excellent operational documentation.
• SuperBankMerkleTreeValidation is a clean, dependency-free smoke test for the leaf hashing invariant.
• The _forceGrantRole helper with explicit OZ 5.x slot documentation is good — it'll save time for whoever needs to maintain it.
• The multi-fork Pigeon relay covering the full ETH→Base→swap path provides coverage that's otherwise very hard to achieve.