Skip to content
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

Add default proposal parameters and custom proposal configuration #127

Merged
merged 5 commits into from
Aug 26, 2024

Conversation

tensojka
Copy link
Contributor

This changes quorum and proposal voting time to come from a storage variable and thus be updatable. Also adds support for custom proposals that have a shorter voting time.

@tensojka tensojka requested a review from scobi7 August 22, 2024 16:53
@tensojka
Copy link
Contributor Author

Still need to fix tests, but otherwise ready for review

Copy link
Collaborator

@scobi7 scobi7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic looks good. Im unfamiliar with the small quorum values that are hard coded so just ensuring those are good and can be left like that should be okay. (like in get quorum or the asserts in set_default_proposal_params)

Also maybe emitting an event for when parameters of proposals are changed may be a good idea.

@@ -554,5 +579,15 @@ mod proposals {
self.custom_proposal_type.write(idx, config);
idx
}

fn set_default_proposal_params(ref self: ComponentState<TContractState>, quorum: u32, proposal_voting_seconds: u32) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for proposal parameter configurations. Only potential suggestion is maybe emitting an event for changes in proposal parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong opinion on that, but don't see a reason to consume that event, so not adding

Was broken because I deployed the treasury contract on Sepolia.
@tensojka tensojka merged commit e79e3b0 into master Aug 26, 2024
1 check passed
tensojka added a commit that referenced this pull request Aug 26, 2024
* Add proposal that updates default prop params (see #127)

* Add display of generic proposal

* Fix dependencies, add eslint CI

* Update CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants