Skip to content

Commit

Permalink
use enum instead for readability
Browse files Browse the repository at this point in the history
  • Loading branch information
arr00 committed Jan 3, 2024
1 parent d4f30ef commit cf9834e
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 41 deletions.
5 changes: 4 additions & 1 deletion contracts/party/PartyGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,10 @@ abstract contract PartyGovernance is
// Must not require a vote to create a distribution, otherwise
// distributions can only be created through a distribution
// proposal.
if (_getSharedProposalStorage().opts.distributionsConfig != 0) {
if (
_getSharedProposalStorage().opts.distributionsConfig !=
DistributionsConfig.AllowedWithoutVote
) {
revert DistributionsRequireVoteError();
}
// Must be an active member.
Expand Down
8 changes: 5 additions & 3 deletions contracts/party/PartyGovernanceNFT.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ abstract contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
name = name_;
symbol = symbol_;
if (rageQuitTimestamp_ != 0) {
if (proposalEngineOpts.distributionsConfig == 0) {
if (proposalEngineOpts.distributionsConfig == DistributionsConfig.AllowedWithoutVote) {
revert CannotEnableRageQuitIfNotDistributionsRequireVoteError();
}

Expand Down Expand Up @@ -346,8 +346,10 @@ abstract contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
}

// Prevent enabling ragequit if distributions can be created without a vote.
if (_getSharedProposalStorage().opts.distributionsConfig == 0)
revert CannotEnableRageQuitIfNotDistributionsRequireVoteError();
if (
_getSharedProposalStorage().opts.distributionsConfig ==
DistributionsConfig.AllowedWithoutVote
) revert CannotEnableRageQuitIfNotDistributionsRequireVoteError();

uint40 oldRageQuitTimestamp = rageQuitTimestamp;

Expand Down
5 changes: 4 additions & 1 deletion contracts/proposals/ProposalExecutionEngine.sol
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,10 @@ contract ProposalExecutionEngine is
_getSharedProposalStorage().opts.allowArbCallsToSpendPartyEth
);
} else if (pt == ProposalType.Distribute) {
if (_getSharedProposalStorage().opts.distributionsConfig != 1) {
if (
_getSharedProposalStorage().opts.distributionsConfig !=
DistributionsConfig.AllowedWithVote
) {
revert ProposalDisabled(pt);
}

Expand Down
10 changes: 8 additions & 2 deletions contracts/proposals/ProposalStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ abstract contract ProposalStorage {
uint96 totalVotingPower;
}

enum DistributionsConfig {
AllowedWithoutVote,
AllowedWithVote,
NotAllowed
}

struct ProposalEngineOpts {
// Whether the party can add new authorities with the add authority proposal.
bool enableAddAuthorityProposal;
Expand All @@ -32,8 +38,8 @@ abstract contract ProposalStorage {
bool allowArbCallsToSpendPartyEth;
// Whether operators can be used.
bool allowOperators;
// 0 = allowed without vote. 1 = allowed with vote. 2 = not allowed.
uint8 distributionsConfig;
// Distributions config for the party.
DistributionsConfig distributionsConfig;
}

uint256 internal constant PROPOSAL_FLAG_UNANIMOUS = 0x1;
Expand Down
19 changes: 10 additions & 9 deletions test/crowdfund/CrowdfundFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import "./MockMarketWrapper.sol";
import "contracts/globals/Globals.sol";
import "contracts/globals/LibGlobals.sol";
import "contracts/renderers/MetadataRegistry.sol";
import { ProposalStorage } from "contracts/proposals/ProposalStorage.sol";
import "contracts/renderers/MetadataProvider.sol";
import { FixedPointMathLib } from "solmate/utils/FixedPointMathLib.sol";
import { LibSafeCast } from "contracts/utils/LibSafeCast.sol";
Expand Down Expand Up @@ -141,7 +142,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
});

Expand Down Expand Up @@ -219,7 +220,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
});

Expand Down Expand Up @@ -422,7 +423,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
});

Expand Down Expand Up @@ -495,7 +496,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
});

Expand Down Expand Up @@ -566,7 +567,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
});

Expand Down Expand Up @@ -638,7 +639,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
}),
preciousTokens: new IERC721[](0),
preciousTokenIds: new uint256[](0),
Expand Down Expand Up @@ -714,7 +715,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
}),
preciousTokens: new IERC721[](0),
preciousTokenIds: new uint256[](0),
Expand Down Expand Up @@ -804,7 +805,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
}),
preciousTokens: new IERC721[](0),
preciousTokenIds: new uint256[](0),
Expand Down Expand Up @@ -870,7 +871,7 @@ contract CrowdfundFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
}),
preciousTokens: new IERC721[](0),
preciousTokenIds: new uint256[](0),
Expand Down
2 changes: 1 addition & 1 deletion test/crowdfund/InitialETHCrowdfund.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1748,7 +1748,7 @@ contract InitialETHCrowdfundTest is InitialETHCrowdfundTestBase {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
}),
preciousTokens: new IERC721[](0),
preciousTokenIds: new uint256[](0),
Expand Down
14 changes: 10 additions & 4 deletions test/party/PartyFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ contract PartyFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
});
uint40 rageQuitTimestamp = uint40(block.timestamp + 30 days);
Expand Down Expand Up @@ -111,7 +111,10 @@ contract PartyFactoryTest is Test, TestUtils {
.getProposalEngineOpts();
assertEq(proposalEngineOpts.allowArbCallsToSpendPartyEth, true);
assertEq(proposalEngineOpts.allowOperators, true);
assertEq(proposalEngineOpts.distributionsConfig, 1);
assertEq(
uint8(proposalEngineOpts.distributionsConfig),
uint8(ProposalStorage.DistributionsConfig.AllowedWithVote)
);
assertEq(party.preciousListHash(), _hashPreciousList(preciousTokens, preciousTokenIds));
}

Expand All @@ -135,7 +138,7 @@ contract PartyFactoryTest is Test, TestUtils {
enableAddAuthorityProposal: true,
allowArbCallsToSpendPartyEth: true,
allowOperators: true,
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
});
bytes memory customMetadata = abi.encodePacked(_randomBytes32());
Expand Down Expand Up @@ -168,7 +171,10 @@ contract PartyFactoryTest is Test, TestUtils {
.getProposalEngineOpts();
assertEq(proposalEngineOpts.allowArbCallsToSpendPartyEth, true);
assertEq(proposalEngineOpts.allowOperators, true);
assertEq(proposalEngineOpts.distributionsConfig, 1);
assertEq(
uint8(proposalEngineOpts.distributionsConfig),
uint8(ProposalStorage.DistributionsConfig.AllowedWithVote)
);
assertEq(party.preciousListHash(), _hashPreciousList(preciousTokens, preciousTokenIds));
assertEq(address(registry.getProvider(address(party))), address(provider));
assertEq(provider.getMetadata(address(party), 0), customMetadata);
Expand Down
36 changes: 18 additions & 18 deletions test/party/PartyGovernanceNFT.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);
uint40 newTimestamp = uint40(block.timestamp + 1);
Expand Down Expand Up @@ -568,7 +568,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: true,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);
address notHost = _randomAddress();
Expand Down Expand Up @@ -596,7 +596,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -632,7 +632,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -665,7 +665,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -693,7 +693,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -788,7 +788,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -923,7 +923,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -990,7 +990,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -1078,7 +1078,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -1148,7 +1148,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 0
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithoutVote
})
);

Expand Down Expand Up @@ -1179,7 +1179,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -1248,7 +1248,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -1318,7 +1318,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -1409,7 +1409,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -1480,7 +1480,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -1549,7 +1549,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down Expand Up @@ -1637,7 +1637,7 @@ contract PartyGovernanceNFTTest is PartyGovernanceNFTTestBase {
allowArbCallsToSpendPartyEth: false,
allowOperators: false,
// Needs to be true to set non-zero rageQuitTimestamp
distributionsConfig: 1
distributionsConfig: ProposalStorage.DistributionsConfig.AllowedWithVote
})
);

Expand Down
4 changes: 3 additions & 1 deletion test/party/PartyGovernanceUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,9 @@ contract PartyGovernanceUnitTest is Test, TestUtils {
uint256[] memory preciousTokenIds
) private returns (TestablePartyGovernance gov) {
defaultGovernanceOpts.totalVotingPower = totalVotingPower;
defaultProposalEngineOpts.distributionsConfig = distributionsRequireVote ? 1 : 0;
defaultProposalEngineOpts.distributionsConfig = ProposalStorage.DistributionsConfig(
distributionsRequireVote ? 1 : 0
);

return
new TestablePartyGovernance(
Expand Down
Loading

0 comments on commit cf9834e

Please sign in to comment.