diff --git a/contracts/party/PartyGovernance.sol b/contracts/party/PartyGovernance.sol index a57f11d0..c04b5ae0 100644 --- a/contracts/party/PartyGovernance.sol +++ b/contracts/party/PartyGovernance.sol @@ -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 @@ -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); @@ -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(); } } @@ -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(); } } _; @@ -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(); } _; } @@ -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(); } @@ -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. @@ -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. @@ -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; diff --git a/contracts/party/PartyGovernanceNFT.sol b/contracts/party/PartyGovernanceNFT.sol index 99723d5c..e8c2d64c 100644 --- a/contracts/party/PartyGovernanceNFT.sol +++ b/contracts/party/PartyGovernanceNFT.sol @@ -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(); @@ -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(); } _; } @@ -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; @@ -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; @@ -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; @@ -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); } @@ -278,7 +280,7 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 { getApproved[tokenId] != msg.sender && !isApprovedForAll[owner][msg.sender] ) { - revert UnauthorizedToBurnError(); + revert NotAuthorized(); } } @@ -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(); @@ -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); diff --git a/test/crowdfund/ReraiseETHCrowdfund.t.sol b/test/crowdfund/ReraiseETHCrowdfund.t.sol index cf98089b..598695be 100644 --- a/test/crowdfund/ReraiseETHCrowdfund.t.sol +++ b/test/crowdfund/ReraiseETHCrowdfund.t.sol @@ -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); } diff --git a/test/party/PartyGovernanceNFT.t.sol b/test/party/PartyGovernanceNFT.t.sol index c81be67e..4118fade 100644 --- a/test/party/PartyGovernanceNFT.t.sol +++ b/test/party/PartyGovernanceNFT.t.sol @@ -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()); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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(); } diff --git a/test/party/PartyGovernanceNFTUnit.sol b/test/party/PartyGovernanceNFTUnit.sol index 1410b333..f0150778 100644 --- a/test/party/PartyGovernanceNFTUnit.sol +++ b/test/party/PartyGovernanceNFTUnit.sol @@ -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); } diff --git a/test/party/PartyGovernanceUnit.t.sol b/test/party/PartyGovernanceUnit.t.sol index e9b6e5b6..cef76b30 100644 --- a/test/party/PartyGovernanceUnit.t.sol +++ b/test/party/PartyGovernanceUnit.t.sol @@ -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); } @@ -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); } @@ -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); }