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 all 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 @@
171501
170896
2 changes: 1 addition & 1 deletion .forge-snapshots/VoteSigComp.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
52138
51979
2 changes: 1 addition & 1 deletion .forge-snapshots/VoteSigCompMetadata.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
53824
53665
2 changes: 1 addition & 1 deletion .forge-snapshots/VoteTxComp.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
45325
45220
2 changes: 1 addition & 1 deletion .forge-snapshots/VoteTxCompMetadata.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
46916
46811
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
139 changes: 80 additions & 59 deletions src/Space.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,15 @@ import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils
import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import { IERC4824 } from "src/interfaces/IERC4824.sol";
import { ISpace, ISpaceActions, ISpaceState, ISpaceOwnerActions } from "src/interfaces/ISpace.sol";
import { Choice, FinalizationStatus, IndexedStrategy, Proposal, ProposalStatus, Strategy } from "src/types.sol";
import {
Choice,
FinalizationStatus,
IndexedStrategy,
Proposal,
ProposalStatus,
Strategy,
UpdateSettingsInput
} from "src/types.sol";
import { IVotingStrategy } from "src/interfaces/IVotingStrategy.sol";
import { IExecutionStrategy } from "src/interfaces/IExecutionStrategy.sol";
import { IProposalValidationStrategy } from "src/interfaces/IProposalValidationStrategy.sol";
Expand All @@ -22,6 +30,18 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
using BitPacker for uint256;
using SXUtils for IndexedStrategy[];

/// @dev Placeholder value to indicate the user does not want to update a string.
/// @dev Evaluates to: `0xf2cda9b13ed04e585461605c0d6e804933ca828111bd94d4e6a96c75e8b048ba`.
bytes32 private constant NO_UPDATE_HASH = keccak256(abi.encodePacked("No update"));
pscott marked this conversation as resolved.
Show resolved Hide resolved

/// @dev Placeholder value to indicate the user does not want to update an address.
/// @dev Evaluates to: `0xf2cda9b13ed04e585461605c0d6e804933ca8281`.
address private constant NO_UPDATE_ADDRESS = address(bytes20(keccak256(abi.encodePacked("No update"))));

/// @dev Placeholder value to indicate the user does not want to update a uint32.
/// @dev Evaluates to: `0xf2cda9b1`.
uint32 private constant NO_UPDATE_UINT32 = uint32(bytes4(keccak256(abi.encodePacked("No update"))));

/// @inheritdoc IERC4824
string public daoURI;
/// @inheritdoc ISpaceState
Expand Down Expand Up @@ -70,6 +90,9 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
_setMinVotingDuration(_minVotingDuration);
_setProposalValidationStrategy(_proposalValidationStrategy);
_setVotingDelay(_votingDelay);

if (_votingStrategies.length == 0) revert EmptyArray();
if (_authenticators.length == 0) revert EmptyArray();
_addVotingStrategies(_votingStrategies);
_addAuthenticators(_authenticators);

Expand Down Expand Up @@ -98,68 +121,70 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
// ------------------------------------

/// @inheritdoc ISpaceOwnerActions
function setMaxVotingDuration(uint32 _maxVotingDuration) external override onlyOwner {
_setMaxVotingDuration(_maxVotingDuration);
emit MaxVotingDurationUpdated(_maxVotingDuration);
}

/// @inheritdoc ISpaceOwnerActions
function setMinVotingDuration(uint32 _minVotingDuration) external override onlyOwner {
_setMinVotingDuration(_minVotingDuration);
emit MinVotingDurationUpdated(_minVotingDuration);
}
// solhint-disable-next-line code-complexity
function updateSettings(UpdateSettingsInput calldata input) external override onlyOwner {
if ((input.minVotingDuration != NO_UPDATE_UINT32) && (input.maxVotingDuration != NO_UPDATE_UINT32)) {
// Check that min and max VotingDuration are valid
// We don't use the internal `_setMinVotingDuration` and `_setMaxVotingDuration` functions because
pscott marked this conversation as resolved.
Show resolved Hide resolved
// it would revert when `_minVotingDuration > maxVotingDuration` (when the new `_min` is
// bigger than the current `max`).
if (input.minVotingDuration > input.maxVotingDuration)
revert InvalidDuration(input.minVotingDuration, input.maxVotingDuration);

minVotingDuration = input.minVotingDuration;
emit MinVotingDurationUpdated(input.minVotingDuration);

maxVotingDuration = input.maxVotingDuration;
emit MaxVotingDurationUpdated(input.maxVotingDuration);
} else if (input.minVotingDuration != NO_UPDATE_UINT32) {
_setMinVotingDuration(input.minVotingDuration);
emit MinVotingDurationUpdated(input.minVotingDuration);
} else if (input.maxVotingDuration != NO_UPDATE_UINT32) {
_setMaxVotingDuration(input.maxVotingDuration);
emit MaxVotingDurationUpdated(input.maxVotingDuration);
}

/// @inheritdoc ISpaceOwnerActions
function setMetadataURI(string calldata _metadataURI) external override onlyOwner {
emit MetadataURIUpdated(_metadataURI);
}
if (input.votingDelay != NO_UPDATE_UINT32) {
_setVotingDelay(input.votingDelay);
emit VotingDelayUpdated(input.votingDelay);
}

/// @inheritdoc ISpaceOwnerActions
function setDaoURI(string calldata newDaoURI) external override onlyOwner {
_setDaoURI(newDaoURI);
emit DaoURIUpdated(newDaoURI);
}
if (keccak256(abi.encodePacked(input.metadataURI)) != NO_UPDATE_HASH) {
emit MetadataURIUpdated(input.metadataURI);
}

/// @inheritdoc ISpaceOwnerActions
function setProposalValidationStrategy(
Strategy calldata _proposalValidationStrategy,
string calldata proposalValidationStrategyMetadataURI
) external override onlyOwner {
_setProposalValidationStrategy(_proposalValidationStrategy);
emit ProposalValidationStrategyUpdated(_proposalValidationStrategy, proposalValidationStrategyMetadataURI);
}
if (keccak256(abi.encodePacked(input.daoURI)) != NO_UPDATE_HASH) {
_setDaoURI(input.daoURI);
emit DaoURIUpdated(input.daoURI);
}

/// @inheritdoc ISpaceOwnerActions
function setVotingDelay(uint32 _votingDelay) external override onlyOwner {
_setVotingDelay(_votingDelay);
emit VotingDelayUpdated(_votingDelay);
}
if (input.proposalValidationStrategy.addr != NO_UPDATE_ADDRESS) {
_setProposalValidationStrategy(input.proposalValidationStrategy);
emit ProposalValidationStrategyUpdated(
input.proposalValidationStrategy,
input.proposalValidationStrategyMetadataURI
);
}

/// @inheritdoc ISpaceOwnerActions
function addVotingStrategies(
Strategy[] calldata _votingStrategies,
string[] calldata votingStrategyMetadataURIs
) external override onlyOwner {
_addVotingStrategies(_votingStrategies);
emit VotingStrategiesAdded(_votingStrategies, votingStrategyMetadataURIs);
}
if (input.authenticatorsToAdd.length > 0) {
_addAuthenticators(input.authenticatorsToAdd);
emit AuthenticatorsAdded(input.authenticatorsToAdd);
}

/// @inheritdoc ISpaceOwnerActions
function removeVotingStrategies(uint8[] calldata _votingStrategyIndices) external override onlyOwner {
_removeVotingStrategies(_votingStrategyIndices);
emit VotingStrategiesRemoved(_votingStrategyIndices);
}
if (input.authenticatorsToRemove.length > 0) {
_removeAuthenticators(input.authenticatorsToRemove);
emit AuthenticatorsRemoved(input.authenticatorsToRemove);
}

/// @inheritdoc ISpaceOwnerActions
function addAuthenticators(address[] calldata _authenticators) external override onlyOwner {
_addAuthenticators(_authenticators);
emit AuthenticatorsAdded(_authenticators);
}
if (input.votingStrategiesToAdd.length > 0) {
_addVotingStrategies(input.votingStrategiesToAdd);
emit VotingStrategiesAdded(input.votingStrategiesToAdd, input.votingStrategyMetadataURIsToAdd);
}

/// @inheritdoc ISpaceOwnerActions
function removeAuthenticators(address[] calldata _authenticators) external override onlyOwner {
_removeAuthenticators(_authenticators);
emit AuthenticatorsRemoved(_authenticators);
if (input.votingStrategiesToRemove.length > 0) {
_removeVotingStrategies(input.votingStrategiesToRemove);
emit VotingStrategiesRemoved(input.votingStrategiesToRemove);
}
}

/// @dev Gates access to whitelisted authenticators only.
Expand Down Expand Up @@ -354,7 +379,6 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra

/// @dev Adds an array of voting strategies.
function _addVotingStrategies(Strategy[] memory _votingStrategies) internal {
if (_votingStrategies.length == 0) revert EmptyArray();
uint256 cachedActiveVotingStrategies = activeVotingStrategies;
uint8 cachedNextVotingStrategyIndex = nextVotingStrategyIndex;
if (cachedNextVotingStrategyIndex >= 256 - _votingStrategies.length) revert ExceedsStrategyLimit();
Expand All @@ -370,7 +394,6 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra

/// @dev Removes an array of voting strategies, specified by their indices.
function _removeVotingStrategies(uint8[] memory _votingStrategyIndices) internal {
if (_votingStrategyIndices.length == 0) revert EmptyArray();
for (uint8 i = 0; i < _votingStrategyIndices.length; i++) {
activeVotingStrategies = activeVotingStrategies.setBit(_votingStrategyIndices[i], false);
}
Expand All @@ -380,15 +403,13 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra

/// @dev Adds an array of authenticators.
function _addAuthenticators(address[] memory _authenticators) internal {
if (_authenticators.length == 0) revert EmptyArray();
for (uint256 i = 0; i < _authenticators.length; i++) {
authenticators[_authenticators[i]] = true;
}
}

/// @dev Removes an array of authenticators.
function _removeAuthenticators(address[] memory _authenticators) internal {
if (_authenticators.length == 0) revert EmptyArray();
for (uint256 i = 0; i < _authenticators.length; i++) {
authenticators[_authenticators[i]] = false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/space/ISpaceEvents.sol
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ interface ISpaceEvents {

/// @notice Emitted when the voting delay is updated.
/// @param newVotingDelay The new voting delay.
event VotingDelayUpdated(uint256 newVotingDelay);
event VotingDelayUpdated(uint32 newVotingDelay);

/// @notice Emitted when a proposal is updated.
/// @param proposalId The proposal id.
Expand Down
67 changes: 19 additions & 48 deletions src/interfaces/space/ISpaceOwnerActions.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

pragma solidity ^0.8.18;

import { Strategy } from "../../types.sol";
import { Strategy, UpdateSettingsInput } from "../../types.sol";

/// @title Space Owner Actions
/// @notice The actions that can be performed by the owner of a Space,
Expand All @@ -13,51 +13,22 @@ interface ISpaceOwnerActions {
/// @param proposalId The proposal to cancel.
function cancel(uint256 proposalId) external;

/// @notice Sets the voting delay.
/// @param delay The new voting delay.
function setVotingDelay(uint32 delay) external;

/// @notice Sets the minimum voting duration.
/// @param duration The new minimum voting duration.
function setMinVotingDuration(uint32 duration) external;

/// @notice Sets the maximum voting duration.
/// @param duration The new maximum voting duration.
function setMaxVotingDuration(uint32 duration) external;

/// @notice Sets the proposal validation strategy.
/// @param proposalValidationStrategy The new proposal validation strategy.
/// @param proposalValidationStrategyMetadataURI The new metadata URI for the proposal validation strategy.
function setProposalValidationStrategy(
Strategy calldata proposalValidationStrategy,
string calldata proposalValidationStrategyMetadataURI
) external;

/// @notice Sets the metadata URI for the Space.
/// @param metadataURI The new metadata URI.
function setMetadataURI(string calldata metadataURI) external;

/// @notice Sets the DAO URI for the Space.
/// @param daoURI The new DAO URI.
function setDaoURI(string calldata daoURI) external;

/// @notice Adds an array of voting strategies.
/// @param votingStrategies The array of voting strategies to add.
/// @param votingStrategyMetadataURIs The array of metadata URIs for `votingStrategies`.
function addVotingStrategies(
Strategy[] calldata votingStrategies,
string[] calldata votingStrategyMetadataURIs
) external;

/// @notice Removes an array of voting strategies.
/// @param indicesToRemove The array of indices of the voting strategies to remove.
function removeVotingStrategies(uint8[] calldata indicesToRemove) external;

/// @notice Adds an array of authenticators.
/// @param authenticators The array of authenticator addresses to add.
function addAuthenticators(address[] calldata authenticators) external;

/// @notice Removes an array of authenticators.
/// @param authenticators The array of authenticator addresses to remove.
function removeAuthenticators(address[] calldata authenticators) external;
/// @notice Updates the settings.
/// @param input The settings to modify
/// @dev The structure should consist of:
/// minVotingDuration The new minimum voting duration. Set to `NO_UPDATE_UINT32` to ignore.
/// maxVotingDuration The new maximum voting duration. Set to `NO_UPDATE_UINT32` to ignore.
/// votingDelay The new voting delay. Set to `NO_UPDATE_UINT32` to ignore.
/// metadataURI The new metadataURI. Set to `NO_UPDATE_STRING` to ignore.
/// daoURI The new daoURI. Set to `NO_UPDATE_STRING` to ignore.
/// proposalValidationStrategy The new proposal validation strategy to use. Set
/// to `NO_UPDATE_STRATEGY` to ignore.
/// proposalValidationStrategyMetadataURI The new metadata URI for the proposal validation strategy.
/// authenticatorsToAdd The authenticators to add. Set to an empty array to ignore.
/// authenticatorsToRemove The authenticators to remove. Set to an empty array to ignore.
/// votingStrategiesToAdd The voting strategies to add. Set to an empty array to ignore.
/// votingStrategyMetadataURIsToAdd The voting strategy metadata uris to add. Set to
/// an empty array to ignore.
/// votignStrategiesToRemove The indices of voting strategies to remove. Set to empty array to ignore.
function updateSettings(UpdateSettingsInput calldata input) external;
}
17 changes: 17 additions & 0 deletions src/types.sol
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,20 @@ struct MetaTransaction {
// We require a salt so that the struct can always be unique and we can use its hash as a unique identifier.
uint256 salt;
}

/// @dev Structure used for the function `updateSettings` because of solidity's stack constraints.
/// For more information, see `ISpaceOwnerActions.sol`.
struct UpdateSettingsInput {
uint32 minVotingDuration;
uint32 maxVotingDuration;
uint32 votingDelay;
string metadataURI;
string daoURI;
Strategy proposalValidationStrategy;
string proposalValidationStrategyMetadataURI;
address[] authenticatorsToAdd;
address[] authenticatorsToRemove;
Strategy[] votingStrategiesToAdd;
string[] votingStrategyMetadataURIsToAdd;
uint8[] votingStrategiesToRemove;
}
24 changes: 20 additions & 4 deletions test/ActiveProposalsLimiterProposalValidationStrategy.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity ^0.8.18;

import { SpaceTest } from "./utils/Space.t.sol";
import { Strategy } from "../src/types.sol";
import { Strategy, UpdateSettingsInput } from "../src/types.sol";
import {
ActiveProposalsLimiterProposalValidationStrategy
} from "../src/proposal-validation-strategies/ActiveProposalsLimiterProposalValidationStrategy.sol";
Expand All @@ -20,9 +20,25 @@ contract ActiveProposalsLimterTest is SpaceTest {

activeProposalsLimiterProposalValidationStrategy = new ActiveProposalsLimiterProposalValidationStrategy();

space.setProposalValidationStrategy(
Strategy(address(activeProposalsLimiterProposalValidationStrategy), abi.encode(cooldown, maxActive)),
""
Strategy memory newProposalStrategy = Strategy(
address(activeProposalsLimiterProposalValidationStrategy),
abi.encode(cooldown, maxActive)
);
space.updateSettings(
UpdateSettingsInput(
NO_UPDATE_UINT32,
NO_UPDATE_UINT32,
NO_UPDATE_UINT32,
NO_UPDATE_STRING,
NO_UPDATE_STRING,
newProposalStrategy,
"",
NO_UPDATE_ADDRESSES,
NO_UPDATE_ADDRESSES,
NO_UPDATE_STRATEGIES,
NO_UPDATE_STRINGS,
NO_UPDATE_UINT8S
)
);
}

Expand Down
Loading