Skip to content

Commit

Permalink
feat: New proposal type SetGovernanceParameterProposal (#299)
Browse files Browse the repository at this point in the history
* shift gov params storage to

* add new proposal

* settable vote duration

* test: add tests (#300)

* set multiply governance parameters at a time

* execute multiple test

* move  out of

* modify codecov

* add back an assertion

---------

Co-authored-by: Brian Le <[email protected]>
  • Loading branch information
arr00 and 0xble committed Sep 22, 2023
1 parent 81fabd3 commit d642adb
Show file tree
Hide file tree
Showing 9 changed files with 257 additions and 43 deletions.
7 changes: 0 additions & 7 deletions codecov.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
ignore:
- "test"
- "deploy"

coverage:
status:
patch:
default:
target: 90%
if_ci_failed: error
41 changes: 15 additions & 26 deletions contracts/party/PartyGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,6 @@ abstract contract PartyGovernance is
address payable feeRecipient;
}

// Subset of `GovernanceOpts` that are commonly read together for
// efficiency.
struct GovernanceValues {
uint40 voteDuration;
uint40 executionDelay;
uint16 passThresholdBps;
uint96 totalVotingPower;
/// @notice Number of hosts for this party
uint8 numHosts;
}

// A snapshot of voting power for a member.
struct VotingPowerSnapshot {
// The timestamp when the snapshot was taken.
Expand Down Expand Up @@ -216,11 +205,11 @@ abstract contract PartyGovernance is
mapping(address => bool) public isHost;
/// @notice The last person a voter delegated its voting power to.
mapping(address => address) public delegationsByVoter;
// Governance parameters for this party.
GovernanceValues internal _governanceValues;
// ProposalState by proposal ID.
/// @notice Number of hosts for this party
uint8 public numHosts;
/// @notice ProposalState by proposal ID.
mapping(uint256 => ProposalState) private _proposalStateByProposalId;
// Snapshots of voting power per user, each sorted by increasing time.
/// @notice Snapshots of voting power per user, each sorted by increasing time.
mapping(address => VotingPowerSnapshot[]) private _votingPowerSnapshotsByVoter;

modifier onlyHost() {
Expand Down Expand Up @@ -302,13 +291,13 @@ abstract contract PartyGovernance is
abi.encode(proposalEngineOpts)
);
// Set the governance parameters.
_governanceValues = GovernanceValues({
_getSharedProposalStorage().governanceValues = GovernanceValues({
voteDuration: govOpts.voteDuration,
executionDelay: govOpts.executionDelay,
passThresholdBps: govOpts.passThresholdBps,
totalVotingPower: govOpts.totalVotingPower,
numHosts: uint8(govOpts.hosts.length)
totalVotingPower: govOpts.totalVotingPower
});
numHosts = uint8(govOpts.hosts.length);
// Set fees.
feeBps = govOpts.feeBps;
feeRecipient = govOpts.feeRecipient;
Expand Down Expand Up @@ -390,8 +379,8 @@ abstract contract PartyGovernance is

/// @notice Retrieve fixed governance parameters.
/// @return gv The governance parameters of this party.
function getGovernanceValues() external view returns (GovernanceValues memory gv) {
return _governanceValues;
function getGovernanceValues() external view returns (GovernanceValues memory) {
return _getSharedProposalStorage().governanceValues;
}

/// @notice Get the hash of a proposal.
Expand Down Expand Up @@ -468,7 +457,7 @@ abstract contract PartyGovernance is
isHost[newPartyHost] = true;
} else {
// Burned the host status
--_governanceValues.numHosts;
--numHosts;
}
isHost[msg.sender] = false;
emit HostStatusTransferred(msg.sender, newPartyHost);
Expand Down Expand Up @@ -513,7 +502,7 @@ abstract contract PartyGovernance is
}
}
// Prevent creating a distribution if the party has not started.
if (_governanceValues.totalVotingPower == 0) {
if (_getSharedProposalStorage().governanceValues.totalVotingPower == 0) {
revert PartyNotStartedError();
}
// Get the address of the token distributor.
Expand Down Expand Up @@ -572,8 +561,8 @@ abstract contract PartyGovernance is
executedTime: 0,
completedTime: 0,
votes: 0,
totalVotingPower: _governanceValues.totalVotingPower,
numHosts: _governanceValues.numHosts,
totalVotingPower: _getSharedProposalStorage().governanceValues.totalVotingPower,
numHosts: numHosts,
numHostsAccepted: 0
}),
getProposalHash(proposal)
Expand Down Expand Up @@ -650,7 +639,7 @@ abstract contract PartyGovernance is
_areVotesPassing(
values.votes,
values.totalVotingPower,
_governanceValues.passThresholdBps
_getSharedProposalStorage().governanceValues.passThresholdBps
)
) {
info.values.passedTime = uint40(block.timestamp);
Expand Down Expand Up @@ -1073,7 +1062,7 @@ abstract contract PartyGovernance is
return ProposalStatus.Defeated;
}
uint40 t = uint40(block.timestamp);
GovernanceValues memory gv = _governanceValues;
GovernanceValues memory gv = _getSharedProposalStorage().governanceValues;
if (pv.passedTime != 0) {
// Ready.
if (pv.passedTime + gv.executionDelay <= t) {
Expand Down
13 changes: 7 additions & 6 deletions contracts/party/PartyGovernanceNFT.sol
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
/// @param tokenId The token ID to query.
/// @return share The voting power percentage of `tokenId`.
function getVotingPowerShareOf(uint256 tokenId) public view returns (uint256) {
uint256 totalVotingPower = _governanceValues.totalVotingPower;
uint256 totalVotingPower = _getSharedProposalStorage().governanceValues.totalVotingPower;
return
totalVotingPower == 0 ? 0 : (votingPowerByTokenId[tokenId] * 1e18) / totalVotingPower;
}
Expand All @@ -177,7 +177,7 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
address delegate
) external onlyAuthority returns (uint256 tokenId) {
uint96 mintedVotingPower_ = mintedVotingPower;
uint96 totalVotingPower = _governanceValues.totalVotingPower;
uint96 totalVotingPower = _getSharedProposalStorage().governanceValues.totalVotingPower;

// Cap voting power to remaining unminted voting power supply.
uint96 votingPower_ = votingPower.safeCastUint256ToUint96();
Expand Down Expand Up @@ -212,7 +212,7 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
/// @param votingPower The amount of voting power to add.
function addVotingPower(uint256 tokenId, uint256 votingPower) external onlyAuthority {
uint96 mintedVotingPower_ = mintedVotingPower;
uint96 totalVotingPower = _governanceValues.totalVotingPower;
uint96 totalVotingPower = _getSharedProposalStorage().governanceValues.totalVotingPower;
// Cap voting power to remaining unminted voting power supply.
uint96 votingPower_ = votingPower.safeCastUint256ToUint96();
// Allow minting past total voting power if minting party cards for
Expand All @@ -234,7 +234,7 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
/// an authority.
/// @param newVotingPower The new total voting power to add.
function increaseTotalVotingPower(uint96 newVotingPower) external onlyAuthority {
_governanceValues.totalVotingPower += newVotingPower;
_getSharedProposalStorage().governanceValues.totalVotingPower += newVotingPower;
}

/// @notice Burn governance NFTs and remove their voting power. Can only
Expand All @@ -243,7 +243,8 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
function burn(uint256[] memory tokenIds) public onlyAuthority {
// Authority needs to be able to burn cards during the initial
// crowdfund to process refunds but not after the party has started.
if (_governanceValues.totalVotingPower != 0) revert UnauthorizedToBurnError();
if (_getSharedProposalStorage().governanceValues.totalVotingPower != 0)
revert UnauthorizedToBurnError();

// Used to update voting power state of party at the end.
_burnAndUpdateVotingPower(tokenIds, false);
Expand Down Expand Up @@ -382,7 +383,7 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
uint96 totalVotingPowerBurned = _burnAndUpdateVotingPower(tokenIds, true);

// Update total voting power of party.
_governanceValues.totalVotingPower -= totalVotingPowerBurned;
_getSharedProposalStorage().governanceValues.totalVotingPower -= totalVotingPowerBurned;
}
{
uint16 feeBps_ = feeBps;
Expand Down
7 changes: 6 additions & 1 deletion contracts/proposals/ProposalExecutionEngine.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import "./DistributeProposal.sol";
import "./AddAuthorityProposal.sol";
import "./OperatorProposal.sol";
import { SetSignatureValidatorProposal } from "./SetSignatureValidatorProposal.sol";
import { SetGovernanceParameterProposal } from "./SetGovernanceParameterProposal.sol";

/// @notice Upgradable implementation of proposal execution logic for parties that use it.
/// @dev This contract will be delegatecall'ed into by `Party` proxy instances.
Expand All @@ -33,6 +34,7 @@ contract ProposalExecutionEngine is
AddAuthorityProposal,
OperatorProposal,
SetSignatureValidatorProposal,
SetGovernanceParameterProposal,
IERC1271
{
using LibRawResult for bytes;
Expand All @@ -54,7 +56,8 @@ contract ProposalExecutionEngine is
Distribute,
AddAuthority,
Operator,
SetSignatureValidatorProposal
SetSignatureValidatorProposal,
SetGovernanceParameterProposal
}

// Explicit storage bucket for "private" state owned by the `ProposalExecutionEngine`.
Expand Down Expand Up @@ -279,6 +282,8 @@ contract ProposalExecutionEngine is
nextProgressData = _executeOperation(params);
} else if (pt == ProposalType.SetSignatureValidatorProposal) {
nextProgressData = _executeSetSignatureValidator(params);
} else if (pt == ProposalType.SetGovernanceParameterProposal) {
nextProgressData = _executeSetGovernanceParameter(params);
} else if (pt == ProposalType.UpgradeProposalEngineImpl) {
_executeUpgradeProposalsImplementation(params.proposalData);
} else {
Expand Down
9 changes: 9 additions & 0 deletions contracts/proposals/ProposalStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ abstract contract ProposalStorage {
struct SharedProposalStorage {
IProposalExecutionEngine engineImpl;
ProposalEngineOpts opts;
GovernanceValues governanceValues;
}

/// @notice Governance values stored for a party
struct GovernanceValues {
uint40 voteDuration;
uint40 executionDelay;
uint16 passThresholdBps;
uint96 totalVotingPower;
}

struct ProposalEngineOpts {
Expand Down
67 changes: 67 additions & 0 deletions contracts/proposals/SetGovernanceParameterProposal.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.20;

import { ProposalStorage } from "./ProposalStorage.sol";
import { IProposalExecutionEngine } from "./IProposalExecutionEngine.sol";

contract SetGovernanceParameterProposal is ProposalStorage {
/// @notice Reverted with when the new governance parameter value is invalid
error InvalidGovernanceParameter(uint256 value);
/// @notice Emitted when the vote duration is set
event VoteDurationSet(uint256 oldValue, uint256 newValue);
/// @notice Emitted when the execution delay is set
event ExecutionDelaySet(uint256 oldValue, uint256 newValue);
/// @notice Emitted when the pass threshold bps is set
event PassThresholdBpsSet(uint256 oldValue, uint256 newValue);

/// @notice Struct containing data required for this proposal type
struct SetGovernanceParameterProposalData {
uint40 voteDuration;
uint40 executionDelay;
uint16 passThresholdBps;
}

/// @notice Execute a `SetGovernanceParameterProposal` which sets the given governance parameter.
function _executeSetGovernanceParameter(
IProposalExecutionEngine.ExecuteProposalParams memory params
) internal returns (bytes memory) {
SetGovernanceParameterProposalData memory proposalData = abi.decode(
params.proposalData,
(SetGovernanceParameterProposalData)
);
if (proposalData.voteDuration != 0) {
if (proposalData.voteDuration < 1 hours) {
revert InvalidGovernanceParameter(proposalData.voteDuration);
}
emit VoteDurationSet(
_getSharedProposalStorage().governanceValues.voteDuration,
proposalData.voteDuration
);
_getSharedProposalStorage().governanceValues.voteDuration = proposalData.voteDuration;
}
if (proposalData.executionDelay != 0) {
if (proposalData.executionDelay > 30 days) {
revert InvalidGovernanceParameter(proposalData.executionDelay);
}
emit ExecutionDelaySet(
_getSharedProposalStorage().governanceValues.executionDelay,
proposalData.executionDelay
);
_getSharedProposalStorage().governanceValues.executionDelay = proposalData
.executionDelay;
}
if (proposalData.passThresholdBps != 0) {
if (proposalData.passThresholdBps > 10000) {
revert InvalidGovernanceParameter(proposalData.passThresholdBps);
}
emit PassThresholdBpsSet(
_getSharedProposalStorage().governanceValues.passThresholdBps,
proposalData.passThresholdBps
);
_getSharedProposalStorage().governanceValues.passThresholdBps = proposalData
.passThresholdBps;
}

return "";
}
}
2 changes: 1 addition & 1 deletion test/party/PartyGovernance.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ contract PartyGovernanceTest is Test, TestUtils {

// Remove second host
party.abdicateHost(address(0));
assertEq(party.getGovernanceValues().numHosts, 1);
assertEq(party.numHosts(), 1);

// Still need to wait for the second host
_assertProposalStatus(party, 1, PartyGovernance.ProposalStatus.Passed, 22);
Expand Down
24 changes: 22 additions & 2 deletions test/party/PartyGovernanceUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,11 @@ contract TestablePartyGovernance is PartyGovernance {
}

function increaseTotalVotingPower(uint96 newVotingPower) external {
_governanceValues.totalVotingPower += newVotingPower;
_getSharedProposalStorage().governanceValues.totalVotingPower += newVotingPower;
}

function getTotalVotingPower() external view returns (uint96) {
return _governanceValues.totalVotingPower;
return _getSharedProposalStorage().governanceValues.totalVotingPower;
}

function transferVotingPower(address from, address to, uint256 power) external {
Expand Down Expand Up @@ -2325,6 +2325,26 @@ contract PartyGovernanceUnitTest is Test, TestUtils {
gov.abdicateHost(nonHost2);
}

// Demonstrate correct functionality of abidacte host along with the `numHost` counter.
function testHostPower_canAbidacteHost() external {
TestablePartyGovernance gov;
(
IERC721[] memory preciousTokens,
uint256[] memory preciousTokenIds
) = _createPreciousTokens(2);
gov = _createGovernance(false, 100e18, preciousTokens, preciousTokenIds);

assertEq(gov.numHosts(), 2);
vm.prank(defaultGovernanceOpts.hosts[0]);
gov.abdicateHost(address(this));
assertFalse(gov.isHost(defaultGovernanceOpts.hosts[0]));
assertTrue(gov.isHost(address(this)));
assertEq(gov.numHosts(), 2);
gov.abdicateHost(address(0));
assertFalse(gov.isHost(address(this)));
assertEq(gov.numHosts(), 1);
}

// voting power of past member is 0 at current time.
function testVotingPower_votingPowerOfPastMemberIsZeroAtCurrentTime() external {
(
Expand Down
Loading

0 comments on commit d642adb

Please sign in to comment.