refactor: use block numbers to track time (#210)

* refactor: timestamps to numbers in space * refactor: block numbers in proposal status funcs * refactored: removed timestamp resolver * refactor: proposal validation to use block nubmers * refactor: removed snapshot block number * chore: updated tests * refactor: make getVotingPower a view function * refactor: revert active proposal limiter to timestamps * refactor: revert timelocks to timestamps * chore: fixed tests * Update IVotingStrategy.sol chore: natspec typo --------- Co-authored-by: Orlando <[email protected]>
snapshot-labs · Jun 19, 2023 · 5c51bbe · 5c51bbe
1 parent e7c71b2
commit 5c51bbe
Show file tree

Hide file tree

Showing 32 changed files with 159 additions and 292 deletions.
diff --git a/.forge-snapshots/ProposeSigComp.snap b/.forge-snapshots/ProposeSigComp.snap
@@ -1 +1 @@
-149997
+148973
diff --git a/.forge-snapshots/VoteSigComp.snap b/.forge-snapshots/VoteSigComp.snap
@@ -1 +1 @@
-50700
+50407
diff --git a/.forge-snapshots/VoteSigCompMetadata.snap b/.forge-snapshots/VoteSigCompMetadata.snap
@@ -1 +1 @@
-52243
+51957
diff --git a/.forge-snapshots/VoteTxComp.snap b/.forge-snapshots/VoteTxComp.snap
@@ -1 +1 @@
-44270
+43954
diff --git a/.forge-snapshots/VoteTxCompMetadata.snap b/.forge-snapshots/VoteTxCompMetadata.snap
@@ -1 +1 @@
-45854
+45545
diff --git a/README.md b/README.md
@@ -41,8 +41,7 @@ src
 │  ├─ SXHash.sol - "Snapshot X Types Hashing Library"
 │  ├─ SXUtils.sol - "Snapshot X Types Utilities Library"
 │  ├─ SignatureVerifier.sol - "Verifies EIP712 Signatures for Snapshot X actions"
-│  ├─ SpaceManager.sol - "Manages a whitelist of Spaces that have permissions to execute transactions"
-│  └─ TimestampResolver.sol - "Resolves timestamps to block numbers"
+│  └─ SpaceManager.sol - "Manages a whitelist of Spaces that have permissions to execute transactions"
 ├─ ProxyFactory.sol - "Handles the deployment and tracking of Space contracts"
 └─ Space.sol - "The base contract for each Snapshot X space"
 └─ types.sol - "Definitions for Snapshot X custom types"

diff --git a/src/Space.sol b/src/Space.sol
@@ -204,9 +204,6 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
         Strategy calldata executionStrategy,
         bytes calldata userProposalValidationParams
     ) external override onlyAuthenticator {
-        // Casting to `uint32` is fine because this gives us until year ~2106.
-        uint32 snapshotTimestamp = uint32(block.timestamp);
-
         if (
             !IProposalValidationStrategy(proposalValidationStrategy.addr).validate(
                 author,
@@ -215,20 +212,20 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
             )
         ) revert FailedToPassProposalValidation();
 
-        uint32 startTimestamp = snapshotTimestamp + votingDelay;
-        uint32 minEndTimestamp = startTimestamp + minVotingDuration;
-        uint32 maxEndTimestamp = startTimestamp + maxVotingDuration;
+        // Max block number of 2^32 - 1 = 4,294,967,295
+        uint32 startBlockNumber = uint32(block.number) + votingDelay;
+        uint32 minEndBlockNumber = startBlockNumber + minVotingDuration;
+        uint32 maxEndBlockNumber = startBlockNumber + maxVotingDuration;
 
         // The execution payload is the params of the supplied execution strategy struct.
         bytes32 executionPayloadHash = keccak256(executionStrategy.params);
 
         Proposal memory proposal = Proposal(
             author,
-            snapshotTimestamp,
-            startTimestamp,
+            startBlockNumber,
             IExecutionStrategy(executionStrategy.addr),
-            minEndTimestamp,
-            maxEndTimestamp,
+            minEndBlockNumber,
+            maxEndBlockNumber,
             FinalizationStatus.Pending,
             executionPayloadHash,
             activeVotingStrategies
@@ -250,16 +247,16 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
     ) external override onlyAuthenticator {
         Proposal memory proposal = proposals[proposalId];
         _assertProposalExists(proposal);
-        if (block.timestamp >= proposal.maxEndTimestamp) revert VotingPeriodHasEnded();
-        if (block.timestamp < proposal.startTimestamp) revert VotingPeriodHasNotStarted();
+        if (block.number >= proposal.maxEndBlockNumber) revert VotingPeriodHasEnded();
+        if (block.number < proposal.startBlockNumber) revert VotingPeriodHasNotStarted();
         if (proposal.finalizationStatus != FinalizationStatus.Pending) revert ProposalFinalized();
         if (voteRegistry[proposalId][voter]) revert UserAlreadyVoted();
 
         voteRegistry[proposalId][voter] = true;
 
         uint256 votingPower = _getCumulativePower(
             voter,
-            proposal.snapshotTimestamp,
+            proposal.startBlockNumber,
             userVotingStrategies,
             proposal.activeVotingStrategies
         );
@@ -315,7 +312,7 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
         Proposal storage proposal = proposals[proposalId];
         _assertProposalExists(proposal);
         if (author != proposal.author) revert InvalidCaller();
-        if (block.timestamp >= proposal.startTimestamp) revert VotingDelayHasPassed();
+        if (block.number >= proposal.startBlockNumber) revert VotingDelayHasPassed();
 
         proposal.executionPayloadHash = keccak256(executionStrategy.params);
         proposal.executionStrategy = IExecutionStrategy(executionStrategy.addr);
@@ -401,16 +398,14 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
 
     /// @dev Reverts if a specified proposal does not exist.
     function _assertProposalExists(Proposal memory proposal) internal pure {
-        // startTimestamp cannot be set to 0 when a proposal is created,
-        // so if proposal.startTimestamp is 0 it means this proposal does not exist
-        // and hence `proposalId` is invalid.
-        if (proposal.startTimestamp == 0) revert InvalidProposal();
+        // If a proposal exists, then its execution payload hash will be non-zero.
+        if (proposal.executionPayloadHash == 0) revert InvalidProposal();
     }
 
     /// @dev Returns the cumulative voting power of a user over a set of voting strategies.
     function _getCumulativePower(
         address userAddress,
-        uint32 timestamp,
+        uint32 blockNumber,
         IndexedStrategy[] calldata userStrategies,
         uint256 allowedStrategies
     ) internal returns (uint256) {
@@ -429,7 +424,7 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
             Strategy memory strategy = votingStrategies[strategyIndex];
 
             totalVotingPower += IVotingStrategy(strategy.addr).getVotingPower(
-                timestamp,
+                blockNumber,
                 userAddress,
                 strategy.params,
                 userStrategies[i].params

diff --git a/src/execution-strategies/EmergencyQuorumExecutionStrategy.sol b/src/execution-strategies/EmergencyQuorumExecutionStrategy.sol
@@ -56,12 +56,12 @@ abstract contract EmergencyQuorumExecutionStrategy is IExecutionStrategy, SpaceM
             return ProposalStatus.Cancelled;
         } else if (proposal.finalizationStatus == FinalizationStatus.Executed) {
             return ProposalStatus.Executed;
-        } else if (block.timestamp < proposal.startTimestamp) {
+        } else if (block.number < proposal.startBlockNumber) {
             return ProposalStatus.VotingDelay;
         } else if (emergencyQuorumReached) {
             if (_supported(votesFor, votesAgainst)) {
                 // Proposal is supported
-                if (block.timestamp < proposal.maxEndTimestamp) {
+                if (block.number < proposal.maxEndBlockNumber) {
                     // New votes can still come in so return `VotingPeriodAccepted`.
                     return ProposalStatus.VotingPeriodAccepted;
                 } else {
@@ -70,19 +70,19 @@ abstract contract EmergencyQuorumExecutionStrategy is IExecutionStrategy, SpaceM
                 }
             } else {
                 // Proposal is not supported
-                if (block.timestamp < proposal.maxEndTimestamp) {
+                if (block.number < proposal.maxEndBlockNumber) {
                     // New votes might still come in so return `VotingPeriod`.
                     return ProposalStatus.VotingPeriod;
                 } else {
                     // New votes can't come in, so it's definitely rejected.
                     return ProposalStatus.Rejected;
                 }
             }
-        } else if (block.timestamp < proposal.minEndTimestamp) {
-            // Proposal has not reached minEndTimestamp yet.
+        } else if (block.number < proposal.minEndBlockNumber) {
+            // Proposal has not reached minEndBlockNumber yet.
             return ProposalStatus.VotingPeriod;
-        } else if (block.timestamp < proposal.maxEndTimestamp) {
-            // Timestamp is between minEndTimestamp and maxEndTimestamp
+        } else if (block.number < proposal.maxEndBlockNumber) {
+            // block number is between minEndBlockNumber and maxEndBlockNumber
             if (accepted) {
                 return ProposalStatus.VotingPeriodAccepted;
             } else {

diff --git a/src/execution-strategies/OptimisticQuorumExecutionStrategy.sol b/src/execution-strategies/OptimisticQuorumExecutionStrategy.sol
@@ -48,19 +48,19 @@ abstract contract OptimisticQuorumExecutionStrategy is IExecutionStrategy, Space
             return ProposalStatus.Cancelled;
         } else if (proposal.finalizationStatus == FinalizationStatus.Executed) {
             return ProposalStatus.Executed;
-        } else if (block.timestamp < proposal.startTimestamp) {
+        } else if (block.number < proposal.startBlockNumber) {
             return ProposalStatus.VotingDelay;
         } else if (rejected) {
             // We're past the vote start. If it has been rejected, we can short-circuit and return Rejected.
             return ProposalStatus.Rejected;
-        } else if (block.timestamp < proposal.minEndTimestamp) {
-            // minEndTimestamp not reached, indicate we're still in the voting period.
+        } else if (block.number < proposal.minEndBlockNumber) {
+            // minEndBlockNumber not reached, indicate we're still in the voting period.
             return ProposalStatus.VotingPeriod;
-        } else if (block.timestamp < proposal.maxEndTimestamp) {
-            // minEndTimestamp < now < maxEndTimestamp ; if not `rejected`, we can indicate it can be `accepted`.
+        } else if (block.number < proposal.maxEndBlockNumber) {
+            // minEndBlockNumber < block.number < maxEndBlockNumber ; if not `rejected`, we can indicate it can be `accepted`.
             return ProposalStatus.VotingPeriodAccepted;
         } else {
-            // maxEndTimestamp < now ; proposal has not been `rejected` ; we can indicate it's `accepted`.
+            // maxEndBlockNumber < block.number ; proposal has not been `rejected` ; we can indicate it's `accepted`.
             return ProposalStatus.Accepted;
         }
     }

diff --git a/src/execution-strategies/SimpleQuorumExecutionStrategy.sol b/src/execution-strategies/SimpleQuorumExecutionStrategy.sol
@@ -50,11 +50,11 @@ abstract contract SimpleQuorumExecutionStrategy is IExecutionStrategy, SpaceMana
             return ProposalStatus.Cancelled;
         } else if (proposal.finalizationStatus == FinalizationStatus.Executed) {
             return ProposalStatus.Executed;
-        } else if (block.timestamp < proposal.startTimestamp) {
+        } else if (block.number < proposal.startBlockNumber) {
             return ProposalStatus.VotingDelay;
-        } else if (block.timestamp < proposal.minEndTimestamp) {
+        } else if (block.number < proposal.minEndBlockNumber) {
             return ProposalStatus.VotingPeriod;
-        } else if (block.timestamp < proposal.maxEndTimestamp) {
+        } else if (block.number < proposal.maxEndBlockNumber) {
             if (accepted) {
                 return ProposalStatus.VotingPeriodAccepted;
             } else {

diff --git a/src/interfaces/IVotingStrategy.sol b/src/interfaces/IVotingStrategy.sol
@@ -4,19 +4,17 @@ pragma solidity ^0.8.18;
 
 /// @title Voting Strategy Interface
 interface IVotingStrategy {
-    /// @notice Gets the voting power of an address at a given timestamp.
-    /// @param timestamp The snapshot timestamp to get the voting power at. If a particular voting strategy
-    ///                  requires a block number instead of a timestamp, the strategy should resolve the
-    ///                  timestamp to a block number.
+    /// @notice Gets the voting power of an address at a given block number.
+    /// @param blockNumber The snapshot block number to get the voting power at.
     /// @param voter The address to get the voting power of.
     /// @param params The global parameters that can configure the voting strategy for a particular Space.
     /// @param userParams The user parameters that can be used in the voting strategy computation.
-    /// @return votingPower The voting power of the address at the given timestamp. If there is no voting power,
+    /// @return votingPower The voting power of the address at the given block number. If there is no voting power,
     ///                     return 0.
     function getVotingPower(
-        uint32 timestamp,
+        uint32 blockNumber,
         address voter,
         bytes calldata params,
         bytes calldata userParams
-    ) external returns (uint256 votingPower);
+    ) external view returns (uint256 votingPower);
 }
diff --git a/src/interfaces/space/ISpaceState.sol b/src/interfaces/space/ISpaceState.sol
@@ -55,12 +55,11 @@ interface ISpaceState {
     /// @dev Returns all zeros if the proposal does not exist.
     /// @param proposalId The ID of the proposal.
     /// @return author The address of the proposal author.
-    /// @return snapshotTimestamp The timestamp of the proposal snapshot.
-    ///         All Voting Power will be calculated at this timestamp.
-    /// @return startTimestamp The timestamp of the start of the voting period.
+    /// @return startBlockNumber The block number of the start of the voting period.
+    ///         This is also the snapshot block number where voting power is calculated at.
     /// @return executionStrategy The address of the execution strategy used in the proposal.
-    /// @return minEndTimestamp The timestamp of the minimum end of the voting period.
-    /// @return maxEndTimestamp The timestamp of the maximum end of the voting period.
+    /// @return minEndBlockNumber The block number of the minimum end of the voting period.
+    /// @return maxEndBlockNumber The block number of the maximum end of the voting period.
     /// @return finalizationStatus The finalization status of the proposal. See `FinalizationStatus`.
     /// @return executionPayloadHash The keccak256 hash of the execution payload.
     /// @return activeVotingStrategies The bit array of the active voting strategies for the proposal.
@@ -71,11 +70,10 @@ interface ISpaceState {
         view
         returns (
             address author,
-            uint32 snapshotTimestamp,
-            uint32 startTimestamp,
+            uint32 startBlockNumber,
             IExecutionStrategy executionStrategy,
-            uint32 minEndTimestamp,
-            uint32 maxEndTimestamp,
+            uint32 minEndBlockNumber,
+            uint32 maxEndBlockNumber,
             FinalizationStatus finalizationStatus,
             bytes32 executionPayloadHash,
             uint256 activeVotingStrategies

diff --git a/...sal-validation-strategies/PropositionPowerAndActiveProposalsLimiterValidationStrategy.sol b/...sal-validation-strategies/PropositionPowerAndActiveProposalsLimiterValidationStrategy.sol
@@ -16,7 +16,7 @@ contract PropositionPowerAndActiveProposalsLimiterValidationStrategy is
     IProposalValidationStrategy
 {
     /// @notice Validates an author by checking if the proposition power of the author exceeds a threshold over a set of
-    ///         strategies and if the author has reached the maximum number of active proposals at the current timestamp.
+    ///         strategies and if the author has reached the maximum number of active proposals at the current block.
     /// @param author Author of the proposal.
     /// @param params ABI encoded array that should contain the following:
     ///                 cooldown: Duration to wait before the proposal counter gets reset.

diff --git a/src/types.sol b/src/types.sol
@@ -11,18 +11,17 @@ struct Proposal {
     // SLOT 1:
     // The address of the proposal creator.
     address author;
-    // The timestamp at which voting power for the proposal is calculated.
-    uint32 snapshotTimestamp;
-    // The timestamp at which the voting period starts.
-    uint32 startTimestamp;
+    // The block number at which the voting period starts.
+    // This is also the snapshot block number where voting power is calculated at.
+    uint32 startBlockNumber;
     //
     // SLOT 2:
     // The address of execution strategy used for the proposal.
     IExecutionStrategy executionStrategy;
-    // The minimum timestamp at which the proposal can be finalized.
-    uint32 minEndTimestamp;
-    // The maximum timestamp at which the proposal can be finalized.
-    uint32 maxEndTimestamp;
+    // The minimum block number at which the proposal can be finalized.
+    uint32 minEndBlockNumber;
+    // The maximum block number at which the proposal can be finalized.
+    uint32 maxEndBlockNumber;
     // An enum that stores whether a proposal is pending, executed, or cancelled.
     FinalizationStatus finalizationStatus;
     //

diff --git a/src/utils/PropositionPower.sol b/src/utils/PropositionPower.sol
@@ -23,14 +23,14 @@ abstract contract PropositionPower {
         Strategy[] memory allowedStrategies,
         IndexedStrategy[] memory userStrategies
     ) internal returns (bool) {
-        uint256 votingPower = _getCumulativePower(author, uint32(block.timestamp), userStrategies, allowedStrategies);
+        uint256 votingPower = _getCumulativePower(author, uint32(block.number), userStrategies, allowedStrategies);
         return (votingPower >= proposalThreshold);
     }
 
-    /// @dev Computes the cumulative proposition power of an address at a given timestamp over a set of strategies.
+    /// @dev Computes the cumulative proposition power of an address at a given block number over a set of strategies.
     function _getCumulativePower(
         address userAddress,
-        uint32 timestamp,
+        uint32 blockNumber,
         IndexedStrategy[] memory userStrategies,
         Strategy[] memory allowedStrategies
     ) internal returns (uint256) {
@@ -44,7 +44,7 @@ abstract contract PropositionPower {
             Strategy memory strategy = allowedStrategies[strategyIndex];
 
             totalVotingPower += IVotingStrategy(strategy.addr).getVotingPower(
-                timestamp,
+                blockNumber,
                 userAddress,
                 strategy.params,
                 userStrategies[i].params

diff --git a/src/utils/TimestampResolver.sol b/src/utils/TimestampResolver.sol