ENG-472 Bridge controller pointer and MintBurnGuard guardrails#918
Open
piotr-roslaniec wants to merge 82 commits into
Open
ENG-472 Bridge controller pointer and MintBurnGuard guardrails#918piotr-roslaniec wants to merge 82 commits into
piotr-roslaniec wants to merge 82 commits into
Conversation
…ocal env; add Sepolia env example
…only, verify library bytecodes before linking
…_PK or named account)
…ack to use governanceDelays(0)
…OZ upgrades plugin + Sepolia env
…es; format; make format script run Prettier only to satisfy CI
This reverts commit 766a6d0.
piotr-roslaniec
requested review from
Shadowfiend,
lukasz-zimnoch,
nkuba and
pdyraga
as code owners
…, and modify IBank.transferBalanceFrom() signature,
miquelcabot
reviewed
Jan 7, 2026
| uint256 newTotal = _increaseTotalMintedInternal(amount); | ||
|
|
||
| emit BankMintExecuted(controller, recipient, amount, newTotal); | ||
| bridge.controllerIncreaseBalance(recipient, _toTbtcBaseUnits(amount)); |
There was a problem hiding this comment.
@piotr-roslaniec Possible Issue: incorrectly calling bridge.controllerIncreaseBalance(recipient, _toTbtcBaseUnits(amount)), converting satoshis (1e8) to base units (1e18) before passing to the Bridge. However, Bridge.controllerIncreaseBalance() → Bank.increaseBalance() expects satoshis (1e8), not base units (1e18)
Contributor
Author
There was a problem hiding this comment.
Good catch, @miquelcabot. Please include that in your changes as well. Thanks
…ounts instead of tbtc (1e18)
… state management library
…n & tests (#921) ## Summary This PR completes the ENG-472 workstream by implementing the `MintBurnGuard` contract with full integration support for Account Control, comprehensive testing, deployment scripts, and Bridge governance wiring. The focus is to establish a robust controller-driven mint/burn path with global exposure tracking, rate limiting, and pause controls while maintaining clean separation between the guard's enforcement logic and Account Control's reserve management. ## Scope - **Bridge controller authorization**: Implemented `setMintingController()` governance function and renamed `IMintingAuthorization` --> `IBridgeController` for clarity. - **Operator terminology standardization**: Unified naming from "controller" to "operator" throughout MintBurnGuard to distinguish from Bridge's "minting controller" role. - **Core burn/unmint functionality**: Implemented `unmintAndBurnFrom()` and `burnFrom()` methods for AccountControl integration, with SafeERC20 usage and comprehensive error handling. - **Interface refinement**: Updated `IMintBurnGuard` with new burn/unmint methods; extended `IBank` and `ITBTCVault` interfaces for integration requirements. - **Deployment infrastructure**: Created deployment and configuration scripts (`44_deploy_mint_burn_guard.ts`, `45_configure_mint_burn_guard.ts`, `46_authorize_mint_burn_guard_in_bridge.ts`). - **Comprehensive test coverage**: Expanded from 28 to 38 tests, including 10 new integration tests with `MockAccountControl` simulating realistic AccountControl usage patterns. - **Mock contracts for testing**: Created `MockAccountControl`, `MockBridgeController`, `MockBurnBank`, and `MockBurnVault` to support integration testing. ## Motivation ENG-472 introduced a least-privilege operator pattern with global mint/burn guardrails. This PR builds on that foundation by: - Completing the MintBurnGuard implementation with all required burn/unmint methods - Establishing clear terminology (operator vs. controller) to avoid confusion - Providing comprehensive integration testing that validates AccountControl usage patterns - Creating production-ready deployment scripts with governance authorization flow - Ensuring the design is aligned with Bridge governance and Account Control expectations The completed implementation enables AccountControl to manage per-reserve accounting while MintBurnGuard enforces system-wide caps, rate limits, and pause controls. ## Notes - This PR is the completion of the initial ENG-472 MintBurnGuard workstream. - **Breaking changes**: Interface renames (`IMintingAuthorization` --> `IBridgeController`, `controller` --> `operator` in MintBurnGuard) require migration for any external integrations. - **Test coverage**: All 38 tests passing with comprehensive coverage of mint/burn flows, operator authorization, cap enforcement, rate limiting, and multi-reserve scenarios.
…ation (#936) # Summary PR#933 upgraded the Sepolia Bridge proxy to repair rebateStaking (using reinitializer(5)), overwriting a prior deployment from new/bank-decreaser that had introduced MintBurnGuard controller support. The current on-chain Bridge is missing: - setMintingController() / getMintingController() - controllerIncreaseBalance() / controllerIncreaseBalances() This PR adds the tooling to re-upgrade the Sepolia Bridge proxy to the new/bank-decreaser implementation, restoring those methods. No reinitializer is called — the mintingController slot defaults to address(0) and is set post-upgrade via governance. ## Changes - solidity/deploy/84_upgrade_bridge_mint_burn_controller.ts — new deploy script that reads the ProxyAdmin from the EIP-1967 admin slot, deploys a fresh Bridge implementation linked to the existing Sepolia libraries, and calls ProxyAdmin.upgrade() directly. Avoids dependency on the OZ network manifest. - solidity/deploy/utils/library-resolution.ts — fix verifyLibraryBytecodes to use FQN contracts/bridge/${name}.sol:${name} to avoid HH701 ambiguity on the Wallets artifact. - solidity/test/bridge/Bridge.MintBurnControllerUpgrade.test.ts — fork test validating the upgrade against a Sepolia fork. Requires Anvil (--network system_tests); also activates when HARDHAT_NETWORK=system_tests so FORKING_URL is not required. - solidity/scripts/snapshot.sh — pre-upgrade snapshot script: records baseline on-chain state and validates abort conditions. - solidity/scripts/verify-upgrade.sh — post-upgrade verification script: asserts implementation changed, new methods exist, rebateStaking preserved, governance and ProxyAdmin unchanged. - solidity/.env.example — documents all env vars required for the upgrade workflow. - docs/bridge-mintburn-controller-upgrade-runbook.md — end-to-end 8-phase runbook covering snapshot, build, standard tests, fork test, dry run with real keys, live upgrade, post-upgrade verification, and skip guard restore. ## Runbook status - [x] Phase 1 — Pre-upgrade snapshot (snapshot.sh) - [x] Phase 2 — Build and lint - [x] Phase 3 — Standard test suite - [x] Phase 4 — Fork test (4/4 passing against Sepolia fork on Anvil) - [x] Phase 5 — Dry run with real keys against Anvil fork (--network system_tests) - [x] **Phase 6 — Execute the live upgrade on Sepolia** - [x] Phase 7 — Post-upgrade verification (verify-upgrade.sh) - [x] Phase 8 — Re-enable func.skip and commit
…ion in Bridge upgrade verifyLibraryBytecodes() defaulted to non-strict mode, logging warnings on bytecode mismatches instead of aborting. This allowed Bridge proxy upgrades to proceed with unreviewed or modified external libraries (Deposit, DepositSweep, Redemption, Wallets, Fraud, MovingFunds). Change the default to strict=true and pass strict explicitly in the upgrade script so any mismatch aborts before upgradeProxy executes.
…ion (#950) ## Summary - Changes `verifyLibraryBytecodes()` default from `strict=false` to `strict=true` so bytecode mismatches abort instead of logging warnings - Passes `strict=true` explicitly in `80_upgrade_bridge_v2.ts` to make intent clear - Prevents Bridge proxy upgrades from proceeding with unreviewed or modified external libraries (Deposit, DepositSweep, Redemption, Wallets, Fraud, MovingFunds) ## Context Trail of Bits finding **TOB-TBTCAC-45**: "Non-fatal library verification allows Bridge upgrades to link unreviewed external libraries". The `verifyLibraryBytecodes` utility logged warnings on bytecode mismatches but did not abort, allowing `upgradeProxy` to execute with potentially compromised library contracts. ## Test plan - [ ] Verify upgrade script aborts when a library bytecode mismatch is detected (mock a stale library deployment) - [ ] Verify upgrade script succeeds when all library bytecodes match - [ ] Verify existing tests pass with no regressions
piotr-roslaniec
force-pushed
the
new/bank-decreaser
branch
from
eecfc53 to
037ee51
Compare
Brings in rebate staking, cross-chain updates, and TypeScript SDK changes from main while preserving the MintBurnGuard / bank-decreaser feature commit history.
piotr-roslaniec
force-pushed
the
new/bank-decreaser
branch
from
037ee51 to
29fcc68
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Motivation
Changes
setControllerBalanceIncreaser(address)(zero clears); controller-guardedcontrollerIncreaseBalanceand batch variant; exposecontrollerBalanceIncreaser(); storage gap adjusted.setControllerBalanceIncreaser(address)to manage the Bridge controller pointer.reduceExposureAndBurn; added owner-onlysetTotalMinted(cap-checked, rate window reset); enforced cap vs rate-limit consistency, pause/rate checks, zero-address guards, custom errors, and safe addition (unchecked removed); event naming aligned toamount.