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.

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

feat: add multiple update* methods #149

Merged
merged 25 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
347a00c
feat: add updateAuthenticators, updateVotingStrategies and updateSett…
pscott Apr 21, 2023
7197a37
feat: add updateAuthenticatorsAndVotingStrategies
pscott Apr 21, 2023
0c3346c
fix: add --via-ir to forge coverage
pscott Apr 21, 2023
9208e15
fix: add via_ir in foundry.toml
pscott Apr 24, 2023
a79d2b1
refactor: split the functions into updateSettings and updateStrategies
pscott Apr 24, 2023
15c6c04
fix: check for metadatauriupdated event when testing
pscott Apr 24, 2023
1d47068
fix: check for _min and _max and do not use internal setters
pscott May 3, 2023
a4a20b7
refactor: move back from votingStrategiesMetadataURIs to votingStrate…
pscott May 10, 2023
efd0006
refactor: remove set* entrypoints
pscott May 11, 2023
4324e2e
Merge branch 'main' of github.com:snapshot-labs/snapshot-oc into feat…
pscott May 12, 2023
a10fd7d
refactor: remove EmptyArray check in internal functions
pscott May 12, 2023
86c8042
fix: Differentiate NO_UPDATE_METADATA_URI and NO_UPDATE_METADATA_URI_…
pscott May 12, 2023
521f20e
fix: fix NO_UPDATE_METADATA_URI_HASH
pscott May 12, 2023
239b816
fix: remove useless comments
pscott May 12, 2023
ab2e158
refactor: changing magic values for NO_UPDATE
pscott May 12, 2023
1c2a5dc
Merge branch 'main' of github.com:snapshot-labs/snapshot-oc into feat…
pscott May 15, 2023
835262b
refactor: use types instead of variables for no_update
pscott May 15, 2023
187c745
Merge branch 'main' of github.com:snapshot-labs/snapshot-oc into feat…
pscott May 15, 2023
bfe8baa
refactor: use input struct for update settings
pscott May 15, 2023
d0d952f
fix: remove extra comments on UpdateSettingsInput
pscott May 17, 2023
9b467fe
chore: remove extra comment in updateSettings function
pscott May 17, 2023
f690143
chore: add comments on values of NO_UPDATE_*
pscott May 17, 2023
c16f7ae
Merge branch 'main' of github.com:snapshot-labs/snapshot-oc into feat…
pscott May 17, 2023
99c9659
chore: fix typo in Space.col comments
pscott May 17, 2023
3de402b
chore: updated comments
Orland0x May 17, 2023
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
2 changes: 1 addition & 1 deletion .forge-snapshots/ProposeSigComp.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170436
170458
2 changes: 1 addition & 1 deletion .forge-snapshots/VoteSigComp.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
52054
52129
2 changes: 1 addition & 1 deletion .forge-snapshots/VoteSigCompMetadata.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
53740
53815
2 changes: 1 addition & 1 deletion .forge-snapshots/VoteTxComp.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
45241
45370
2 changes: 1 addition & 1 deletion .forge-snapshots/VoteTxCompMetadata.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
46832
46961
2 changes: 1 addition & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"}]

[profile.ci]
fuzz = { runs = 1_000 }
verbosity = 4
verbosity = 4
78 changes: 73 additions & 5 deletions src/Space.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract Space is ISpace, Initializable, UUPSUpgradeable, OwnableUpgradeable, Re
Strategy public proposalValidationStrategy;

// Mapping of allowed authenticators.
mapping(address auth => bool allowed) private authenticators;
mapping(address auth => bool allowed) public authenticators;
// Mapping of all `Proposal`s of this space (past and present).
mapping(uint256 proposalId => Proposal proposal) private proposalRegistry;
// Mapping used to know if a voter already voted on a specific proposal. Here to prevent double voting.
Expand All @@ -70,7 +70,7 @@ contract Space is ISpace, Initializable, UUPSUpgradeable, OwnableUpgradeable, Re
Strategy memory _proposalValidationStrategy,
string memory _metadataURI,
Strategy[] memory _votingStrategies,
string[] memory _votingStrategyMetadataURIs,
string[] memory _votingStrategiesMetadataURIs,
address[] memory _authenticators
) public initializer {
__Ownable_init();
Expand All @@ -93,7 +93,7 @@ contract Space is ISpace, Initializable, UUPSUpgradeable, OwnableUpgradeable, Re
_proposalValidationStrategy,
_metadataURI,
_votingStrategies,
_votingStrategyMetadataURIs,
_votingStrategiesMetadataURIs,
_authenticators
);
}
Expand Down Expand Up @@ -273,10 +273,10 @@ contract Space is ISpace, Initializable, UUPSUpgradeable, OwnableUpgradeable, Re

function addVotingStrategies(
Strategy[] calldata _votingStrategies,
string[] calldata votingStrategyMetadataURIs
string[] calldata _votingStrategiesMetadataURIs
) external override onlyOwner {
_addVotingStrategies(_votingStrategies);
emit VotingStrategiesAdded(_votingStrategies, votingStrategyMetadataURIs);
emit VotingStrategiesAdded(_votingStrategies, _votingStrategiesMetadataURIs);
}

function removeVotingStrategies(uint8[] calldata _votingStrategyIndices) external override onlyOwner {
Expand All @@ -294,6 +294,74 @@ contract Space is ISpace, Initializable, UUPSUpgradeable, OwnableUpgradeable, Re
emit AuthenticatorsRemoved(_authenticators);
}

function updateAuthenticators(address[] calldata _toAdd, address[] calldata _toRemove) external override onlyOwner {
_addAuthenticators(_toAdd);
emit AuthenticatorsAdded(_toAdd);

_removeAuthenticators(_toRemove);
emit AuthenticatorsRemoved(_toRemove);
}

function updateVotingStrategies(
Strategy[] calldata _votingStrategiesToAdd,
string[] calldata _votingStrategiesMetadataURIsToAdd,
uint8[] calldata _indicesToRemove
) external override onlyOwner {
_addVotingStrategies(_votingStrategiesToAdd);
emit VotingStrategiesAdded(_votingStrategiesToAdd, _votingStrategiesMetadataURIsToAdd);

_removeVotingStrategies(_indicesToRemove);
emit VotingStrategiesRemoved(_indicesToRemove);
}

function updateStrategies(
Strategy calldata _proposalValidationStrategy,
address[] calldata _authenticatorsToAdd,
address[] calldata _authenticatorsToRemove,
Strategy[] calldata _votingStrategiesToAdd,
string[] calldata _votingStrategiesMetadataURIsToAdd,
uint8[] calldata _votingIndicesToRemove
) external override onlyOwner {
_setProposalValidationStrategy(_proposalValidationStrategy);
emit ProposalValidationStrategyUpdated(_proposalValidationStrategy);

_addAuthenticators(_authenticatorsToAdd);
emit AuthenticatorsAdded(_authenticatorsToAdd);

_removeAuthenticators(_authenticatorsToRemove);
emit AuthenticatorsRemoved(_authenticatorsToRemove);

_addVotingStrategies(_votingStrategiesToAdd);
emit VotingStrategiesAdded(_votingStrategiesToAdd, _votingStrategiesMetadataURIsToAdd);

_removeVotingStrategies(_votingIndicesToRemove);
emit VotingStrategiesRemoved(_votingIndicesToRemove);
}

function updateSettings(
uint32 _minVotingDuration,
uint32 _maxVotingDuration,
uint32 _votingDelay,
string calldata _metadataURI
) external override onlyOwner {
// Check that min and max VotingDuration are valid
// We don't use the internal `_setMinVotingDuration` and `_setMaxVotingDuration` functions because
// it would revert when `_minVotingDuration > maxVotingDuration` (when the new `_min` is
// bigger than the current `max`).
if (_minVotingDuration > _maxVotingDuration) revert InvalidDuration(_minVotingDuration, _maxVotingDuration);

minVotingDuration = _minVotingDuration;
emit MinVotingDurationUpdated(_minVotingDuration);

maxVotingDuration = _maxVotingDuration;
emit MaxVotingDurationUpdated(_maxVotingDuration);

_setVotingDelay(_votingDelay);
emit VotingDelayUpdated(_votingDelay);

emit MetadataURIUpdated(_metadataURI);
}

// ------------------------------------
// | |
// | GETTERS |
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces/space/ISpaceEvents.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface ISpaceEvents {
Strategy proposalValidationStrategy,
string metadataURI,
Strategy[] votingStrategies,
string[] votingStrategyMetadataURIs,
string[] votingStrategiesMetadataURIs,
address[] authenticators
);
event ProposalCreated(uint256 nextProposalId, address author, Proposal proposal, string metadataUri, bytes payload);
Expand All @@ -38,6 +38,6 @@ interface ISpaceEvents {
event MetadataURIUpdated(string newMetadataURI);
event ProposalValidationStrategyUpdated(Strategy newProposalValidationStrategy);
event QuorumUpdated(uint256 newQuorum);
event VotingDelayUpdated(uint256 newVotingDelay);
event VotingDelayUpdated(uint32 newVotingDelay);
event ProposalUpdated(uint256 proposalId, Strategy newExecutionStrategy, string newMetadataURI);
}
26 changes: 25 additions & 1 deletion src/interfaces/space/ISpaceOwnerActions.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,36 @@ interface ISpaceOwnerActions {

function addVotingStrategies(
Strategy[] calldata votingStrategies,
string[] calldata votingStrategyMetadataURIs
string[] calldata votingStrategiesMetadataURIs
pscott marked this conversation as resolved.
Show resolved Hide resolved
) external;

function removeVotingStrategies(uint8[] calldata indicesToRemove) external;

function addAuthenticators(address[] calldata _authenticators) external;

function removeAuthenticators(address[] calldata _authenticators) external;

function updateAuthenticators(address[] calldata _toAdd, address[] calldata _toRemove) external;

function updateVotingStrategies(
Strategy[] calldata _votingStrategies,
string[] calldata _votingStrategiesMetadataURIs,
uint8[] calldata _indicesToRemove
) external;

function updateStrategies(
Strategy calldata _proposalValidationStrategy,
address[] calldata _authenticatorsToAdd,
address[] calldata _authenticatorsToRemove,
Strategy[] calldata _votingStrategiesToAdd,
string[] calldata _votingStrategiesMetadataURIsToAdd,
uint8[] calldata _votingIndicesToRemove
) external;

function updateSettings(
uint32 _minVotingDuration,
uint32 _maxVotingDuration,
uint32 _votingDelay,
string calldata _metadataURI
) external;
}
12 changes: 6 additions & 6 deletions test/ProxyFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ contract SpaceFactoryTest is Test, IProxyFactoryEvents, IProxyFactoryErrors {
Strategy public proposalValidationStrategy;
uint32 public quorum;
string public metadataURI = "SX-EVM";
string[] public votingStrategyMetadataURIs;
string[] public votingStrategiesMetadataURIs;
string[] public executionStrategyMetadataURIs;

function setUp() public {
Expand Down Expand Up @@ -78,7 +78,7 @@ contract SpaceFactoryTest is Test, IProxyFactoryEvents, IProxyFactoryErrors {
proposalValidationStrategy,
metadataURI,
votingStrategies,
votingStrategyMetadataURIs,
votingStrategiesMetadataURIs,
authenticators,
executionStrategies,
executionStrategyMetadataURIs
Expand All @@ -100,7 +100,7 @@ contract SpaceFactoryTest is Test, IProxyFactoryEvents, IProxyFactoryErrors {
proposalValidationStrategy,
metadataURI,
votingStrategies,
votingStrategyMetadataURIs,
votingStrategiesMetadataURIs,
authenticators,
executionStrategies,
executionStrategyMetadataURIs
Expand All @@ -121,7 +121,7 @@ contract SpaceFactoryTest is Test, IProxyFactoryEvents, IProxyFactoryErrors {
proposalValidationStrategy,
metadataURI,
votingStrategies,
votingStrategyMetadataURIs,
votingStrategiesMetadataURIs,
authenticators,
executionStrategies,
executionStrategyMetadataURIs
Expand All @@ -143,7 +143,7 @@ contract SpaceFactoryTest is Test, IProxyFactoryEvents, IProxyFactoryErrors {
proposalValidationStrategy,
metadataURI,
votingStrategies,
votingStrategyMetadataURIs,
votingStrategiesMetadataURIs,
authenticators,
executionStrategies,
executionStrategyMetadataURIs
Expand All @@ -162,7 +162,7 @@ contract SpaceFactoryTest is Test, IProxyFactoryEvents, IProxyFactoryErrors {
proposalValidationStrategy,
metadataURI,
votingStrategies,
votingStrategyMetadataURIs,
votingStrategiesMetadataURIs,
authenticators
);
}
Expand Down
106 changes: 101 additions & 5 deletions test/SpaceOwnerActions.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import { SpaceV2 } from "./mocks/SpaceV2.sol";
import { SpaceTest } from "./utils/Space.t.sol";
import { Choice, IndexedStrategy, Strategy } from "../src/types.sol";
import { VanillaExecutionStrategy } from "../src/execution-strategies/VanillaExecutionStrategy.sol";
import { BitPacker } from "../src/utils/BitPacker.sol";

contract SpaceOwnerActionsTest is SpaceTest {
using BitPacker for uint256;

event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

// ------- Transfer Ownership -------
Expand Down Expand Up @@ -201,15 +204,15 @@ contract SpaceOwnerActionsTest is SpaceTest {
uint8[] memory newIndices = new uint8[](1);
newIndices[0] = 1;

string[] memory votingStrategyMetadataURIs = new string[](0);
string[] memory votingStrategiesMetadataURIs = new string[](0);

IndexedStrategy[] memory newUserVotingStrategies = new IndexedStrategy[](1);
newUserVotingStrategies[0] = IndexedStrategy(newIndices[0], new bytes(0));

vm.expectEmit(true, true, true, true);
emit VotingStrategiesAdded(newVotingStrategies, votingStrategyMetadataURIs);
emit VotingStrategiesAdded(newVotingStrategies, votingStrategiesMetadataURIs);
vm.prank(owner);
space.addVotingStrategies(newVotingStrategies, votingStrategyMetadataURIs);
space.addVotingStrategies(newVotingStrategies, votingStrategiesMetadataURIs);

// Create a proposal using the default proposal validation strategy
uint256 proposalId1 = _createProposal(author, proposalMetadataURI, executionStrategy, new bytes(0));
Expand Down Expand Up @@ -261,9 +264,9 @@ contract SpaceOwnerActionsTest is SpaceTest {
function testAddVotingStrategiesUnauthorized() public {
vm.expectRevert("Ownable: caller is not the owner");
vm.prank(unauthorized);
string[] memory votingStrategyMetadataURIs = new string[](0);
string[] memory votingStrategiesMetadataURIs = new string[](0);

space.addVotingStrategies(votingStrategies, votingStrategyMetadataURIs);
space.addVotingStrategies(votingStrategies, votingStrategiesMetadataURIs);
}

function testRemoveVotingStrategiesUnauthorized() public {
Expand Down Expand Up @@ -308,6 +311,99 @@ contract SpaceOwnerActionsTest is SpaceTest {
space.removeAuthenticators(authenticators);
}

function testUpdateAuthenticators() public {
address[] memory newAuths = new address[](2);
newAuths[0] = address(111);
newAuths[1] = address(222);

space.updateAuthenticators(newAuths, authenticators);

// Ensure authenticators were correctly updated
assertEq(space.authenticators(newAuths[0]), true);
assertEq(space.authenticators(newAuths[1]), true);
assertEq(space.authenticators(authenticators[0]), false);
}

function testUpdateVotingStrategies() public {
Strategy[] memory _votingStrategiesToAdd = new Strategy[](2);
_votingStrategiesToAdd[0] = Strategy(address(0xc), new bytes(0));
_votingStrategiesToAdd[1] = Strategy(address(0xd), new bytes(0));

string[] memory _votingStrategiesMetadataURIsToAdd = new string[](2);
_votingStrategiesMetadataURIsToAdd[0] = "test456";
_votingStrategiesMetadataURIsToAdd[1] = "test789";

uint8[] memory _indicesToRemove = new uint8[](1);
_indicesToRemove[0] = 0;

space.updateVotingStrategies(_votingStrategiesToAdd, _votingStrategiesMetadataURIsToAdd, _indicesToRemove);
// Ensure voting strategies were correctly updated
assertEq(space.activeVotingStrategies().isBitSet(0), false);
assertEq(space.activeVotingStrategies().isBitSet(1), true);
assertEq(space.activeVotingStrategies().isBitSet(2), true);
}

function testUpdateStrategies() public {
Strategy memory _proposalValidationStrategy = Strategy(address(111), new bytes(0));

address[] memory newAuths = new address[](2);
newAuths[0] = address(111);
newAuths[1] = address(222);

Strategy[] memory _votingStrategiesToAdd = new Strategy[](2);
_votingStrategiesToAdd[0] = Strategy(address(0xc), new bytes(0));
_votingStrategiesToAdd[1] = Strategy(address(0xd), new bytes(0));

string[] memory _votingStrategiesMetadataURIsToAdd = new string[](2);
_votingStrategiesMetadataURIsToAdd[0] = "test456";
_votingStrategiesMetadataURIsToAdd[1] = "test789";

uint8[] memory _indicesToRemove = new uint8[](1);
_indicesToRemove[0] = 0;

space.updateStrategies(
_proposalValidationStrategy,
newAuths,
authenticators,
_votingStrategiesToAdd,
_votingStrategiesMetadataURIsToAdd,
_indicesToRemove
);

// Ensure the proposal validation strategy was correctly updated
(address addr, bytes memory params) = space.proposalValidationStrategy();
assertEq(addr, _proposalValidationStrategy.addr);
assertEq(params, _proposalValidationStrategy.params);

// Ensure authenticators were correctly updated
assertEq(space.authenticators(newAuths[0]), true);
assertEq(space.authenticators(newAuths[1]), true);
assertEq(space.authenticators(authenticators[0]), false);

// Ensure voting strategies were correctly updated
assertEq(space.activeVotingStrategies().isBitSet(0), false);
assertEq(space.activeVotingStrategies().isBitSet(1), true);
assertEq(space.activeVotingStrategies().isBitSet(2), true);
}

function testUpdateSettings() public {
uint32 _maxVotingDuration = maxVotingDuration + 1;
uint32 _minVotingDuration = minVotingDuration + 1;
string memory _metadataURI = "test123";
uint32 _votingDelay = 42;

vm.expectEmit(true, true, true, true);
emit MetadataURIUpdated(_metadataURI);
space.updateSettings(_minVotingDuration, _maxVotingDuration, _votingDelay, _metadataURI);

// Ensure durations were correctly updated
assertEq(space.maxVotingDuration(), _maxVotingDuration);
assertEq(space.minVotingDuration(), _minVotingDuration);

// Ensure voting delay was correctly updated
assertEq(space.votingDelay(), _votingDelay);
}

// ------- Upgrading a Space ----

event Upgraded(address indexed implementation);
Expand Down
Loading