-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix spec and add spec for supra governance #63
base: dev
Are you sure you want to change the base?
Conversation
aborts_if !table::spec_contains(voting_forum.proposals, proposal_id); | ||
aborts_if is_voting_period_over(proposal); | ||
aborts_if proposal.is_resolved; | ||
aborts_if !exists<timestamp::CurrentTimeMicroseconds>(@supra_framework); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@axiongsupra not sure why this aborts_if
is required and what purpose does it serve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for checking the whether time already started. This is required if we have call to timestamp::now_seconds()
// }; | ||
let timestamp_secs_bytes = std::bcs::serialize(timestamp::spec_now_seconds()); | ||
let key = std::string::spec_utf8(RESOLVABLE_TIME_METADATA_KEY); | ||
ensures simple_map::spec_get(post_proposal.metadata, key) == timestamp_secs_bytes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This property only ensures that time is recorded. Other properties we should aim at adding are
- vote by the voter is recorded (if not already voted)
- vote by the voter is recorded (if vote is flipped)
- total yes/no votes should tally (increase/decrease appropriately)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in e65045b
aptos-move/framework/supra-framework/sources/multisig_voting.spec.move
Outdated
Show resolved
Hide resolved
aborts_if has_multi_step_key && !from_bcs::deserializable<bool>(simple_map::spec_get(proposal.metadata, multi_step_key)); | ||
aborts_if has_multi_step_key && from_bcs::deserialize<bool>(simple_map::spec_get(proposal.metadata, multi_step_key)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand the role of these. @axiongsupra
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks strange to me. I will take a look and submit a patch.
|
||
let post post_voting_forum = global<VotingForum<ProposalType>>(voting_forum_address); | ||
let post post_proposal = table::spec_get(post_voting_forum.proposals, proposal_id); | ||
aborts_if !exists<timestamp::CurrentTimeMicroseconds>(@supra_framework); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the role of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for checking the whether time already started. This is required if we have call to timestamp::now_seconds()
from_bcs::deserialize(simple_map::spec_get(proposal.metadata, multi_step_key)); | ||
aborts_if !is_multi_step && len(next_execution_hash) != 0; | ||
|
||
aborts_if len(next_execution_hash) == 0 && !exists<timestamp::CurrentTimeMicroseconds>(@supra_framework); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, not sure at this point why !exist<timestamp::CurrentTimeMicroseconds>(@supra_framework)
is needed. I do not see this in the source code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for checking the whether time already started. This is required if we have call to timestamp::now_seconds()
No description provided.