refactor: LayerZero

[supercontracts]

Summary by CodeRabbit

• Bug Fixes

  • Corrected a typographical error in documentation describing trust assumptions related to withdrawal validation and admin controls.

• Code Improvements

  • Restructured internal token transfer architecture to improve code organization, enhance maintainability, and facilitate better code modularity and reusability. All existing functionality and public-facing interfaces have been fully preserved without changes to user experience or features.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

• 🔍 Trigger a full review

Walkthrough

The PR refactors LayerZero token transfer logic by extracting multi-step manual operations from MainnetController into a new LayerZeroLib library, consolidating recipient validation, rate limiting, approvals, fee quoting, and sending into a single library function. A typo in README.md is also corrected.

Changes

Cohort / File(s)	Summary
Documentation README.md	Fixed typographical error: "invalided" → "invalidated" in trust assumptions regarding admin withdrawal invalidation.
Core Refactoring src/MainnetController.sol, src/libraries/LayerZeroLib.sol	Extracted LayerZero transfer logic from MainnetController into new LayerZeroLib.transferTokenLayerZero() function; replaces inlined multi-step quote/fee/send flow with library-driven implementation encapsulating proxy, rate limits, OFT address, amount, destination endpoint, rate limit ID, and recipient handling; removes direct OptionsBuilder and LayerZero send calls from controller.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• spark-alm-controller#200: Modifies LayerZero token transfer flow by centralizing recipient lookup and adding non-zero recipient validation—shares similar control flow changes with this refactoring.
• spark-alm-controller#162: Updates LayerZero token transfer and approval handling using ApproveLib patterns—directly related to the approval and transfer logic now consolidated in LayerZeroLib.

Suggested reviewers

• supercontracts

Poem

🐰 Transfer logic hops from place to place,
LayerZeroLib now takes the race,
Rate limits guard and approvals flow,
One function calls what once was slow,
A tidy warren, clean and bright! 🌟

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor: LayerZero' follows the required format (prefix with colon) and is directly related to the main change—consolidating LayerZero logic into a dedicated library.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/under-size

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[lucas-manuel]

@coderabbitai review this and ensure equivalent functionality with existing code

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Get PR details and list of changed files
echo "=== PR Details ==="
gh pr view 216 --json title,body,files --jq '{title, body, files: [.files[].path]}'

echo -e "\n=== Changed Files ==="
gh pr diff 216 --name-only

echo -e "\n=== Full Diff ==="
gh pr diff 216

Length of output: 8239

---

🏁 Script executed:

#!/bin/bash

# Get the RateLimitHelpers contract to understand _rateLimited implementation
echo "=== RateLimitHelpers.sol ==="
cat src/RateLimitHelpers.sol

# Check if there are any tests for the transferTokenLayerZero function
echo -e "\n=== Searching for tests ==="
fd -e sol -e t.sol . test/ | xargs grep -l "transferTokenLayerZero" 2>/dev/null || echo "No test files found"

# Check the full context of MainnetController to see rateLimits state variable
echo -e "\n=== MainnetController state variables (focusing on rateLimits and proxy) ==="
sed -n '/contract MainnetController/,/function/p' src/MainnetController.sol | head -100

Length of output: 6049

---

🏁 Script executed:

#!/bin/bash

# Search for _rateLimited function in MainnetController
echo "=== Looking for _rateLimited in MainnetController ==="
grep -n "_rateLimited" src/MainnetController.sol | head -20

# Get the full _rateLimited implementation from MainnetController
echo -e "\n=== _rateLimited implementation in MainnetController ==="
ast-grep --pattern 'function _rateLimited($$$) {
  $$$
}'

# Also check the entire file for the pattern more broadly
echo -e "\n=== Searching for rate limit helper function pattern ==="
rg -A 5 "function _rateLimited" src/MainnetController.sol

Length of output: 2676

---

🏁 Script executed:

#!/bin/bash

# Look at the LayerZero test file to understand the test coverage
echo "=== LayerZero.t.sol test file ==="
cat test/mainnet-fork/LayerZero.t.sol

Length of output: 21899

---

@lucas-manuel I've completed a thorough review of this LayerZero refactoring. The refactored code maintains full equivalence with the original implementation. ✅

Equivalence Analysis

Execution Flow Comparison

Original (MainnetController.sol):

1. _checkRole(RELAYER) → Role validation
2. _rateLimited(...) → Rate limit check using state variables
3. layerZeroRecipients[destinationEndpointId] → Retrieve recipient
4. require(recipient != bytes32(0), ...) → Validate recipient
5. ApproveLib.approve(...) → Conditional approval
6. Build SendParam struct
7. Query OFT and update minAmountLD
8. Get fee quote
9. Execute proxy.doCallWithValue(...)

Refactored (MainnetController + LzLib):

1. _checkRole(RELAYER) → Role validation (stays in controller)
2. Build params struct with state variables (proxy, rateLimits, LIMIT_LAYERZERO_TRANSFER, layerZeroRecipients[destinationEndpointId])
3. LzLib.transferTokenLayerZero(...) executes same logic:
   • Rate limit check with passed parameters
   • Recipient validation
   • Conditional approval
   • Build SendParam struct
   • Query OFT and update minAmountLD
   • Get fee quote
   • Execute proxy.doCallWithValue(...)

Key Verifications

✅ Rate Limiting: Identical key construction and validation

// Both construct: keccak256(abi.encode(LIMIT_LAYERZERO_TRANSFER, oftAddress, destinationEndpointId))
// Both call: rateLimits.triggerRateLimitDecrease(key, amount)

✅ Recipient Validation: Same check, same error message

// Both: require(recipient != bytes32(0), "MC/recipient-not-set")

✅ Approval Logic: Identical conditional approval with same parameters

✅ Options: Same constants (200_000, 0)

✅ SendParam Construction: All fields identical

✅ Quote Operations: Same sequence and usage

✅ Final Call: Identical doCallWithValue with same parameters

Test Coverage

The existing test suite in test/mainnet-fork/LayerZero.t.sol validates:

• ✅ Reentrancy protection (remains at controller level)
• ✅ Access control (RELAYER role check)
• ✅ Rate limiting (zero max, boundary conditions)
• ✅ Recipient validation
• ✅ Full success flow with event emission
• ✅ Tests pass for both MainnetController and ForeignController

Benefits of Refactoring

1. Code Reusability: ForeignController can use the same library
2. Better Organization: LayerZero logic separated into dedicated library
3. Cleaner Imports: LayerZero dependencies isolated to LzLib
4. Maintainability: Single source of truth for LayerZero transfer logic

Conclusion

This is a safe, clean refactoring with zero functional changes. All state transitions, external calls, validations, and error handling remain identical. The only differences are organizational improvements.

The base branch was changed.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/libraries/LayerZeroLib.sol`:
- Around line 49-58: The comment flags that the branch where
ILayerZero(params.oftAddress).approvalRequired() == false is untested; either
verify in deployment that no OFT implements approvalRequired() returning false
or add an integration test that sets up an OFT mock returning false and
exercises the code path in LayerZeroLib (call the function that contains the
approvalRequired check using params.oftAddress/params.amount/params.proxy) to
ensure behavior is correct when approval is not required; if behavior must
change, update the logic around ILayerZero.approvalRequired() and
ApproveLib.approve usage accordingly and document the assumption in
LayerZeroLib.sol.
- Line 34: The controller's bytecode references the library LayerZeroLib because
transferTokenLayerZero is declared external in that library; update your
deployment tooling to deploy LayerZeroLib on each target chain and link its
address into the controller before deploying/verifying the controller contract.
Concretely, modify your deploy scripts (e.g., the Hardhat deploy task or Truffle
migrations) to: 1) deploy LayerZeroLib first, 2) inject or link the returned
library address into the controller artifact (use artifact.linkLibrary or
solc/library linking settings), and 3) then deploy and verify the controller
using the linked bytecode; also ensure your per-chain config stores the
LayerZeroLib address so Etherscan/verification steps use the linked library
address. Ensure references to transferTokenLayerZero and LayerZeroLib are the
ones updated in the scripts.

📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13894e3 and 4621780.

📒 Files selected for processing (3)

• README.md
• src/MainnetController.sol
• src/libraries/LayerZeroLib.sol

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-16T16:47:34.627Z

Learnt from: supercontracts
Repo: sparkdotfi/spark-alm-controller PR: 170
File: src/MainnetController.sol:1153-1181
Timestamp: 2025-10-16T16:47:34.627Z
Learning: In src/MainnetController.sol, the USDS token always returns true on successful transfer operations, so it does not require the empty return data handling that some non-standard ERC20 tokens (like USDT) need.

Applied to files:

• src/MainnetController.sol

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: build
• GitHub Check: test
• GitHub Check: coverage

🔇 Additional comments (6)

README.md (1)

144-144: Doc typo fix looks good.

Clearer wording; no further changes needed.

src/libraries/LayerZeroLib.sol (2)

20-28: Parameter struct keeps the call-site tidy.

Nice consolidation of transfer inputs into a single struct.

---

90-92: Rate-limit helper wrapper is clean.

Simple delegation keeps the call sites readable.

src/MainnetController.sol (3)

19-25: LayerZeroLib import aligns with the refactor.

No issues with the new dependency.

---

502-506: Signature formatting change is fine.

No behavioral impact.

---

970-978: Library call wiring looks consistent—please re-run LayerZero fork coverage.

Field mapping into LayerZeroLib.TransferTokenLayerZeroParams matches the previous inline flow; please confirm test/mainnet-fork/LayerZero.t.sol still exercises this path after the refactor.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Verify library linking in deployment tooling.

transferTokenLayerZero is external in a library, so controller bytecode will require a linked library address. Please confirm build/deploy scripts deploy and link LayerZeroLib for every target chain.

🤖 Prompt for AI Agents

In `@src/libraries/LayerZeroLib.sol` at line 34, The controller's bytecode
references the library LayerZeroLib because transferTokenLayerZero is declared
external in that library; update your deployment tooling to deploy LayerZeroLib
on each target chain and link its address into the controller before
deploying/verifying the controller contract. Concretely, modify your deploy
scripts (e.g., the Hardhat deploy task or Truffle migrations) to: 1) deploy
LayerZeroLib first, 2) inject or link the returned library address into the
controller artifact (use artifact.linkLibrary or solc/library linking settings),
and 3) then deploy and verify the controller using the linked bytecode; also
ensure your per-chain config stores the LayerZeroLib address so
Etherscan/verification steps use the linked library address. Ensure references
to transferTokenLayerZero and LayerZeroLib are the ones updated in the scripts.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Confirm the untested approvalRequired == false branch before production use.

The inline note indicates missing integration coverage for this path. Please verify no deployed OFT returns false, or add a targeted integration test (happy to help).

🤖 Prompt for AI Agents

In `@src/libraries/LayerZeroLib.sol` around lines 49 - 58, The comment flags that
the branch where ILayerZero(params.oftAddress).approvalRequired() == false is
untested; either verify in deployment that no OFT implements approvalRequired()
returning false or add an integration test that sets up an OFT mock returning
false and exercises the code path in LayerZeroLib (call the function that
contains the approvalRequired check using
params.oftAddress/params.amount/params.proxy) to ensure behavior is correct when
approval is not required; if behavior must change, update the logic around
ILayerZero.approvalRequired() and ApproveLib.approve usage accordingly and
document the assumption in LayerZeroLib.sol.

[github-actions]

Coverage after merging refactor/under-size into dev will be

99.40%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
deploy
ControllerDeploy.sol	100%	100%	100%	100%
ForeignControllerInit.sol	100%	100%	100%	100%
MainnetControllerInit.sol	97.37%	93.33%	100%	100%	152, 90
src
ALMProxy.sol	100%	100%	100%	100%
ALMProxyFreezable.sol	100%	100%	100%	100%
ForeignController.sol	94.90%	84.62%	95.65%	97.22%	128–129, 129, 129, 316–317, 573
MainnetController.sol	99.18%	100%	98.36%	99.24%	583–584
OTCBuffer.sol	100%	100%	100%	100%
RateLimitHelpers.sol	100%	100%	100%	100%
RateLimits.sol	100%	100%	100%	100%
WeEthModule.sol	92.86%	75%	100%	100%	26, 38
src/libraries
AaveLib.sol	100%	100%	100%	100%
ApproveLib.sol	100%	100%	100%	100%
CCTPLib.sol	100%	100%	100%	100%
CurveLib.sol	100%	100%	100%	100%
ERC4626Lib.sol	96%	75%	100%	100%	108
LayerZeroLib.sol	100%	100%	100%	100%
PSMLib.sol	100%	100%	100%	100%
UniswapV4Lib.sol	99.33%	96%	100%	100%	284
WeETHLib.sol	100%	100%	100%	100%