diff --git a/packages/contracts/contracts/SignatureBridge.sol b/packages/contracts/contracts/SignatureBridge.sol index 5e2fa975..2f3dad35 100644 --- a/packages/contracts/contracts/SignatureBridge.sol +++ b/packages/contracts/contracts/SignatureBridge.sol @@ -46,7 +46,9 @@ contract SignatureBridge is Governable, ChainIdWithType, ProposalNonceTracker { /// @notice Initializes SignatureBridge with a governor /// @param initialGovernor Addresses that should be initially granted the relayer role. - constructor(address initialGovernor, uint32 nonce) Governable(initialGovernor, nonce) {} + /// @param jobId JobId of the governor. + /// @param votingThreshold Number of votes required to force set the governor. + constructor(address initialGovernor, uint32 jobId, uint32 votingThreshold) Governable(initialGovernor, jobId, votingThreshold) {} /// @notice Sets a new resource for handler contracts that use the IExecutor interface, /// and maps the {handlerAddress} to {newResourceID} in {_resourceIdToHandlerAddress}. diff --git a/packages/contracts/contracts/utils/Governable.sol b/packages/contracts/contracts/utils/Governable.sol index b84b65fe..c7dcb267 100644 --- a/packages/contracts/contracts/utils/Governable.sol +++ b/packages/contracts/contracts/utils/Governable.sol @@ -8,15 +8,11 @@ pragma solidity ^0.8.18; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; /// @title The Vote struct that defines a vote in the governance mechanism -/// @param nonce nonce of the proposal -/// @param leafIndex leafIndex of the proposer in the proposer set Merkle tree -/// @param siblingPathNodes Merkle proof path of sibling nodes +/// @param jobId JobId of the proposaed governor. /// @param proposedGovernor the governor that the voter wants to force reset to struct Vote { - uint32 nonce; - uint32 leafIndex; + uint32 jobId; address proposedGovernor; - bytes32[] siblingPathNodes; } /// @title The Governable contract that defines the governance mechanism @@ -25,26 +21,14 @@ struct Vote { contract Governable { address public governor; - /// Refresh nonce is for rotating the DKG - uint32 public refreshNonce = 0; + /// Job Id of the rotating the DKG + uint32 public jobId = 0; /// Last time ownership was transferred to a new governor uint256 public lastGovernorUpdateTime; - /// The root of the proposer set Merkle tree - bytes32 public voterMerkleRoot; - - /// The average session length in millisecs - /// Note: This default is set to the max value of a uint64 so that there - /// is no chance of a new governor being voted for before the governance has successfully - /// been transferred. In this first transferral, the actual session length will be set. - uint64 public averageSessionLengthInMillisecs = 2 ** 64 - 1; - - /// The session length multiplier (see the voteInFavorForceSetGovernor function below) - uint256 public sessionLengthMultiplier = 2; - - /// The number of proposers - uint32 public voterCount; + /// Threshold for the number of votes required to force set the governor. + uint32 public votingThreshold = 1; /// (votingPeriod/refreshNonce => (proposer => (vote of new governor))) /// whether a proposer has voted in this period and who they're voting for @@ -56,17 +40,20 @@ contract Governable { event GovernanceOwnershipTransferred( address indexed previousOwner, + uint32 previousOwnerJobId, address indexed newOwner, - uint256 timestamp, - uint32 indexed refreshNonce + uint32 indexed jobId, + uint256 timestamp + ); event RecoveredAddress(address indexed recovered); - constructor(address _governor, uint32 _refreshNonce) { + constructor(address _governor, uint32 _jobId, uint32 _votingThreshold) { governor = _governor; - refreshNonce = _refreshNonce; + jobId = _jobId; + votingThreshold = _votingThreshold; lastGovernorUpdateTime = block.timestamp; - emit GovernanceOwnershipTransferred(address(0), _governor, block.timestamp, refreshNonce); + emit GovernanceOwnershipTransferred(address(0), 0, _governor, _jobId, block.timestamp); } /// @notice Throws if called by any account other than the owner. @@ -75,25 +62,13 @@ contract Governable { _; } - /// @notice Checks if its a valid time to vote. - modifier isValidTimeToVote() { - // Check block time stamp is some length greater than the last time - // ownership transferred - require( - block.timestamp > - lastGovernorUpdateTime + - ((sessionLengthMultiplier * averageSessionLengthInMillisecs) / 1000), - "Governable: Invalid time for vote" - ); - _; - } /// @notice Checks if the vote nonces are valid. modifier areValidVotes(Vote[] memory votes) { for (uint i = 0; i < votes.length; i++) { require( - votes[i].nonce == refreshNonce, - "Governable: Nonce of vote must match refreshNonce" + votes[i].jobId < jobId, + "Governable: JobId of vote must match jobId" ); require( votes[i].proposedGovernor != address(0x0), @@ -129,74 +104,51 @@ contract Governable { /// @notice Renouncing ownership will leave the contract without an owner, /// thereby removing any functionality that is only available to the owner. function renounceOwnership() public onlyGovernor { - voterMerkleRoot = bytes32(0); - averageSessionLengthInMillisecs = 1 << (64 - 1); - voterCount = 0; - refreshNonce++; + votingThreshold = 0; + jobId = 0; governor = address(0); - emit GovernanceOwnershipTransferred(governor, address(0), block.timestamp, refreshNonce); + emit GovernanceOwnershipTransferred(governor, jobId, address(0), jobId, block.timestamp); } /// @notice Transfers ownership of the contract to a new account (`newOwner`). /// @param newOwner The new owner of the contract. - /// @param nonce The nonce of the proposal. + /// @param jobId JobId of the new governor. /// @notice Can only be called by the current owner. - function transferOwnership(address newOwner, uint32 nonce) public onlyGovernor { - _transferOwnership(newOwner); - refreshNonce = nonce; + function transferOwnership(address newOwner, uint32 jobId) public onlyGovernor { + _transferOwnership(newOwner, jobId); } /// @notice Transfers ownership of the contract to a new account associated with the publicKey - /// and update other storage values relevant to the emergency voting process. - /// @param _voterMerkleRoot The new voter merkle root. - /// @param _averageSessionLengthInMillisecs The new average session length in milliseconds. - /// @param _voterCount The new number of voters. - /// @param _nonce The nonce of the proposal. + /// @param _jobId The nonce of the proposal. /// @param _publicKey The public key of the new governor. /// @param _sig The signature of the propsal data. function transferOwnershipWithSignature( - bytes32 _voterMerkleRoot, - uint64 _averageSessionLengthInMillisecs, - uint32 _voterCount, - uint32 _nonce, + uint32 _jobId, bytes memory _publicKey, bytes memory _sig ) public { - require(_nonce == refreshNonce + 1, "Governable: Nonce must increment by 1"); - require(_averageSessionLengthInMillisecs > 0, "Governable: Invalid session length"); + require(_jobId > jobId, "Governable: JobId must be greater than current jobId"); bytes32 pubKeyHash = keccak256(_publicKey); address newOwner = address(uint160(uint256(pubKeyHash))); require( isSignatureFromGovernor( abi.encodePacked( - _voterMerkleRoot, - _averageSessionLengthInMillisecs, - _voterCount, - _nonce, _publicKey ), _sig ), "Governable: caller is not the governor" ); - voterMerkleRoot = _voterMerkleRoot; - averageSessionLengthInMillisecs = _averageSessionLengthInMillisecs; - voterCount = _voterCount; - _transferOwnership(newOwner); + _transferOwnership(newOwner, _jobId); } /// @notice Casts a vote in favor of force refreshing the governor /// @param vote A vote struct function voteInFavorForceSetGovernor( Vote memory vote - ) external isValidTimeToVote areValidVotes(arrayifyVote(vote)) { + ) external areValidVotes(arrayifyVote(vote)) { // Check merkle proof is valid address proposerAddress = msg.sender; - require( - _isValidMerkleProof(vote.siblingPathNodes, proposerAddress, vote.leafIndex), - "Governable: Invalid merkle proof" - ); - _processVote(vote, proposerAddress); } @@ -206,49 +158,40 @@ contract Governable { function voteInFavorForceSetGovernorWithSig( Vote[] memory votes, bytes[] memory sigs - ) external isValidTimeToVote areValidVotes(votes) { + ) external areValidVotes(votes) { require(votes.length == sigs.length, "Governable: Invalid number of votes and signatures"); for (uint i = 0; i < votes.length; i++) { // Recover the address from the signature address proposerAddress = recover(abi.encode(votes[i]), sigs[i]); - - // Check merkle proof is valid - bool isValid = _isValidMerkleProof( - votes[i].siblingPathNodes, - proposerAddress, - votes[i].leafIndex - ); - if (isValid) { - // Since we require voterCount / 2 votes to be in favor of a new governor, - // we can stop processing votes if we have enough votes for a new governor. - // Since we have nonces on votes, we can safely assume that the votes from - // previous rounds cannot be processed. We process and terminate the vote early - // even if the vote is not the last vote in the array by choice. - if (_processVote(votes[i], proposerAddress)) { - return; - } + // Since we require voterCount / 2 votes to be in favor of a new governor, + // we can stop processing votes if we have enough votes for a new governor. + // Since we have nonces on votes, we can safely assume that the votes from + // previous rounds cannot be processed. We process and terminate the vote early + // even if the vote is not the last vote in the array by choice. + if (_processVote(votes[i], proposerAddress)) { + return; } } } - + /// @notice Process a vote /// @param vote A vote struct /// @param voter The address of the voter function _processVote(Vote memory vote, address voter) internal returns (bool) { // If the proposer has already voted, remove their previous vote - if (alreadyVoted[vote.nonce][voter] != address(0x0)) { - address previousVote = alreadyVoted[vote.nonce][voter]; - numOfVotesForGovernor[vote.nonce][previousVote] -= 1; + if (alreadyVoted[vote.jobId][voter] != address(0x0)) { + address previousVote = alreadyVoted[vote.jobId][voter]; + numOfVotesForGovernor[vote.jobId][previousVote] -= 1; } // Update the vote mappings - alreadyVoted[vote.nonce][voter] = vote.proposedGovernor; - numOfVotesForGovernor[vote.nonce][vote.proposedGovernor] += 1; + alreadyVoted[vote.jobId][voter] = vote.proposedGovernor; + numOfVotesForGovernor[vote.jobId][vote.proposedGovernor] += 1; // Try to resolve the vote if enough votes for a proposed governor have been cast. // Note: `voterCount` is also assumed to be the maximum # of voters in the system. // Therefore, if `voterCount / 2` votes are in favor of a new governor, we can // safely assume that there is no other governor that has more votes. - if (numOfVotesForGovernor[vote.nonce][vote.proposedGovernor] > voterCount / 2) { - _transferOwnership(vote.proposedGovernor); + if (numOfVotesForGovernor[vote.jobId][vote.proposedGovernor] > votingThreshold) { + _transferOwnership(vote.proposedGovernor, vote.jobId); // If we transferred ownership, we return true to indicate the election is over. return true; } @@ -256,46 +199,22 @@ contract Governable { return false; } - /// @notice Checks a merkle proof given a leaf and merkle path of sibling nodes. - /// @param siblingPathNodes the path of sibling nodes of the Merkle proof - /// @param leaf the leaf to prove membership of in the Merkle tree - /// @param leafIndex the index of the leaf in the Merkle tree - function _isValidMerkleProof( - bytes32[] memory siblingPathNodes, - address leaf, - uint32 leafIndex - ) internal view returns (bool) { - require( - siblingPathNodes.length == getVoterMerkleTreeDepth(), - "Governable: Invalid merkle proof length" - ); - bytes32 leafHash = keccak256(abi.encodePacked(leaf)); - bytes32 currNodeHash = leafHash; - uint32 nodeIndex = leafIndex; - for (uint8 i = 0; i < siblingPathNodes.length; i++) { - if (nodeIndex % 2 == 0) { - currNodeHash = keccak256(abi.encodePacked(currNodeHash, siblingPathNodes[i])); - } else { - currNodeHash = keccak256(abi.encodePacked(siblingPathNodes[i], currNodeHash)); - } - nodeIndex = nodeIndex / 2; - } - return voterMerkleRoot == currNodeHash; - } - /// @notice Transfers ownership of the contract to a new account (`newOwner`). /// @param newOwner The new owner of the contract - function _transferOwnership(address newOwner) internal { + /// @param _jobId JobId of the new governor. + function _transferOwnership(address newOwner, uint32 _jobId) internal { require(newOwner != address(0), "Governable: New governor is the zero address"); address previousGovernor = governor; + uint32 previousGovernorJobId = jobId; governor = newOwner; lastGovernorUpdateTime = block.timestamp; - refreshNonce++; + jobId = _jobId; emit GovernanceOwnershipTransferred( previousGovernor, + previousGovernorJobId, newOwner, - lastGovernorUpdateTime, - refreshNonce + _jobId, + lastGovernorUpdateTime ); } @@ -319,48 +238,15 @@ contract Governable { } /// @notice Helper function for creating a vote struct - /// @param _nonce The nonce of the proposal - /// @param _leafIndex The leaf index of the vote + /// @param _jobId Job id of the proposed governor /// @param _proposedGovernor The proposed governor - /// @param _siblingPathNodes The sibling path nodes of the vote /// @return Vote The vote struct function createVote( - uint32 _nonce, - uint32 _leafIndex, - address _proposedGovernor, - bytes32[] memory _siblingPathNodes + uint32 _jobId, + address _proposedGovernor ) public pure returns (Vote memory) { - return Vote(_nonce, _leafIndex, _proposedGovernor, _siblingPathNodes); + return Vote(_jobId, _proposedGovernor); } - /// @notice Helper function to return the depth of the voter merkle tree. - /// @return uint8 The depth of the voter merkle tree - /// @notice It is assumed that the number of voters never exceeds 4096. - function getVoterMerkleTreeDepth() public view returns (uint8) { - if (voterCount <= 2) { - return 1; - } else if (voterCount <= 4) { - return 2; - } else if (voterCount <= 8) { - return 3; - } else if (voterCount <= 16) { - return 4; - } else if (voterCount <= 32) { - return 5; - } else if (voterCount <= 64) { - return 6; - } else if (voterCount <= 128) { - return 7; - } else if (voterCount <= 256) { - return 8; - } else if (voterCount <= 512) { - return 9; - } else if (voterCount <= 1024) { - return 10; - } else if (voterCount <= 2048) { - return 11; - } else { - return 12; - } - } + } diff --git a/packages/vbridge/src/SignatureBridgeSide.ts b/packages/vbridge/src/SignatureBridgeSide.ts index 2c111b70..22f76118 100644 --- a/packages/vbridge/src/SignatureBridgeSide.ts +++ b/packages/vbridge/src/SignatureBridgeSide.ts @@ -15,6 +15,7 @@ export class SignatureBridgeSide implements IBridgeSide contract: SignatureBridge; admin: ethers.Signer; governor: ethers.Wallet | string; + jobId: number; anchorHandler: AnchorHandler; tokenHandler: TokenWrapperHandler; treasuryHandler: TreasuryHandler; @@ -42,26 +43,27 @@ export class SignatureBridgeSide implements IBridgeSide * @param admin - The deployer and governor upon creation. */ public static async createBridgeSide( - admin: ethers.Wallet + admin: ethers.Wallet, ): Promise> { const bridgeFactory = new SignatureBridge__factory(admin); - const deployedBridge = await bridgeFactory.deploy(admin.address, 0); + const deployedBridge = await bridgeFactory.deploy(admin.address, 0, 1); await deployedBridge.deployed(); const bridgeSide = new SignatureBridgeSide(deployedBridge, (data: any) => { return Promise.resolve(signMessage(admin, data)); }); bridgeSide.admin = admin; bridgeSide.governor = admin; + bridgeSide.jobId = 0; return bridgeSide; } public static async create2BridgeSide( deployer: Deployer, saltHex: string, - admin: ethers.Wallet + admin: ethers.Wallet, ): Promise> { const argTypes = ['address', 'uint32']; - const args = [admin.address, 0]; + const args = [admin.address, 0, 1]; const { contract: deployedBridge } = await deployer.deploy( SignatureBridge__factory, saltHex, @@ -75,6 +77,7 @@ export class SignatureBridgeSide implements IBridgeSide }); bridgeSide.admin = admin; bridgeSide.governor = admin; + bridgeSide.jobId = 0; return bridgeSide; } @@ -124,9 +127,10 @@ export class SignatureBridgeSide implements IBridgeSide * Transfers ownership directly from the current governor to the new governor. * Note that this requires an externally-signed transaction from the current governor. * @param newOwner The new owner of the bridge + * @param jobId The JobId of the new owner */ - public async transferOwnership(newOwner: string, nonce: number) { - return this.contract.transferOwnership(newOwner, nonce, { + public async transferOwnership(newOwner: string, jobId: number) { + return this.contract.transferOwnership(newOwner, jobId, { gasLimit: '0x5B8D80', }); }