-
Notifications
You must be signed in to change notification settings - Fork 1
feat: added voting delay, configurable delay, voting, and submission periods, and checkpoints for periods #66
Conversation
uint16 voteQuorumPct; | ||
uint16 vetoQuorumPct; |
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.
We can bump this up right to fill the entire word. I know it makes no difference to the actual Pct, but if I remember right it's cheaper to access a full word (i.e 256) vs less than that.
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.
But we can leave it as is if we want to be consistent on using uint16
for Percentages everywhere.
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.
uint16 makes the most sense because the max value is 10000 anyways and uint8 is not enough.
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.
let's let the auditors golf this i don't think we should worry about that in this pr
Co-authored-by: Rajath Alex <[email protected]>
Co-authored-by: Rajath Alex <[email protected]>
@@ -205,6 +213,7 @@ abstract contract LlamaTokenCaster is Initializable { | |||
clockAdapter = _clockAdapter; | |||
role = _role; | |||
_setQuorumPct(_voteQuorumPct, _vetoQuorumPct); | |||
_setPeriodPcts(2500, 5000, 2500); // default to 25% delay, 50% casting, 25% submission periods |
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.
@AustinGreen @dd0sxx Something for us to consider as a follow up to this PR: We can pass this in at deploy time similar to Quorum Pct and have it all wrapped as a struct called CasterConfig
. This gives us way more flexibility at deployment time to put in another configuration instead of having to create an action to first deploy and then update.
I would bias towards doing that.
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.
@0xrajath yeah let's track this an issue and address it in a follow up PR
Coverage after merging theo/update-submission-period into main will be
Coverage Report
|
Motivation:
closes #3
Modifications:
nonrelated changes in this pr:
Result:
we now have a voting delay, and checkpointed / configurable period pcts.