Route rebate to extraData receiver for sponsored depositors - #970
Route rebate to extraData receiver for sponsored depositors#970mswilkison wants to merge 2 commits into
Conversation
piotr-roslaniec
left a comment
There was a problem hiding this comment.
LGTM. Verified the rebate routing is safe (extraData is committed to in the P2(W)SH script hash, so a sponsored relayer cannot forge a different receiver), storage layout is upgrade-safe (gap 48 -> 47), and the require() guards added in 3ba6515 make the sponsored-depositor branch fail loud on missing/zero-address extraData.
When a user mints through a relayer-style depositor contract such as NativeBTCDepositor, the Bridge sees `msg.sender = depositor contract` and applies the RebateStaking lookup against that contract — which has no T stake of its own — instead of the actual L1 receiver encoded in `extraData`. The user's existing rebate budget is bypassed and the full deposit treasury fee (20 bps on mainnet today) is charged. This change adds a governance-managed `sponsoredDepositors` allowlist to `BridgeState`. When `_revealDeposit` is called by an allowlisted depositor and `extraData != bytes32(0)`, the rebate is keyed off the L1 address decoded from `extraData` rather than `msg.sender`. `deposit.depositor` is unchanged so refund and finalize accounting are not affected; delegation through `RebateStaking.getStaker` continues to work transparently. Bridge exposes `setSponsoredDepositor(address, bool)` (onlyGovernance) and `isSponsoredDepositor(address)`. BridgeGovernance gains a matching forwarder following the existing `setRebateStaking` pattern. A new event `SponsoredDepositorSet(address indexed depositor, bool sponsored)` is emitted on allowlist changes. The allowlist consumes one slot from the existing `__gap` (48 → 47) so storage layout remains upgrade-safe. The allowlist is intentionally narrow: only depositors whose `extraData` is provably the L1 address that should be charged belong on it. Today that's `NativeBTCDepositor` only (not yet deployed to mainnet). Cross-chain depositors whose `extraData` is an L2 user identifier (Sui, StarkNet, Arbitrum/Base via Wormhole) must NOT be allowlisted and are handled separately by the cross-chain rebate work in #952. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the sponsored-depositor branch silently fell back to keying the rebate off the relay contract (which has no stake) if extraData was absent or decoded to the zero address. That regressed the user to the full 20 bps fee without surfacing the misconfiguration. Require an allowlisted depositor to always provide a non-zero extraData that decodes to a non-zero L1 address. NativeBTCDepositor already satisfies this via AbstractBTCDepositor.revealDepositWithExtraData, so production behavior is unchanged; the guard prevents a future allowlist addition from silently regressing. Add a test covering the no-extraData revert path.
3ba6515 to
d81ae59
Compare
|
Warning Review limit reached
Next review available in: 55 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
Summary
Closes the gap where a T staker who mints through a relayer-style depositor (e.g.
NativeBTCDepositor) loses their fee waiver because the Bridge sees the depositor contract asmsg.senderand routes theRebateStakinglookup against that contract's (empty) stake instead of the L1 receiver's.sponsoredDepositorsallowlist toBridgeState.Deposit._revealDeposit, when the caller is allowlisted andextraData != bytes32(0), the rebate is keyed off the address decoded fromextraDatarather thanmsg.sender.deposit.depositoris unchanged — refund and finalize accounting are untouched; delegation viaRebateStaking.getStakerkeeps working.Bridge.setSponsoredDepositor(address, bool)(onlyGovernance) +isSponsoredDepositor(address)view; matchingBridgeGovernance.setSponsoredDepositor(...)forwarder.SponsoredDepositorSet(address indexed depositor, bool sponsored).__gap(48 → 47), so layout remains upgrade-safe.Why
depositTreasuryFeeDivisor = 500on mainnet today (20 bps), so this is the fee a user with T staked would expect their rebate to reduce on every deposit. Without this routing, gasless minting throughNativeBTCDepositor(live on mainnet at0xad7c6d46f4a4bc2d3a227067d03218d6d7c9aaa5) always pays the full 20 bps because the depositor contract has no stake. After this change, the user's existingRebateStakingbudget is honored exactly as it would be on a direct L1 reveal.Allowlist scope
The allowlist is intentionally narrow. Membership means "
extraDataon reveals from this contract is the L1 address whose stake should be charged." OnlyNativeBTCDepositorsatisfies that today. The allowlist ships empty; after merge, a single governance call adds the mainnet proxy:Explicitly out of scope: cross-chain depositors (Sui, StarkNet, Arbitrum/Base via Wormhole). Their
extraDatais an L2/non-EVM identifier, not an L1 staker. Cross-chain rebates are handled by #952 with its proper EIP-712/EIP-1271 authorization model.Test plan
🤖 Generated with Claude Code