Configure NTT migration chains - #999
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mswilkison
changed the title
2b5690c to
a0c36ee
Compare
acb96b1 to
c4148ca
Compare
9f45f3e to
721978d
Compare
- Reformat L1BTCDepositorNtt.sol and MockNttManagerWithExecutor.sol per prettier (contracts-format CI). - Regenerate typescript API reference for the new normalizeNttRecipient export and shifted ntt.ts line references (typescript-docs CI). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The executor variant hardcoded the destination-chain unused-gas refund to bytes32(uint256(uint160(msg.sender))) — the relayer's L1 address left-padded. On EVM destinations that is the canonical Wormhole convention, but on non-EVM destinations (Solana, Sui) it is not a controllable account, so any unused-gas refund is silently black-holed. Add a per-instance, owner-settable `destinationRefundAddress` (32-byte universal address format). When unset it falls back to the deposit recipient, which is always a controllable account on the destination chain — safe-by-default on any chain with no governance action. This removes the only chain-type assumption in the contract, so an instance can be initialized for any destinationChainId without silently losing refunds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed a merge commit ( Rationale for how it was resolved: since the merge-base, Note this does not touch the open design question of the destination-recipient encoding (full 32-byte vs chain-ID-packed) — that remains as designed in this PR. |
Reconciles the NTT migration branch with main. main's only change since the merge-base is the Sei removal (#998), which this branch had already applied via its fixed-destination NTT redesign; all conflicts resolved in favor of this branch's redesign. Resulting tree is identical to the branch tip plus removal of the residual cross-chain/sei artifacts.
Regenerates the TypeScript api-reference docs from the merged source to resolve conflicts between this branch's NTT migration doc regen and dev's StarkNet relayer conflict-handling doc regen (#1025). No manual content was hand-merged; the api-reference tree is the typedoc output for the merged src/.
e9538fb to
cc6757f
Compare
…pported-chains # Conflicts: # solidity/test/cross-chain/wormhole/L1BTCDepositorNtt.core.test.ts # solidity/test/cross-chain/wormhole/L1BTCDepositorNtt.ntt.test.ts # solidity/test/cross-chain/wormhole/L1BTCDepositorNtt.utils.test.ts
## Summary - Pay the deposit finalization reimbursement before the deferred initialization reimbursement, so gas consumed by the initialization receiver cannot be counted again in the finalizer reimbursement. - Apply the same ordering to the flattened legacy Arbitrum and Base V2 implementations. - Update focused and existing depositor tests to require the safe reimbursement order. This only reorders the two terminal reimbursement calls. It does not change storage, the ABI, deposit state transitions, or token-transfer behavior. ## PR #908 evaluation This is the focused code-level follow-up extracted from #908: - **Deferred reimbursement double counting:** validated against current dev; the regression assertion failed before the fix and passes afterward. - **Refund blocking:** no fix warranted. The canonical ReimbursementPool uses a non-reverting low-level send and emits SendingEtherFailed if the pool is empty or the receiver rejects Ether, so those cases do not block finalization. - **Initialization/finalization gas-price mismatch:** confirmed as an accounting limitation, but #908 proposes an unsafe conversion because the pool adds staticGas at the later transaction gas price. A correct fix needs a pool-level fixed-wei reimbursement design rather than an upgradeable depositor storage change. - **Executor fee and parameter validation:** already covered by #962, so none of that work is duplicated here. I also checked current open PRs immediately before submission. #999 touches AbstractL1BTCDepositor only to add NTT migration hooks and recipient transformation; it does not change reimbursement accounting. No open PR other than stale #908 implements this fix. ## Validation - yarn build - yarn typecheck - yarn format (passes with repository-existing warnings) - AbstractL1BTCDepositor, BTCDepositorWormhole, L1BTCDepositorWormhole, L1BTCDepositorWormholeV2Arbitrum, L1BTCDepositorWormholeV2Base, and NativeBTCDepositor suites - All L1BTCDepositorNtt suites - All TypeScript StarkNetBitcoinDepositor suites (34 passing, 4 pre-existing pending) Refs: #908, #962, #999 --------- Co-authored-by: Piotr Rosłaniec <p.roslaniec@gmail.com>
2 participants
Summary
Notes
This is stacked on #998 because the NTT target set intentionally excludes Sei.
The Base mainnet configuration now uses Wormhole chain ID
30instead of the EVM chain ID.Executor transfers default the destination gas refund address to the destination recipient so refunds remain deliverable on non-EVM chains. EVM deployments can set
destinationRefundAddressif they want a relayer or treasury refund account instead.Validation
git diff --checktypescript/node_modules/.bin/prettier --check cross-chain/solana/tests/01__tbtc.ts cross-chain/solana/tests/helpers/tbtc.ts cross-chain/solana/target/idl/tbtc.json typescript/src/lib/solana/target/idl/tbtc.json solidity/scripts/configure-l1-btc-depositor-ntt.tsrustfmt --check cross-chain/solana/programs/tbtc/src/error.rs cross-chain/solana/programs/tbtc/src/processor/admin/transfer_mint_authority.rsnpm run test -- --grep "L1BTCDepositorNtt fixed destination|L1BTCDepositorNttWithExecutor fixed destination"fromsolidityLocal toolchain blockers:
anchoris not installed locally, so Anchor build/test was not runnable here.cargo check -p tbtcis blocked before project code bywasm-bindgen v0.2.87incompatibility with the installed Rust toolchain.sui move testis blocked by the missing localTokenBridgedependency path.