Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 45 additions & 4 deletions ethereum/src/MultisigProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ contract MultisigProxy is IMultisigProxy {
/// @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`.
Expand Down Expand Up @@ -183,16 +189,19 @@ 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();
if (timelockDuration_ >= MAX_PROPOSAL_LIFETIME) revert TimelockTooLong();

_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_;
Expand Down Expand Up @@ -869,17 +878,19 @@ 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);

} 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);
Expand Down Expand Up @@ -1078,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++) {
Expand Down
2 changes: 2 additions & 0 deletions ethereum/src/interfaces/IMultisigProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -63,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();
Expand Down
77 changes: 77 additions & 0 deletions ethereum/test/Bridge.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,83 @@ contract BridgeTest is Test {
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`)
// ========================================================================
Expand Down
Loading