refactor: use block numbers to track time #210
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 10 commits
June 15, 2023 18:30
Codecov Report
@@ Coverage Diff @@
## main #210 +/- ##
==========================================
- Coverage 81.66% 80.82% -0.84%
==========================================
Files 29 28 -1
Lines 469 459 -10
Branches 114 111 -3
==========================================
- Hits 383 371 -12
Misses 73 73
- Partials 13 15 +2
|
pscott
previously approved these changes
Jun 19, 2023
Comment on lines
+399
to
+400
| // If a proposal exists, then its execution payload hash will be non-zero. | ||
| if (proposal.executionPayloadHash == 0) revert InvalidProposal(); |
There was a problem hiding this comment.
Is it really impossible to create a proposal with an empty execution payload? Imagine we want to "ratify" something on-chian, then the execution payload could be empty right?
I think proposal.startBlockNumber == 0 would be a better check here
There was a problem hiding this comment.
but then the payload hash would be keccak(0) so non zero. Wheras it would technically be possible to have a start block number of zero (ie the proposal was created in the genesis block and there is no voting delay)
src/interfaces/IVotingStrategy.sol
Outdated
Comment on lines
8
to
10
| /// @param blockNumber The snapshot block number to get the voting power at. If a particular voting strategy | ||
| /// requires a block number instead of a block number, the strategy should resolve the | ||
| /// block number to a block number. |
There was a problem hiding this comment.
can remove the whole "resolve" part
|
Question: how does this PR help #193 ? |
there is now no state changes in the voting strategies, so they are view functions and can be queried off chain fairly simply |
didn't notice the |
chore: natspec typo
pscott
approved these changes
Jun 19, 2023
pscott
added a commit
that referenced
this pull request
Jun 19, 2023
* 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]>
Orland0x
added a commit
that referenced
this pull request
Jun 19, 2023
* audit: use uint256 instead of bool in mappings * opti: use uint256 instead of bools in other mappings * chore: define TRUE, FALSE in types * refactor: use TRUE, FALSE consts from types.sol * chore: comment explain pupose of boolean aliases * audit: prevent reentrancy issues in execute function (#207) * audit: prevent reentrancy issues in execute function * comment: add comment for cached proposal * comment: remove comment for finalization status modification * chore: CEI explanation * chore: typo --------- Co-authored-by: Orlando <[email protected]> * 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]> --------- Co-authored-by: Orlando <[email protected]> Co-authored-by: Orland0x <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #193, #174, #168
Migrates the space contract and execution strategy proposal status functions. to use block numbers instead of timestamps. Note that this does not include the ActiveProposalLimiter, and Timelock contracts which still use timestamps.