-
Notifications
You must be signed in to change notification settings - Fork 15
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
Moved action specific predicates to actionWellFormed and fix a conformance failure #673
Conversation
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.
Actually, now that I look at it I think it's better to move it into a separate helper function. The idea is that actionWellFormed
follows this pattern: #31 And that's destroyed by adding these extra arguments.
a716f3a
to
7c35621
Compare
35e0909
to
10973d3
Compare
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.
Looks pretty good, but actionValid
and actionWellFormed
are now in hidden code blocks, i.e. this removes actual logic from the PDF. I think just making the two functions but not the decision procedures visible is good enough right now, but it probably needs to be improved later.
Also, I'm not so sure about the usage of ∙
in the new functions. It makes sense, but it's not consistent with how we write non-STS predicates elsewhere. Maybe remove them for now and we can open a discussion about that in a new issue?
5e773c0
to
ce34026
Compare
ce34026
to
90229ae
Compare
Sorry @Soupstraw I didn't see this earlier. I'll review it today! |
I've moved |
+ added some explanatory documentation + split figure involving types and functions used the GOV sts so they fit on the page + added `\clearpage` commands to make prose line up better with the figures they're explaining
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.
I just pushed my suggested changes directly to your branch since they merely reorganize and document the code a bit. Otherwise, looks really nice. Good work!
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.
LGTM
Description
This PR moves the action-specific predicates in
GOV-Propose
toactionWellFormed
and adds a check that makes sure all the receiving staking credentials of aTreasuryWithdrawal
are registered when the proposal is made.This probably requires some changes to the prose. It seemed like the prose about
actionWellFormed
was already outdated before these changes, because it only mentioned we check the parameter updates, but we also had some conditions about treasury withdrawals as well.I also added two other predicate checks to the GOV rule which are present in the implementation:
GOV-Propose
: the return address of a new proposal must be a registered staking credentialGOV-Vote
: the proposal being voted for must not be expiredChecklist
CHANGELOG.md