feat(bridge,vault): add PSBT covenant migration debt and reveal guard#957
feat(bridge,vault): add PSBT covenant migration debt and reveal guard#957lrsaturnino wants to merge 12 commits into
Conversation
Introduces the on-chain machinery that lets a migration vault settle deposit obligations against the Bridge in lockstep with the Bitcoin sweep flow. The mechanism enables a one-way migration channel where a designated vault accumulates "migration debt" as deposits are revealed under a tagged extra-data byte string, and that debt is repaid when the matching sweep transaction is proven on-chain. Bridge changes: - Adds a canonical migration debt vault pointer maintained by governance via setMigrationDebtVault and an atomic rotateMigrationDebtVault that refuses to leave the prior canonical vault holding outstanding debt. - Tightens setVaultStatus so the canonical vault cannot be untrusted while it is the canonical pointer, and so any vault that still reports outstanding migration debt cannot be untrusted (fail-open staticcall; vaults not implementing the interface are unaffected). - Adds a reveal-time guard in Deposit.revealDeposit: deposits whose extra-data carries the migration tag must come from a registered migration revealer in the per-deposit vault and must have outstanding debt to repay; conversely, deposits from a registered migration revealer that lack the tag are rejected. - Exposes explicit failure reasons via MigrationRevealRejected with reason codes covering missing debt, missing tag, mismatched revealer, unauthorised revealer, vault call failure and bad vault response. - Reduces __gap from 48 to 47 to introduce migrationDebtVault while preserving storage layout continuity for proxy upgrades; documented in docs/BRIDGE_STORAGE_UPGRADE_NOTES.md. DepositSweep changes: - After a sweep is proven, notifies the canonical migration vault via ITBTCVaultMigrationSweepHook with a fail-open batch call followed by bounded per-revealer fallback retries so a single bad entry cannot strand a completion batch. Vault changes: - TBTCOptimisticMinting now implements ITBTCVaultMigrationDebt with per-revealer migration debt tracking, a registration counter that enforces hasOutstandingMigrationDebt across rotations, and a TBTCMigrationDebtOperations library that isolates the debt state transitions (register, repay, residual clear). - Adds MIN_OPTIMISTIC_MINTING_FEE_DIVISOR = 10 to bound the maximum optimistic minting fee rate at 10%. - Adds MAX_MIGRATION_SWEEP_BATCH_SIZE = 100 to cap per-sweep callback fan-out. - Wires migrationSweepNotifier and migrationSweepReserve so a downstream contract can be informed exactly once when a revealer's migration debt reaches zero, with pending-completion bookkeeping that survives partial batch outcomes. Test infrastructure: - Adds harness contracts (BridgeForVaultHarness, TBTCVaultHarness, DepositRevealGuardHarness, DepositSweepCallbackHarness, plus mocks for migration debt vault, sweep notifier, sweep vault and a regular comparison vault) so the migration paths can be exercised in isolation from the full Bridge fixture. - Adds a NatSpec verification script that asserts the required documentation surface for the migration debt interface remains present. Deploy: - Resolves wallet registry and reimbursement pool against deployment artifacts when present so downstream networks can wire the new debt vault pointer through the bridge governance flow.
Bridge deployed bytecode exceeded the EIP-170 24,576-byte limit. Two public reinitializers — initializeV2_FixVaultZeroDeposit and initializeV5_RepairRebateStaking — were permanently consumed on every live proxy: slot 50 _initialized reads 0x05 on mainnet and Sepolia, with Initialized events at mainnet blocks 24282346 and 24800704. OpenZeppelin Initializable does not decrement, so the functions could never run again. Removing both reinitializers plus a Bridge-only optimizer override (runs 500) brings deployed bytecode to 23,522 bytes — 1,054 under the limit. Storage layout is unchanged. The DepositVaultFixed and RebateStakingRepaired event declarations are retained so historical logs decode through the current ABI. Deploy scripts that targeted the removed selectors are hard-disabled (func.skip = true) with on-chain evidence captured in the deprecation headers. The exported encoder helpers in script 85 remain importable as historical calldata-format regression coverage. The Bridge.RebateRecovery test is deleted — it exercised the removed reinitializer through helpers.upgrades.upgradeProxy. Incidental: prettier collapsed the PreviousMigrationDebtVaultMismatch declaration into a single line.
| revert MigrationRevealRejected(MIGRATION_DEBT_MISSING_REASON); | ||
| } | ||
| } else { | ||
| if (isRegisteredMigrationRevealer(self, reveal.vault, msg.sender)) { |
There was a problem hiding this comment.
The NatSpec says fail-closed is intentional because a failed canonical vault call makes "migration-tag enforcement indeterminate." Makes sense for the migration grant path. But this function is also called in the non-migration else branch, so a broken canonical vault locks out regular depositors too, not just migration ones. Was that consequence considered, or should the blocking check be fail-open?
There was a problem hiding this comment.
Intentional fail-closed — covered by solidity/test/bridge/Deposit.MigrationRevealGuard.Harness.test.ts:445-478. The non-migration-depositor lockout observation is correct, but a failed canonical lookup makes migration-tag enforcement indeterminate, so reverting is the safer policy. A future hardening could be a set-time interface probe on setMigrationDebtVault to convert this to a deploy-time check; tracking as a separate follow-up.
|
|
||
| // Best-effort fallback: retry each revealer in isolation so one bad | ||
| // entry cannot strand the whole completion batch. | ||
| for (uint256 i = 0; i < revealers.length; i++) { |
There was a problem hiding this comment.
The raw revealers array gets passed to the vault, which rejects batches over 100. There is no cap on sweep inputs at the Bridge level (inputsCount comes straight from the Bitcoin tx varint), so a sweep with 101+ deposits always fails the batch and falls through to O(N) individual vault calls inside submitDepositSweepProof. The callback tests only cover 1 revealer -- was this tested for gas with large migration sweeps?
There was a problem hiding this comment.
Addressed in commit d14c2e4 — chunked at MAX_MIGRATION_SWEEP_BATCH_SIZE = 100 (solidity/contracts/bridge/DepositSweep.sol:51,307-380). Tests at solidity/test/bridge/DepositSweep.MigrationCallback.test.ts:93,125 cover the 150-revealer happy path and the chunked fallback (2 batch failures + 150 single-hook successes).
| emit MigrationSweepNotifierUpdated(previousNotifier, notifier); | ||
| } | ||
|
|
||
| function setMigrationSweepReserve(address revealer, address reserve) |
There was a problem hiding this comment.
The dedicated MigrationSweepCompletionPendingCleared event makes it clear the coupling is intentional. But setMigrationSweepReserve has no NatSpec at all, so an operator zeroing the reserve during an incident would have no hint that it also kills the pending callback. Worth adding a note there.
There was a problem hiding this comment.
Addressed in commit d14c2e4 — NatSpec block added above solidity/contracts/vault/TBTCOptimisticMinting.sol:799 documenting that reserve == address(0) clears pendingMigrationSweepCompletion[revealer] and emits MigrationSweepCompletionPendingCleared.
| "Migration debt already registered" | ||
| ); | ||
|
|
||
| uint256 nextDebt = migrationDebt[revealer] + amount; |
There was a problem hiding this comment.
The NatSpec return value already says "equals amount since registration starts from zero," and the require(migrationDebt[revealer] == 0) three lines above guarantees it. Safe to drop the addition and assign directly?
There was a problem hiding this comment.
Addressed in commit d14c2e4 — solidity/contracts/vault/TBTCMigrationDebtOperations.sol:37 now assigns amount directly. The existing require(migrationDebt[revealer] == 0) makes the prior addition equivalent.
| ); | ||
| } | ||
|
|
||
| function isMigrationReveal(bytes32 extraData) internal pure returns (bool) { |
There was a problem hiding this comment.
Both Deposit.sol (line 478) and this file have an identical isMigrationReveal helper. MigrationExtraData already owns the tag constant; was putting the helper there considered?
There was a problem hiding this comment.
Addressed in commit d14c2e4 — helper moved into solidity/contracts/bridge/MigrationExtraData.sol:13-15. Callers updated at Deposit.sol:227, TBTCOptimisticMinting.sol:337,399, and DepositRevealGuardHarness.sol:73.
| emit MigrationRevealerSet(revealer, allowed); | ||
| } | ||
|
|
||
| function setMigrationSweepNotifier(address notifier) external onlyOwner { |
There was a problem hiding this comment.
No code.length > 0 check on the setter (no other setter in vault/bridge does this either, so may be accepted practice). Worth noting: an EOA notifier causes every notifyMigrationSweep to revert inside the vault via Solidity's EXTCODESIZE check. Those reverts get swallowed by the low-level .call in notifyMigrationSweepCallback, so MigrationSweepCallbackRetryFailed fires for every revealer silently. Intentional risk or worth a guard at set-time?
There was a problem hiding this comment.
Addressed in commit d14c2e4 — solidity/contracts/vault/TBTCOptimisticMinting.sol:766 now requires notifier == address(0) || notifier.code.length > 0. EOA notifier reverts at set-time; address(0) remains a valid disable state. Tests at solidity/test/vault/TBTCVault.MigrationDebt.test.ts:458,464.
- Chunk migration sweep callback batches at the vault's 100-revealer limit to avoid the O(N) per-revealer fallback path on oversized sweeps (DepositSweep.notifyMigrationSweepCallback, MAX_MIGRATION_SWEEP_BATCH_SIZE). - Reject EOA / non-contract addresses at setMigrationSweepNotifier set time so misconfiguration surfaces before sweep callbacks fire; address(0) remains a valid disable state. - Guard registerMigrationDebt against re-registration while a pending sweep completion is unresolved, closing a state-machine gap where a stale callback could clear a fresh reserve. - Document the side effect of zeroing setMigrationSweepReserve (clears pendingMigrationSweepCompletion) via NatSpec. - Simplify registerDebt in TBTCMigrationDebtOperations to a direct assign; the existing require(migrationDebt == 0) makes the prior addition equivalent. - Consolidate the duplicate isMigrationReveal helper into MigrationExtraData; update callers in Deposit, TBTCOptimisticMinting, and DepositRevealGuardHarness. Tests: 4 new in TBTCVault.MigrationDebt (EOA notifier reverts, zero disable, pending-sweep guard, re-register after pending clears) and 2 new in DepositSweep.MigrationCallback (chunking happy path, chunked fallback). Local yarn build clean; all 48 migration-tagged tests pass. Pre-existing unrelated failures (cross-chain/Wormhole, OM divisor-math) remain untouched.
…ivisor Restores divisor=0 as the explicit "fee disabled" governance state alongside the existing non-zero minimum bound. The previous guard rejected zero, but the fee formula at TBTCOptimisticMinting.sol still retained the `divisor > 0 ? amount / divisor : 0` ternary — leaving the zero branch as permanently dead code while three test contexts that asserted truncation- to-zero with `divisor = type(uint32).max` empirically failed (e.g. 199_900_000_000_000 / 4_294_967_295 = 46_542, not 0). Both setter guards (begin and finalize) now accept `divisor == 0 || divisor >= MIN_OPTIMISTIC_MINTING_FEE_DIVISOR`, aligning the setter with the fee formula's existing zero-handling branch. The non-zero floor of 10 (capping non-zero fee rate at 10%) is preserved, so divisors in [1, 9] remain blocked. Test changes: - Two zero-fee test contexts restored to their pre-port `divisor = 0` baseline; arithmetic comments and labels updated. - The setter test that asserted `divisor = 0` reverts is replaced with a boundary test asserting `divisor = 1` reverts, matching the new guard shape. - AbstractBTCDepositor and the OM fee formula already handle `divisor = 0` correctly via `divisor > 0 ? ... : 0`; no other code paths needed changes. NatSpec on MIN_OPTIMISTIC_MINTING_FEE_DIVISOR and on the finalize setter documents that 0 is a valid out-of-band sentinel meaning "no fee". Local CI: yarn build clean, yarn lint 0 errors, 101/101 tests pass in TBTCVault.OptimisticMinting.test.ts (was 98/101 with 3 failures). TBTCVault contract size 19.449 KB (well under EIP-170).
Mechanical prettier reformatting on PR-#132 ported migration files plus targeted slither-disable directives with one-line rationale on the intentional fail-open vault.call sites and the bounded migration sweep loop. Slither suppressions: - TBTCOptimisticMinting.registerMigrationDebt -> registerDebt: unused-return is intentional; the library returns the registered amount which equals the caller's input by invariant. - TBTCOptimisticMinting.repayDebt counter decrement: costly-loop suppressed with rationale that per-revealer atomicity is the contract's guarantee. - TBTCOptimisticMinting notifier dispatch + emit: calls-loop and reentrancy-events suppressed; notifier is governance-controlled, the array is bounded by MAX_MIGRATION_SWEEP_BATCH_SIZE, partial failure is the intended fail-open semantic. - DepositSweep.notifyMigrationSweepCallback batch + per-revealer fallback emits: reentrancy-events suppressed; the sweep proof is finalized before these informational retry signals fire. Format: - yarn format:fix applied across migration files; multi-line return tuples and call expressions reformatted to prettier defaults. - One whitespace fix in test/verify-migration-debt-natspec.sh. No behavior changes. yarn build, yarn format, and the full test suite pass: 2973 passing / 0 failing / 31 pending. Bridge bytecode 23,522 bytes (1,054 under EIP-170). 44 lint warnings remain (function ordering and max-states-count) but yarn format exits 0 — they are non-blocking and pre-existing across the repo, not specific to this port.
lrsaturnino
requested review from
Shadowfiend,
lukasz-zimnoch,
nkuba and
pdyraga
as code owners
…ry NatSpec Add a @dev note to setMigrationDebtVault explaining it skips the outstanding-debt guard (use rotateMigrationDebtVault for live rotations). Add @notice/@param NatSpec to setRevealer and setSweepReserve in TBTCMigrationDebtOperations, consistent with registerDebt above them.
| "Vault is not trusted" | ||
| ); | ||
|
|
||
| if (MigrationExtraData.isMigrationReveal(extraData)) { |
There was a problem hiding this comment.
Migration reveals gate on reveal.vault directly, bypassing the canonical vault check that the non-migration else branch runs via isRegisteredMigrationRevealer. So any trusted vault implementing ITBTCVaultMigrationDebt can authorize a migration reveal, not just the canonical one pointed to by migrationDebtVault. Is that the intended trust model, or should the canonical vault be the sole authority here?
There was a problem hiding this comment.
Good catch — pinned reveal.vault to self.migrationDebtVault in the migration branch in 02f448b; tests cover non-canonical (interface or not) and unset canonical.
The migration grant path in _revealDeposit authorized against reveal.vault directly, so any trusted vault implementing ITBTCVaultMigrationDebt could authorize a migration-tagged reveal - not just the canonical migrationDebtVault. Pin reveal.vault to self.migrationDebtVault inside the migration branch so the grant path and the post-sweep callback (DepositSweep) share a single source of truth, and migration reveals are rejected while migration is disabled at the Bridge level (canonical address zero). Adds tests for: trusted non-canonical vault, non-canonical migration-debt vault with registered revealer + debt, and unset canonical vault.
Four independently-scoped fixes from an audit pass on the migration debt subsystem: 1. Bridge ETH rescue (Blocker) submitFraudChallenge accepts ETH that can be orphaned when the downstream Fraud library refunds via a bounded-gas call whose return value is not checked: the challenge is marked resolved regardless of payout success, leaving funds custodied in the Bridge with no exit path. Adds recoverETH(payable, uint256) onlyGovernance with a checked low-level transfer, plus a forwarder on BridgeGovernance. 2. Canonical migration debt vault setter (Must-fix) setMigrationDebtVault now (a) probes the target's ITBTCVaultMigrationDebt interface conformance fail-closed -- the deposit-reveal guard fail-closes on staticcall failure to the canonical vault, so a non-conforming pointer would brick the pipeline -- and (b) rejects overwriting a canonical vault that still has outstanding migration debt, forcing governance to use rotateMigrationDebtVault (which atomically untrusts the previous vault). Emergency disable to address(0) still works because the previous-debt check uses a fail-open staticcall. The corresponding harness mirrors the same logic. 3. TBTCVault upgrade in-flight guard (Must-fix) finalizeUpgrade transferred TBTC ownership and the full Bank balance to a fresh new-vault contract that starts with empty migrationDebt, pendingMigrationSweepCompletion, migrationSweepReserve, isMigrationRevealer, and _outstandingMigrationDebtCount. Reverts when this vault is the canonical migration debt vault and hasOutstandingMigrationDebt is true so governance must drain debt before rotating ownership. Also widens hasOutstandingMigrationDebt from external to public so subclasses can read it without an external self-call. 4. Sweep solvency invariant on deposit parameters (Must-fix) BridgeState.updateDepositParameters only enforced dust > txMaxFee, but DepositSweep performs amount - treasuryFee - txFee and treasury fee is amount / treasuryFeeDivisor. A misconfigured divisor combined with the existing rule could create revealed deposits that always underflow at sweep time. Now also enforces dust > dust/treasuryFeeDivisor + txMaxFee when the divisor is non-zero, matching the worst-case sweep arithmetic. Bytecode budget: trimmed Bridge.sol require strings to custom errors to fit the EIP-170 limit after adding the rescue function and the canonical-vault setter logic.
The slither-disable-next-line directive was followed by a separate solhint-disable-next-line line, so slither's "next-line" applied to the solhint comment and the low-level-calls detector still fired on recipient.call. Inlines the solhint suppression on the call line and keeps slither-disable-next-line directly above it.
Bridge was below the bytecode ceiling by only 0.063 KiB, so replace remaining Bridge revert strings with custom errors and share the migration-debt staticcall helper across strict and fail-open paths. Rotation now probes non-zero target vaults for ITBTCVaultMigrationDebt, while setMigrationDebtVault fail-closes only on regular non-zero overwrites and preserves the emergency-disable lane. Fraud challenge ETH is tracked as open escrow and recoverETH is capped to the non-escrowed balance so governance cannot drain live challenge refund backing.
Probe canonical migration debt vault targets for every static selector used by Bridge reveal paths, while keeping outgoing-debt checks fail-closed and the zero-address disable lane. Add one-shot fraud challenge escrow seeding for upgraded deployments, gate rescue and new challenges until seeded, and skip legacy challenge decrements before the seed. Track whether newly submitted challenges are counted so seeded legacy challenges and post-seed challenges both release escrow without underflow. Update the hotfix deploy script to compute the seed from fraud challenge events and fail closed when a direct transaction value is insufficient evidence. Cover partial migration vault rejection, seeded legacy resolution/rescue bounds, and post-seed fraud challenge tracking.
The hotfix script previously linked the new Bridge implementation to the legacy mainnet Deposit, DepositSweep, and Fraud libraries. Those libraries' source was modified earlier in this PR chain, so the migration-debt reveal guard, post-sweep callbacks, and fraud-challenge escrow accounting would all be unreachable at runtime even though the new Bridge bytecode references them. Deploy fresh DepositTIP109Hotfix, DepositSweepTIP109Hotfix, and FraudTIP109Hotfix copies alongside the existing RedemptionTIP109Hotfix, and link the new Bridge implementation to the fresh addresses for every modified linked library. Add a deploy-script test that asserts the linkage: each modified library resolves to a freshly-deployed address rather than the legacy mainnet deployment, and the Bridge implementation libraries map matches.
2 participants
Audit positioning
This PR is the Bridge + Vault audit baseline for the Trail of Bits PSBT covenant assessment. Per the engagement-letter amendment of 2026-05-09, the SOW Scope URLs were rewritten to:
tlabs-xyz/tbtc-v2-ac#263— AC smart contracts (consolidated baseline; supersedestbtc/#132andtbtc/#217)threshold-network/keep-core#3882— covenant signerCoverage of this PR: Bridge migration-debt accounting and reveal guard (
Bridge.sol,BridgeState.sol,BridgeGovernance.sol,Deposit.sol,DepositSweep.sol,MigrationExtraData.sol); vault migration-sweep hooks and optimistic-minting migration guard (TBTCVault.sol,TBTCMigrationDebtOperations.sol,TBTCOptimisticMinting.sol); storage-layout delta for proxy upgrade —BridgeState.__gap48 → 47, newmigrationDebtVaultfield.Where to start reading
solidity/docs/BRIDGE_STORAGE_UPGRADE_NOTES.md— required reading for SOW Smart-Contracts Q8 (storage upgrade considerations: compromisedGOVERNANCE_ROLEupgrade, ERC-7201 namespace collisions,_revertIfLegacyStorageOccupied)solidity/contracts/bridge/MigrationExtraData.sol— encoding of extra-data fields used during covenant migrationsolidity/contracts/bridge/Deposit.sol— new migration-aware reveal guard withMigrationRevealRejected(bytes32)reason codessolidity/contracts/bridge/BridgeState.sol— storage layout deltasolidity/contracts/bridge/Bridge.sol— migration-debt vault wiring,setMigrationDebtVault/rotateMigrationDebtVaultsolidity/contracts/vault/TBTCMigrationDebtOperations.sol— migration-debt accounting librarysolidity/contracts/vault/TBTCOptimisticMinting.sol—MIN_OPTIMISTIC_MINTING_FEE_DIVISOR = 10, migration-aware mint gatesolidity/contracts/bridge/DepositSweep.sol— migration-sweep callback path with bounded fan-outInvariants and reason codes (to verify)
Invariants asserted by this PR:
outstandingMigrationDebt(revealer) = sum(registered_deposits.amount) − sum(repaid_deposits.amount)(per-revealer accounting inTBTCMigrationDebtOperations).setVaultStatus(canonicalMigrationDebtVault, untrusted)always reverts while it is the canonical pointer.rotateMigrationDebtVaultrefuses to leave the prior canonical vault holding outstanding debt.MIN_OPTIMISTIC_MINTING_FEE_DIVISOR (= 10)— caps the maximum minting fee at 10%.MAX_MIGRATION_SWEEP_BATCH_SIZE = 100caps per-sweep callback fan-out.migrationSweepNotifieris informed exactly once when a revealer's migration debt reaches zero, with pending-completion bookkeeping that survives partial batch outcomes.MigrationRevealRejected(bytes32)reason codes (raised inDeposit.revealDeposit):missing debtmissing tagmismatched revealerunauthorised revealervault call failureITBTCVaultMigrationDebtinterface revertsbad vault responseFail-open staticcall semantics: when a vault does not implement
ITBTCVaultMigrationDebtat all (no selector), thesetVaultStatusand migration-debt callback paths treat the vault as having zero migration debt — by design, so non-migration vaults are not punished. Vaults that do implement the interface but return malformed data are caught bybad vault response.Cross-repo coupling
This PR is the ABI source-of-truth that
tlabs-xyz/tbtc-v2-acvendors asexternal/tbtc-v2/<sha>/. The interfaces touched here (ITBTCVaultMigrationDebt,ITBTCVaultMigrationSweepHook,ITBTCVaultMigrationSweepNotifier) are consumed by covenant code in the sibling repo. After this PR merges,tbtc-v2-acwill re-pin its Bridge artifact tarball to the merged SHA.Deferred items (intentionally out of scope of this PR)
setMigrationDebtVaultrotation-guard bypass — flagged during PR feat(bridge,vault): add PSBT covenant migration debt and reveal guard #957 development and explicitly deferred to a follow-up. Auditors should note the current rotation flow and flag any new exploit path beyond this known concern; the known concern itself is tracked separately.Bridgecontract size exceeds EIP-170 —Bridgecompiles to 26.156 KB vs the 24.576 KB cap. Resolution (delegatecall library extraction orcontractSizercarve-out) is a separate change; see Known issues for upstream policy below.TBTCVault.OptimisticMinting.test.ts— divisortype(uint32).maxdoes not actually truncate to zero against the fixture amount; documented in detail below.Introduction
This PR ports the on-chain machinery for the PSBT covenant migration flow into the canonical
threshold-network/tbtc-v2source tree. The mechanism enables a one-way migration channel where a designated vault accumulates migration debt as deposits are revealed under a tagged extra-data byte string, and that debt is repaid when the matching sweep transaction is proven on-chain. The intent is for this repository to be the source of truth and audit baseline for the Bridge half of the covenant stack, in parallel with the covenant contracts and services that land intlabs-xyz/tbtc-v2-ac.Provenance
The same code and tests merged into
tlabs-xyz/tbtcon PR tlabs-xyz/tbtc#132 at merge commit tlabs-xyz/tbtc@02ee4bf1762ea2b8000b988345849645b15b672a. This PR ports the 37 files that live undercontracts/tbtc-v2/...in the monorepo to their canonical paths undersolidity/...here.Bridge contracts that diverged between the monorepo's vendored copy and
threshold-network/tbtc-v2/main(Bridge.sol,Deposit.sol) were three-way merged so that the TIP-109 hotfix (PR #948) is preserved alongside the migration debt additions; verified bygreponinitializeV5_RepairRebateStaking,RebateStakingRepaired,TreasuryFeeType.Depositand the newMigrationDebtVaultUpdated/MigrationRevealRejectedsymbols.Details
Bridge changes
Five contracts modified (
Bridge.sol,BridgeGovernance.sol,BridgeState.sol,Deposit.sol,DepositSweep.sol) plus one new (MigrationExtraData.sol).A canonical migration debt vault pointer is maintained by governance via
setMigrationDebtVaultand an atomicrotateMigrationDebtVaultthat refuses to leave the prior canonical vault holding outstanding debt.setVaultStatusis tightened so the canonical migration debt vault cannot be untrusted while it is the canonical pointer, and any vault that still reports outstanding migration debt cannot be untrusted (fail-open staticcall — vaults not implementing the interface are unaffected).A reveal-time guard is added in
Deposit.revealDeposit: deposits whose extra data carries the migration tag must come from a registered migration revealer in the per-deposit vault and must have outstanding debt to repay; deposits from a registered migration revealer that lack the tag are rejected. Failures surface explicit reasons viaMigrationRevealRejected(bytes32)with reason codes covering missing debt, missing tag, mismatched revealer, unauthorised revealer, vault call failure and bad vault response.__gapinBridgeStateis reduced from 48 to 47 to introducemigrationDebtVaultwhile preserving storage layout continuity for proxy upgrades; documented insolidity/docs/BRIDGE_STORAGE_UPGRADE_NOTES.md.DepositSweep changes
After a sweep is proven, the canonical migration vault is notified via
ITBTCVaultMigrationSweepHookwith a fail-open batch call followed by bounded per-revealer fallback retries so a single bad entry cannot strand a completion batch.Vault changes
Two contracts modified (
TBTCOptimisticMinting.sol,TBTCVault.sol) plus three new interfaces and one library.TBTCOptimisticMintingnow implementsITBTCVaultMigrationDebtwith per-revealer migration debt tracking, a registration counter that enforceshasOutstandingMigrationDebtacross rotations, and aTBTCMigrationDebtOperationslibrary that isolates the debt state transitions (register, repay, residual clear).MIN_OPTIMISTIC_MINTING_FEE_DIVISOR = 10is introduced to bound the maximum optimistic minting fee rate at 10%, andMAX_MIGRATION_SWEEP_BATCH_SIZE = 100caps per-sweep callback fan-out.migrationSweepNotifierandmigrationSweepReserveallow a downstream contract to be informed exactly once when a revealer's migration debt reaches zero, with pending-completion bookkeeping that survives partial batch outcomes.Test infrastructure
Eleven harness contracts are added under
solidity/contracts/test/so the migration paths can be exercised in isolation from the full Bridge fixture (BridgeForVaultHarness,TBTCVaultHarness, theDeposit*andDepositSweep*harnesses, plus mocks for migration debt vault, sweep notifier, sweep vault and a regular comparison vault). A NatSpec verification script atsolidity/test/verify-migration-debt-natspec.shasserts that the required documentation surface for the migration debt interface remains present.Solution
The 37-file diff decomposes as 22 added files and 15 modified files. For 13 of the 15 modified files the monorepo's pre-PR-132 vendored copy was byte-identical to
threshold-network/tbtc-v2/main, so they were applied as direct overwrites. The remaining 2 (Bridge.sol,Deposit.sol) were three-way merged withgit merge-fileagainst the common base; both merges were clean (zero conflict markers) and verified by symbol grep to contain both branches' additions.One deliberate adaptation versus the monorepo source: the rewrite of
solidity/test/fixtures/bridge.tsintroduced by PR tlabs-xyz/tbtc#132 was an accommodation for the monorepo's deploy-script chain. Inthreshold-network/tbtc-v2/mainthe existing fixture works against the in-tree deploy chain, so this PR keeps the bare-main fixture and only adapts what the migrated tests actually need. None of the migrated tests destructure additional fixture fields, so no signature change was required. Verified locally: with PR tlabs-xyz/tbtc#132's fixture rewrite, all modified tests fail in thebefore allhook with a JSON-RPC QUANTITY validator error from the inlinehelpers.upgrades.deployProxycall; with the bare-main fixture they execute cleanly.Tests
Local verification on Node 14.21.3 with
yarn 1.22.22againstsolidity/.yarn lintreports 0 errors and 363 warnings (warnings are pre-existing).yarn format(root) reports clean ("All matched files use Prettier code style!").npx hardhat compileis clean.solidity/test/verify-migration-debt-natspec.shreports 12/12 PASS.The five new migration test files (
Bridge.CanonicalMigrationDebtVaultGuard.test.ts,Deposit.MigrationRevealGuard.Harness.test.ts,DepositSweep.MigrationCallback.test.ts,TBTCVault.MigrationDebt.test.ts,TBTCVault.OptimisticMintMigrationGuard.test.ts) report 52/52 PASS in 15s.The three modified test files (
Bridge.Governance.test.ts,Bridge.Parameters.test.ts,TBTCVault.OptimisticMinting.test.ts) report 539/542 PASS, 3 FAIL — see Known issues for upstream policy below.Known issues for upstream policy
These two conditions exist in the source PR tlabs-xyz/tbtc#132 and are surfaced here intentionally rather than papered over. Both are policy decisions for this repository's reviewers.
1.
Bridgecontract sizeBridgecompiles to 26.156 KB with this PR vs 23.939 KB on baremain. The added migration-vault rotation, reveal-guard helpers, custom errors and event accounting add ~2.2 KB. EIP-170 caps mainnet deployable code at 24.576 KB, sohardhat-contract-sizer(which is configuredstrict: trueinhardhat.config.ts) fails the build.The options are to extract a chunk of Bridge into a delegatecall library (preferred for long-term carrying capacity, but a non-trivial refactor); to accept a temporary
except: ["Bridge$"]entry incontractSizerwith a tracked follow-up issue; or to leave the build red until extraction lands. This PR does not modify thecontractSizerconfiguration. The deployment blocker is real and should be reviewed alongside the rest of the change.2. Three test failures in
TBTCVault.OptimisticMinting.test.tsPR tlabs-xyz/tbtc#132 introduced
MIN_OPTIMISTIC_MINTING_FEE_DIVISOR = 10and rewrote the long-standing "when the optimistic minting fee is zero" context (which previously set divisor0) into a new "when the optimistic minting fee rounds to zero" context withdivisor = type(uint32).max = 4294967295. The test comments assert that integer division truncates the fee to zero. The arithmetic does not actually truncate to zero for the deposit amount used in the fixture:199_900_000_000_000 / 4_294_967_295 = 46_542, so the treasury receives 46 542 wei and the depositor receives that amount less than the assertion expects. The three failing assertions areshould send no optimistic mint fee to treasury(expected0, got46542),should mint TBTC to depositor(expected199_900_000_000_000, got199_899_999_953_458), and a secondshould mint TBTC to depositor(expected200_000_000_000_000, got199_999_999_953_434).This was not caught upstream because the monorepo CI's
ci:corescript only runspnpm --filter @keep-network/tbtc-v2 run buildfor the contracts package, notrun test. Resolution requires either picking a divisor that actually truncates against the fixture amount (e.g. one larger than the deposit amount) or revisiting theMIN_OPTIMISTIC_MINTING_FEE_DIVISORenforcement. This PR keeps the tests as PR tlabs-xyz/tbtc#132 wrote them so the situation is visible to upstream.Storage upgrade considerations
BridgeState.StorageaddsmigrationDebtVaultand reduces the trailing__gapfromuint256[48]touint256[47]. Any future storage-layout diff against current proxies must reflect this and any proxy upgrade plan should include a layout review.