From abd8c431d827799043121fa64103dd3722343ebf Mon Sep 17 00:00:00 2001 From: sh3ifu Date: Fri, 12 Jun 2026 14:56:22 +0300 Subject: [PATCH 1/2] test(R-I-07,R-I-08): min fundsIn amount + zero-amount guard regressions --- ethereum/test/Bridge.t.sol | 144 ++++++++++++++++++++++++++++-- ethereum/test/Integration.t.sol | 3 +- ethereum/test/MultisigProxy.t.sol | 3 +- 3 files changed, 143 insertions(+), 7 deletions(-) diff --git a/ethereum/test/Bridge.t.sol b/ethereum/test/Bridge.t.sol index 8f0db86..b438b15 100644 --- a/ethereum/test/Bridge.t.sol +++ b/ethereum/test/Bridge.t.sol @@ -50,6 +50,7 @@ contract BridgeTest is Test { ); event LZAdapterUpdated(address indexed oldAdapter, address indexed newAdapter); event RouteRegistryUpdated(address indexed oldRegistry, address indexed newRegistry); + event MinFundsInAmountUpdated(uint256 oldMinimum, uint256 newMinimum); Bridge bridge; MockERC20 usdt0; @@ -101,7 +102,8 @@ contract BridgeTest is Test { address(usdt0), address(routeRegistry), payable(address(cm)), - address(0) + address(0), + 1 // minFundsInAmount: smallest non-zero floor; cases that need a higher floor deploy their own Bridge ); rgbVerifier = new RGBVerifier(address(btcRelay)); @@ -221,24 +223,24 @@ contract BridgeTest is Test { function test_constructor_revertsOnZeroToken() public { vm.expectRevert(BridgeBase.InvalidTokenAddress.selector); - new Bridge(address(0), address(routeRegistry), payable(address(cm)), address(0)); + new Bridge(address(0), address(routeRegistry), payable(address(cm)), address(0), 1); } function test_constructor_revertsOnZeroRouteRegistry() public { vm.expectRevert(IBridge.InvalidRouteRegistryAddress.selector); - new Bridge(address(usdt0), address(0), payable(address(cm)), address(0)); + new Bridge(address(usdt0), address(0), payable(address(cm)), address(0), 1); } function test_constructor_revertsOnZeroCommissionManager() public { vm.expectRevert(IBridge.InvalidCommissionManagerAddress.selector); - new Bridge(address(usdt0), address(routeRegistry), payable(address(0)), address(0)); + new Bridge(address(usdt0), address(routeRegistry), payable(address(0)), address(0), 1); } function test_constructor_storesInitialLZAdapter() public { address initialAdapter = makeAddr('initial-adapter'); vm.prank(deployer); Bridge b = new Bridge( - address(usdt0), address(routeRegistry), payable(address(cm)), initialAdapter + address(usdt0), address(routeRegistry), payable(address(cm)), initialAdapter, 1 ); assertEq(b.lzAdapter(), initialAdapter, 'lzAdapter set in constructor'); } @@ -301,6 +303,138 @@ contract BridgeTest is Test { bridge.setRouteRegistry(makeAddr('newReg')); } + // ======================================================================== + // Minimum fundsIn amount + zero-amount guards + // + // `minFundsInAmount` is a non-zero floor enforced on the inbound path: it + // rejects zero-amount deposits (R-I-07) and dust whose commission would + // round to zero (R-I-08). `fundsOut` is an authorized release and only + // rejects amount == 0. The harness deploys with the smallest floor (1), so + // tests that need a higher floor raise it via `setMinFundsInAmount`. + // ======================================================================== + + function test_constructor_storesMinFundsInAmount() public { + vm.prank(deployer); + Bridge b = new Bridge( + address(usdt0), address(routeRegistry), payable(address(cm)), address(0), 1234 + ); + assertEq(b.minFundsInAmount(), 1234, 'minFundsInAmount stored from constructor'); + } + + function test_constructor_revertsOnZeroMinFundsInAmount() public { + vm.expectRevert(IBridge.InvalidMinFundsInAmount.selector); + new Bridge(address(usdt0), address(routeRegistry), payable(address(cm)), address(0), 0); + } + + function test_setMinFundsInAmount_updatesAndEmits() public { + vm.expectEmit(false, false, false, true, address(bridge)); + emit MinFundsInAmountUpdated(1, 1000); + + vm.prank(multisig); + bridge.setMinFundsInAmount(1000); + assertEq(bridge.minFundsInAmount(), 1000); + } + + function test_setMinFundsInAmount_revertsOnZero() public { + vm.prank(multisig); + vm.expectRevert(IBridge.InvalidMinFundsInAmount.selector); + bridge.setMinFundsInAmount(0); + } + + function test_setMinFundsInAmount_revertsIfNotOwner() public { + vm.prank(user); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, user)); + bridge.setMinFundsInAmount(1000); + } + + // --- R-I-07: zero amount --- + + function test_fundsIn_revertsOnZeroAmount() public { + // Floor is 1 (setUp), so a zero deposit is below the minimum. + vm.expectRevert(abi.encodeWithSelector(IBridge.AmountBelowMinimum.selector, uint256(0), uint256(1))); + vm.prank(user); + bridge.fundsIn(0, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + } + + function test_fundsOut_revertsOnZeroAmount() public { + // fundsOut has no minimum — only the zero-amount no-op guard, which + // fires before the burn-id and balance checks. + vm.expectRevert(IBridge.ZeroAmount.selector); + vm.prank(multisig); + bridge.fundsOut( + recipient, 0, BURN_ID, + RGB_CHAIN_ID, SOURCE_CHAIN_ID, SRC_ADDR, + _proof(), _settlement(_singleFundsInId()) + ); + } + + // --- R-I-08: dust / below-minimum on the inbound path --- + + function test_fundsIn_revertsBelowMinimum() public { + vm.prank(multisig); + bridge.setMinFundsInAmount(1000); + + vm.expectRevert(abi.encodeWithSelector(IBridge.AmountBelowMinimum.selector, uint256(999), uint256(1000))); + vm.prank(user); + bridge.fundsIn(999, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + } + + function test_fundsIn_acceptsExactlyAtMinimum() public { + vm.prank(multisig); + bridge.setMinFundsInAmount(1000); + + vm.prank(user); + bridge.fundsIn(1000, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + assertEq(rgbModule.fundsInRecords(TX_ID), 1000, 'deposit at the floor is accepted'); + } + + function test_fundsIn_acceptsAboveMinimum() public { + vm.prank(multisig); + bridge.setMinFundsInAmount(1000); + + vm.prank(user); + bridge.fundsIn(1001, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + assertEq(rgbModule.fundsInRecords(TX_ID), 1001, 'deposit above the floor is accepted'); + } + + function test_fundsIn_dustThatRoundsCommissionToZeroIsRejected() public { + // 4% token commission. Below 25 units the fee floors to zero + // (24 * 400 / 100 / 100 == 0) — the exact dust case R-I-08 describes. + // A floor of 25 keeps such dust off the inbound path; at 25 the fee is + // a non-zero 1 unit. + _setFundsInTokenRule(400); + vm.prank(multisig); + bridge.setMinFundsInAmount(25); + + assertEq(cm.calculateStableFee(24, 400, 100), 0, 'sanity: 24 pays zero commission'); + + vm.expectRevert(abi.encodeWithSelector(IBridge.AmountBelowMinimum.selector, uint256(24), uint256(25))); + vm.prank(user); + bridge.fundsIn(24, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + + // At the floor the deposit is accepted and pays a non-zero commission. + vm.prank(user); + bridge.fundsIn(25, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + assertEq(rgbModule.fundsInRecords(TX_ID), 24, 'net = 25 - 1 commission'); + } + + function test_fundsInFromAdapter_revertsBelowMinimum() public { + // The adapter overload shares `_fundsIn`, so the floor applies there too. + address mockAdapter = makeAddr('mock-adapter'); + vm.prank(multisig); + bridge.setLZAdapter(mockAdapter); + vm.prank(multisig); + bridge.setMinFundsInAmount(1000); + + usdt0.mint(mockAdapter, 999); + vm.prank(mockAdapter); + usdt0.approve(address(bridge), 999); + + vm.expectRevert(abi.encodeWithSelector(IBridge.AmountBelowMinimum.selector, uint256(999), uint256(1000))); + vm.prank(mockAdapter); + bridge.fundsIn(999, SOURCE_CHAIN_ID, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + } + // ======================================================================== // fundsIn — adapter overload (`onlyLZAdapter`) // ======================================================================== diff --git a/ethereum/test/Integration.t.sol b/ethereum/test/Integration.t.sol index 34c780a..8be1d67 100644 --- a/ethereum/test/Integration.t.sol +++ b/ethereum/test/Integration.t.sol @@ -161,7 +161,8 @@ contract IntegrationTest is Test { address(token), address(routeRegistry), payable(address(cm)), - address(0) + address(0), + 1 // minFundsInAmount: smallest non-zero floor for tests ); rgbVerifier = new RGBVerifier(address(btcRelay)); diff --git a/ethereum/test/MultisigProxy.t.sol b/ethereum/test/MultisigProxy.t.sol index ca79ae0..d8d21a5 100644 --- a/ethereum/test/MultisigProxy.t.sol +++ b/ethereum/test/MultisigProxy.t.sol @@ -149,7 +149,8 @@ contract MultisigProxyTest is Test { address(token), address(routeRegistry), payable(address(cm)), - address(0) + address(0), + 1 // minFundsInAmount: smallest non-zero floor for tests ); rgbVerifier = new RGBVerifier(address(btcRelay)); From a48a6e2acf626be33d59a6888fd4376c234eb5f3 Mon Sep 17 00:00:00 2001 From: Denys <53053282+sh3ifu@users.noreply.github.com> Date: Wed, 17 Jun 2026 15:19:19 +0300 Subject: [PATCH 2/2] chore(R-I-07,R-I-08): pass minFundsInAmount to Bridge in deploy scripts (#47) * chore(R-I-07,R-I-08): pass minFundsInAmount to Bridge in deploy scripts * fix(R-I-01): bound TEE execute/executeBatch deadline to MAX_TEE_DEADLINE (#48) * fix(R-I-01): bound TEE execute/executeBatch deadline to MAX_TEE_DEADLINE * test(R-I-01): TEE execute/executeBatch deadline upper-bound regressions (#49) * test(R-I-01): TEE execute/executeBatch deadline upper-bound regressions * fix(R-I-18): reject proposals with deadline shorter than the timelock (#51) * fix(R-I-18): reject proposals with deadline shorter than the timelock * test(R-I-18): proposal deadline-vs-timelock lower-bound regressions (#52) * test(R-I-18): proposal deadline-vs-timelock lower-bound regressions * fix(R-I-11): cap destinationAddress / sourceAddress length in Bridge (#55) * fix(R-I-11): cap destinationAddress / sourceAddress length in Bridge * test(R-I-11): destinationAddress / sourceAddress length-cap regressions (#56) * test(R-I-11): destinationAddress / sourceAddress length-cap regressions * fix(R-I-12): cap proof length in Bridge.fundsOut (#57) * fix(R-I-12): cap proof length in Bridge.fundsOut * test(R-I-12): proof length-cap regressions for fundsOut (#58) * test(R-I-12): proof length-cap regressions for fundsOut * fix(R-W-12): enforce strict-majority signer threshold floor (#60) * fix(R-W-12): enforce strict-majority signer threshold floor * test(R-W-12): strict-majority signer threshold regressions (#61) * test(R-W-12): strict-majority signer threshold regressions * fix(R-W-14): require disjoint enclave and federation signer sets (#62) * fix(R-W-14): require disjoint enclave and federation signer sets * test(R-W-14): disjoint signer-set regressions (#63) * test(R-W-14): disjoint signer-set regressions * fix(R-I-06): cap signer-set size at MAX_SIGNERS (#64) * fix(R-I-06): cap signer-set size at MAX_SIGNERS * test(R-I-06): signer-set size cap regressions (#65) --- ethereum/script/deploy/DeployAll.s.sol | 7 +- ethereum/script/deploy/DeployBridge.s.sol | 10 +- ethereum/src/Bridge.sol | 22 ++ ethereum/src/MultisigProxy.sol | 63 +++- ethereum/src/interfaces/IBridge.sol | 15 +- ethereum/src/interfaces/IMultisigProxy.sol | 3 + ethereum/test/Bridge.t.sol | 134 +++++++ ethereum/test/MultisigProxy.t.sol | 403 ++++++++++++++++++++- 8 files changed, 617 insertions(+), 40 deletions(-) diff --git a/ethereum/script/deploy/DeployAll.s.sol b/ethereum/script/deploy/DeployAll.s.sol index 244b958..61a3ce1 100644 --- a/ethereum/script/deploy/DeployAll.s.sol +++ b/ethereum/script/deploy/DeployAll.s.sol @@ -37,7 +37,8 @@ import { RgbSettlementModule } from '../../src/settlement/RgbSettlementModule.so /// PRIVATE_KEY, USDT0_ADDRESS, BTC_RELAY_ADDRESS, /// ENCLAVE_SIGNERS, ENCLAVE_THRESHOLD, /// FEDERATION_SIGNERS, FEDERATION_THRESHOLD, -/// COMMISSION_RECIPIENT, TIMELOCK_DURATION, MIN_TIMELOCK +/// COMMISSION_RECIPIENT, TIMELOCK_DURATION, MIN_TIMELOCK, +/// MIN_FUNDS_IN_AMOUNT /// /// Env (optional): /// ETH_USD_FEED — Chainlink ETH/USD aggregator (wired in before CM @@ -74,6 +75,7 @@ contract DeployAll is Script { uint256 minTimelock = vm.envUint('MIN_TIMELOCK'); address ethUsdFeed = vm.envOr('ETH_USD_FEED', address(0)); uint256 ethUsdHb = vm.envOr('ETH_USD_HEARTBEAT', uint256(0)); + uint256 minFundsIn = vm.envUint('MIN_FUNDS_IN_AMOUNT'); address deployer = vm.addr(pk); uint64 startNonce = vm.getNonce(deployer); @@ -96,7 +98,8 @@ contract DeployAll is Script { usdt0, address(routeRegistry), payable(address(cm)), - address(0) + address(0), + minFundsIn ); // ---- 5. Route plugins (nonce n+3, n+4) --------------------------- diff --git a/ethereum/script/deploy/DeployBridge.s.sol b/ethereum/script/deploy/DeployBridge.s.sol index 6f194e9..c6190b1 100644 --- a/ethereum/script/deploy/DeployBridge.s.sol +++ b/ethereum/script/deploy/DeployBridge.s.sol @@ -29,6 +29,12 @@ import { Bridge } from '../../src/Bridge.sol'; /// `Bridge.setLZAdapter(adapter)`. The Bridge /// accepts adapter-only `fundsInFromAdapter` /// calls *only* from the configured adapter. +/// MIN_FUNDS_IN_AMOUNT — Minimum accepted `fundsIn` deposit in token +/// smallest units (USDT0 has 6 decimals, so e.g. +/// 10000 = 0.01 USDT0). Required and must be +/// non-zero; retune later via federation +/// governance with +/// `Bridge.setMinFundsInAmount(newMinimum)`. /// /// Usage: /// forge script script/deploy/DeployBridge.s.sol \ @@ -40,9 +46,10 @@ contract DeployBridge is Script { address routeRegistry = vm.envAddress('ROUTE_REGISTRY_ADDRESS'); address commissionManager = vm.envAddress('COMMISSION_MANAGER'); address lzAdapter = vm.envOr('LZ_ADAPTER', address(0)); + uint256 minFundsInAmount = vm.envUint('MIN_FUNDS_IN_AMOUNT'); vm.startBroadcast(pk); - bridge = new Bridge(usdt0, routeRegistry, payable(commissionManager), lzAdapter); + bridge = new Bridge(usdt0, routeRegistry, payable(commissionManager), lzAdapter, minFundsInAmount); vm.stopBroadcast(); console2.log('Bridge deployed at: ', address(bridge)); @@ -51,5 +58,6 @@ contract DeployBridge is Script { console2.log('RouteRegistry: ', address(bridge.routeRegistry())); console2.log('CommissionManager: ', address(bridge.commissionManager())); console2.log('LZ adapter: ', bridge.lzAdapter()); + console2.log('Min fundsIn amount: ', bridge.minFundsInAmount()); } } diff --git a/ethereum/src/Bridge.sol b/ethereum/src/Bridge.sol index 3fc3eb5..c4cf0d0 100644 --- a/ethereum/src/Bridge.sol +++ b/ethereum/src/Bridge.sol @@ -40,6 +40,21 @@ contract Bridge is BridgeBase, IBridge, ReentrancyGuard { // State // ========================================================================= + /// @notice Upper bound on the `destinationAddress` (fundsIn) and + /// `sourceAddress` (fundsOut) byte length. These strings are echoed + /// into event logs, so an unbounded value would let a caller inflate + /// log-storage and indexer cost. 512 bytes covers every supported + /// destination representation (RGB invoices and other chains) with + /// ample headroom. + uint256 public constant MAX_ADDRESS_LENGTH = 512; + + /// @notice Upper bound on the `fundsOut` `proof` byte length. `proof` is + /// forwarded to the route verifier, so an unbounded blob would let a + /// release inflate calldata + verifier gas. 1024 bytes comfortably + /// fits the verifier formats (RGB is 64 bytes today; a future SPV + /// inclusion proof stays well under this). + uint256 public constant MAX_PROOF_LENGTH = 1024; + /// @notice CommissionManager that receives and custodies protocol fees. ICommissionManager public immutable commissionManager; @@ -203,6 +218,10 @@ contract Bridge is BridgeBase, IBridge, ReentrancyGuard { if (recipient == address(0)) revert InvalidRecipientAddress(); if (sourceChainId == 0) revert InvalidSourceChainId(); if (destinationChainId == 0) revert InvalidDestinationChainId(); + if (bytes(sourceAddress).length > MAX_ADDRESS_LENGTH) { + revert AddressTooLong(bytes(sourceAddress).length, MAX_ADDRESS_LENGTH); + } + if (proof.length > MAX_PROOF_LENGTH) revert ProofTooLong(proof.length, MAX_PROOF_LENGTH); if (amount > IERC20(TOKEN).balanceOf(address(this))) revert AmountExceedBridgePool(); // Common replay guard. Set the flag before any external interaction @@ -290,6 +309,9 @@ contract Bridge is BridgeBase, IBridge, ReentrancyGuard { ) private { if (amount < minFundsInAmount) revert AmountBelowMinimum(amount, minFundsInAmount); if (bytes(destinationAddress).length == 0) revert InvalidDestinationAddress(); + if (bytes(destinationAddress).length > MAX_ADDRESS_LENGTH) { + revert AddressTooLong(bytes(destinationAddress).length, MAX_ADDRESS_LENGTH); + } if (sourceChainId == 0) revert InvalidSourceChainId(); if (destinationChainId == 0) revert InvalidDestinationChainId(); diff --git a/ethereum/src/MultisigProxy.sol b/ethereum/src/MultisigProxy.sol index 8956c22..76e872b 100644 --- a/ethereum/src/MultisigProxy.sol +++ b/ethereum/src/MultisigProxy.sol @@ -62,9 +62,22 @@ contract MultisigProxy is IMultisigProxy { /// @notice Maximum allowed time between proposal creation and its deadline. uint256 public constant MAX_PROPOSAL_LIFETIME = 30 days; + /// @notice Maximum allowed lifetime of a TEE-signed `execute` / `executeBatch` + /// deadline. Tighter than `MAX_PROPOSAL_LIFETIME` because enclave + /// operations (e.g. `fundsOut`) are meant to be executed promptly + /// after signing; a short ceiling limits how long a leaked or + /// pre-signed payload stays executable while its nonce is unconsumed. + uint256 public constant MAX_TEE_DEADLINE = 1 days; + /// @notice Hard upper bound on `executeBatch` size to keep gas use bounded. uint256 public constant MAX_BATCH_SIZE = 3; + /// @notice Hard upper bound on the size of either signer set. Bounds the + /// O(N^2) duplicate/disjointness checks and stays far within the + /// `uint256` signature-bitmap width (so no signer index is ever + /// unreachable). 20 is ample for an enclave/federation committee. + uint256 public constant MAX_SIGNERS = 20; + /// @notice Length of a Solidity function selector (`bytes4`) in bytes. Used /// to guard `callData` against payloads too short to even carry a /// selector before it is sliced via `calldataload`. @@ -176,9 +189,9 @@ contract MultisigProxy is IMultisigProxy { if (bridge_ == address(0)) revert ZeroBridge(); if (commissionManager_ == address(0)) revert ZeroCommissionManager(); if (enclaveSigners_.length == 0) revert NoSigners(); - if (enclaveThreshold_ == 0 || enclaveThreshold_ > enclaveSigners_.length) revert InvalidThreshold(); + _requireValidThreshold(enclaveThreshold_, enclaveSigners_.length); if (federationSigners_.length == 0) revert NoSigners(); - if (federationThreshold_ == 0 || federationThreshold_ > federationSigners_.length) revert InvalidThreshold(); + _requireValidThreshold(federationThreshold_, federationSigners_.length); if (commissionRecipient_ == address(0)) revert ZeroCommissionRecipient(); if (minTimelock_ == 0 || minTimelock_ >= MAX_PROPOSAL_LIFETIME) revert InvalidMinTimelock(); if (timelockDuration_ < minTimelock_) revert TimelockTooShort(); @@ -186,6 +199,9 @@ contract MultisigProxy is IMultisigProxy { _validateSigners(enclaveSigners_); _validateSigners(federationSigners_); + // The enclave and federation are distinct trust domains; an address in + // both would count toward quorum in each, collapsing that separation. + _requireDisjoint(enclaveSigners_, federationSigners_); bridge = bridge_; commissionManager = commissionManager_; @@ -228,6 +244,10 @@ contract MultisigProxy is IMultisigProxy { bytes[] calldata enclaveSigs ) external { if (block.timestamp > deadline) revert Expired(); + // Bound how far in the future a signed deadline may sit, so a leaked or + // pre-signed payload cannot stay executable indefinitely while its + // nonce is unconsumed. + if (deadline > block.timestamp + MAX_TEE_DEADLINE) revert DeadlineTooFar(); if (callData.length < SELECTOR_LENGTH) revert CallDataTooShort(); bytes4 selector; @@ -258,6 +278,8 @@ contract MultisigProxy is IMultisigProxy { bytes[] calldata enclaveSigs ) external payable { if (block.timestamp > deadline) revert Expired(); + // Bound the signed deadline's distance into the future; + if (deadline > block.timestamp + MAX_TEE_DEADLINE) revert DeadlineTooFar(); uint256 n = targets.length; if (n == 0) revert BatchEmpty(); if (n > MAX_BATCH_SIZE) revert BatchTooLarge(); @@ -818,6 +840,7 @@ contract MultisigProxy is IMultisigProxy { ) private returns (bytes32 proposalId) { if (block.timestamp > deadline) revert Expired(); if (deadline > block.timestamp + MAX_PROPOSAL_LIFETIME) revert DeadlineTooFar(); + if (deadline < block.timestamp + timelockDuration) revert DeadlineBeforeTimelock(); if (nonce != proposalNonce) revert InvalidNonce(); bytes32 digest = _hashTypedData(structHash); @@ -855,8 +878,9 @@ contract MultisigProxy is IMultisigProxy { } else if (opType == OperationType.UpdateEnclaveSigners) { (address[] memory newSigners, uint256 newThreshold) = abi.decode(opData, (address[], uint256)); if (newSigners.length == 0) revert NoSigners(); - if (newThreshold == 0 || newThreshold > newSigners.length) revert InvalidThreshold(); + _requireValidThreshold(newThreshold, newSigners.length); _validateSigners(newSigners); + _requireDisjoint(newSigners, _federationSigners); _enclaveSigners = newSigners; enclaveThreshold = newThreshold; emit EnclaveSignersUpdated(newSigners, newThreshold); @@ -864,8 +888,9 @@ contract MultisigProxy is IMultisigProxy { } else if (opType == OperationType.UpdateFederationSigners) { (address[] memory newSigners, uint256 newThreshold) = abi.decode(opData, (address[], uint256)); if (newSigners.length == 0) revert NoSigners(); - if (newThreshold == 0 || newThreshold > newSigners.length) revert InvalidThreshold(); + _requireValidThreshold(newThreshold, newSigners.length); _validateSigners(newSigners); + _requireDisjoint(newSigners, _enclaveSigners); _federationSigners = newSigners; federationThreshold = newThreshold; emit FederationSignersUpdated(newSigners, newThreshold); @@ -1064,7 +1089,37 @@ contract MultisigProxy is IMultisigProxy { // ========================================================================= /// @dev Validates no zero addresses and no duplicates. O(n^2), fine for <20 signers. + /// @dev Enforces the quorum policy shared by both signer sets (enclave and + /// federation), in the constructor and the signer-update handlers: + /// - at least 2 signers (no single-key set); + /// - threshold of at least 2 (no single signer can authorise alone); + /// - threshold a strict majority (`2 * threshold > n`), which rejects + /// 1-of-N and any sub-majority quorum; + /// - threshold not exceeding the signer count. + /// The smallest valid sets are 2-of-2 and 2-of-3. This upholds the + /// spec invariant "one signer MUST NOT authorise fundsOut alone". + function _requireValidThreshold(uint256 threshold, uint256 n) private pure { + if (n < 2 || threshold < 2 || threshold > n || 2 * threshold <= n) { + revert InvalidThreshold(); + } + } + + /// @dev Reverts if any address in `candidate` also appears in `counterpart`. + /// Used to keep the enclave and federation signer sets disjoint so the + /// two trust domains stay independent (R-W-14). O(n*m), bounded by the + /// signer-set sizes. + function _requireDisjoint(address[] memory candidate, address[] memory counterpart) private pure { + for (uint256 i = 0; i < candidate.length; i++) { + for (uint256 j = 0; j < counterpart.length; j++) { + if (candidate[i] == counterpart[j]) revert SignerSetsOverlap(candidate[i]); + } + } + } + function _validateSigners(address[] memory signers) private pure { + // Bound the set before the O(N^2) duplicate scan below, and keep every + // signer index within the uint256 signature bitmap (R-I-06). + if (signers.length > MAX_SIGNERS) revert TooManySigners(signers.length, MAX_SIGNERS); for (uint256 i = 0; i < signers.length; i++) { if (signers[i] == address(0)) revert ZeroAddressSigner(); for (uint256 j = i + 1; j < signers.length; j++) { diff --git a/ethereum/src/interfaces/IBridge.sol b/ethereum/src/interfaces/IBridge.sol index 7bc1397..7d39dc0 100644 --- a/ethereum/src/interfaces/IBridge.sol +++ b/ethereum/src/interfaces/IBridge.sol @@ -9,22 +9,11 @@ interface IBridge { error InvalidDestinationAddress(); error InvalidDestinationChainId(); error InvalidSourceChainId(); - /// @notice `fundsOut` was called with `amount == 0`. A zero-amount release - /// is a no-op that only emits a redundant event, so it is rejected. - /// (The inbound path is guarded by `AmountBelowMinimum` instead, - /// since its non-zero `minFundsInAmount` already excludes zero.) error ZeroAmount(); - /// @notice A `fundsIn` deposit was below the configured `minFundsInAmount`. - /// The minimum is always non-zero, so this also rejects zero-amount - /// deposits, and it keeps dust — small enough that its commission - /// rounds to zero and only adds event/processing noise — off the - /// inbound path. Does not apply to `fundsOut` (authorized releases - /// of already-recorded amounts only check `amount > 0`). error AmountBelowMinimum(uint256 amount, uint256 minimum); - /// @notice `minFundsInAmount` was set to zero at construction or via - /// `setMinFundsInAmount`. The floor must be non-zero so that it - /// meaningfully rejects zero-amount and dust deposits. error InvalidMinFundsInAmount(); + error AddressTooLong(uint256 length, uint256 maxLength); + error ProofTooLong(uint256 length, uint256 maxLength); error InvalidRouteRegistryAddress(); error InvalidCommissionManagerAddress(); error NotLZAdapter(); diff --git a/ethereum/src/interfaces/IMultisigProxy.sol b/ethereum/src/interfaces/IMultisigProxy.sol index a6d1323..915ccd0 100644 --- a/ethereum/src/interfaces/IMultisigProxy.sol +++ b/ethereum/src/interfaces/IMultisigProxy.sol @@ -54,6 +54,7 @@ interface IMultisigProxy { error ProposalExpired(); error DataMismatch(); error DeadlineTooFar(); + error DeadlineBeforeTimelock(); error ProposalExists(); error IndexOutOfRange(); error BitmapOutOfRange(); @@ -62,6 +63,8 @@ interface IMultisigProxy { error InvalidSignature(); error ZeroAddressSigner(); error DuplicateSigner(); + error SignerSetsOverlap(address signer); + error TooManySigners(uint256 count, uint256 max); error CallFailed(); error UnknownOperationType(); error ZeroRecipient(); diff --git a/ethereum/test/Bridge.t.sol b/ethereum/test/Bridge.t.sol index b438b15..0b893ab 100644 --- a/ethereum/test/Bridge.t.sol +++ b/ethereum/test/Bridge.t.sol @@ -154,6 +154,15 @@ contract BridgeTest is Test { return abi.encode(ids); } + /// @dev Build an ASCII string of exactly `len` bytes (for address-length caps). + function _str(uint256 len) internal pure returns (string memory) { + bytes memory b = new bytes(len); + for (uint256 i = 0; i < len; i++) { + b[i] = 'a'; + } + return string(b); + } + function _setFundsInTokenRule(uint256 percent) internal { vm.prank(deployer); cm.setCommissionRule( @@ -435,6 +444,131 @@ contract BridgeTest is Test { bridge.fundsIn(999, SOURCE_CHAIN_ID, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); } + function test_fundsIn_revertsOnDestinationAddressTooLong() public { + uint256 max = bridge.MAX_ADDRESS_LENGTH(); + string memory tooLong = _str(max + 1); + + vm.expectRevert(abi.encodeWithSelector(IBridge.AddressTooLong.selector, max + 1, max)); + vm.prank(user); + bridge.fundsIn(AMOUNT, RGB_CHAIN_ID, tooLong, TX_ID, ''); + } + + function test_fundsIn_acceptsDestinationAddressAtMaxLength() public { + uint256 max = bridge.MAX_ADDRESS_LENGTH(); + + vm.prank(user); + bridge.fundsIn(AMOUNT, RGB_CHAIN_ID, _str(max), TX_ID, ''); + assertEq(rgbModule.fundsInRecords(TX_ID), AMOUNT, 'deposit at the address-length cap is accepted'); + } + + function test_fundsOut_revertsOnSourceAddressTooLong() public { + vm.prank(user); + bridge.fundsIn(AMOUNT, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + + uint256 max = bridge.MAX_ADDRESS_LENGTH(); + string memory tooLong = _str(max + 1); + + vm.expectRevert(abi.encodeWithSelector(IBridge.AddressTooLong.selector, max + 1, max)); + vm.prank(multisig); + bridge.fundsOut( + recipient, AMOUNT, BURN_ID, + RGB_CHAIN_ID, SOURCE_CHAIN_ID, tooLong, + _proof(), _settlement(_singleFundsInId()) + ); + } + + function test_fundsOut_acceptsSourceAddressAtMaxLength() public { + vm.prank(user); + bridge.fundsIn(AMOUNT, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + + uint256 max = bridge.MAX_ADDRESS_LENGTH(); + + vm.prank(multisig); + bridge.fundsOut( + recipient, AMOUNT, BURN_ID, + RGB_CHAIN_ID, SOURCE_CHAIN_ID, _str(max), + _proof(), _settlement(_singleFundsInId()) + ); + assertEq(usdt0.balanceOf(recipient), AMOUNT, 'release with sourceAddress at the cap succeeds'); + } + + // ======================================================================== + // Proof length cap (R-I-12) + // + // fundsOut forwards `proof` to the route verifier, so it is capped at + // MAX_PROOF_LENGTH to bound calldata + verifier gas. The exact cap is + // accepted; one byte over reverts ProofTooLong. The real RGB proof + // (abi.encode(uint256, bytes32) = 64 bytes) is far under the cap. + // ======================================================================== + + /// @dev Build a `bytes` blob of exactly `len` bytes. + function _bytesOfLength(uint256 len) internal pure returns (bytes memory b) { + b = new bytes(len); + for (uint256 i = 0; i < len; i++) { + b[i] = 0x61; + } + } + + function test_fundsOut_revertsOnProofTooLong() public { + vm.prank(user); + bridge.fundsIn(AMOUNT, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + + uint256 max = bridge.MAX_PROOF_LENGTH(); + bytes memory tooLong = _bytesOfLength(max + 1); + + vm.expectRevert(abi.encodeWithSelector(IBridge.ProofTooLong.selector, max + 1, max)); + vm.prank(multisig); + bridge.fundsOut( + recipient, AMOUNT, BURN_ID, + RGB_CHAIN_ID, SOURCE_CHAIN_ID, SRC_ADDR, + tooLong, _settlement(_singleFundsInId()) + ); + } + + function test_fundsOut_acceptsProofAtMaxLength() public { + vm.prank(user); + bridge.fundsIn(AMOUNT, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + + // A proof at the exact cap passes the length guard. It then reverts in + // the verifier (the blob is not a valid (height, commitment) pair), so + // the cap check is isolated by asserting it is NOT ProofTooLong: the + // call reaches the verifier instead. + uint256 max = bridge.MAX_PROOF_LENGTH(); + bytes memory atMax = _bytesOfLength(max); + + vm.prank(multisig); + try bridge.fundsOut( + recipient, AMOUNT, BURN_ID, + RGB_CHAIN_ID, SOURCE_CHAIN_ID, SRC_ADDR, + atMax, _settlement(_singleFundsInId()) + ) { + // a decodable proof would succeed; this blob won't, so we don't + // expect to land here — but if a future verifier accepts it, the + // length guard still passed, which is what this test asserts. + } catch (bytes memory reason) { + // Must NOT be the length guard — proving max-length passes it. + bytes4 sel = bytes4(reason); + assertTrue(sel != IBridge.ProofTooLong.selector, 'max-length proof must clear the length guard'); + } + } + + function test_fundsOut_acceptsRealProofUnderCap() public { + // The production-shaped 64-byte RGB proof is well under the cap and the + // happy path still succeeds. + vm.prank(user); + bridge.fundsIn(AMOUNT, RGB_CHAIN_ID, DST_ADDR, TX_ID, ''); + + assertLt(_proof().length, bridge.MAX_PROOF_LENGTH(), 'sanity: real proof under cap'); + + vm.prank(multisig); + bridge.fundsOut( + recipient, AMOUNT, BURN_ID, + RGB_CHAIN_ID, SOURCE_CHAIN_ID, SRC_ADDR, + _proof(), _settlement(_singleFundsInId()) + ); + assertEq(usdt0.balanceOf(recipient), AMOUNT, 'release with a normal proof succeeds'); + } + // ======================================================================== // fundsIn — adapter overload (`onlyLZAdapter`) // ======================================================================== diff --git a/ethereum/test/MultisigProxy.t.sol b/ethereum/test/MultisigProxy.t.sol index d8d21a5..8bc3a2f 100644 --- a/ethereum/test/MultisigProxy.t.sol +++ b/ethereum/test/MultisigProxy.t.sol @@ -222,6 +222,20 @@ contract MultisigProxyTest is Test { ids[0] = TX_ID; } + /// @dev Valid strict-majority signer sets (2-of-2) used as filler in + /// constructor-revert tests that target a non-threshold revert. + function _validEnc() internal view returns (address[] memory a) { + a = new address[](2); + a[0] = encA1; + a[1] = encA2; + } + + function _validFed() internal view returns (address[] memory a) { + a = new address[](2); + a[0] = fedA1; + a[1] = fedA2; + } + function _fundsOutCalldata() internal view returns (bytes memory) { bytes memory proof = abi.encode(BLOCK_HEIGHT, COMMITMENT_HASH); bytes memory settlementData = abi.encode(_fundsInIds()); @@ -284,44 +298,34 @@ contract MultisigProxyTest is Test { } function test_constructor_revertsOnZeroCommission() public { - address[] memory enc = new address[](1); enc[0] = encA1; - address[] memory fed = new address[](1); fed[0] = fedA1; vm.expectRevert(IMultisigProxy.ZeroCommissionRecipient.selector); - new MultisigProxy(address(bridge), address(cm), enc, 1, fed, 1, address(0), TIMELOCK, MIN_TIMELOCK); + new MultisigProxy(address(bridge), address(cm), _validEnc(), 2, _validFed(), 2, address(0), TIMELOCK, MIN_TIMELOCK); } function test_constructor_revertsOnTimelockTooLong() public { - address[] memory enc = new address[](1); enc[0] = encA1; - address[] memory fed = new address[](1); fed[0] = fedA1; vm.expectRevert(IMultisigProxy.TimelockTooLong.selector); - new MultisigProxy(address(bridge), address(cm), enc, 1, fed, 1, commissionReceiver, 30 days, MIN_TIMELOCK); + new MultisigProxy(address(bridge), address(cm), _validEnc(), 2, _validFed(), 2, commissionReceiver, 30 days, MIN_TIMELOCK); } // ---- R-W-11: MIN_TIMELOCK floor (post-fix) ---- /// @dev UT-FIX-13: deploying with a timelock below the requested floor reverts. function test_constructor_revertsOnTimelockBelowMinTimelock() public { - address[] memory enc = new address[](1); enc[0] = encA1; - address[] memory fed = new address[](1); fed[0] = fedA1; // timelock (1h) is below the requested floor (2h) -> TimelockTooShort vm.expectRevert(IMultisigProxy.TimelockTooShort.selector); - new MultisigProxy(address(bridge), address(cm), enc, 1, fed, 1, commissionReceiver, 1 hours, 2 hours); + new MultisigProxy(address(bridge), address(cm), _validEnc(), 2, _validFed(), 2, commissionReceiver, 1 hours, 2 hours); } /// @dev A zero floor is rejected — it would defeat the purpose of the fix. function test_constructor_revertsOnZeroMinTimelock() public { - address[] memory enc = new address[](1); enc[0] = encA1; - address[] memory fed = new address[](1); fed[0] = fedA1; vm.expectRevert(IMultisigProxy.InvalidMinTimelock.selector); - new MultisigProxy(address(bridge), address(cm), enc, 1, fed, 1, commissionReceiver, TIMELOCK, 0); + new MultisigProxy(address(bridge), address(cm), _validEnc(), 2, _validFed(), 2, commissionReceiver, TIMELOCK, 0); } /// @dev A floor at/above the upper bound leaves no valid range — rejected. function test_constructor_revertsOnMinTimelockTooLong() public { - address[] memory enc = new address[](1); enc[0] = encA1; - address[] memory fed = new address[](1); fed[0] = fedA1; vm.expectRevert(IMultisigProxy.InvalidMinTimelock.selector); - new MultisigProxy(address(bridge), address(cm), enc, 1, fed, 1, commissionReceiver, TIMELOCK, 30 days); + new MultisigProxy(address(bridge), address(cm), _validEnc(), 2, _validFed(), 2, commissionReceiver, TIMELOCK, 30 days); } function test_minTimelock_returnsConfiguredFloor() public view { @@ -329,17 +333,265 @@ contract MultisigProxyTest is Test { } function test_constructor_revertsOnDuplicateSigner() public { + // Duplicate enclave signer with an otherwise-valid 2-of-2 threshold, so + // the call clears the threshold guard and reverts in _validateSigners. address[] memory enc = new address[](2); enc[0] = encA1; enc[1] = encA1; - address[] memory fed = new address[](1); fed[0] = fedA1; vm.expectRevert(IMultisigProxy.DuplicateSigner.selector); - new MultisigProxy(address(bridge), address(cm), enc, 1, fed, 1, commissionReceiver, TIMELOCK, MIN_TIMELOCK); + new MultisigProxy(address(bridge), address(cm), enc, 2, _validFed(), 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK); } function test_constructor_revertsOnZeroAddressSigner() public { - address[] memory enc = new address[](1); enc[0] = address(0); - address[] memory fed = new address[](1); fed[0] = fedA1; + // Zero-address enclave signer in a 2-signer set with a valid 2-of-2 + // threshold, so the call clears the threshold guard and reverts in + // _validateSigners. + address[] memory enc = new address[](2); enc[0] = address(0); enc[1] = encA2; vm.expectRevert(IMultisigProxy.ZeroAddressSigner.selector); - new MultisigProxy(address(bridge), address(cm), enc, 1, fed, 1, commissionReceiver, TIMELOCK, MIN_TIMELOCK); + new MultisigProxy(address(bridge), address(cm), enc, 2, _validFed(), 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK); + } + + // ======================================================================== + // Strict-majority signer threshold floor (R-W-12 / UT-FIX-14) + // + // Both signer sets, at construction and on signer-update, must be a strict + // majority of at least 2 signers: n >= 2, threshold >= 2, threshold <= n, + // 2*threshold > n. Rejects 1-of-N, the degenerate 1-of-1, and sub-majority + // quorums; the smallest valid sets are 2-of-2 and 2-of-3. Signer-update + // validation runs at executeProposal time (in _executeByType), so the bad + // proposal is created successfully and reverts only on execution. + // ======================================================================== + + /// @dev Build an n-element signer array from a seed (distinct non-zero addrs). + function _signers(uint256 n) internal pure returns (address[] memory a) { + a = new address[](n); + for (uint256 i = 0; i < n; i++) { + a[i] = address(uint160(0xA000 + i)); + } + } + + /// @dev A second n-element signer array disjoint from `_signers` — used as + /// the counterpart set so the two trust domains do not overlap. + function _signersB(uint256 n) internal pure returns (address[] memory a) { + a = new address[](n); + for (uint256 i = 0; i < n; i++) { + a[i] = address(uint160(0xB000 + i)); + } + } + + // ---- Constructor ---- + + function test_constructor_rejectsOneOfThree_enclave() public { + vm.expectRevert(IMultisigProxy.InvalidThreshold.selector); + new MultisigProxy(address(bridge), address(cm), _signers(3), 1, _validFed(), 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK); + } + + function test_constructor_rejectsOneOfOne_enclave() public { + // n < 2 — single-key set, rejected even though 2*1 > 1. + vm.expectRevert(IMultisigProxy.InvalidThreshold.selector); + new MultisigProxy(address(bridge), address(cm), _signers(1), 1, _validFed(), 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK); + } + + function test_constructor_rejectsSubMajority_enclave() public { + // 2-of-4: 2*2 == 4, not a strict majority. + vm.expectRevert(IMultisigProxy.InvalidThreshold.selector); + new MultisigProxy(address(bridge), address(cm), _signers(4), 2, _validFed(), 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK); + } + + function test_constructor_rejectsOneOfThree_federation() public { + // Enclave valid, federation 1-of-3 — the floor applies to both sets. + vm.expectRevert(IMultisigProxy.InvalidThreshold.selector); + new MultisigProxy(address(bridge), address(cm), _validEnc(), 2, _signers(3), 1, commissionReceiver, TIMELOCK, MIN_TIMELOCK); + } + + function test_constructor_acceptsTwoOfTwo() public { + MultisigProxy p = new MultisigProxy( + address(bridge), address(cm), _signers(2), 2, _signersB(2), 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK + ); + assertEq(p.enclaveThreshold(), 2); + assertEq(p.federationThreshold(), 2); + } + + function test_constructor_acceptsThreeOfFour() public { + // Strict majority with a non-trivial set (2*3 > 4). + MultisigProxy p = new MultisigProxy( + address(bridge), address(cm), _signers(4), 3, _signersB(4), 3, commissionReceiver, TIMELOCK, MIN_TIMELOCK + ); + assertEq(p.enclaveThreshold(), 3); + assertEq(p.federationThreshold(), 3); + } + + // ---- Signer update (validated at executeProposal) ---- + + function test_proposeUpdateEnclaveSigners_rejectsOneOfNAtExecute() public { + address[] memory newSigners = _signers(3); + uint256 badThreshold = 1; // 1-of-3 + uint256 nonce = proxy.proposalNonce(); + uint256 deadline = block.timestamp + 1 days; + + bytes32 digest = MultisigHelper.digestProposeUpdateEnclaveSigners( + domainSep, newSigners, badThreshold, nonce, deadline + ); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + // Propose succeeds — threshold validity is enforced on execution. + bytes32 id = proxy.proposeUpdateEnclaveSigners(newSigners, badThreshold, nonce, deadline, bitmap, sigs); + + vm.warp(block.timestamp + TIMELOCK + 1); + vm.expectRevert(IMultisigProxy.InvalidThreshold.selector); + proxy.executeProposal(id, abi.encode(newSigners, badThreshold)); + } + + function test_proposeUpdateFederationSigners_rejectsSubMajorityAtExecute() public { + address[] memory newSigners = _signers(4); + uint256 badThreshold = 2; // 2-of-4, sub-majority + uint256 nonce = proxy.proposalNonce(); + uint256 deadline = block.timestamp + 1 days; + + bytes32 digest = MultisigHelper.digestProposeUpdateFederationSigners( + domainSep, newSigners, badThreshold, nonce, deadline + ); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + bytes32 id = proxy.proposeUpdateFederationSigners(newSigners, badThreshold, nonce, deadline, bitmap, sigs); + + vm.warp(block.timestamp + TIMELOCK + 1); + vm.expectRevert(IMultisigProxy.InvalidThreshold.selector); + proxy.executeProposal(id, abi.encode(newSigners, badThreshold)); + } + + function test_proposeUpdateFederationSigners_acceptsStrictMajority() public { + address[] memory newSigners = _signers(3); + uint256 newThreshold = 2; // 2-of-3 + uint256 nonce = proxy.proposalNonce(); + uint256 deadline = block.timestamp + 1 days; + + bytes32 digest = MultisigHelper.digestProposeUpdateFederationSigners( + domainSep, newSigners, newThreshold, nonce, deadline + ); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + bytes32 id = proxy.proposeUpdateFederationSigners(newSigners, newThreshold, nonce, deadline, bitmap, sigs); + + vm.warp(block.timestamp + TIMELOCK + 1); + proxy.executeProposal(id, abi.encode(newSigners, newThreshold)); + assertEq(proxy.federationThreshold(), 2); + assertEq(proxy.getFederationSigners().length, 3); + } + + // ======================================================================== + // Disjoint signer sets (R-W-14 / UT-FIX-16) + // + // The enclave and federation sets must not share an address, at construction + // and on signer-update (validated at executeProposal). The setUp sets are + // already disjoint (encA* vs fedA*). + // ======================================================================== + + function test_constructor_rejectsOverlappingSignerSets() public { + // encA1 is present in both the enclave and the federation set. + address[] memory enc = new address[](2); enc[0] = encA1; enc[1] = encA2; + address[] memory fed = new address[](2); fed[0] = encA1; fed[1] = fedA2; + vm.expectRevert(abi.encodeWithSelector(IMultisigProxy.SignerSetsOverlap.selector, encA1)); + new MultisigProxy(address(bridge), address(cm), enc, 2, fed, 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK); + } + + function test_constructor_acceptsDisjointSignerSets() public { + MultisigProxy p = new MultisigProxy( + address(bridge), address(cm), _signers(2), 2, _signersB(2), 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK + ); + assertEq(p.getEnclaveSigners().length, 2); + assertEq(p.getFederationSigners().length, 2); + } + + function test_proposeUpdateEnclaveSigners_rejectsOverlapWithFederationAtExecute() public { + // New enclave set includes fedA1, a current federation signer. + address[] memory newSigners = new address[](2); newSigners[0] = fedA1; newSigners[1] = encA2; + uint256 newThreshold = 2; + uint256 nonce = proxy.proposalNonce(); + uint256 deadline = block.timestamp + 1 days; + + bytes32 digest = MultisigHelper.digestProposeUpdateEnclaveSigners( + domainSep, newSigners, newThreshold, nonce, deadline + ); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + bytes32 id = proxy.proposeUpdateEnclaveSigners(newSigners, newThreshold, nonce, deadline, bitmap, sigs); + + vm.warp(block.timestamp + TIMELOCK + 1); + vm.expectRevert(abi.encodeWithSelector(IMultisigProxy.SignerSetsOverlap.selector, fedA1)); + proxy.executeProposal(id, abi.encode(newSigners, newThreshold)); + } + + function test_proposeUpdateFederationSigners_rejectsOverlapWithEnclaveAtExecute() public { + // New federation set includes encA1, a current enclave signer. + address[] memory newSigners = new address[](2); newSigners[0] = encA1; newSigners[1] = fedA2; + uint256 newThreshold = 2; + uint256 nonce = proxy.proposalNonce(); + uint256 deadline = block.timestamp + 1 days; + + bytes32 digest = MultisigHelper.digestProposeUpdateFederationSigners( + domainSep, newSigners, newThreshold, nonce, deadline + ); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + bytes32 id = proxy.proposeUpdateFederationSigners(newSigners, newThreshold, nonce, deadline, bitmap, sigs); + + vm.warp(block.timestamp + TIMELOCK + 1); + vm.expectRevert(abi.encodeWithSelector(IMultisigProxy.SignerSetsOverlap.selector, encA1)); + proxy.executeProposal(id, abi.encode(newSigners, newThreshold)); + } + + // ======================================================================== + // Signer-set size cap (R-I-06 / UT-FIX-27) + // + // Either set is capped at MAX_SIGNERS, at construction and on signer-update + // (validated in _validateSigners at executeProposal). The cap bounds the + // O(N^2) scan and keeps every index within the uint256 signature bitmap. + // ======================================================================== + + function test_constructor_rejectsTooManySigners() public { + uint256 max = proxy.MAX_SIGNERS(); + address[] memory enc = _signers(max + 1); // 21 distinct enclave signers + uint256 encThreshold = (max + 1) / 2 + 1; // strict majority, so it clears the threshold guard + + vm.expectRevert(abi.encodeWithSelector(IMultisigProxy.TooManySigners.selector, max + 1, max)); + new MultisigProxy( + address(bridge), address(cm), enc, encThreshold, _validFed(), 2, commissionReceiver, TIMELOCK, MIN_TIMELOCK + ); + } + + function test_constructor_acceptsSignersAtMax() public { + uint256 max = proxy.MAX_SIGNERS(); + uint256 threshold = max / 2 + 1; // 11-of-20 strict majority + + MultisigProxy p = new MultisigProxy( + address(bridge), address(cm), _signers(max), threshold, _signersB(max), threshold, commissionReceiver, TIMELOCK, MIN_TIMELOCK + ); + assertEq(p.getEnclaveSigners().length, max); + assertEq(p.getFederationSigners().length, max); + } + + function test_proposeUpdateEnclaveSigners_rejectsTooManySignersAtExecute() public { + uint256 max = proxy.MAX_SIGNERS(); + address[] memory newSigners = _signers(max + 1); + uint256 newThreshold = (max + 1) / 2 + 1; // valid strict majority + uint256 nonce = proxy.proposalNonce(); + uint256 deadline = block.timestamp + 1 days; + + bytes32 digest = MultisigHelper.digestProposeUpdateEnclaveSigners( + domainSep, newSigners, newThreshold, nonce, deadline + ); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + bytes32 id = proxy.proposeUpdateEnclaveSigners(newSigners, newThreshold, nonce, deadline, bitmap, sigs); + + vm.warp(block.timestamp + TIMELOCK + 1); + vm.expectRevert(abi.encodeWithSelector(IMultisigProxy.TooManySigners.selector, max + 1, max)); + proxy.executeProposal(id, abi.encode(newSigners, newThreshold)); } // ======================================================================== @@ -377,6 +629,68 @@ contract MultisigProxyTest is Test { proxy.execute(callData, nonce, deadline, bitmap, sigs); } + // ======================================================================== + // TEE deadline upper bound (R-I-01 / UT-FIX-23) + // + // execute / executeBatch reject a signed deadline further than + // MAX_TEE_DEADLINE into the future, so a leaked or pre-signed payload cannot + // stay executable indefinitely. The boundary (exactly now + MAX_TEE_DEADLINE) + // is still accepted — the guard is a strict `>`. + // ======================================================================== + + function test_execute_revertsOnDeadlineTooFar() public { + bytes memory callData = _fundsOutCalldata(); + uint256 nonce = proxy.getNonce(FUNDS_OUT_SELECTOR); + uint256 deadline = block.timestamp + proxy.MAX_TEE_DEADLINE() + 1; + + bytes32 digest = MultisigHelper.digestBridgeOp(domainSep, FUNDS_OUT_SELECTOR, callData, nonce, deadline); + (uint256[] memory pks, uint256 bitmap) = _encSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + vm.expectRevert(IMultisigProxy.DeadlineTooFar.selector); + proxy.execute(callData, nonce, deadline, bitmap, sigs); + } + + function test_execute_acceptsDeadlineAtMaxBoundary() public { + bytes memory callData = _fundsOutCalldata(); + uint256 nonce = proxy.getNonce(FUNDS_OUT_SELECTOR); + uint256 deadline = block.timestamp + proxy.MAX_TEE_DEADLINE(); // exact boundary, strict `>` lets it through + + bytes32 digest = MultisigHelper.digestBridgeOp(domainSep, FUNDS_OUT_SELECTOR, callData, nonce, deadline); + (uint256[] memory pks, uint256 bitmap) = _encSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + proxy.execute(callData, nonce, deadline, bitmap, sigs); + assertEq(token.balanceOf(recipient), AMOUNT, 'executes at the exact deadline ceiling'); + } + + function test_executeBatch_revertsOnDeadlineTooFar() public { + (address[] memory targets, bytes[] memory callDatas, uint256[] memory values) = _singleFundsOutBatch(); + uint256 nonce = proxy.batchNonce(); + uint256 deadline = block.timestamp + proxy.MAX_TEE_DEADLINE() + 1; + + bytes32 digest = MultisigHelper.digestBridgeBatchOp(domainSep, targets, callDatas, values, nonce, deadline); + (uint256[] memory pks, uint256 bitmap) = _encSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + vm.expectRevert(IMultisigProxy.DeadlineTooFar.selector); + proxy.executeBatch(targets, callDatas, values, nonce, deadline, bitmap, sigs); + } + + function test_executeBatch_acceptsDeadlineAtMaxBoundary() public { + (address[] memory targets, bytes[] memory callDatas, uint256[] memory values) = _singleFundsOutBatch(); + uint256 nonce = proxy.batchNonce(); + uint256 deadline = block.timestamp + proxy.MAX_TEE_DEADLINE(); // exact boundary + + bytes32 digest = MultisigHelper.digestBridgeBatchOp(domainSep, targets, callDatas, values, nonce, deadline); + (uint256[] memory pks, uint256 bitmap) = _encSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + proxy.executeBatch(targets, callDatas, values, nonce, deadline, bitmap, sigs); + assertEq(token.balanceOf(recipient), AMOUNT, 'batch executes at the exact deadline ceiling'); + assertEq(proxy.batchNonce(), nonce + 1, 'batchNonce incremented'); + } + function test_execute_revertsOnWrongNonce() public { bytes memory callData = _fundsOutCalldata(); uint256 nonce = 99; @@ -786,6 +1100,55 @@ contract MultisigProxyTest is Test { proxy.proposeUpdateBridge(makeAddr('nb'), nonce, deadline, bitmap, sigs); } + function test_proposeUpdateBridge_revertsOnDeadlineBeforeTimelock() public { + uint256 nonce = proxy.proposalNonce(); + // One second short of the timelock window → dead on arrival. + uint256 deadline = block.timestamp + proxy.timelockDuration() - 1; + + bytes32 digest = MultisigHelper.digestProposeUpdateBridge(domainSep, makeAddr('nb'), nonce, deadline); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + vm.expectRevert(IMultisigProxy.DeadlineBeforeTimelock.selector); + proxy.proposeUpdateBridge(makeAddr('nb'), nonce, deadline, bitmap, sigs); + } + + function test_proposeUpdateBridge_acceptsDeadlineAtTimelockBoundary() public { + uint256 nonce = proxy.proposalNonce(); + uint256 deadline = block.timestamp + proxy.timelockDuration(); // exact boundary, `==` allowed + + bytes32 digest = MultisigHelper.digestProposeUpdateBridge(domainSep, makeAddr('nb'), nonce, deadline); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + bytes32 proposalId = proxy.proposeUpdateBridge(makeAddr('nb'), nonce, deadline, bitmap, sigs); + assertEq( + uint8(proxy.getProposal(proposalId).status), + uint8(IMultisigProxy.ProposalStatus.Pending), + 'proposal at the exact boundary is created' + ); + } + + function test_proposeUpdateBridge_executableAtTimelockBoundary() public { + address newBridge = makeAddr('boundaryBridge'); + uint256 nonce = proxy.proposalNonce(); + uint256 timelock = proxy.timelockDuration(); + uint256 deadline = block.timestamp + timelock; // deadline == proposedAt + timelock + + bytes32 digest = MultisigHelper.digestProposeUpdateBridge(domainSep, newBridge, nonce, deadline); + (uint256[] memory pks, uint256 bitmap) = _fedSigSet2of3(); + bytes[] memory sigs = MultisigHelper.signAll(vm, digest, pks); + + bytes32 proposalId = proxy.proposeUpdateBridge(newBridge, nonce, deadline, bitmap, sigs); + + // Execute at the single instant where block.timestamp == proposedAt + + // timelock == deadline: timelock has just elapsed and the deadline has + // not yet passed (both checks use strict comparisons). + vm.warp(block.timestamp + timelock); + proxy.executeProposal(proposalId, abi.encode(newBridge)); + assertEq(proxy.bridge(), newBridge, 'boundary proposal executes at the exact instant'); + } + // ======================================================================== // Propose + Execute — UpdateEnclaveSigners // ========================================================================