feat: Add xlayer integration tests - #177
Conversation
Summary by OctaneNew Contracts
Updated Contracts
🔗 Commit Hash: 32ee908 |
|
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:
WalkthroughThis PR adds support for two new chains, Robinhood and XLayer, across the codebase: chain-id utilities, payload dispatch logic in SparkPayloadEthereum, new proposal contracts (SparkEthereum_20260716, SparkRobinhood_20260716), test-harness fork/bridge/executor wiring, and extensive test suites. Also updates submodule pointers and CI config. ChangesRobinhood/XLayer chain integration
Estimated code review effort: 4 (Complex) | ~60 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/test-harness/SpellRunner.sol (1)
100-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded XLayer RPC URL breaks established env-var pattern.
Every other chain (including the newly added Robinhood chain) sources its RPC URL via
vm.envString(...), but XLayer is hardcoded to"https://rpc.xlayer.tech". This public endpoint is rate-limited (100 req/s per IP per X Layer docs) and not overridable, unlike Robinhood'sRH_RPC_URLwhich was added to CI. This makes XLayer tests more prone to CI flakiness/rate-limiting and inconsistent with the rest of the harness.♻️ Suggested fix
setChain("xlayer", ChainData({ name : "XLayer", - rpcUrl : "https://rpc.xlayer.tech", + rpcUrl : vm.envOr("XLAYER_RPC_URL", string("https://rpc.xlayer.tech")), chainId : 196 }));🤖 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/test-harness/SpellRunner.sol` around lines 100 - 110, The XLayer chain setup in SpellRunner currently hardcodes the RPC endpoint, unlike the rest of the harness that uses vm.envString for overridable URLs. Update the xlayer entry in setChain to read the RPC URL from an environment variable, following the same pattern used for robinhood_chain and the other chain definitions, so CI and local runs can avoid the public rate-limited endpoint.src/proposals/20260716/Spell_20260716.t.sol (2)
64-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest contract names reference the wrong spell date (
20260702).All three test contracts are named
SparkEthereum_20260702_*but target_spellId = 20260716. Rename to20260716for consistency and to avoid confusion with the prior spell.Also applies to: 96-96, 111-111
🤖 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/20260716/Spell_20260716.t.sol` at line 64, The three test contract names still use the old spell date, which conflicts with the current _spellId value. Rename each SparkEthereum_20260702_* test contract to SparkEthereum_20260716_* in the affected proposal test file so the contract names match the 20260716 spell and avoid confusion with the prior spell.
136-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRELAYER role changes are not asserted.
SparkRobinhood_20260716.execute()also grants RELAYER toALM_BACKSTOP_RELAYER_MULTISIGand revokes it fromALM_RELAYER_MULTISIG_1, but this test only verifies the FREEZER role transitions. Consider adding parallel assertions for RELAYER to cover the full payload.🤖 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/20260716/Spell_20260716.t.sol` around lines 136 - 150, The Robinhood role-change test only checks FREEZER transitions, so it misses the RELAYER changes applied by SparkRobinhood_20260716.execute(). Update test_ROBINHOOD_controllerRoleChanges in Spell_20260716.t.sol to add matching assertions for controller.RELAYER() before and after _executeAllPayloadsAndBridges(), verifying ALM_RELAYER_MULTISIG_1 is revoked and ALM_BACKSTOP_RELAYER_MULTISIG is granted.src/proposals/20260716/SparkEthereum_20260716.sol (1)
26-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMultiple placeholder values still marked
// TODO: change.
ANCHORAGE_FEES_RECIPIENT,INCENTIVES_RECIPIENT,XLAYER_ALM_PROXY, andLZ_ENDPOINT_XLAYERare placeholders. These must be finalized before this spell can be executed on mainnet. Want me to open a tracking issue?Also applies to: 38-38
🤖 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/20260716/SparkEthereum_20260716.sol` around lines 26 - 30, The proposal still contains placeholder constants marked TODO, so finalize the real mainnet values before execution. Update SparkEthereum_20260716.sol by replacing ANCHORAGE_FEES_RECIPIENT, INCENTIVES_RECIPIENT, and XLAYER_ALM_PROXY with their production addresses, and also resolve LZ_ENDPOINT_XLAYER wherever it is defined in the same contract. Keep the existing constant names and only swap in the verified values so the deploy/execution path no longer relies on placeholders.src/proposals/20260716/XLayer_20260716.t.sol (1)
72-73: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid hardcoding the XLayer RPC URL in fork tests
vm.createSelectFork("https://rpc.xlayer.tech", 64662930)is used in bothXLayerConfigTestsandXLayerE2ETests, and the same URL is also embedded inXLayerCrosschainE2ETests. That couples CI to a public endpoint and a fixed fork block, which can make these tests flaky. Source the RPC from config/env like the other networks and move the block number into a shared constant.🤖 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/20260716/XLayer_20260716.t.sol` around lines 72 - 73, The fork setup in XLayerConfigTests, XLayerE2ETests, and XLayerCrosschainE2ETests is hardcoding the public XLayer RPC and a fixed block, which should be replaced with configurable values. Update the setUp path that calls vm.createSelectFork to read the RPC URL from the same config/env mechanism used by the other networks, and move the fork block number into a shared constant so all XLayer tests use the same source of truth.
🤖 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 `@src/proposals/20260716/SparkEthereum_20260716.sol`:
- Around line 26-27: `INCENTIVES_RECIPIENT` and `ANCHORAGE_FEES_RECIPIENT` are
both using the same placeholder value, so the transfers in the proposal
currently route to one address and break the treasury test. Update the constants
in `SparkEthereum_20260716` to use two distinct, real recipient addresses before
merge, and verify the balance expectations in
`test_ETHEREUM_sparkTreasury_transfers` still match the separate incentive and
anchorage-fees transfers.
- Around line 40-41: The constructor in SparkEthereum_20260716 currently leaves
the inherited payload addresses unset, so the PAYLOAD_ROBINHOOD and
PAYLOAD_XLAYER branches in SparkPayloadEthereum.execute() never activate. Update
the constructor to accept the Robinhood and XLayer payload addresses and assign
them to the corresponding inherited immutables/fields so those dispatch paths
are enabled. Make sure the existing execute() checks will now see nonzero values
and queue both payloads correctly.
In `@src/proposals/20260716/XLayer_20260716.t.sol`:
- Line 99: The comments in this test suite still refer to USDG/SPUSDG even
though the exercised vault is the USDT-based spUSDT vault, so update the
affected comments in the XLayer_20260716_t.sol scenario to match the actual
asset and vault names. In particular, revise the placeholders around the role
setup, vault state, and withdrawal sections so they reference USDT/Spark Savings
USDT instead of USDG/SPUSDG, and fix the deposit-state comment in the section
around the vault balance check to describe the 1e6 totalAssets/totalSupply
assertion rather than an 8.01e6 USDG amount.
---
Nitpick comments:
In `@src/proposals/20260716/SparkEthereum_20260716.sol`:
- Around line 26-30: The proposal still contains placeholder constants marked
TODO, so finalize the real mainnet values before execution. Update
SparkEthereum_20260716.sol by replacing ANCHORAGE_FEES_RECIPIENT,
INCENTIVES_RECIPIENT, and XLAYER_ALM_PROXY with their production addresses, and
also resolve LZ_ENDPOINT_XLAYER wherever it is defined in the same contract.
Keep the existing constant names and only swap in the verified values so the
deploy/execution path no longer relies on placeholders.
In `@src/proposals/20260716/Spell_20260716.t.sol`:
- Line 64: The three test contract names still use the old spell date, which
conflicts with the current _spellId value. Rename each SparkEthereum_20260702_*
test contract to SparkEthereum_20260716_* in the affected proposal test file so
the contract names match the 20260716 spell and avoid confusion with the prior
spell.
- Around line 136-150: The Robinhood role-change test only checks FREEZER
transitions, so it misses the RELAYER changes applied by
SparkRobinhood_20260716.execute(). Update test_ROBINHOOD_controllerRoleChanges
in Spell_20260716.t.sol to add matching assertions for controller.RELAYER()
before and after _executeAllPayloadsAndBridges(), verifying
ALM_RELAYER_MULTISIG_1 is revoked and ALM_BACKSTOP_RELAYER_MULTISIG is granted.
In `@src/proposals/20260716/XLayer_20260716.t.sol`:
- Around line 72-73: The fork setup in XLayerConfigTests, XLayerE2ETests, and
XLayerCrosschainE2ETests is hardcoding the public XLayer RPC and a fixed block,
which should be replaced with configurable values. Update the setUp path that
calls vm.createSelectFork to read the RPC URL from the same config/env mechanism
used by the other networks, and move the fork block number into a shared
constant so all XLayer tests use the same source of truth.
In `@src/test-harness/SpellRunner.sol`:
- Around line 100-110: The XLayer chain setup in SpellRunner currently hardcodes
the RPC endpoint, unlike the rest of the harness that uses vm.envString for
overridable URLs. Update the xlayer entry in setChain to read the RPC URL from
an environment variable, following the same pattern used for robinhood_chain and
the other chain definitions, so CI and local runs can avoid the public
rate-limited endpoint.
🪄 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: f3c7c161-c91d-4693-b9ab-125de5275b5c
📒 Files selected for processing (20)
.github/workflows/test.ymlarchive/20260702/SparkArbitrumOne_20260702.solarchive/20260702/SparkAvalanche_20260702.solarchive/20260702/SparkBase_20260702.solarchive/20260702/SparkEthereum_20260702.solarchive/20260702/SparkOptimism_20260702.solarchive/20260702/SparkUnichain_20260702.solarchive/20260702/Spell_20260702.t.soldiffs/20260716-Ethereum-0xC13e21B648A5Ee794902342038FF3aDAB66BE987-pre_20260716-Ethereum-0xC13e21B648A5Ee794902342038FF3aDAB66BE987-post.mdlib/spark-address-registrylib/xchain-helperssrc/SparkPayloadEthereum.solsrc/interfaces/Interfaces.solsrc/libraries/ChainIdUtils.solsrc/proposals/20260716/SparkEthereum_20260716.solsrc/proposals/20260716/SparkRobinhood_20260716.solsrc/proposals/20260716/Spell_20260716.t.solsrc/proposals/20260716/XLayer_20260716.t.solsrc/test-harness/SpellRunner.solsrc/test-harness/SpellTests.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: 3
🧹 Nitpick comments (5)
src/test-harness/SpellRunner.sol (1)
100-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded XLayer RPC URL breaks established env-var pattern.
Every other chain (including the newly added Robinhood chain) sources its RPC URL via
vm.envString(...), but XLayer is hardcoded to"https://rpc.xlayer.tech". This public endpoint is rate-limited (100 req/s per IP per X Layer docs) and not overridable, unlike Robinhood'sRH_RPC_URLwhich was added to CI. This makes XLayer tests more prone to CI flakiness/rate-limiting and inconsistent with the rest of the harness.♻️ Suggested fix
setChain("xlayer", ChainData({ name : "XLayer", - rpcUrl : "https://rpc.xlayer.tech", + rpcUrl : vm.envOr("XLAYER_RPC_URL", string("https://rpc.xlayer.tech")), chainId : 196 }));🤖 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/test-harness/SpellRunner.sol` around lines 100 - 110, The XLayer chain setup in SpellRunner currently hardcodes the RPC endpoint, unlike the rest of the harness that uses vm.envString for overridable URLs. Update the xlayer entry in setChain to read the RPC URL from an environment variable, following the same pattern used for robinhood_chain and the other chain definitions, so CI and local runs can avoid the public rate-limited endpoint.src/proposals/20260716/Spell_20260716.t.sol (2)
64-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest contract names reference the wrong spell date (
20260702).All three test contracts are named
SparkEthereum_20260702_*but target_spellId = 20260716. Rename to20260716for consistency and to avoid confusion with the prior spell.Also applies to: 96-96, 111-111
🤖 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/20260716/Spell_20260716.t.sol` at line 64, The three test contract names still use the old spell date, which conflicts with the current _spellId value. Rename each SparkEthereum_20260702_* test contract to SparkEthereum_20260716_* in the affected proposal test file so the contract names match the 20260716 spell and avoid confusion with the prior spell.
136-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRELAYER role changes are not asserted.
SparkRobinhood_20260716.execute()also grants RELAYER toALM_BACKSTOP_RELAYER_MULTISIGand revokes it fromALM_RELAYER_MULTISIG_1, but this test only verifies the FREEZER role transitions. Consider adding parallel assertions for RELAYER to cover the full payload.🤖 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/20260716/Spell_20260716.t.sol` around lines 136 - 150, The Robinhood role-change test only checks FREEZER transitions, so it misses the RELAYER changes applied by SparkRobinhood_20260716.execute(). Update test_ROBINHOOD_controllerRoleChanges in Spell_20260716.t.sol to add matching assertions for controller.RELAYER() before and after _executeAllPayloadsAndBridges(), verifying ALM_RELAYER_MULTISIG_1 is revoked and ALM_BACKSTOP_RELAYER_MULTISIG is granted.src/proposals/20260716/SparkEthereum_20260716.sol (1)
26-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMultiple placeholder values still marked
// TODO: change.
ANCHORAGE_FEES_RECIPIENT,INCENTIVES_RECIPIENT,XLAYER_ALM_PROXY, andLZ_ENDPOINT_XLAYERare placeholders. These must be finalized before this spell can be executed on mainnet. Want me to open a tracking issue?Also applies to: 38-38
🤖 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/20260716/SparkEthereum_20260716.sol` around lines 26 - 30, The proposal still contains placeholder constants marked TODO, so finalize the real mainnet values before execution. Update SparkEthereum_20260716.sol by replacing ANCHORAGE_FEES_RECIPIENT, INCENTIVES_RECIPIENT, and XLAYER_ALM_PROXY with their production addresses, and also resolve LZ_ENDPOINT_XLAYER wherever it is defined in the same contract. Keep the existing constant names and only swap in the verified values so the deploy/execution path no longer relies on placeholders.src/proposals/20260716/XLayer_20260716.t.sol (1)
72-73: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid hardcoding the XLayer RPC URL in fork tests
vm.createSelectFork("https://rpc.xlayer.tech", 64662930)is used in bothXLayerConfigTestsandXLayerE2ETests, and the same URL is also embedded inXLayerCrosschainE2ETests. That couples CI to a public endpoint and a fixed fork block, which can make these tests flaky. Source the RPC from config/env like the other networks and move the block number into a shared constant.🤖 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/20260716/XLayer_20260716.t.sol` around lines 72 - 73, The fork setup in XLayerConfigTests, XLayerE2ETests, and XLayerCrosschainE2ETests is hardcoding the public XLayer RPC and a fixed block, which should be replaced with configurable values. Update the setUp path that calls vm.createSelectFork to read the RPC URL from the same config/env mechanism used by the other networks, and move the fork block number into a shared constant so all XLayer tests use the same source of truth.
🤖 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 `@src/proposals/20260716/SparkEthereum_20260716.sol`:
- Around line 26-27: `INCENTIVES_RECIPIENT` and `ANCHORAGE_FEES_RECIPIENT` are
both using the same placeholder value, so the transfers in the proposal
currently route to one address and break the treasury test. Update the constants
in `SparkEthereum_20260716` to use two distinct, real recipient addresses before
merge, and verify the balance expectations in
`test_ETHEREUM_sparkTreasury_transfers` still match the separate incentive and
anchorage-fees transfers.
- Around line 40-41: The constructor in SparkEthereum_20260716 currently leaves
the inherited payload addresses unset, so the PAYLOAD_ROBINHOOD and
PAYLOAD_XLAYER branches in SparkPayloadEthereum.execute() never activate. Update
the constructor to accept the Robinhood and XLayer payload addresses and assign
them to the corresponding inherited immutables/fields so those dispatch paths
are enabled. Make sure the existing execute() checks will now see nonzero values
and queue both payloads correctly.
In `@src/proposals/20260716/XLayer_20260716.t.sol`:
- Line 99: The comments in this test suite still refer to USDG/SPUSDG even
though the exercised vault is the USDT-based spUSDT vault, so update the
affected comments in the XLayer_20260716_t.sol scenario to match the actual
asset and vault names. In particular, revise the placeholders around the role
setup, vault state, and withdrawal sections so they reference USDT/Spark Savings
USDT instead of USDG/SPUSDG, and fix the deposit-state comment in the section
around the vault balance check to describe the 1e6 totalAssets/totalSupply
assertion rather than an 8.01e6 USDG amount.
---
Nitpick comments:
In `@src/proposals/20260716/SparkEthereum_20260716.sol`:
- Around line 26-30: The proposal still contains placeholder constants marked
TODO, so finalize the real mainnet values before execution. Update
SparkEthereum_20260716.sol by replacing ANCHORAGE_FEES_RECIPIENT,
INCENTIVES_RECIPIENT, and XLAYER_ALM_PROXY with their production addresses, and
also resolve LZ_ENDPOINT_XLAYER wherever it is defined in the same contract.
Keep the existing constant names and only swap in the verified values so the
deploy/execution path no longer relies on placeholders.
In `@src/proposals/20260716/Spell_20260716.t.sol`:
- Line 64: The three test contract names still use the old spell date, which
conflicts with the current _spellId value. Rename each SparkEthereum_20260702_*
test contract to SparkEthereum_20260716_* in the affected proposal test file so
the contract names match the 20260716 spell and avoid confusion with the prior
spell.
- Around line 136-150: The Robinhood role-change test only checks FREEZER
transitions, so it misses the RELAYER changes applied by
SparkRobinhood_20260716.execute(). Update test_ROBINHOOD_controllerRoleChanges
in Spell_20260716.t.sol to add matching assertions for controller.RELAYER()
before and after _executeAllPayloadsAndBridges(), verifying
ALM_RELAYER_MULTISIG_1 is revoked and ALM_BACKSTOP_RELAYER_MULTISIG is granted.
In `@src/proposals/20260716/XLayer_20260716.t.sol`:
- Around line 72-73: The fork setup in XLayerConfigTests, XLayerE2ETests, and
XLayerCrosschainE2ETests is hardcoding the public XLayer RPC and a fixed block,
which should be replaced with configurable values. Update the setUp path that
calls vm.createSelectFork to read the RPC URL from the same config/env mechanism
used by the other networks, and move the fork block number into a shared
constant so all XLayer tests use the same source of truth.
In `@src/test-harness/SpellRunner.sol`:
- Around line 100-110: The XLayer chain setup in SpellRunner currently hardcodes
the RPC endpoint, unlike the rest of the harness that uses vm.envString for
overridable URLs. Update the xlayer entry in setChain to read the RPC URL from
an environment variable, following the same pattern used for robinhood_chain and
the other chain definitions, so CI and local runs can avoid the public
rate-limited endpoint.
🪄 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: f3c7c161-c91d-4693-b9ab-125de5275b5c
📒 Files selected for processing (20)
.github/workflows/test.ymlarchive/20260702/SparkArbitrumOne_20260702.solarchive/20260702/SparkAvalanche_20260702.solarchive/20260702/SparkBase_20260702.solarchive/20260702/SparkEthereum_20260702.solarchive/20260702/SparkOptimism_20260702.solarchive/20260702/SparkUnichain_20260702.solarchive/20260702/Spell_20260702.t.soldiffs/20260716-Ethereum-0xC13e21B648A5Ee794902342038FF3aDAB66BE987-pre_20260716-Ethereum-0xC13e21B648A5Ee794902342038FF3aDAB66BE987-post.mdlib/spark-address-registrylib/xchain-helperssrc/SparkPayloadEthereum.solsrc/interfaces/Interfaces.solsrc/libraries/ChainIdUtils.solsrc/proposals/20260716/SparkEthereum_20260716.solsrc/proposals/20260716/SparkRobinhood_20260716.solsrc/proposals/20260716/Spell_20260716.t.solsrc/proposals/20260716/XLayer_20260716.t.solsrc/test-harness/SpellRunner.solsrc/test-harness/SpellTests.sol
🛑 Comments failed to post (3)
src/proposals/20260716/SparkEthereum_20260716.sol (2)
26-27: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
INCENTIVES_RECIPIENTandANCHORAGE_FEES_RECIPIENTare the same placeholder address.Both constants are set to
0x2002…02, so the incentives transfer (Line 84) and the anchorage-fees transfer (Line 88) currently go to the same recipient. This is incorrect production behavior and it also breakstest_ETHEREUM_sparkTreasury_transfersinSpell_20260716.t.sol: with both recipients equal, the final balance becomesINCENTIVES_AMOUNT + ANCHORAGE_FEES_AMOUNT, so the firstassertEq(..., incentivesRecipientBalanceBefore + INCENTIVES_AMOUNT)will fail. Set distinct real addresses before merge.🤖 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/20260716/SparkEthereum_20260716.sol` around lines 26 - 27, `INCENTIVES_RECIPIENT` and `ANCHORAGE_FEES_RECIPIENT` are both using the same placeholder value, so the transfers in the proposal currently route to one address and break the treasury test. Update the constants in `SparkEthereum_20260716` to use two distinct, real recipient addresses before merge, and verify the balance expectations in `test_ETHEREUM_sparkTreasury_transfers` still match the separate incentive and anchorage-fees transfers.
40-41: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # How prior spells wire foreign PAYLOAD_* immutables in their constructors rg -nP 'PAYLOAD_(ROBINHOOD|XLAYER|ARBITRUM|BASE)\s*=' src -C2Repository: sparkdotfi/spark-spells
Length of output: 161
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Files matching SparkEthereum_20260716 ==\n' fd -a 'SparkEthereum_20260716.sol' src || true printf '\n== Search PAYLOAD_ROBINHOOD / PAYLOAD_XLAYER declarations and usages ==\n' rg -n --hidden -S 'PAYLOAD_(ROBINHOOD|XLAYER)\b' src -C 3 || true printf '\n== Search for constructor assignments to payload-like immutables ==\n' rg -n --hidden -S 'constructor\s*\(|immutable|PAYLOAD_' src/proposals src -C 2 || trueRepository: sparkdotfi/spark-spells
Length of output: 18276
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Outline proposal file ==\n' ast-grep outline src/proposals/20260716/SparkEthereum_20260716.sol --view expanded || true printf '\n== Proposal file with line numbers ==\n' cat -n src/proposals/20260716/SparkEthereum_20260716.sol | sed -n '1,220p' printf '\n== Find SparkPayloadEthereum definition ==\n' rg -n --hidden -S 'contract\s+SparkPayloadEthereum|PAYLOAD_ROBINHOOD|PAYLOAD_XLAYER|PAYLOAD_' src -C 4 || trueRepository: sparkdotfi/spark-spells
Length of output: 14258
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== SparkPayloadEthereum constructor/initializer slice ==\n' cat -n src/SparkPayloadEthereum.sol | sed -n '1,220p' printf '\n== AaveV3PayloadBase constructor slice ==\n' cat -n src/AaveV3PayloadBase.sol | sed -n '1,140p' printf '\n== Search for inline initialization of PAYLOAD_* in src ==\n' rg -n --hidden -S 'PAYLOAD_(ARBITRUM|BASE|GNOSIS|OPTIMISM|UNICHAIN|AVALANCHE|ROBINHOOD|XLAYER)\s*(=|:)' src -C 2 || trueRepository: sparkdotfi/spark-spells
Length of output: 17382
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== SparkPayloadEthereum constructor/initializer slice ==\n' sed -n '1,220p' src/SparkPayloadEthereum.sol | cat -n printf '\n== AaveV3PayloadBase constructor slice ==\n' sed -n '1,180p' src/AaveV3PayloadBase.sol | cat -n printf '\n== Search for inline initialization of PAYLOAD_* in src ==\n' rg -n --hidden -S 'PAYLOAD_(ARBITRUM|BASE|GNOSIS|OPTIMISM|UNICHAIN|AVALANCHE|ROBINHOOD|XLAYER)\s*(=|:)' src -C 2 || trueRepository: sparkdotfi/spark-spells
Length of output: 18767
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Constructors in proposal contracts ==\n' rg -n --hidden -S 'constructor\s*\(' src/proposals -C 2 || true printf '\n== Proposal contracts that mention PAYLOAD_ ==\n' rg -n --hidden -S 'PAYLOAD_' src/proposals -C 2 || trueRepository: sparkdotfi/spark-spells
Length of output: 2764
Wire
PAYLOAD_ROBINHOOD/PAYLOAD_XLAYERin the constructor
SparkPayloadEthereum.execute()only queues these branches when the inherited immutables are nonzero, but this contract never assigns them. Pass the Robinhood/XLayer payload addresses into the constructor and set those fields, or the new dispatches will stay disabled.🤖 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/20260716/SparkEthereum_20260716.sol` around lines 40 - 41, The constructor in SparkEthereum_20260716 currently leaves the inherited payload addresses unset, so the PAYLOAD_ROBINHOOD and PAYLOAD_XLAYER branches in SparkPayloadEthereum.execute() never activate. Update the constructor to accept the Robinhood and XLayer payload addresses and assign them to the corresponding inherited immutables/fields so those dispatch paths are enabled. Make sure the existing execute() checks will now see nonzero values and queue both payloads correctly.src/proposals/20260716/XLayer_20260716.t.sol (1)
99-99: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Comments reference USDG/SPUSDG but this suite exercises the USDT (spUSDT) vault.
The vault under test is USDT-based (
asset() == USDT,name() == "Spark Savings USDT", Lines 125-127), yet several comments were carried over from the Robinhood/USDG tests: Line 99 ("Spark Savings USDG Vault roles"), Line 108, Line 215, Line 259, and Lines 312-333. Additionally, Line 215 states the vault "already has 8.01e6 of USDG", but the assertions on Lines 226-227 verifytotalAssets/totalSupply == 1e6. Please correct these to avoid confusion about which asset/amount is being validated.Also applies to: 108-108, 215-215, 259-259, 312-333
🤖 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/20260716/XLayer_20260716.t.sol` at line 99, The comments in this test suite still refer to USDG/SPUSDG even though the exercised vault is the USDT-based spUSDT vault, so update the affected comments in the XLayer_20260716_t.sol scenario to match the actual asset and vault names. In particular, revise the placeholders around the role setup, vault state, and withdrawal sections so they reference USDT/Spark Savings USDT instead of USDG/SPUSDG, and fix the deposit-state comment in the section around the vault balance check to describe the 1e6 totalAssets/totalSupply assertion rather than an 8.01e6 USDG amount.
Overview
🔗 Commit Hash: 32ee908 |
* initial commit * feat: Add RH testing infra (#175) feat: add rh test * feat: Add XLayer infra (#176) * add: rh tests * add: grove transfer usds * add: description for robinhood chain * feat: Add xlayer integration tests (#177) * feat: add xlayer integration tests * feat: update to merge latest * fix: Update e2e tests (#178) * fix: update e2e tests * fix: rm prompt diff * add: xlayer tests * add: rh tests * fix: review * fix: review * add: roles to rh alm proxy freezable and tests * fix * fix: partial review * add: fix approx * fix: review * fix: review * add deploy * fix: tests --------- Co-authored-by: Lucas Manuel <lucasmanuel.tech@gmail.com> Co-authored-by: Charan (aka TheMj0ln1r) <104960642+TheMj0ln1r@users.noreply.github.com>
1 participant
Summary by CodeRabbit