Skip to content

Commit

Permalink
remove none and simplify governance voting
Browse files Browse the repository at this point in the history
  • Loading branch information
@salman01zp
salman01zp committed May 2, 2024
1 parent 03a6c58 commit ea4d0ec
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 176 deletions.
4 changes: 3 additions & 1 deletion packages/contracts/contracts/SignatureBridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down
224 changes: 55 additions & 169 deletions packages/contracts/contracts/utils/Governable.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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),
Expand Down Expand Up @@ -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);
}

Expand All @@ -206,96 +158,63 @@ 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;
}

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
);
}

Expand All @@ -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;
}
}

}
Loading

0 comments on commit ea4d0ec

Please sign in to comment.