Governance factory #284
Merged
Governance factory #284
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
Co-authored-by: Bayological <[email protected]>
After a few iterations, we landed on a simple Airgrab scheme: - With a list of <address>,<token amount> we generate a merkle tree and record the root in the contract - A user does KYC, gets the merkle proof from our backend and submits the claim - The contract verifies the merkle proof and KYC and locks the tokens on behalf of the user for a predetermined cliff and slope. - The airgrab has an endTimestamp after witch any leftover tokens in the contract can be transferred to the treasury. More details on specifics around the KYC can be extracted from the comments in the code which contain links to documentation. There is a single open question around how we encode the expected credential from fractal. The expected credential is basically the set of restrictions we want to enforce and it's a string that looks like `"level:plus;residency_not:ca,us"`. I currently provided that statically in the code and it's probably not the final version that we want. But to parameterize in a naive way would incur a big gas because we can only store it in storage, and not as an immutable parameter because only immutable base types are allowed as of now in solidity. If we want to parameterize it the approach I would take would be to store a keccak of it as an immutable bytes32 parameter, and provide the full string as a parameter to the claim function and when verifying the credential we would first verify that the keccak matches and then use it in the construction of the full credential. I'm not sure if it's worth it or if it's fine to just come up with the value and keep it static in line as is right now. N/A Yes - Fixes #282 N/A N/A --------- Co-authored-by: baroooo <[email protected]>
nvtaveras
reviewed
Oct 12, 2023
contracts/governance/Factory.sol
Outdated
| // Airgrab configuration | ||
| uint32 public constant AIRGRAB_LOCK_SLOPE = 104; // Slope duration for the airgrabed tokens in weeks | ||
| uint32 public constant AIRGRAB_LOCK_CLIFF = 0; // Cliff duration for the airgrabed tokens | ||
| uint256 public constant AIRGRAB_DURATION = 365 days; // Duration for the airgrab |
There was a problem hiding this comment.
A lot of these comments are redundant given the variable names. Maybe only leave the ones that you think are not as clear or could provide additional context to the reader?
For example L56 seems like useful context:
uint256 public constant GOVERNOR_VOTING_DELAY = 1; // Voting start the next block
on another note, is the airgrab supposed to last one year?
There was a problem hiding this comment.
yes, after one year remaining funds will be drained
contracts/governance/Factory.sol
Outdated
| // Governor configuration | ||
| uint256 public constant GOVERNOR_VOTING_DELAY = 1; // Voting start the next block | ||
| uint256 public constant GOVERNOR_VOTING_PERIOD = 120_960; // Voting period for the governor (7 days in blocks CELO) | ||
| uint256 public constant GOVERNOR_PROPOSAL_THRESHOLD = 1_000e18; // Proposal threshold for the governor |
There was a problem hiding this comment.
How can I verify that this value is correct?
There was a problem hiding this comment.
it is tentatively decided and noted here:
https://www.notion.so/Mento-Token-and-Governance-ae6acced7b4d465db90f92f1408ee23c?pvs=4#ac6b141b682b4c1b9986c2d50dcca811
philbow61
approved these changes
Oct 12, 2023
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.
Description
A factory contract that deploys and configures all token and governance related contracts.
It will be initiated by Celo governance.
Treasury and vesting contracts won't be created in this PR. Those changes will be added later.
Other changes
Airgrab's init function is updated to resolve triple circular dependence between token, locking and airgrab
Governor config is parameterized to keep all config in the Factory
Added TODO comments as reminders
The branch is based on locking-contracts branch so locking related changes can be ignored in this review (contracts/governance/locking, test/Locking, test/mocks, test/utils)
Tested
Tested with unit tests
The issue #275 aims to add integration tests that will test the main flows
Related issues