feat: Spell 2026-08-13 (SC-1605) - #182
Conversation
Summary by OctaneNew Contracts
Updated Contracts
🔗 Commit Hash: b9013b1 |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChangesSpark spell deployments
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Overview
🔗 Commit Hash: b9013b1 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
archive/20260716/SparkEthereum_20260716.sol (1)
52-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
LZ_ENDPOINT_XLAYERto reflect that it holds a LayerZero endpoint ID.The value
30274is a LayerZero endpoint ID (EID), not an endpoint address. The test file usesLZ_EID_XLAYERfor the same concept. Use one term for the same concept.♻️ Proposed rename
- uint32 internal constant LZ_ENDPOINT_XLAYER = 30274; + uint32 internal constant LZ_EID_XLAYER = 30274;Update the two usages on lines 80 and 85 accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/SparkEthereum_20260716.sol` at line 52, Rename the constant LZ_ENDPOINT_XLAYER to LZ_EID_XLAYER and update both references on lines 80 and 85 to use the renamed LayerZero endpoint ID symbol.archive/20260716/XLayer_20260716.t.sol (3)
304-311: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_assertUnlimitedRateLimitis never called.No test in
XLayerE2ETestscalls this helper. TherateLimitsmember set on line 258 is also only read by this helper. The unlimited rate limits are already asserted inXLayerConfigTests.test_rateLimits_configon lines 163-184.Either delete the helper, or add a test that uses it for the rate-limit keys not covered by
test_rateLimits_config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/XLayer_20260716.t.sol` around lines 304 - 311, Remove the unused _assertUnlimitedRateLimit helper and the rateLimits member used only by it, unless adding assertions for otherwise uncovered rate-limit keys in XLayerE2ETests; avoid duplicating coverage already provided by XLayerConfigTests.test_rateLimits_config.
402-403: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
L1_EXECUTORis unused and the step comment names the wrong target.Line 402 declares
L1_EXECUTOR, but the call on line 461 targetsEthereum.SPARK_PROXY. The comment on line 458 states thatL1_PAUSE_PROXYtriggersL1_EXECUTOR, which does not match the code.Delete the unused constant and correct the comment to name
Ethereum.SPARK_PROXY.♻️ Proposed fix
// Ethereum mainnet governance contracts - address constant L1_EXECUTOR = 0x3300f198988e4C9C63F75dF86De36421f06af8c4; address constant L1_PAUSE_PROXY = 0xBE8E3e3618f7474F8cB1d074A26afFef007E98FB;- // Step 3: L1_PAUSE_PROXY triggers L1_EXECUTOR to execute the crosschain payload. + // Step 3: L1_PAUSE_PROXY makes Ethereum.SPARK_PROXY execute the crosschain payload.Also applies to: 458-464
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/XLayer_20260716.t.sol` around lines 402 - 403, Remove the unused L1_EXECUTOR constant and update the step comment near the call to identify Ethereum.SPARK_PROXY as the target, matching the actual invocation while retaining L1_PAUSE_PROXY only where it is used.
88-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the hardcoded RPC URL with a configured endpoint.
Three fork setups embed
https://rpc.xlayer.techdirectly. This bypasses the repository RPC configuration, so CI cannot point the tests at a private or archival node, and a public-endpoint outage fails the suite.Use an alias resolved through
foundry.tomlrpc_endpoints, or read the URL from an environment variable withvm.envOr.Run the following script to check how other tests in the repository resolve fork URLs:
#!/bin/bash # Inspect declared RPC endpoints and existing fork-creation patterns. fd -t f 'foundry.toml' --exec cat -n rg -nP 'createSelectFork|createFork|rpcUrl' --iglob '*.sol' -C2 | head -80Also applies to: 253-254, 425-429
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/XLayer_20260716.t.sol` around lines 88 - 89, Replace the hardcoded https://rpc.xlayer.tech URL in all three fork setup blocks, including setUp and the locations around the other referenced ranges, with the repository’s configured RPC endpoint resolution. Reuse the established foundry.toml rpc_endpoints alias or an environment-variable fallback via vm.envOr, matching existing fork-creation patterns and preserving each fork’s block number.archive/20260716/Spell_20260716.t.sol (1)
235-238: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the tautological assertions.
Several assertions compare a value against the variable that was just assigned from it, so they always pass:
- Line 237 compares
usdt0XLayer.balanceOf(XLayer.ALM_PROXY)withxLayerProxyUsdt0Starting, assigned on line 235.- Lines 278-279 compare the two mainnet balances with
ethProxyUsdtStartingandethOFTUsdtStarting, assigned on lines 275-276.- Line 323 repeats the assertion already made on line 311 with no intervening state change.
Delete these lines. Keep the assertions that check a balance change.
Also applies to: 275-279, 323-324
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/Spell_20260716.t.sol` around lines 235 - 238, Remove the tautological balance assertions in the test: the ALM proxy assertion using xLayerProxyUsdt0Starting, the mainnet assertions using ethProxyUsdtStarting and ethOFTUsdtStarting, and the duplicate assertion repeated after the earlier check. Preserve assertions that verify actual balance changes, including the vault balance assertion.archive/20260716/SparkRobinhood_20260716.sol (1)
13-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThree files redeclare the ALM freezable proxy interface.
src/interfaces/Interfaces.sollines 15-21 already declareIALMProxyFreezableLike, but it lacksgrantRoleandrevokeRole. Each new file works around that by declaring its own copy, so the same role surface now exists in four places with four different member sets. Extend the shared interface once, then import it.
archive/20260716/SparkRobinhood_20260716.sol#L13-L21: delete the local interface and addIALMProxyFreezableLiketo the existing named import on line 11.archive/20260716/SparkXLayer_20260716.sol#L6-L14: delete the local interface and importIALMProxyFreezableLikefrom../../interfaces/Interfaces.sol.archive/20260716/XLayer_20260716.t.sol#L41-L51: deleteIXLayerALMProxyFreezableand use the sharedIALMProxyFreezableLike, which already declaresDEFAULT_ADMIN_ROLE,ALLOCATOR_ROLE,FREEZER_ROLE, andhasRole.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/SparkRobinhood_20260716.sol` around lines 13 - 21, Extend the shared IALMProxyFreezableLike interface in src/interfaces/Interfaces.sol with grantRole and revokeRole, then remove the duplicate local interfaces: in archive/20260716/SparkRobinhood_20260716.sol lines 13-21, add IALMProxyFreezableLike to the existing named import; in archive/20260716/SparkXLayer_20260716.sol lines 6-14, import it from ../../interfaces/Interfaces.sol; and in archive/20260716/XLayer_20260716.t.sol lines 41-51, delete IXLayerALMProxyFreezable and use the shared interface.src/proposals/20260813/Spell_20260813.t.sol (1)
56-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the inherited state-view constant and normalization helpers.
SparkLiquidityLayerTestsalready definesUNISWAP_V4_STATE_VIEWwith the same address, and it already defines_toNormalizedAmount/_fromNormalizedAmountforaddresstokens. This contract inherits both. The local copies can drift from the harness values when the state view address or the decimal handling changes.Use the inherited constant, and convert
Currencytoaddressat the call sites.♻️ Proposed change
uint256 internal constant USDG_BALANCES_SLOT_INDEX = 1; uint256 internal constant _V4_SWAP = 0x10; - - address internal constant _STATE_VIEW = 0x7fFE42C4a5DEeA5b0feC41C94C136Cf115597227;- function _toNormalizedAmount(Currency currency, uint256 amount) - internal view returns (uint256 normalizedAmount) - { - return amount * 1e18 / (10 ** IERC20Metadata(Currency.unwrap(currency)).decimals()); - } - - function _fromNormalizedAmount(Currency currency, uint256 normalizedAmount) internal view returns (uint256 amount) { - return normalizedAmount * (10 ** IERC20Metadata(Currency.unwrap(currency)).decimals()) / 1e18; - } + function _toNormalizedAmount(Currency currency, uint256 amount) + internal view returns (uint256 normalizedAmount) + { + return _toNormalizedAmount(Currency.unwrap(currency), amount); + } + + function _fromNormalizedAmount(Currency currency, uint256 normalizedAmount) internal view returns (uint256 amount) { + return _fromNormalizedAmount(Currency.unwrap(currency), normalizedAmount); + }Then replace
_STATE_VIEWwithUNISWAP_V4_STATE_VIEWin_getCurrentTick(line 216).Also applies to: 381-389
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/proposals/20260813/Spell_20260813.t.sol` at line 56, Remove the locally defined _STATE_VIEW and reuse the inherited UNISWAP_V4_STATE_VIEW constant in _getCurrentTick. Remove the local normalization helpers and use the inherited _toNormalizedAmount and _fromNormalizedAmount helpers, converting Currency values to address at their call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@archive/20260716/Spell_20260716.t.sol`:
- Around line 87-102: Update test_ETHEREUM_sll_deactivateOldMorphoUsdtVault to
verify before _executeAllPayloadsAndBridges that Ethereum.ALM_PROXY holds zero
shares of OLD_MORPHO_VAULT_V2_USDT, using the vault’s established share-balance
query or an existing position-exited assertion.
- Around line 383-388: Update the interest derivation comment near the
USDT0XLayer assertion to use the exact FIVE_PCT_APY value, and align the
USDT0XLayer and Robinhood redemption assertions by using the same approximate
absolute comparison and tolerance for the calculated amount.
---
Nitpick comments:
In `@archive/20260716/SparkEthereum_20260716.sol`:
- Line 52: Rename the constant LZ_ENDPOINT_XLAYER to LZ_EID_XLAYER and update
both references on lines 80 and 85 to use the renamed LayerZero endpoint ID
symbol.
In `@archive/20260716/SparkRobinhood_20260716.sol`:
- Around line 13-21: Extend the shared IALMProxyFreezableLike interface in
src/interfaces/Interfaces.sol with grantRole and revokeRole, then remove the
duplicate local interfaces: in archive/20260716/SparkRobinhood_20260716.sol
lines 13-21, add IALMProxyFreezableLike to the existing named import; in
archive/20260716/SparkXLayer_20260716.sol lines 6-14, import it from
../../interfaces/Interfaces.sol; and in archive/20260716/XLayer_20260716.t.sol
lines 41-51, delete IXLayerALMProxyFreezable and use the shared interface.
In `@archive/20260716/Spell_20260716.t.sol`:
- Around line 235-238: Remove the tautological balance assertions in the test:
the ALM proxy assertion using xLayerProxyUsdt0Starting, the mainnet assertions
using ethProxyUsdtStarting and ethOFTUsdtStarting, and the duplicate assertion
repeated after the earlier check. Preserve assertions that verify actual balance
changes, including the vault balance assertion.
In `@archive/20260716/XLayer_20260716.t.sol`:
- Around line 304-311: Remove the unused _assertUnlimitedRateLimit helper and
the rateLimits member used only by it, unless adding assertions for otherwise
uncovered rate-limit keys in XLayerE2ETests; avoid duplicating coverage already
provided by XLayerConfigTests.test_rateLimits_config.
- Around line 402-403: Remove the unused L1_EXECUTOR constant and update the
step comment near the call to identify Ethereum.SPARK_PROXY as the target,
matching the actual invocation while retaining L1_PAUSE_PROXY only where it is
used.
- Around line 88-89: Replace the hardcoded https://rpc.xlayer.tech URL in all
three fork setup blocks, including setUp and the locations around the other
referenced ranges, with the repository’s configured RPC endpoint resolution.
Reuse the established foundry.toml rpc_endpoints alias or an
environment-variable fallback via vm.envOr, matching existing fork-creation
patterns and preserving each fork’s block number.
In `@src/proposals/20260813/Spell_20260813.t.sol`:
- Line 56: Remove the locally defined _STATE_VIEW and reuse the inherited
UNISWAP_V4_STATE_VIEW constant in _getCurrentTick. Remove the local
normalization helpers and use the inherited _toNormalizedAmount and
_fromNormalizedAmount helpers, converting Currency values to address at their
call sites.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f66e2a47-241c-40ae-9dca-34ee3bfd9644
📒 Files selected for processing (9)
archive/20260716/SparkEthereum_20260716.solarchive/20260716/SparkRobinhood_20260716.solarchive/20260716/SparkXLayer_20260716.solarchive/20260716/Spell_20260716.t.solarchive/20260716/XLayer_20260716.t.soldiffs/20260813-Ethereum-0xC13e21B648A5Ee794902342038FF3aDAB66BE987-pre_20260813-Ethereum-0xC13e21B648A5Ee794902342038FF3aDAB66BE987-post.mdsrc/proposals/20260813/SparkEthereum_20260813.solsrc/proposals/20260813/Spell_20260813.t.solsrc/test-harness/SparkLiquidityLayerTests.sol
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (7)
archive/20260716/SparkEthereum_20260716.sol (1)
52-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
LZ_ENDPOINT_XLAYERto reflect that it holds a LayerZero endpoint ID.The value
30274is a LayerZero endpoint ID (EID), not an endpoint address. The test file usesLZ_EID_XLAYERfor the same concept. Use one term for the same concept.♻️ Proposed rename
- uint32 internal constant LZ_ENDPOINT_XLAYER = 30274; + uint32 internal constant LZ_EID_XLAYER = 30274;Update the two usages on lines 80 and 85 accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/SparkEthereum_20260716.sol` at line 52, Rename the constant LZ_ENDPOINT_XLAYER to LZ_EID_XLAYER and update both references on lines 80 and 85 to use the renamed LayerZero endpoint ID symbol.archive/20260716/XLayer_20260716.t.sol (3)
304-311: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_assertUnlimitedRateLimitis never called.No test in
XLayerE2ETestscalls this helper. TherateLimitsmember set on line 258 is also only read by this helper. The unlimited rate limits are already asserted inXLayerConfigTests.test_rateLimits_configon lines 163-184.Either delete the helper, or add a test that uses it for the rate-limit keys not covered by
test_rateLimits_config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/XLayer_20260716.t.sol` around lines 304 - 311, Remove the unused _assertUnlimitedRateLimit helper and the rateLimits member used only by it, unless adding assertions for otherwise uncovered rate-limit keys in XLayerE2ETests; avoid duplicating coverage already provided by XLayerConfigTests.test_rateLimits_config.
402-403: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
L1_EXECUTORis unused and the step comment names the wrong target.Line 402 declares
L1_EXECUTOR, but the call on line 461 targetsEthereum.SPARK_PROXY. The comment on line 458 states thatL1_PAUSE_PROXYtriggersL1_EXECUTOR, which does not match the code.Delete the unused constant and correct the comment to name
Ethereum.SPARK_PROXY.♻️ Proposed fix
// Ethereum mainnet governance contracts - address constant L1_EXECUTOR = 0x3300f198988e4C9C63F75dF86De36421f06af8c4; address constant L1_PAUSE_PROXY = 0xBE8E3e3618f7474F8cB1d074A26afFef007E98FB;- // Step 3: L1_PAUSE_PROXY triggers L1_EXECUTOR to execute the crosschain payload. + // Step 3: L1_PAUSE_PROXY makes Ethereum.SPARK_PROXY execute the crosschain payload.Also applies to: 458-464
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/XLayer_20260716.t.sol` around lines 402 - 403, Remove the unused L1_EXECUTOR constant and update the step comment near the call to identify Ethereum.SPARK_PROXY as the target, matching the actual invocation while retaining L1_PAUSE_PROXY only where it is used.
88-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the hardcoded RPC URL with a configured endpoint.
Three fork setups embed
https://rpc.xlayer.techdirectly. This bypasses the repository RPC configuration, so CI cannot point the tests at a private or archival node, and a public-endpoint outage fails the suite.Use an alias resolved through
foundry.tomlrpc_endpoints, or read the URL from an environment variable withvm.envOr.Run the following script to check how other tests in the repository resolve fork URLs:
#!/bin/bash # Inspect declared RPC endpoints and existing fork-creation patterns. fd -t f 'foundry.toml' --exec cat -n rg -nP 'createSelectFork|createFork|rpcUrl' --iglob '*.sol' -C2 | head -80Also applies to: 253-254, 425-429
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/XLayer_20260716.t.sol` around lines 88 - 89, Replace the hardcoded https://rpc.xlayer.tech URL in all three fork setup blocks, including setUp and the locations around the other referenced ranges, with the repository’s configured RPC endpoint resolution. Reuse the established foundry.toml rpc_endpoints alias or an environment-variable fallback via vm.envOr, matching existing fork-creation patterns and preserving each fork’s block number.archive/20260716/Spell_20260716.t.sol (1)
235-238: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the tautological assertions.
Several assertions compare a value against the variable that was just assigned from it, so they always pass:
- Line 237 compares
usdt0XLayer.balanceOf(XLayer.ALM_PROXY)withxLayerProxyUsdt0Starting, assigned on line 235.- Lines 278-279 compare the two mainnet balances with
ethProxyUsdtStartingandethOFTUsdtStarting, assigned on lines 275-276.- Line 323 repeats the assertion already made on line 311 with no intervening state change.
Delete these lines. Keep the assertions that check a balance change.
Also applies to: 275-279, 323-324
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/Spell_20260716.t.sol` around lines 235 - 238, Remove the tautological balance assertions in the test: the ALM proxy assertion using xLayerProxyUsdt0Starting, the mainnet assertions using ethProxyUsdtStarting and ethOFTUsdtStarting, and the duplicate assertion repeated after the earlier check. Preserve assertions that verify actual balance changes, including the vault balance assertion.archive/20260716/SparkRobinhood_20260716.sol (1)
13-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThree files redeclare the ALM freezable proxy interface.
src/interfaces/Interfaces.sollines 15-21 already declareIALMProxyFreezableLike, but it lacksgrantRoleandrevokeRole. Each new file works around that by declaring its own copy, so the same role surface now exists in four places with four different member sets. Extend the shared interface once, then import it.
archive/20260716/SparkRobinhood_20260716.sol#L13-L21: delete the local interface and addIALMProxyFreezableLiketo the existing named import on line 11.archive/20260716/SparkXLayer_20260716.sol#L6-L14: delete the local interface and importIALMProxyFreezableLikefrom../../interfaces/Interfaces.sol.archive/20260716/XLayer_20260716.t.sol#L41-L51: deleteIXLayerALMProxyFreezableand use the sharedIALMProxyFreezableLike, which already declaresDEFAULT_ADMIN_ROLE,ALLOCATOR_ROLE,FREEZER_ROLE, andhasRole.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/SparkRobinhood_20260716.sol` around lines 13 - 21, Extend the shared IALMProxyFreezableLike interface in src/interfaces/Interfaces.sol with grantRole and revokeRole, then remove the duplicate local interfaces: in archive/20260716/SparkRobinhood_20260716.sol lines 13-21, add IALMProxyFreezableLike to the existing named import; in archive/20260716/SparkXLayer_20260716.sol lines 6-14, import it from ../../interfaces/Interfaces.sol; and in archive/20260716/XLayer_20260716.t.sol lines 41-51, delete IXLayerALMProxyFreezable and use the shared interface.src/proposals/20260813/Spell_20260813.t.sol (1)
56-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the inherited state-view constant and normalization helpers.
SparkLiquidityLayerTestsalready definesUNISWAP_V4_STATE_VIEWwith the same address, and it already defines_toNormalizedAmount/_fromNormalizedAmountforaddresstokens. This contract inherits both. The local copies can drift from the harness values when the state view address or the decimal handling changes.Use the inherited constant, and convert
Currencytoaddressat the call sites.♻️ Proposed change
uint256 internal constant USDG_BALANCES_SLOT_INDEX = 1; uint256 internal constant _V4_SWAP = 0x10; - - address internal constant _STATE_VIEW = 0x7fFE42C4a5DEeA5b0feC41C94C136Cf115597227;- function _toNormalizedAmount(Currency currency, uint256 amount) - internal view returns (uint256 normalizedAmount) - { - return amount * 1e18 / (10 ** IERC20Metadata(Currency.unwrap(currency)).decimals()); - } - - function _fromNormalizedAmount(Currency currency, uint256 normalizedAmount) internal view returns (uint256 amount) { - return normalizedAmount * (10 ** IERC20Metadata(Currency.unwrap(currency)).decimals()) / 1e18; - } + function _toNormalizedAmount(Currency currency, uint256 amount) + internal view returns (uint256 normalizedAmount) + { + return _toNormalizedAmount(Currency.unwrap(currency), amount); + } + + function _fromNormalizedAmount(Currency currency, uint256 normalizedAmount) internal view returns (uint256 amount) { + return _fromNormalizedAmount(Currency.unwrap(currency), normalizedAmount); + }Then replace
_STATE_VIEWwithUNISWAP_V4_STATE_VIEWin_getCurrentTick(line 216).Also applies to: 381-389
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/proposals/20260813/Spell_20260813.t.sol` at line 56, Remove the locally defined _STATE_VIEW and reuse the inherited UNISWAP_V4_STATE_VIEW constant in _getCurrentTick. Remove the local normalization helpers and use the inherited _toNormalizedAmount and _fromNormalizedAmount helpers, converting Currency values to address at their call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@archive/20260716/Spell_20260716.t.sol`:
- Around line 87-102: Update test_ETHEREUM_sll_deactivateOldMorphoUsdtVault to
verify before _executeAllPayloadsAndBridges that Ethereum.ALM_PROXY holds zero
shares of OLD_MORPHO_VAULT_V2_USDT, using the vault’s established share-balance
query or an existing position-exited assertion.
- Around line 383-388: Update the interest derivation comment near the
USDT0XLayer assertion to use the exact FIVE_PCT_APY value, and align the
USDT0XLayer and Robinhood redemption assertions by using the same approximate
absolute comparison and tolerance for the calculated amount.
---
Nitpick comments:
In `@archive/20260716/SparkEthereum_20260716.sol`:
- Line 52: Rename the constant LZ_ENDPOINT_XLAYER to LZ_EID_XLAYER and update
both references on lines 80 and 85 to use the renamed LayerZero endpoint ID
symbol.
In `@archive/20260716/SparkRobinhood_20260716.sol`:
- Around line 13-21: Extend the shared IALMProxyFreezableLike interface in
src/interfaces/Interfaces.sol with grantRole and revokeRole, then remove the
duplicate local interfaces: in archive/20260716/SparkRobinhood_20260716.sol
lines 13-21, add IALMProxyFreezableLike to the existing named import; in
archive/20260716/SparkXLayer_20260716.sol lines 6-14, import it from
../../interfaces/Interfaces.sol; and in archive/20260716/XLayer_20260716.t.sol
lines 41-51, delete IXLayerALMProxyFreezable and use the shared interface.
In `@archive/20260716/Spell_20260716.t.sol`:
- Around line 235-238: Remove the tautological balance assertions in the test:
the ALM proxy assertion using xLayerProxyUsdt0Starting, the mainnet assertions
using ethProxyUsdtStarting and ethOFTUsdtStarting, and the duplicate assertion
repeated after the earlier check. Preserve assertions that verify actual balance
changes, including the vault balance assertion.
In `@archive/20260716/XLayer_20260716.t.sol`:
- Around line 304-311: Remove the unused _assertUnlimitedRateLimit helper and
the rateLimits member used only by it, unless adding assertions for otherwise
uncovered rate-limit keys in XLayerE2ETests; avoid duplicating coverage already
provided by XLayerConfigTests.test_rateLimits_config.
- Around line 402-403: Remove the unused L1_EXECUTOR constant and update the
step comment near the call to identify Ethereum.SPARK_PROXY as the target,
matching the actual invocation while retaining L1_PAUSE_PROXY only where it is
used.
- Around line 88-89: Replace the hardcoded https://rpc.xlayer.tech URL in all
three fork setup blocks, including setUp and the locations around the other
referenced ranges, with the repository’s configured RPC endpoint resolution.
Reuse the established foundry.toml rpc_endpoints alias or an
environment-variable fallback via vm.envOr, matching existing fork-creation
patterns and preserving each fork’s block number.
In `@src/proposals/20260813/Spell_20260813.t.sol`:
- Line 56: Remove the locally defined _STATE_VIEW and reuse the inherited
UNISWAP_V4_STATE_VIEW constant in _getCurrentTick. Remove the local
normalization helpers and use the inherited _toNormalizedAmount and
_fromNormalizedAmount helpers, converting Currency values to address at their
call sites.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f66e2a47-241c-40ae-9dca-34ee3bfd9644
📒 Files selected for processing (9)
archive/20260716/SparkEthereum_20260716.solarchive/20260716/SparkRobinhood_20260716.solarchive/20260716/SparkXLayer_20260716.solarchive/20260716/Spell_20260716.t.solarchive/20260716/XLayer_20260716.t.soldiffs/20260813-Ethereum-0xC13e21B648A5Ee794902342038FF3aDAB66BE987-pre_20260813-Ethereum-0xC13e21B648A5Ee794902342038FF3aDAB66BE987-post.mdsrc/proposals/20260813/SparkEthereum_20260813.solsrc/proposals/20260813/Spell_20260813.t.solsrc/test-harness/SparkLiquidityLayerTests.sol
🛑 Comments failed to post (2)
archive/20260716/Spell_20260716.t.sol (2)
87-102: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Confirm that the old USDT Morpho V2 vault holds no ALM proxy balance.
The payload sets the withdraw limit for
OLD_MORPHO_VAULT_V2_USDTto0. IfEthereum.ALM_PROXYstill holds shares in that vault, the zeroed withdraw limit strands them, becauseLIMIT_4626_WITHDRAWgates the withdrawal path.Add an assertion that the ALM proxy share balance is zero before execution, or confirm that the position was already exited.
💚 Proposed assertion
_assertRateLimit(depositKey, 100_000_000e6, 1_000_000_000e6 / uint256(1 days)); _assertRateLimit(withdrawKey, type(uint256).max, 0); + // No funds are stranded by zeroing the withdraw limit. + assertEq(IERC20(OLD_MORPHO_VAULT_V2_USDT).balanceOf(Ethereum.ALM_PROXY), 0); + _executeAllPayloadsAndBridges();📝 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.function test_ETHEREUM_sll_deactivateOldMorphoUsdtVault() external onChain(ChainIdUtils.Ethereum()) { SparkLiquidityLayerContext memory ctx = _getSparkLiquidityLayerContext(); MainnetController controller = MainnetController(ctx.controller); bytes32 depositKey = RateLimitHelpers.makeAddressKey(controller.LIMIT_4626_DEPOSIT(), OLD_MORPHO_VAULT_V2_USDT); bytes32 withdrawKey = RateLimitHelpers.makeAddressKey(controller.LIMIT_4626_WITHDRAW(), OLD_MORPHO_VAULT_V2_USDT); _assertRateLimit(depositKey, 100_000_000e6, 1_000_000_000e6 / uint256(1 days)); _assertRateLimit(withdrawKey, type(uint256).max, 0); // No funds are stranded by zeroing the withdraw limit. assertEq(IERC20(OLD_MORPHO_VAULT_V2_USDT).balanceOf(Ethereum.ALM_PROXY), 0); _executeAllPayloadsAndBridges(); _assertRateLimit(depositKey, 0, 0); _assertRateLimit(withdrawKey, 0, 0); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/Spell_20260716.t.sol` around lines 87 - 102, Update test_ETHEREUM_sll_deactivateOldMorphoUsdtVault to verify before _executeAllPayloadsAndBridges that Ethereum.ALM_PROXY holds zero shares of OLD_MORPHO_VAULT_V2_USDT, using the vault’s established share-balance query or an existing position-exited assertion.
383-388: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the derivation comment and align the two redemption assertions.
The comment on line 384 uses the rate
1.000000001547125957863212448. The constantFIVE_PCT_APYon line 72 is1.000000001547125957863212167. The comment does not match the value used by the test.The equivalent Robinhood assertion on line 534 uses
assertApproxEqAbs(..., 1_001_337.610629e6, 1)for the same math, while line 386 uses exact equality against1_001_337.610630e6. Use one form for both tests so a one-unit rounding difference does not make one test brittle.💚 Proposed fix
// 1m + 10 days of interest at 5% APY - // bc -l <<< 'scale=27; e( l(1.000000001547125957863212448)*(60*60*24*10) )' + // bc -l <<< 'scale=27; e( l(1.000000001547125957863212167)*(60*60*24*10) )' // 1.001337610630706965933736950 - assertEq(usdt0XLayer.balanceOf(user), 1_001_337.610630e6); + assertApproxEqAbs(usdt0XLayer.balanceOf(user), 1_001_337.610630e6, 1); assertEq(vault.balanceOf(user), 0);📝 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.// 1m + 10 days of interest at 5% APY // bc -l <<< 'scale=27; e( l(1.000000001547125957863212167)*(60*60*24*10) )' // 1.001337610630706965933736950 assertApproxEqAbs(usdt0XLayer.balanceOf(user), 1_001_337.610630e6, 1); assertEq(vault.balanceOf(user), 0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@archive/20260716/Spell_20260716.t.sol` around lines 383 - 388, Update the interest derivation comment near the USDT0XLayer assertion to use the exact FIVE_PCT_APY value, and align the USDT0XLayer and Robinhood redemption assertions by using the same approximate absolute comparison and tolerance for the calculated amount.
TheMj0ln1r
left a comment
TheMj0ln1r
left a comment
There was a problem hiding this comment.
lgtm, just few formats.
| _assertRateLimit(withdrawPoolId, 0, 0); | ||
| _assertRateLimit(swapPoolId, 0, 0); | ||
|
|
||
| (int24 _tickLowerMin, int24 _tickUpperMax, uint24 _maxTickSpacing) = controller.uniswapV4TickLimits(USDG_USDS_POOL_ID); |
There was a problem hiding this comment.
space after bracket. (Apply to entire file)
| assertEq(controller.maxSlippages(address(uint160(uint256(USDG_USDS_POOL_ID)))), 0.999e18); | ||
|
|
||
| _assertRateLimit(depositPoolId, 10_000_000e18, 100_000_000e18 / uint256(1 days)); | ||
| _assertRateLimit(withdrawPoolId, type(uint256).max, 0); |
| vm.revertTo(snapshot); // To allow swapping through higher liquidity below | ||
| } else { | ||
| // Deposit is disabled | ||
| assertEq(vars.depositAmounts[0], 0); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
lucas-manuel
left a comment
lucas-manuel
left a comment
There was a problem hiding this comment.
Reviewed spell, LGTM after these changes.
TheMj0ln1r
left a comment
TheMj0ln1r
left a comment
There was a problem hiding this comment.
Approving the Spell for deployment
- Check forum post link - https://forum.skyeco.com/t/august-13-2026-proposed-changes-to-spark-for-upcoming-spell/28135/8
- Check description for all payloads
- Check poll links
- Check spark-address-registry commit matches current HEAD
- SLL - Uniswap V4 SLL Onboarding [Ethereum] - [1] - DONE
-
Verify Pool ID in Uniswap UI -
USDG_USDS_POOL_ID=0x28adc7179a8a83c3379955d59563c0fec33eadfa83946b447af289190ff5fcff -
Independently convert ticks to prices from forum post to validate correctness
>>> (1.0001 ** -276_334) * 10**12 0.9990031909716848 >>> (1.0001 ** -276_314) * 10**12 1.0010030965990384
-
Review all parameters in
configureUniswapV4Poolagainst forum post -
Review
configureUniswapV4Poolfunction in SLLHelpers, ensuring that- Tick limits are set -
-276_334,-276_314 - Max slippage is set -
0.999e18(0.1%) - Rate limits are set
- Deposit
- maxAmount -
10_000_000e18 - slope -
100_000_000e18/day
- maxAmount -
- Withdraw
- maxAmount - unlimited
- slope - unlimited
- Swap
- maxAmount -
5_000_000e18 - slope -
200_000_000e18/day
- maxAmount -
- Deposit
- Tick limits are set -
-
Review test -
test_ETHEREUM_sparkLiquidityLayer_onboardUniswapV4USDGUSDS- Check rate limits have been set
- Check maxSlippage has been set
- Check tick limits have been set
- Check that E2E testing is properly conducting deposit/withdraw/swap with proper state assertions and rate limit checks
-
- SLL - Uniswap V4 SLL Onboarding [Ethereum] - [2] - DONE
-
Verify Pool ID in Uniswap UI -
RLUSD_USDS_POOL_ID=0x9035721b23481db3888fd201b9c2b26dbc3af60258bca65e669f2ed98dc8eb4f -
Independently convert ticks to prices from forum post to validate correctness
>>> (1.0001 ** -10) 0.9990005497800716 >>> (1.0001 ** 10) 1.001000450120021
-
Review all parameters in
configureUniswapV4Poolagainst forum post -
Review
configureUniswapV4Poolfunction in SLLHelpers, ensuring that- Tick limits are set -
-10,10 - Max slippage is set -
0.999e18(0.1%) - Rate limits are set
- Deposit
- maxAmount -
10_000_000e18 - slope -
50_000_000e18/day
- maxAmount -
- Withdraw
- maxAmount - unlimited
- slope - unlimited
- Swap
- maxAmount -
5_000_000e18 - slope -
100_000_000e18/day
- maxAmount -
- Deposit
- Tick limits are set -
-
Review test -
test_ETHEREUM_sparkLiquidityLayer_onboardUniswapV4RLUSDUSDS- Check rate limits have been set
- Check maxSlippage has been set
- Check tick limits have been set
- Check that E2E testing is properly conducting deposit/withdraw/swap with proper state assertions and rate limit checks
-
- SLL - Curve SLL onboarding [Ethereum] - [3] - DONE
- Check pool address against forum post -
CURVE_RLUSD_USDC = 0xD001aE433f254283FeCE51d4ACcE8c53263aa186 - Ensure that the pool is of type
stableswap-ng(check curve.fi UI) - Check that pool address is verified
- Ensure that the pool has been seeded with LP shares burned at
address(1) - Ensure that maxSlippage is set according to forum post -
0.999e18 - Check maxAmount against forum post -
5_000_000e18 - Check slope against forum post -
25_000_000e18/day - Verify the precision of the rate limit is 1e18
- Check rate limit before and after state in a test -
test_ETHEREUM_curvePoolOnboarding_RLUSDUSDC - Ensure that
_configureCurvePoolhelper function is used in spell - Ensure that
_testCurveOnboardinghelper test is used in test
- Check pool address against forum post -
- Withdraw reserves [Ethereum] - [4] - DONE
- Review SparkPayloadEthereum code for claiming all reserves
- Ensure stablecoins go to ALMProxy and everything else goes to Liquidation multisig
- Review that test decrements and increments funds from correct addresses for all reserves
- Treasury Transfer [Ethereum] - [5] - DONE
- Check receiver address against forum post or address registry -
ALM_OPS_MULTISIG - Check amount against forum post -
1_756_359e18 - Review that test decrements and increments funds from correct addresses -
test_ETHEREUM_sparkTreasury_transferExcessUSDSForBuybacks
- Check receiver address against forum post or address registry -
bc34f01
lucas-manuel
left a comment
lucas-manuel
left a comment
There was a problem hiding this comment.
Good to deploy.
- Check forum post link - https://forum.skyeco.com/t/august-13-2026-proposed-changes-to-spark-for-upcoming-spell/28135/8
- Check description for all payloads
- Check poll links
- Check spark-address-registry commit matches current HEAD
- SLL - Uniswap V4 SLL Onboarding [Ethereum] - [1] DONE
-
Verify Pool ID in Uniswap UI -
USDG_USDS_POOL_ID=0x28adc7179a8a83c3379955d59563c0fec33eadfa83946b447af289190ff5fcff -
Verify pool is seeded through UI
-
Check pool token ordering through Etherscan and fee
-
Independently convert ticks to prices from forum post to validate correctness
>>> (1.0001 ** -276_334) * 10**12 0.9990031909716848 >>> (1.0001 ** -276_314) * 10**12 1.0010030965990384
-
Review all parameters in
configureUniswapV4Poolagainst forum post -
Review
configureUniswapV4Poolfunction in SLLHelpers, ensuring that- Tick limits are set -
-276_334,-276_314 - Max slippage is set -
0.999e18(0.1%) - Rate limits are set
- Deposit
- maxAmount -
10_000_000e18 - slope -
100_000_000e18/day
- maxAmount -
- Withdraw
- maxAmount - unlimited
- slope - unlimited
- Swap
- maxAmount -
5_000_000e18 - slope -
200_000_000e18/day
- maxAmount -
- Deposit
- Tick limits are set -
-
Review test -
test_ETHEREUM_sparkLiquidityLayer_onboardUniswapV4USDGUSDS- Check rate limits have been set
- Check maxSlippage has been set
- Check tick limits have been set
- Check that E2E testing is properly conducting deposit/withdraw/swap with proper state assertions and rate limit checks
-
- SLL - Uniswap V4 SLL Onboarding [Ethereum] - [2] - DONE
-
Verify Pool ID in Uniswap UI -
RLUSD_USDS_POOL_ID=0x9035721b23481db3888fd201b9c2b26dbc3af60258bca65e669f2ed98dc8eb4f -
Verify pool is seeded through UI
-
Check pool token ordering through Etherscan and fee
-
Independently convert ticks to prices from forum post to validate correctness
>>> (1.0001 ** -10) 0.9990005497800716 >>> (1.0001 ** 10) 1.001000450120021
-
Review all parameters in
configureUniswapV4Poolagainst forum post -
Review
configureUniswapV4Poolfunction in SLLHelpers, ensuring that- Tick limits are set -
-10,10 - Max slippage is set -
0.999e18(0.1%) - Rate limits are set
- Deposit
- maxAmount -
10_000_000e18 - slope -
50_000_000e18/day
- maxAmount -
- Withdraw
- maxAmount - unlimited
- slope - unlimited
- Swap
- maxAmount -
5_000_000e18 - slope -
100_000_000e18/day
- maxAmount -
- Deposit
- Tick limits are set -
-
Review test -
test_ETHEREUM_sparkLiquidityLayer_onboardUniswapV4RLUSDUSDS- Check rate limits have been set
- Check maxSlippage has been set
- Check tick limits have been set
- Check that E2E testing is properly conducting deposit/withdraw/swap with proper state assertions and rate limit checks
-
- SLL - Curve SLL onboarding [Ethereum] - [3] - DONE
- Check pool address against forum post -
CURVE_RLUSD_USDC = 0xD001aE433f254283FeCE51d4ACcE8c53263aa186 - Check seeded
- Ensure that the pool is of type
stableswap-ng(check curve.fi UI) - Check that pool address is verified
- Ensure that the pool has been seeded with LP shares burned at
address(1) - Ensure that maxSlippage is set according to forum post -
0.999e18 - Check maxAmount against forum post -
5_000_000e18 - Check slope against forum post -
25_000_000e18/day - Verify the precision of the rate limit is 1e18
- Check rate limit before and after state in a test -
test_ETHEREUM_curvePoolOnboarding_RLUSDUSDC - Ensure that
_configureCurvePoolhelper function is used in spell - Ensure that
_testCurveOnboardinghelper test is used in test
- Check pool address against forum post -
- Withdraw reserves [Ethereum] - [4] - DONE
- Review SparkPayloadEthereum code for claiming all reserves
- Ensure stablecoins go to ALMProxy and everything else goes to Liquidation multisig
- Review that test decrements and increments funds from correct addresses for all reserves
- Treasury Transfer [Ethereum] - [5] - DONE
- Check receiver address against forum post or address registry -
ALM_OPS_MULTISIG - Check amount against forum post -
1_756_359e18 - Review that test decrements and increments funds from correct addresses -
test_ETHEREUM_sparkTreasury_transferExcessUSDSForBuybacks
- Check receiver address against forum post or address registry -
lucas-manuel
left a comment
lucas-manuel
left a comment
There was a problem hiding this comment.
Approving payload deployed at 0xc827237CB91Fa8E78B8dfA4F77838eDf924C04e9
TheMj0ln1r
left a comment
TheMj0ln1r
left a comment
There was a problem hiding this comment.
Approving spell deployment at 0xc827237CB91Fa8E78B8dfA4F77838eDf924C04e9
There was a problem hiding this comment.
Confirming and approving the spell deployment at 0xc827237CB91Fa8E78B8dfA4F77838eDf924C04e9
Completed checklist for both the development and deployment stages: https://gist.github.com/unvariant-dev/9ec09866fed86c5b96b7f9d8e6035e20
4 participants
2026-08-13 Spark Spell
Forum Post
https://forum.skyeco.com/t/august-13-2026-proposed-changes-to-spark-for-upcoming-spell/28135#p-106996-summary-1
Dependencies on Sky Core spell
None
Notes for Reviewers
None
Conditions for Spell Deployment
Conditions for Spell Handoff to Sky
Conditions for Spell Merge
Summary by CodeRabbit