Timestamp to block number #183
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
* Bump @types/node from 17.0.38 to 17.0.40 (#169) Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 17.0.38 to 17.0.40. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump ts-node from 10.8.0 to 10.8.1 (#167) Bumps [ts-node](https://github.com/TypeStrong/ts-node) from 10.8.0 to 10.8.1. - [Release notes](https://github.com/TypeStrong/ts-node/releases) - [Commits](TypeStrong/ts-node@v10.8.0...v10.8.1) --- updated-dependencies: - dependency-name: ts-node dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump ethereumjs-util from 7.1.4 to 7.1.5 (#168) Bumps [ethereumjs-util](https://github.com/ethereumjs/ethereumjs-monorepo) from 7.1.4 to 7.1.5. - [Release notes](https://github.com/ethereumjs/ethereumjs-monorepo/releases) - [Commits](https://github.com/ethereumjs/ethereumjs-monorepo/compare/ethereumjs-util@7.1.4...ethereumjs-util@7.1.5) --- updated-dependencies: - dependency-name: ethereumjs-util dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Orland0x
commented
Jun 15, 2022
| # In the current implementation, we cant store arrays with a zero as the last element because the read function would just treat | ||
| # the array is 1 element shorter. Hence, we offset the slot_index by 1 so that it is never zero. | ||
| # Probably worth changing our array handling so this is not necessary. | ||
| let slot_index = params[1] - 1 |
There was a problem hiding this comment.
this isnt ideal. we could change how we store arrays.
There was a problem hiding this comment.
Yeah didn't comment on that since I saw this comment. Def agree 🤝
There was a problem hiding this comment.
opened an issue: #188
Orland0x
commented
Jun 15, 2022
| let (fact_registry_addr) = fact_registry_store.read() | ||
|
|
||
| let (eth_block_number) = get_eth_block_number(timestamp) | ||
| let eth_block_number = eth_block_number - 1 # temp shift - waiting for Fossil fix |
There was a problem hiding this comment.
pscott
approved these changes
Jun 15, 2022
| # This branch will be taken whenever a vote is cast as the mapping value would be set at proposal creation. | ||
| return (number) | ||
| else: | ||
| # The timestamp has not yet been queried in fossil. Therefore we must query Fossil for the latest eth block |
There was a problem hiding this comment.
This means a proposer can't create a proposal with a snapshot that was BEFORE the creation of the proposal right? Like if I create a proposal on second of Jan and I want the snapshot to be taken on the 1st of Jan (one day before), that's not possible, correct?
There was a problem hiding this comment.
Well technically you could if the latest block number stored in Fossil was on the 1st. This shouldnt happen though as I expect block hashes to be sent hourly.
The ethereum block number snapshot will always be sometime before the proposal creation, the length of which depends on the regularity of block hashes being sent from L1
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.
With this update, the snapshot of a proposal is made using a timestamp rather than a block number. Specifically, the snapshot is given by the block timestamp at proposal creation obtained via
get_block_timestamp(). This allows SX to be natively multi-chain as timestamps are universal whereas block numbers are chain dependent.Certain voting strategies do however require a block number, such as the single slot proof strategy. This resolution between timestamp and block number should occur at the voting strategy level and is up to the strategy creator to ensure the validity of the conversion.
For the single slot proof strategy, we provide this functionality within
lib/timestamp.cairo. It works as follows:timestamp_to_eth_block_number.timestamp_to_eth_block_numbermapping is then queried with this timestamp which will return the same ethereum block number as before. We therefore ensure that the snapshot block number of the proposal remains constant even if later block hashes are sent to fossil during the period of the proposal.We also add more extensive testing of the single slot proof strategy in this PR.