fix(dataworker): skip orbit gas funding for Universal chains (Robinhood)#3526
fix(dataworker): skip orbit gas funding for Universal chains (Robinhood)#3526droplet-rl wants to merge 2 commits into
Conversation
Robinhood is technically an Arbitrum Orbit chain (chainIsOrbit returns true) but relays HubPool->SpokePool roots and tokens via the Succinct/Helios SP1 light-client bridge (HubPoolStore), not the Orbit Inbox. It therefore needs no ETH pre-loaded for L1->L2 message fees and has no entry in ARBITRUM_ORBIT_L1L2_MESSAGE_FEE_DATA. _executePoolRebalanceLeaves routed any chainIsArbitrum/chainIsOrbit leaf through _getRequiredEthForOrbitPoolRebalanceLeaf, which destructured the (undefined) fee data and threw, crashing the entire pool-leaf execution run: TypeError: Cannot destructure property 'amountWei' of 'ARBITRUM_ORBIT_L1L2_MESSAGE_FEE_DATA[leaf.chainId]' as it is undefined. Exclude UNIVERSAL_CHAINS from the orbit gas-funding path, consistent with the finalizer's existing UNIVERSAL_CHAINS handling. Adds a regression test asserting a Robinhood leaf enqueues executeRootBundle with no loadEthForL2Calls funding tx. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66d77809d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // For non-Universal orbit leaves we need to check we have enough gas tokens to pay for the | ||
| // L1 to L2 message. | ||
| if ( | ||
| UNIVERSAL_CHAINS.includes(leaf.chainId) || |
There was a problem hiding this comment.
Document the Universal-chain execution path
This changes dataworker pool-rebalance execution behavior by bypassing Orbit gas funding for Universal chains, but the repo guideline in AGENTS.md says to “Keep all relevant AGENTS.md and README.md files updated in the same change whenever behavior, configuration, interfaces, or runtime flow changes.” Please update the relevant dataworker docs (for example src/dataworker/README.md) so operators/contributors can see why Universal Orbit leaves skip loadEthForL2Calls.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good call — added a short "Orbit gas funding during pool-leaf execution" subsection to src/dataworker/README.md (commit a4263a3) explaining the Arbitrum/Orbit loadEthForL2Calls pre-funding step and why UNIVERSAL_CHAINS (Robinhood) skip it, mirroring the finalizer's Helios L1→L2 path.
| UNIVERSAL_CHAINS.includes(leaf.chainId) || | ||
| (!sdkUtils.chainIsArbitrum(leaf.chainId) && !sdkUtils.chainIsOrbit(leaf.chainId)) |
There was a problem hiding this comment.
Preserve funding for Robinhood token sends
This early-continues all Robinhood leaves before the token-send fee reservation, but the current config still enables Robinhood WETH pool leaves (src/common/Constants.ts:374) and maps Robinhood WETH to ArbitrumOrbitBridge (src/common/Constants.ts:548-550), whose constructL1ToL2Txn sends an outboundTransfer with nonzero ETH value for Robinhood (src/adapter/bridges/ArbitrumOrbitBridge.ts:24-27, :92-96). In a Robinhood leaf with a positive WETH netSendAmount, this now queues only executeRootBundle; if the HubPool has no preloaded ETH for L2 calls, the token bridge leg can fail even though the root message itself uses the Universal/Helios path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, but I believe this is a false positive — it conflates two independent subsystems.
_executePoolRebalanceLeaves (this PR) only enqueues two HubPool txs: loadEthForL2Calls (pre-funding the HubPool to pay the Arbitrum Inbox L1→L2 submission fee) and executeRootBundle. The ArbitrumOrbitBridge.constructL1ToL2Txn / outboundTransfer you reference lives in the inventory-rebalancing subsystem: the CUSTOM_BRIDGE map (Constants.ts:548) is consumed only by AdapterManager, Refiller, and jussi — never by the dataworker pool-leaf path. That adapter sends outboundTransfer from the relayer's own l1Signer wallet with its own value (l1SubmitValue), which is entirely independent of the HubPool's preloaded ETH. This PR does not touch that path, so it is unchanged.
For the HubPool's own root-bundle token relay, Robinhood is a Universal chain: it relays roots and net token amounts via the Succinct/Helios SP1 bridge (HubPoolStore / Universal SpokePool), not the Orbit Inbox — so the HubPool needs no preloaded ETH. This is confirmed by the existing @TODO at ARBITRUM_ORBIT_L1L2_MESSAGE_FEE_DATA ("Do we even need this for Robinhood? We are using Universal SpokePool"), Robinhood having no fee-data entry, and the finalizer routing UNIVERSAL_CHAINS through heliosL1toL2Finalizer.
Note also that before this PR the missing fee-data entry made _getRequiredEthForOrbitPoolRebalanceLeaf throw for Robinhood, so no Robinhood leaf could execute at all — there was never a working "fund the Robinhood token send" path to regress. No code change here.
Codex review (P2) flagged that AGENTS.md asks READMEs to track runtime-flow changes. Document why pool-leaf execution skips loadEthForL2Calls for UNIVERSAL_CHAINS (Robinhood) in src/dataworker/README.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @codex for the review. Addressed both: P2 (docs) — accepted. Added an "Orbit gas funding during pool-leaf execution" subsection to P1 (Robinhood WETH token sends) — respectfully disagree, no code change. This conflates two independent subsystems. @codex please re-review the updated diff. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66d77809d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // For non-Universal orbit leaves we need to check we have enough gas tokens to pay for the | ||
| // L1 to L2 message. | ||
| if ( | ||
| UNIVERSAL_CHAINS.includes(leaf.chainId) || |
There was a problem hiding this comment.
Update dataworker docs for the funding skip
This changes the dataworker runtime path by skipping the Orbit loadEthForL2Calls pre-funding step for Universal chains, but this commit only updates code/tests. The repo-level AGENTS.md says to “Keep all relevant AGENTS.md and README.md files updated in the same change whenever behavior, configuration, interfaces, or runtime flow changes,” so src/dataworker/README.md should describe why Robinhood/Universal leaves bypass Orbit gas funding to avoid future operators or contributors reintroducing the old pre-funding assumption.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already addressed — this review re-ran on the original commit 66d77809 before the docs landed. The follow-up commit a4263a35 adds an "Orbit gas funding during pool-leaf execution" subsection to src/dataworker/README.md explaining the Arbitrum/Orbit loadEthForL2Calls pre-funding step and why UNIVERSAL_CHAINS (Robinhood) bypass it, so future operators/contributors won't reintroduce the old pre-funding assumption.
|
This second Codex review re-ran on the original commit |
Summary
Testing
|
|
Confirmed — we're aligned. The docs requested by the duplicate P2 are already on the branch in commit (Note: the stray uncommitted |
Problem
The
zion-across-l1-executor(dataworker) crashed during pool-leaf execution:This throws out of the whole
_executePoolLeavesAndSyncL1Tokensrun, so it blocks all pool-rebalance-leaf execution in the bundle, not just Robinhood's leaf.Root cause
Robinhood (
CHAIN_IDs.ROBINHOOD) is technically an Arbitrum Orbit chain, sosdkUtils.chainIsOrbit()returnstruefor it._executePoolRebalanceLeavesroutes everychainIsArbitrum || chainIsOrbitleaf through_getRequiredEthForOrbitPoolRebalanceLeaf, which destructuresARBITRUM_ORBIT_L1L2_MESSAGE_FEE_DATA[leaf.chainId].But Robinhood relays HubPool→SpokePool roots and tokens via the Succinct/Helios SP1 light-client bridge (
HubPoolStore), not the Orbit Inbox — it's inUNIVERSAL_CHAINS. So it needs no ETH pre-loaded for L1→L2 message fees and has no entry inARBITRUM_ORBIT_L1L2_MESSAGE_FEE_DATA(there's an existing@TODOabout this inConstants.ts). The destructure hitsundefinedand throws.Fix
Exclude
UNIVERSAL_CHAINSfrom the Orbit gas-funding path in_executePoolRebalanceLeaves. This mirrors the finalizer, which already special-casesUNIVERSAL_CHAINSfor the Helios L1→L2 path. Universal leaves now fall straight through toexecuteRootBundlewith noloadEthForL2Callsfunding tx.Testing
yarn build✅yarn lint/ prettier ✅_executePoolRebalanceLeavessuite ✅ (6 passing) — addeddoes not fund universal orbit leaf (e.g. Robinhood)asserting a Robinhood leaf enqueuesexecuteRootBundlewith noloadEthForL2Calls; existingfunds arbitrum leafstill passes, so real Arbitrum funding is unchanged.Context: Slack thread where Paul noted "For Robinhood we are using succinct, not the orbit message bridge."
🤖 Generated with Claude Code