Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(BridgeManager): require min governors in manager #9

Merged
merged 3 commits into from
Mar 25, 2024
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
1 change: 1 addition & 0 deletions script/interfaces/ISharedArgument.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ interface ISharedArgument is IGeneralConfig {
uint96[] voteWeights;
GlobalProposal.TargetOption[] targetOptions;
address[] targets;
uint256 minRequiredGovernor;
}

struct MainchainGatewayV3Param {
Expand Down
20 changes: 20 additions & 0 deletions src/extensions/bridge-operator-governance/BridgeManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ abstract contract BridgeManager is IBridgeManager, HasContracts, BridgeManagerQu
mapping(address operator => uint96 weight) _operatorWeight;
/// @dev Total weight of all governors / operators.
uint256 _totalWeight;
/// @dev The minimum number of governors that must exist in the contract, to avoid the contract become non-accessible.
uint256 _minRequiredGovernor;
}

// keccak256(abi.encode(uint256(keccak256("ronin.storage.BridgeManagerStorageLocation")) - 1)) & ~bytes32(uint256(0xff))
Expand Down Expand Up @@ -71,6 +73,7 @@ abstract contract BridgeManager is IBridgeManager, HasContracts, BridgeManagerQu
);

_addBridgeOperators(voteWeights, governors, bridgeOperators);
_setMinRequiredGovernor(3);
}

// ===================== CONFIG ========================
Expand All @@ -83,6 +86,20 @@ abstract contract BridgeManager is IBridgeManager, HasContracts, BridgeManagerQu
_setContract(contractType, addr);
}

/**
* @inheritdoc IBridgeManager
*/
function setMinRequiredGovernor(uint min) external override onlySelfCall {
_setMinRequiredGovernor(min);
}

function _setMinRequiredGovernor(uint min) internal {
if (min < 3) revert ErrInvalidInput();
BridgeManagerStorage storage $ = _getBridgeManagerStorage();
$._minRequiredGovernor = min;
emit MinRequiredGovernorUpdated(min);
}

/**
* @dev Internal function to require that the caller has governor role access.
*/
Expand Down Expand Up @@ -289,6 +306,9 @@ abstract contract BridgeManager is IBridgeManager, HasContracts, BridgeManagerQu

// simply skip remove operations if inputs are empty.
if (length == 0) return removeds;
if ($._governors.length - length < $._minRequiredGovernor) {
revert ErrBelowMinRequiredGovernors();
}

address iGovernor;
address iOperator;
Expand Down
10 changes: 10 additions & 0 deletions src/interfaces/bridge/IBridgeManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ interface IBridgeManager is IBridgeManagerEvents {
error ErrGovernorNotFound(address governor);
/// @notice Error indicating that the msg.sender is not match the required governor
error ErrGovernorNotMatch(address required, address sender);
/// @notice Error indicating that the governors list will go below minimum number of required governor.
error ErrBelowMinRequiredGovernors();
/// @notice Common invalid input error
error ErrInvalidInput();

/**
* @dev The domain separator used for computing hash digests in the contract.
Expand Down Expand Up @@ -187,4 +191,10 @@ interface IBridgeManager is IBridgeManagerEvents {
* @param newOperator The new address of the operator.
*/
function updateBridgeOperator(address currOperator, address newOperator) external;

/**
* @dev Self-call to update the minimum required governor.
* @param min The minimum number, this must not less than 3.
*/
function setMinRequiredGovernor(uint min) external;
}
5 changes: 5 additions & 0 deletions src/interfaces/bridge/events/IBridgeManagerEvents.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,9 @@ interface IBridgeManagerEvents {
* @dev Emitted when a bridge operator is updated.
*/
event BridgeOperatorUpdated(address indexed governor, address indexed fromBridgeOperator, address indexed toBridgeOperator);

/**
* @dev Emitted when the minimum number of required governors is updated.
*/
event MinRequiredGovernorUpdated(uint min);
}
4 changes: 2 additions & 2 deletions src/libraries/AddressArrayUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ library AddressArrayUtils {
c = new address[](lengthA + lengthB);
}
uint256 i;
for (; i < lengthA; ) {
for (; i < lengthA;) {
c[i] = a[i];
unchecked {
++i;
}
}
for (uint256 j; j < lengthB; ) {
for (uint256 j; j < lengthB;) {
c[i] = b[j];
unchecked {
++i;
Expand Down
27 changes: 27 additions & 0 deletions src/libraries/Uint96ArrayUtils.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// SPDX-License-Identifier: UNLICENSED

pragma solidity ^0.8.0;

library Uint96ArrayUtils {
function extend(uint96[] memory a, uint96[] memory b) internal pure returns (uint96[] memory c) {
uint256 lengthA = a.length;
uint256 lengthB = b.length;
unchecked {
c = new uint96[](lengthA + lengthB);
}
uint256 i;
for (; i < lengthA;) {
c[i] = a[i];
unchecked {
++i;
}
}
for (uint256 j; j < lengthB;) {
c[i] = b[j];
unchecked {
++i;
++j;
}
}
}
}
10 changes: 2 additions & 8 deletions src/mainchain/MainchainBridgeManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract MainchainBridgeManager is BridgeManager, GovernanceRelay, GlobalGoverna
CoreGovernance(DEFAULT_EXPIRY_DURATION)
GlobalCoreGovernance(targetOptions, targets)
BridgeManager(num, denom, roninChainId, bridgeContract, callbackRegisters, bridgeOperators, governors, voteWeights)
{}
{ }

/**
* @dev See `GovernanceRelay-_relayProposal`.
Expand All @@ -56,13 +56,7 @@ contract MainchainBridgeManager is BridgeManager, GovernanceRelay, GlobalGoverna
Ballot.VoteType[] calldata supports_,
Signature[] calldata signatures
) external onlyGovernor {
_relayGlobalProposal({
globalProposal: globalProposal,
supports_: supports_,
signatures: signatures,
domainSeparator: DOMAIN_SEPARATOR,
creator: msg.sender
});
_relayGlobalProposal({ globalProposal: globalProposal, supports_: supports_, signatures: signatures, domainSeparator: DOMAIN_SEPARATOR, creator: msg.sender });
}

/**
Expand Down
26 changes: 9 additions & 17 deletions src/ronin/gateway/RoninBridgeManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ pragma solidity ^0.8.0;

import { ContractType, RoleAccess, ErrUnauthorized, BridgeManager } from "../../extensions/bridge-operator-governance/BridgeManager.sol";
import { Ballot, GlobalProposal, Proposal, GovernanceProposal } from "../../extensions/sequential-governance/governance-proposal/GovernanceProposal.sol";
import { CoreGovernance, GlobalCoreGovernance, GlobalGovernanceProposal } from "../../extensions/sequential-governance/governance-proposal/GlobalGovernanceProposal.sol";
import {
CoreGovernance,
GlobalCoreGovernance,
GlobalGovernanceProposal
} from "../../extensions/sequential-governance/governance-proposal/GlobalGovernanceProposal.sol";
import { VoteStatusConsumer } from "../../interfaces/consumers/VoteStatusConsumer.sol";
import { ErrQueryForEmptyVote } from "../../utils/CommonErrors.sol";

Expand All @@ -25,7 +29,7 @@ contract RoninBridgeManager is BridgeManager, GovernanceProposal, GlobalGovernan
CoreGovernance(expiryDuration)
GlobalCoreGovernance(targetOptions, targets)
BridgeManager(num, denom, roninChainId, bridgeContract, callbackRegisters, bridgeOperators, governors, voteWeights)
{}
{ }

/**
* CURRENT NETWORK
Expand Down Expand Up @@ -101,21 +105,14 @@ contract RoninBridgeManager is BridgeManager, GovernanceProposal, GlobalGovernan
* - The method caller is governor.
*
*/
function castProposalVoteForCurrentNetwork(
Proposal.ProposalDetail calldata proposal,
Ballot.VoteType support
) external onlyGovernor {
function castProposalVoteForCurrentNetwork(Proposal.ProposalDetail calldata proposal, Ballot.VoteType support) external onlyGovernor {
_castProposalVoteForCurrentNetwork(msg.sender, proposal, support);
}

/**
* @dev See `GovernanceProposal-_castProposalBySignatures`.
*/
function castProposalBySignatures(
Proposal.ProposalDetail calldata proposal,
Ballot.VoteType[] calldata supports_,
Signature[] calldata signatures
) external {
function castProposalBySignatures(Proposal.ProposalDetail calldata proposal, Ballot.VoteType[] calldata supports_, Signature[] calldata signatures) external {
_castProposalBySignatures(proposal, supports_, signatures, DOMAIN_SEPARATOR);
}

Expand Down Expand Up @@ -176,12 +173,7 @@ contract RoninBridgeManager is BridgeManager, GovernanceProposal, GlobalGovernan
Ballot.VoteType[] calldata supports_,
Signature[] calldata signatures
) external {
_castGlobalProposalBySignatures({
globalProposal: globalProposal,
supports_: supports_,
signatures: signatures,
domainSeparator: DOMAIN_SEPARATOR
});
_castGlobalProposalBySignatures({ globalProposal: globalProposal, supports_: supports_, signatures: signatures, domainSeparator: DOMAIN_SEPARATOR });
}

/**
Expand Down
39 changes: 38 additions & 1 deletion test/bridge/integration/BaseIntegration.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { SignatureConsumer } from "@ronin/contracts/interfaces/consumers/Signatu
import { Ballot } from "@ronin/contracts/libraries/Ballot.sol";
import { Transfer as LibTransfer } from "@ronin/contracts/libraries/Transfer.sol";
import { GlobalCoreGovernance } from "@ronin/contracts/extensions/sequential-governance/GlobalCoreGovernance.sol";
import { IBridgeManager } from "@ronin/contracts/interfaces/bridge/IBridgeManager.sol";
import { IHasContracts } from "@ronin/contracts/interfaces/collections/IHasContracts.sol";
import { IBridgeManagerCallbackRegister } from "@ronin/contracts/interfaces/bridge/IBridgeManagerCallbackRegister.sol";
import { ContractType } from "@ronin/contracts/utils/ContractType.sol";
Expand Down Expand Up @@ -298,6 +299,7 @@ contract BaseIntegration_Test is Base_Test {
_param.roninBridgeManager.callbackRegisters = wrapAddress(address(_bridgeSlash));
_param.roninBridgeManager.targetOptions = options;
_param.roninBridgeManager.targets = targets;
_param.roninBridgeManager.minRequiredGovernor = 3;

ISharedArgument.BridgeManagerParam memory param = _param.roninBridgeManager;
uint256 length = param.governors.length;
Expand Down Expand Up @@ -355,6 +357,23 @@ contract BaseIntegration_Test is Base_Test {
vm.prank(_param.roninBridgeManager.governors[0]);
_roninBridgeManager.proposeGlobalProposalStructAndCastVotes(globalProposal, supports_, signatures);
}

{
// set min governors
GlobalProposal.GlobalProposalDetail memory globalProposal = _roninProposalUtils.createGlobalProposal({
expiryTimestamp: block.timestamp + 10,
targetOption: GlobalProposal.TargetOption.BridgeManager,
value: 0,
calldata_: abi.encodeCall(IBridgeManager.setMinRequiredGovernor, (_param.roninBridgeManager.minRequiredGovernor)),
gasAmount: 500_000,
nonce: _roninBridgeManager.round(0) + 1
});

SignatureConsumer.Signature[] memory signatures = _roninProposalUtils.generateSignaturesGlobal(globalProposal, _param.test.governorPKs);

vm.prank(_param.roninBridgeManager.governors[0]);
_roninBridgeManager.proposeGlobalProposalStructAndCastVotes(globalProposal, supports_, signatures);
}
}

function _constructForMainchainBridgeManager() internal {
Expand All @@ -369,6 +388,7 @@ contract BaseIntegration_Test is Base_Test {
_param.mainchainBridgeManager.callbackRegisters = getEmptyAddressArray();
_param.mainchainBridgeManager.targetOptions = options;
_param.mainchainBridgeManager.targets = targets;
_param.mainchainBridgeManager.minRequiredGovernor = 3;

ISharedArgument.BridgeManagerParam memory param = _param.mainchainBridgeManager;
uint256 length = param.governors.length;
Expand Down Expand Up @@ -426,6 +446,23 @@ contract BaseIntegration_Test is Base_Test {
vm.prank(_param.roninBridgeManager.governors[0]);
_mainchainBridgeManager.relayGlobalProposal(globalProposal, supports_, signatures);
}

{
// set min governors
GlobalProposal.GlobalProposalDetail memory globalProposal = _roninProposalUtils.createGlobalProposal({
expiryTimestamp: block.timestamp + 10,
targetOption: GlobalProposal.TargetOption.BridgeManager,
value: 0,
calldata_: abi.encodeCall(IBridgeManager.setMinRequiredGovernor, (_param.roninBridgeManager.minRequiredGovernor)),
gasAmount: 500_000,
nonce: _mainchainBridgeManager.round(0) + 1
});

SignatureConsumer.Signature[] memory signatures = _mainchainProposalUtils.generateSignaturesGlobal(globalProposal, _param.test.governorPKs);

vm.prank(_param.roninBridgeManager.governors[0]);
_mainchainBridgeManager.relayGlobalProposal(globalProposal, supports_, signatures);
}
}

function _mainchainGatewayV3Initialize() internal {
Expand Down Expand Up @@ -576,7 +613,7 @@ contract BaseIntegration_Test is Base_Test {
LibTransfer.Receipt memory receipt,
uint256[] memory signerPKs,
bytes32 domainSeparator
) internal pure returns (SignatureConsumer.Signature[] memory sigs) {
) internal pure returns (SignatureConsumer.Signature[] memory sigs) {
sigs = new SignatureConsumer.Signature[](signerPKs.length);

for (uint256 i; i < signerPKs.length; i++) {
Expand Down
20 changes: 13 additions & 7 deletions test/bridge/unit/concrete/bridge-manager/BridgeManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,39 +35,45 @@ contract BridgeManager_Unit_Concrete_Test is Base_Test {
}

function setUp() public virtual {
address[] memory bridgeOperators = new address[](3);
address[] memory bridgeOperators = new address[](5);
bridgeOperators[0] = address(0x10000);
bridgeOperators[1] = address(0x10001);
bridgeOperators[2] = address(0x10002);
bridgeOperators[3] = address(0x10003);
bridgeOperators[4] = address(0x10004);

address[] memory governors = new address[](3);
address[] memory governors = new address[](5);
governors[0] = address(0x20000);
governors[1] = address(0x20001);
governors[2] = address(0x20002);
governors[3] = address(0x20003);
governors[4] = address(0x20004);

uint96[] memory voteWeights = new uint96[](3);
uint96[] memory voteWeights = new uint96[](5);
voteWeights[0] = 100;
voteWeights[1] = 100;
voteWeights[2] = 100;
voteWeights[3] = 100;
voteWeights[4] = 100;

for (uint i; i < bridgeOperators.length; i++) {
_bridgeOperators.push(bridgeOperators[i]);
_governors.push(governors[i]);
_voteWeights.push(voteWeights[i]);
}

_totalWeight = 300;
_totalOperator = 3;
_totalWeight = 500;
_totalOperator = 5;

_bridgeManager = new MockBridgeManager(bridgeOperators, governors, voteWeights);
}

function _generateNewOperators() internal pure returns (address[] memory operators, address[] memory governors, uint96[] memory weights) {
operators = new address[](1);
operators[0] = address(0x10003);
operators[0] = address(0x10099);

governors = new address[](1);
governors[0] = address(0x20003);
governors[0] = address(0x20099);

weights = new uint96[](1);
weights[0] = 100;
Expand Down
2 changes: 1 addition & 1 deletion test/bridge/unit/concrete/bridge-manager/constructor.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ contract Constructor_BridgeManager_Unit_Concrete_Test is BridgeManager_Unit_Conc
comparingWeights: _voteWeights,
expectingWeights: voteWeights
});
assertEq(_bridgeManager.totalBridgeOperator(), 3);
assertEq(_bridgeManager.totalBridgeOperator(), 5);
assertEq(_bridgeManager.getTotalWeight(), _totalWeight);
}

Expand Down
Loading