Skip to content
This repository has been archived by the owner on Nov 27, 2024. It is now read-only.

feat: added voting delay, configurable delay, voting, and submission periods, and checkpoints for periods #66

Merged
merged 33 commits into from
Dec 15, 2023

Conversation

dd0sxx
Copy link
Contributor

@dd0sxx dd0sxx commented Dec 15, 2023

Motivation:

closes #3

Modifications:

  • Added checkpoints for period pcts
  • added voting delay
  • added default split of 25% delay, 50% voting period, 25% submission period
  • added setters and getters that use checkpoints
  • added new tests to make sure voting delay works
  • refactored old tests to work with voting delay

nonrelated changes in this pr:

  • reorganized some events alphabetically
  • deleted commented out constructor tests

Result:

we now have a voting delay, and checkpointed / configurable period pcts.

Comment on lines 27 to 28
uint16 voteQuorumPct;
uint16 vetoQuorumPct;
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

@@ -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
Copy link
Contributor

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.

Copy link
Contributor

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

Copy link

Coverage after merging theo/update-submission-period into main will be

88.38%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src/token-voting
   LlamaERC20TokenActionCreator.sol87.50%50%100%100%43–44
   LlamaERC20TokenCaster.sol92.31%50%100%100%43
   LlamaERC721TokenActionCreator.sol84.21%50%100%100%43, 45–46
   LlamaERC721TokenCaster.sol87.50%50%100%100%43, 45
   LlamaTokenActionCreator.sol84.62%68.18%85.71%91.67%125–126, 271, 271–274, 274, 274, 280–281
   LlamaTokenCaster.sol81.61%79.27%78.13%83.67%209–210, 403, 408, 408, 408–410, 410, 410–414, 416, 434, 443, 443, 443–445, 445, 445–449, 451, 535–536, 544, 544–547, 547, 547, 553–554, 559–560
   LlamaTokenVotingFactory.sol100%100%100%100%

@AustinGreen AustinGreen merged commit 2637a6f into main Dec 15, 2023
5 checks passed
@AustinGreen AustinGreen deleted the theo/update-submission-period branch December 15, 2023 05:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2/3 voting period 1/3 confirmation period should be able to be updated
3 participants