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.

Sign up for GitHub

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

Update governor with jobId and simplify voting mechanism #355

Closed
wants to merge 5 commits into from
Closed

Update governor with jobId and simplify voting mechanism #355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea4d0ec
remove none and simplify governance voting
salman01zp May 2, 2024
c22840d
update governor with jobId
salman01zp May 6, 2024
ef3c2c7
yarn format
salman01zp May 6, 2024
8d8fdd2
Update tests
salman01zp May 7, 2024
145c04c
yarn format
salman01zp May 7, 2024
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
8 changes: 7 additions & 1 deletion packages/contracts/contracts/SignatureBridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,13 @@ 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
231 changes: 54 additions & 177 deletions packages/contracts/contracts/utils/Governable.sol
View file
Open in desktop
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,19 @@ 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 +61,12 @@ 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.
/// @notice Checks if the vote JobId 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 be greater than current jobId"
);
require(
votes[i].proposedGovernor != address(0x0),
Expand Down Expand Up @@ -129,74 +102,46 @@ 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
),
isSignatureFromGovernor(abi.encodePacked(_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,27 +151,18 @@ 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;
}
}
}
Expand All @@ -236,66 +172,42 @@ contract Governable {
/// @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 newJobId JobId of the new governor.
function _transferOwnership(address newOwner, uint32 newJobId) 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 = newJobId;
emit GovernanceOwnershipTransferred(
previousGovernor,
previousGovernorJobId,
newOwner,
lastGovernorUpdateTime,
refreshNonce
newJobId,
lastGovernorUpdateTime
);
}

Expand All @@ -319,48 +231,13 @@ 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);
}

/// @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;
}
return Vote(_jobId, _proposedGovernor);
}
}
Loading
Loading