From 394f512e68597ff4413b27380d3632012ee980ad Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Thu, 13 Nov 2025 15:50:47 -0500 Subject: [PATCH 1/2] add ownable setter functions to bridge validator --- base/script/Deploy.s.sol | 2 +- base/src/BridgeValidator.sol | 61 +++++++++++++++++++++++++++++---- base/test/BridgeValidator.t.sol | 6 ++-- 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/base/script/Deploy.s.sol b/base/script/Deploy.s.sol index 5da9f3d8..0ee0c6cf 100644 --- a/base/script/Deploy.s.sol +++ b/base/script/Deploy.s.sol @@ -88,7 +88,7 @@ contract DeployScript is DevOps { admin: cfg.initialOwner, data: abi.encodeCall( BridgeValidator.initialize, - (cfg.baseValidators, cfg.baseSignatureThreshold, cfg.partnerValidatorThreshold) + (cfg.baseValidators, cfg.baseSignatureThreshold, cfg.partnerValidatorThreshold, cfg.initialOwner) ) }); } diff --git a/base/src/BridgeValidator.sol b/base/src/BridgeValidator.sol index 28ae2d77..6b400144 100644 --- a/base/src/BridgeValidator.sol +++ b/base/src/BridgeValidator.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.28; +import {Ownable} from "solady/auth/Ownable.sol"; import {ECDSA} from "solady/utils/ECDSA.sol"; import {Initializable} from "solady/utils/Initializable.sol"; @@ -15,7 +16,7 @@ import {Bridge} from "./Bridge.sol"; /// /// @notice A validator contract to be used during the Stage 0 phase of Base Bridge. This will likely later be replaced /// by `CrossL2Inbox` from the OP Stack. -contract BridgeValidator is Initializable { +contract BridgeValidator is Initializable, Ownable { using ECDSA for bytes32; /// @notice Container for data used to derive a unique `messageHash` for registration. @@ -146,17 +147,63 @@ contract BridgeValidator is Initializable { /// /// @dev Callable only once due to `initializer` modifier. /// - /// @param baseValidators The initial list of Base validators. - /// @param baseThreshold The minimum number of Base validator signatures required. + /// @param baseValidators The initial list of Base validators. + /// @param baseThreshold The minimum number of Base validator signatures required. /// @param partnerThreshold The minimum number of partner validator signatures required. - function initialize(address[] calldata baseValidators, uint128 baseThreshold, uint256 partnerThreshold) - external - initializer - { + /// @param owner The owner of the bridge validator contract. Has permission to add / remove validators + /// and update threshold values + function initialize( + address[] calldata baseValidators, + uint128 baseThreshold, + uint256 partnerThreshold, + address owner + ) external initializer { VerificationLib.initialize(baseValidators, baseThreshold); require(partnerThreshold <= MAX_PARTNER_VALIDATOR_THRESHOLD, ThresholdTooHigh()); + require(owner != address(0), ZeroAddress()); partnerValidatorThreshold = partnerThreshold; + + _initializeOwner(owner); + } + + /// @notice Updates the Base signature threshold. + /// + /// @dev Only callable by the BridgeValidator owner. + /// + /// @param newThreshold The new threshold value. + function setThreshold(uint256 newThreshold) external onlyOwner { + VerificationLib.setThreshold(newThreshold); + } + + /// @notice Updates the partner signature threshold. + /// + /// @dev Only callable by the BridgeValidator owner. + /// + /// @param newThreshold The new partner validator threshold value. + function setPartnerThreshold(uint256 newThreshold) external onlyOwner { + require(newThreshold <= MAX_PARTNER_VALIDATOR_THRESHOLD, ThresholdTooHigh()); + uint256 oldThreshold = partnerValidatorThreshold; + partnerValidatorThreshold = newThreshold; + emit PartnerThresholdUpdated(oldThreshold, newThreshold); + } + + /// @notice Adds a Base validator. + /// + /// @dev Only callable by the BridgeValidator owner. + /// + /// @param validator The validator address to add. + function addValidator(address validator) external onlyOwner { + VerificationLib.addValidator(validator); + } + + /// @notice Removes a Base validator. + /// + /// @dev Only callable by the BridgeValidator owner. + /// + /// @param validator The validator address to remove. + function removeValidator(address validator) external onlyOwner { + VerificationLib.removeValidator(validator); } /// @notice Pre-validates a batch of Solana → Base messages. diff --git a/base/test/BridgeValidator.t.sol b/base/test/BridgeValidator.t.sol index d47a12da..3bf5e37f 100644 --- a/base/test/BridgeValidator.t.sol +++ b/base/test/BridgeValidator.t.sol @@ -412,7 +412,7 @@ contract BridgeValidatorTest is CommonTest { } vm.expectRevert(VerificationLib.BaseSignerCountTooHigh.selector); - bridgeValidator.initialize(validators, 3, 1); + bridgeValidator.initialize(validators, 3, 1, cfg.initialOwner); } ////////////////////////////////////////////////////////////// @@ -421,7 +421,9 @@ contract BridgeValidatorTest is CommonTest { function test_initialize_revertsWhenCalledTwice() public { vm.expectRevert(Initializable.InvalidInitialization.selector); - bridgeValidator.initialize(cfg.baseValidators, cfg.baseSignatureThreshold, cfg.partnerValidatorThreshold); + bridgeValidator.initialize( + cfg.baseValidators, cfg.baseSignatureThreshold, cfg.partnerValidatorThreshold, cfg.initialOwner + ); } function test_nextNonce_incrementsByBatchLength() public { From cd191ad8e186e66bc3c0d7310fd0be00a11ba28a Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Thu, 13 Nov 2025 16:01:45 -0500 Subject: [PATCH 2/2] add reinitializable function to BridgeValidator --- base/src/BridgeValidator.sol | 14 ++ base/test/BridgeValidator.t.sol | 256 ++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+) diff --git a/base/src/BridgeValidator.sol b/base/src/BridgeValidator.sol index 6b400144..1b66e965 100644 --- a/base/src/BridgeValidator.sol +++ b/base/src/BridgeValidator.sol @@ -167,6 +167,20 @@ contract BridgeValidator is Initializable, Ownable { _initializeOwner(owner); } + /// @notice Reinitializes Base validator. + /// + /// @param partnerThreshold The minimum number of partner validator signatures required. + /// @param newOwner The owner of the bridge validator contract. Has permission to add / remove validators + /// and update threshold values + function reinitialize(uint256 partnerThreshold, address newOwner) external reinitializer(2) { + require(partnerThreshold <= MAX_PARTNER_VALIDATOR_THRESHOLD, ThresholdTooHigh()); + require(newOwner != address(0), ZeroAddress()); + + partnerValidatorThreshold = partnerThreshold; + + _initializeOwner(newOwner); + } + /// @notice Updates the Base signature threshold. /// /// @dev Only callable by the BridgeValidator owner. diff --git a/base/test/BridgeValidator.t.sol b/base/test/BridgeValidator.t.sol index 3bf5e37f..2603b28a 100644 --- a/base/test/BridgeValidator.t.sol +++ b/base/test/BridgeValidator.t.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.28; +import {Ownable} from "solady/auth/Ownable.sol"; import {Initializable} from "solady/utils/Initializable.sol"; import {DeployScript} from "../script/Deploy.s.sol"; @@ -442,6 +443,237 @@ contract BridgeValidatorTest is CommonTest { assertEq(bridgeValidator.nextNonce(), 3); } + ////////////////////////////////////////////////////////////// + /// reinitialize Tests /// + ////////////////////////////////////////////////////////////// + + function test_reinitialize_success() public { + address newOwner = vm.addr(999); + uint256 newPartnerThreshold = 2; + + bridgeValidator.reinitialize(newPartnerThreshold, newOwner); + + assertEq(bridgeValidator.partnerValidatorThreshold(), newPartnerThreshold); + assertEq(bridgeValidator.owner(), newOwner); + } + + function test_reinitialize_revertsWhenThresholdTooHigh() public { + address newOwner = vm.addr(999); + uint256 invalidThreshold = 6; // MAX_PARTNER_VALIDATOR_THRESHOLD is 5 + + vm.expectRevert(BridgeValidator.ThresholdTooHigh.selector); + bridgeValidator.reinitialize(invalidThreshold, newOwner); + } + + function test_reinitialize_revertsWhenZeroOwner() public { + uint256 newPartnerThreshold = 2; + + vm.expectRevert(BridgeValidator.ZeroAddress.selector); + bridgeValidator.reinitialize(newPartnerThreshold, address(0)); + } + + ////////////////////////////////////////////////////////////// + /// setThreshold Tests /// + ////////////////////////////////////////////////////////////// + + function test_setThreshold_success() public { + uint256 newThreshold = 1; + + vm.prank(cfg.initialOwner); + vm.expectEmit(false, false, false, true); + emit ThresholdUpdated(newThreshold); + bridgeValidator.setThreshold(newThreshold); + + assertEq(bridgeValidator.getBaseThreshold(), newThreshold); + } + + function test_setThreshold_revertsWhenNotOwner() public { + vm.expectRevert(abi.encodeWithSelector(Ownable.Unauthorized.selector)); + vm.prank(vm.addr(999)); + bridgeValidator.setThreshold(1); + } + + function test_setThreshold_revertsWhenZero() public { + vm.prank(cfg.initialOwner); + vm.expectRevert(VerificationLib.InvalidThreshold.selector); + bridgeValidator.setThreshold(0); + } + + function test_setThreshold_revertsWhenExceedsValidatorCount() public { + uint256 validatorCount = bridgeValidator.getBaseValidatorCount(); + uint256 invalidThreshold = validatorCount + 1; + + vm.prank(cfg.initialOwner); + vm.expectRevert(VerificationLib.InvalidThreshold.selector); + bridgeValidator.setThreshold(invalidThreshold); + } + + ////////////////////////////////////////////////////////////// + /// setPartnerThreshold Tests /// + ////////////////////////////////////////////////////////////// + + function test_setPartnerThreshold_success() public { + uint256 newThreshold = 2; + uint256 oldThreshold = bridgeValidator.partnerValidatorThreshold(); + + vm.prank(cfg.initialOwner); + vm.expectEmit(false, false, false, true); + emit PartnerThresholdUpdated(oldThreshold, newThreshold); + bridgeValidator.setPartnerThreshold(newThreshold); + + assertEq(bridgeValidator.partnerValidatorThreshold(), newThreshold); + } + + function test_setPartnerThreshold_revertsWhenNotOwner() public { + vm.expectRevert(abi.encodeWithSelector(Ownable.Unauthorized.selector)); + vm.prank(vm.addr(999)); + bridgeValidator.setPartnerThreshold(1); + } + + function test_setPartnerThreshold_revertsWhenThresholdTooHigh() public { + uint256 invalidThreshold = 6; // MAX_PARTNER_VALIDATOR_THRESHOLD is 5 + + vm.prank(cfg.initialOwner); + vm.expectRevert(BridgeValidator.ThresholdTooHigh.selector); + bridgeValidator.setPartnerThreshold(invalidThreshold); + } + + function test_setPartnerThreshold_allowsMaxThreshold() public { + uint256 maxThreshold = 5; // MAX_PARTNER_VALIDATOR_THRESHOLD + uint256 oldThreshold = bridgeValidator.partnerValidatorThreshold(); + + vm.prank(cfg.initialOwner); + vm.expectEmit(false, false, false, true); + emit PartnerThresholdUpdated(oldThreshold, maxThreshold); + bridgeValidator.setPartnerThreshold(maxThreshold); + + assertEq(bridgeValidator.partnerValidatorThreshold(), maxThreshold); + } + + function test_setPartnerThreshold_allowsZero() public { + uint256 oldThreshold = bridgeValidator.partnerValidatorThreshold(); + + vm.prank(cfg.initialOwner); + vm.expectEmit(false, false, false, true); + emit PartnerThresholdUpdated(oldThreshold, 0); + bridgeValidator.setPartnerThreshold(0); + + assertEq(bridgeValidator.partnerValidatorThreshold(), 0); + } + + ////////////////////////////////////////////////////////////// + /// addValidator Tests /// + ////////////////////////////////////////////////////////////// + + function test_addValidator_success() public { + address newValidator = vm.addr(100); + uint256 oldCount = bridgeValidator.getBaseValidatorCount(); + + vm.prank(cfg.initialOwner); + vm.expectEmit(false, false, false, true); + emit ValidatorAdded(newValidator); + bridgeValidator.addValidator(newValidator); + + assertTrue(bridgeValidator.isBaseValidator(newValidator)); + assertEq(bridgeValidator.getBaseValidatorCount(), oldCount + 1); + } + + function test_addValidator_revertsWhenNotOwner() public { + address newValidator = vm.addr(100); + + vm.expectRevert(abi.encodeWithSelector(Ownable.Unauthorized.selector)); + vm.prank(vm.addr(999)); + bridgeValidator.addValidator(newValidator); + } + + function test_addValidator_revertsWhenZeroAddress() public { + vm.prank(cfg.initialOwner); + vm.expectRevert(VerificationLib.InvalidValidatorAddress.selector); + bridgeValidator.addValidator(address(0)); + } + + function test_addValidator_revertsWhenAlreadyAdded() public { + address existingValidator = cfg.baseValidators[0]; + + vm.prank(cfg.initialOwner); + vm.expectRevert(VerificationLib.ValidatorAlreadyAdded.selector); + bridgeValidator.addValidator(existingValidator); + } + + function test_addValidator_revertsWhenMaxCountReached() public { + // Add validators until we reach MAX_BASE_SIGNER_COUNT + uint256 currentCount = bridgeValidator.getBaseValidatorCount(); + uint256 validatorsToAdd = VerificationLib.MAX_BASE_SIGNER_COUNT - currentCount; + + vm.startPrank(cfg.initialOwner); + for (uint256 i = 0; i < validatorsToAdd; i++) { + bridgeValidator.addValidator(vm.addr(200 + i)); + } + + // Now adding one more should fail + vm.expectRevert(VerificationLib.BaseSignerCountTooHigh.selector); + bridgeValidator.addValidator(vm.addr(300)); + vm.stopPrank(); + } + + ////////////////////////////////////////////////////////////// + /// removeValidator Tests /// + ////////////////////////////////////////////////////////////// + + function test_removeValidator_success() public { + address validatorToRemove = cfg.baseValidators[0]; + uint256 oldCount = bridgeValidator.getBaseValidatorCount(); + + // Ensure we have enough validators to maintain threshold + // If threshold is 2 and we have 2 validators, we need to add one first + uint256 currentThreshold = bridgeValidator.getBaseThreshold(); + if (oldCount - 1 < currentThreshold) { + // Add a validator first so we can remove one + vm.prank(cfg.initialOwner); + bridgeValidator.addValidator(vm.addr(150)); + oldCount = bridgeValidator.getBaseValidatorCount(); + } + + vm.prank(cfg.initialOwner); + vm.expectEmit(false, false, false, true); + emit ValidatorRemoved(validatorToRemove); + bridgeValidator.removeValidator(validatorToRemove); + + assertFalse(bridgeValidator.isBaseValidator(validatorToRemove)); + assertEq(bridgeValidator.getBaseValidatorCount(), oldCount - 1); + } + + function test_removeValidator_revertsWhenNotOwner() public { + address validatorToRemove = cfg.baseValidators[0]; + + vm.expectRevert(abi.encodeWithSelector(Ownable.Unauthorized.selector)); + vm.prank(vm.addr(999)); + bridgeValidator.removeValidator(validatorToRemove); + } + + function test_removeValidator_revertsWhenNotAValidator() public { + address nonValidator = vm.addr(200); + + vm.prank(cfg.initialOwner); + vm.expectRevert(VerificationLib.ValidatorNotExisted.selector); + bridgeValidator.removeValidator(nonValidator); + } + + function test_removeValidator_revertsWhenWouldViolateThreshold() public { + // If we have exactly threshold validators, removing one should fail + uint256 currentCount = bridgeValidator.getBaseValidatorCount(); + uint256 currentThreshold = bridgeValidator.getBaseThreshold(); + + // If count equals threshold, removing one would violate threshold + if (currentCount == currentThreshold) { + address validatorToRemove = cfg.baseValidators[0]; + + vm.prank(cfg.initialOwner); + vm.expectRevert(VerificationLib.ValidatorCountLessThanThreshold.selector); + bridgeValidator.removeValidator(validatorToRemove); + } + } + ////////////////////////////////////////////////////////////// /// View Function Tests /// ////////////////////////////////////////////////////////////// @@ -452,6 +684,30 @@ contract BridgeValidatorTest is CommonTest { assertFalse(bridgeValidator.validMessages(bytes32(0))); } + function test_getBaseThreshold_returnsCurrentThreshold() public view { + uint256 threshold = bridgeValidator.getBaseThreshold(); + assertGt(threshold, 0); + assertLe(threshold, bridgeValidator.getBaseValidatorCount()); + } + + function test_getBaseValidatorCount_returnsCurrentCount() public view { + uint256 count = bridgeValidator.getBaseValidatorCount(); + assertGt(count, 0); + assertLe(count, VerificationLib.MAX_BASE_SIGNER_COUNT); + } + + function test_isBaseValidator_returnsTrueForValidators() public view { + for (uint256 i = 0; i < cfg.baseValidators.length; i++) { + assertTrue(bridgeValidator.isBaseValidator(cfg.baseValidators[i])); + } + } + + function test_isBaseValidator_returnsFalseForNonValidators() public view { + assertFalse(bridgeValidator.isBaseValidator(vm.addr(999))); + assertFalse(bridgeValidator.isBaseValidator(address(0))); + assertFalse(bridgeValidator.isBaseValidator(address(this))); + } + function test_validMessages_afterRegistration() public { BridgeValidator.SignedMessage[] memory signedMessages = new BridgeValidator.SignedMessage[](2); signedMessages[0] = BridgeValidator.SignedMessage({