Skip to content

Commit

Permalink
reduce party size below threshold (#314)
Browse files Browse the repository at this point in the history
  • Loading branch information
arr00 committed Oct 16, 2023
1 parent 485c39d commit 676683f
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 60 deletions.
57 changes: 27 additions & 30 deletions contracts/party/PartyGovernance.sol
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.20;

import "../distribution/ITokenDistributor.sol";
import "../utils/ReadOnlyDelegateCall.sol";
import "../tokens/IERC721.sol";
import "../tokens/IERC20.sol";
import { ITokenDistributor } from "../distribution/ITokenDistributor.sol";
import { ReadOnlyDelegateCall } from "../utils/ReadOnlyDelegateCall.sol";
import { IERC721 } from "../tokens/IERC721.sol";
import { IERC20 } from "../tokens/IERC20.sol";
import { IERC721Receiver } from "../tokens/IERC721Receiver.sol";
import { ERC1155TokenReceiverBase } from "../vendor/solmate/ERC1155.sol";
import "../utils/LibERC20Compat.sol";
import "../utils/LibRawResult.sol";
import "../utils/LibSafeCast.sol";
import "../utils/IERC4906.sol";
import "../globals/IGlobals.sol";
import "../globals/LibGlobals.sol";
import "../proposals/IProposalExecutionEngine.sol";
import "../proposals/LibProposal.sol";
import "../proposals/ProposalStorage.sol";

import "./Party.sol";
import { LibERC20Compat } from "../utils/LibERC20Compat.sol";
import { LibRawResult } from "../utils/LibRawResult.sol";
import { LibSafeCast } from "../utils/LibSafeCast.sol";
import { IERC4906 } from "../utils/IERC4906.sol";
import { IGlobals } from "../globals/IGlobals.sol";
import { LibGlobals } from "../globals/LibGlobals.sol";
import { IProposalExecutionEngine } from "../proposals/IProposalExecutionEngine.sol";
import { LibProposal } from "../proposals/LibProposal.sol";
import { ProposalStorage } from "../proposals/ProposalStorage.sol";
import { Implementation } from "../utils/Implementation.sol";
import { Party } from "./Party.sol";

/// @notice Base contract for a Party encapsulating all governance functionality.
abstract contract PartyGovernance is
Expand Down Expand Up @@ -169,13 +169,9 @@ abstract contract PartyGovernance is
error BadProposalStatusError(ProposalStatus status);
error BadProposalHashError(bytes32 proposalHash, bytes32 actualHash);
error ExecutionTimeExceededError(uint40 maxExecutableTime, uint40 timestamp);
error OnlyPartyHostError();
error OnlyActiveMemberError();
error OnlyTokenDistributorOrSelfError();
error NotAuthorized();
error InvalidDelegateError();
error BadPreciousListError();
error OnlyPartyDaoError(address notDao, address partyDao);
error OnlyPartyDaoOrHostError(address notDao, address partyDao);
error OnlyWhenEmergencyActionsAllowedError();
error OnlyWhenEnabledError();
error AlreadyVotedError(address voter);
Expand Down Expand Up @@ -217,18 +213,17 @@ abstract contract PartyGovernance is
/// @notice Snapshots of voting power per user, each sorted by increasing time.
mapping(address => VotingPowerSnapshot[]) private _votingPowerSnapshotsByVoter;

modifier onlyHost() {
function _assertHost() internal view {
if (!isHost[msg.sender]) {
revert OnlyPartyHostError();
revert NotAuthorized();
}
_;
}

function _assertActiveMember() internal view {
VotingPowerSnapshot memory snap = _getLastVotingPowerSnapshotForVoter(msg.sender);
// Must have either delegated voting power or intrinsic voting power.
if (snap.intrinsicVotingPower == 0 && snap.delegatedVotingPower == 0) {
revert OnlyActiveMemberError();
revert NotAuthorized();
}
}

Expand All @@ -237,7 +232,7 @@ abstract contract PartyGovernance is
{
address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET);
if (msg.sender != partyDao) {
revert OnlyPartyDaoError(msg.sender, partyDao);
revert NotAuthorized();
}
}
_;
Expand All @@ -247,7 +242,7 @@ abstract contract PartyGovernance is
modifier onlyPartyDaoOrHost() {
address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET);
if (msg.sender != partyDao && !isHost[msg.sender]) {
revert OnlyPartyDaoOrHostError(msg.sender, partyDao);
revert NotAuthorized();
}
_;
}
Expand All @@ -260,7 +255,7 @@ abstract contract PartyGovernance is
_;
}

function _assertNotGloballyDisabled() internal {
function _assertNotGloballyDisabled() internal view {
if (_GLOBALS.getBool(LibGlobals.GLOBAL_DISABLE_PARTY_ACTIONS)) {
revert OnlyWhenEnabledError();
}
Expand Down Expand Up @@ -459,7 +454,8 @@ abstract contract PartyGovernance is

/// @notice Transfer party host status to another.
/// @param newPartyHost The address of the new host.
function abdicateHost(address newPartyHost) external onlyHost {
function abdicateHost(address newPartyHost) external {
_assertHost();
// 0 is a special case burn address.
if (newPartyHost != address(0)) {
// Cannot transfer host status to an existing host.
Expand Down Expand Up @@ -507,7 +503,7 @@ abstract contract PartyGovernance is
// Must be an active member.
VotingPowerSnapshot memory snap = _getLastVotingPowerSnapshotForVoter(msg.sender);
if (snap.intrinsicVotingPower == 0 && snap.delegatedVotingPower == 0) {
revert OnlyActiveMemberError();
revert NotAuthorized();
}
}
// Prevent creating a distribution if the party has not started.
Expand Down Expand Up @@ -666,7 +662,8 @@ abstract contract PartyGovernance is
/// A proposal that has been already executed at least once (in the `InProgress` status)
/// cannot be vetoed.
/// @param proposalId The ID of the proposal to veto.
function veto(uint256 proposalId) external onlyHost {
function veto(uint256 proposalId) external {
_assertHost();
// Setting `votes` to -1 indicates a veto.
ProposalState storage info = _proposalStateByProposalId[proposalId];
ProposalStateValues memory values = info.values;
Expand Down
36 changes: 20 additions & 16 deletions contracts/party/PartyGovernanceNFT.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
using LibERC20Compat for IERC20;
using LibAddress for address payable;

error OnlyAuthorityError();
error OnlySelfError();
error UnauthorizedToBurnError();
error FixedRageQuitTimestampError(uint40 rageQuitTimestamp);
error CannotRageQuitError(uint40 rageQuitTimestamp);
error CannotDisableRageQuitAfterInitializationError();
Expand Down Expand Up @@ -61,16 +58,15 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
/// @notice Address with authority to mint cards and update voting power for the party.
mapping(address => bool) public isAuthority;

modifier onlyAuthority() {
function _assertAuthority() internal view {
if (!isAuthority[msg.sender]) {
revert OnlyAuthorityError();
revert NotAuthorized();
}
_;
}

modifier onlySelf() {
if (msg.sender != address(this)) {
revert OnlySelfError();
revert NotAuthorized();
}
_;
}
Expand Down Expand Up @@ -171,7 +167,8 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
address owner,
uint256 votingPower,
address delegate
) external onlyAuthority returns (uint256 tokenId) {
) external returns (uint256 tokenId) {
_assertAuthority();
uint96 mintedVotingPower_ = mintedVotingPower;
uint96 totalVotingPower = _getSharedProposalStorage().governanceValues.totalVotingPower;

Expand Down Expand Up @@ -208,7 +205,8 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
/// authority.
/// @param tokenId The ID of the NFT to add voting power to.
/// @param votingPower The amount of voting power to add.
function addVotingPower(uint256 tokenId, uint96 votingPower) external onlyAuthority {
function addVotingPower(uint256 tokenId, uint96 votingPower) external {
_assertAuthority();
uint96 mintedVotingPower_ = mintedVotingPower;
uint96 totalVotingPower = _getSharedProposalStorage().governanceValues.totalVotingPower;

Expand All @@ -235,7 +233,8 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
/// authority.
/// @param tokenId The ID of the NFT to remove voting power from.
/// @param votingPower The amount of voting power to remove.
function removeVotingPower(uint256 tokenId, uint96 votingPower) external onlyAuthority {
function removeVotingPower(uint256 tokenId, uint96 votingPower) external {
_assertAuthority();
mintedVotingPower -= votingPower;
votingPowerByTokenId[tokenId] -= votingPower;

Expand All @@ -245,21 +244,24 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
/// @notice Increase the total voting power of the party. Only callable by
/// an authority.
/// @param votingPower The new total voting power to add.
function increaseTotalVotingPower(uint96 votingPower) external onlyAuthority {
function increaseTotalVotingPower(uint96 votingPower) external {
_assertAuthority();
_getSharedProposalStorage().governanceValues.totalVotingPower += votingPower;
}

/// @notice Decrease the total voting power of the party. Only callable by
/// an authority.
/// @param votingPower The new total voting power to add.
function decreaseTotalVotingPower(uint96 votingPower) external onlyAuthority {
function decreaseTotalVotingPower(uint96 votingPower) external {
_assertAuthority();
_getSharedProposalStorage().governanceValues.totalVotingPower -= votingPower;
}

/// @notice Burn governance NFTs and remove their voting power. Can only
/// be called by an authority before the party has started.
/// @param tokenIds The IDs of the governance NFTs to burn.
function burn(uint256[] memory tokenIds) public onlyAuthority {
function burn(uint256[] memory tokenIds) public {
_assertAuthority();
_burnAndUpdateVotingPower(tokenIds, false);
}

Expand All @@ -278,7 +280,7 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
getApproved[tokenId] != msg.sender &&
!isApprovedForAll[owner][msg.sender]
) {
revert UnauthorizedToBurnError();
revert NotAuthorized();
}
}

Expand Down Expand Up @@ -313,7 +315,8 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {

/// @notice Set the timestamp until which ragequit is enabled.
/// @param newRageQuitTimestamp The new ragequit timestamp.
function setRageQuit(uint40 newRageQuitTimestamp) external onlyHost {
function setRageQuit(uint40 newRageQuitTimestamp) external {
_assertHost();
// Prevent disabling ragequit after initialization.
if (newRageQuitTimestamp == DISABLE_RAGEQUIT_PERMANENTLY) {
revert CannotDisableRageQuitAfterInitializationError();
Expand Down Expand Up @@ -480,7 +483,8 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
}

/// @notice Relinquish the authority role.
function abdicateAuthority() external onlyAuthority {
function abdicateAuthority() external {
_assertAuthority();
delete isAuthority[msg.sender];

emit AuthorityRemoved(msg.sender);
Expand Down
2 changes: 1 addition & 1 deletion test/crowdfund/ReraiseETHCrowdfund.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ contract ReraiseETHCrowdfundTest is LintJSON, TestUtils, ERC721Receiver {
// Will fail because initial contribution should trigger crowdfund to
// try to finalize a win but it will fail because it is not yet set as
// an authority on the party
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
crowdfund.initialize{ value: initialContribution }(opts);
}

Expand Down
18 changes: 9 additions & 9 deletions test/party/PartyGovernanceNFT.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
})
);
address notAuthority = _randomAddress();
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
vm.prank(notAuthority);
party.mint(_randomAddress(), 1, _randomAddress());
}
Expand Down Expand Up @@ -235,7 +235,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {

address notAuthority = _randomAddress();
vm.prank(notAuthority);
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
party.increaseTotalVotingPower(votingPower);
}

Expand Down Expand Up @@ -284,7 +284,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {

address notAuthority = _randomAddress();
vm.prank(notAuthority);
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
party.decreaseTotalVotingPower(votingPower);
}

Expand Down Expand Up @@ -346,7 +346,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {

address notAuthority = _randomAddress();
vm.prank(notAuthority);
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
party.addVotingPower(tokenId, votingPower);
}

Expand Down Expand Up @@ -408,7 +408,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {

address notAuthority = _randomAddress();
vm.prank(notAuthority);
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
party.removeVotingPower(tokenId, votingPower);
}

Expand Down Expand Up @@ -485,7 +485,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
uint256 tokenId = party.mint(recipient, 10, recipient);

vm.prank(_randomAddress());
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
party.burn(tokenId);
}

Expand Down Expand Up @@ -527,7 +527,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
);
address notHost = _randomAddress();
vm.prank(notHost);
vm.expectRevert(PartyGovernance.OnlyPartyHostError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
party.setRageQuit(0);
}

Expand Down Expand Up @@ -992,7 +992,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {

address notOwner = _randomAddress();
vm.prank(notOwner);
vm.expectRevert(PartyGovernanceNFT.UnauthorizedToBurnError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
party.rageQuit(tokenIds, tokens, minWithdrawAmounts, recipient);
}

Expand Down Expand Up @@ -1475,7 +1475,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
})
);
address notAuthority = _randomAddress();
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
vm.prank(notAuthority);
party.abdicateAuthority();
}
Expand Down
2 changes: 1 addition & 1 deletion test/party/PartyGovernanceNFTUnit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ contract PartyGovernanceNFTUnitTest is TestUtils {
uint256 vp = _randomUint256() % defaultGovernanceOpts.totalVotingPower;
address notAuthority = _randomAddress();
vm.prank(notAuthority);
vm.expectRevert(PartyGovernanceNFT.OnlyAuthorityError.selector);
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
nft.mint(from, vp, from);
}

Expand Down
6 changes: 3 additions & 3 deletions test/party/PartyGovernanceUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,7 @@ contract PartyGovernanceUnitTest is Test, TestUtils {
assertEq(gov.propose(proposal, 0), proposalId);

// Non-host tries to veto.
vm.expectRevert(abi.encodeWithSelector(PartyGovernance.OnlyPartyHostError.selector));
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
vm.prank(undelegatedVoter);
gov.veto(proposalId);
}
Expand Down Expand Up @@ -2321,7 +2321,7 @@ contract PartyGovernanceUnitTest is Test, TestUtils {
address nonHost2 = _randomAddress();

vm.prank(nonHost);
vm.expectRevert(abi.encodeWithSelector(PartyGovernance.OnlyPartyHostError.selector));
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
gov.abdicateHost(nonHost2);
}

Expand Down Expand Up @@ -2602,7 +2602,7 @@ contract PartyGovernanceUnitTest is Test, TestUtils {

// Try to create a distribution.
vm.deal(address(gov), 1337e18);
vm.expectRevert(abi.encodeWithSelector(PartyGovernance.OnlyActiveMemberError.selector));
vm.expectRevert(PartyGovernance.NotAuthorized.selector);
vm.prank(member);
gov.distribute(address(gov).balance, ITokenDistributor.TokenType.Native, ETH_ADDRESS, 0);
}
Expand Down

0 comments on commit 676683f

Please sign in to comment.