Skip to content

Added upgradable contraintLimits #798

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

Merged
merged 31 commits into from
Oct 16, 2020

Conversation

nicoelzer
Copy link
Contributor

@nicoelzer nicoelzer commented Oct 13, 2020

@orenyodfat
Copy link
Contributor

so the idea is to have other genericscheme with different gov params which will white list the schemeConstraint contract ?
other 2 alternative to do so are :

  1. add updateSchemeConstraint function to the genericSchemeMultiCall contract.
  2. use the already exist plugin manager scheme to update the genericSchemeMultiCall as a whole.

Please note that option 2 is already available and exist any how.. what is the rational/gain to add another way to do that ?

@nicoelzer
Copy link
Contributor Author

so the idea is to have other genericscheme with different gov params which will white list the schemeConstraint contract ?
other 2 alternative to do so are :

  1. add updateSchemeConstraint function to the genericSchemeMultiCall contract.
  2. use the already exist plugin manager scheme to update the genericSchemeMultiCall as a whole.

Please note that option 2 is already available and exist any how.. what is the rational/gain to add another way to do that ?

Hi @orenyodfat,

exactly the idea would be to use another generic scheme with super safe governance parameters. A proposal should not be able to pass faster then 14 days.

The reasons why a separate GenericScheme might be helpful for me are:

  1. Differenciated, safer governance params then MultiCall & Plugin Manager
  2. Being able to update the limits while the periodSpending are not lost. If we update the MultiCall as a whole we would also reset the spendingLimits of any token/eth.

@orenyodfat
Copy link
Contributor

so the idea is to have other genericscheme with different gov params which will white list the schemeConstraint contract ?
other 2 alternative to do so are :

  1. add updateSchemeConstraint function to the genericSchemeMultiCall contract.
  2. use the already exist plugin manager scheme to update the genericSchemeMultiCall as a whole.

Please note that option 2 is already available and exist any how.. what is the rational/gain to add another way to do that ?

Hi @orenyodfat,

exactly the idea would be to use another generic scheme with super safe governance parameters. A proposal should not be able to pass faster then 14 days.

The reasons why a separate GenericScheme might be helpful for me are:

  1. Differenciated, safer governance params then MultiCall & Plugin Manager
  2. Being able to update the limits while the periodSpending are not lost. If we update the MultiCall as a whole we would also reset the spendingLimits of any token/eth.

this add another complexity to the system . another things to look on and monitor .

Copy link
Contributor

@orenyodfat orenyodfat left a comment

Choose a reason for hiding this comment

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

please emit an event on all updates functions

@orenyodfat orenyodfat changed the base branch from schemeconstraint to master October 15, 2020 12:39
@orenyodfat orenyodfat changed the base branch from master to schemeconstraint October 15, 2020 12:40
Copy link
Contributor

@orenyodfat orenyodfat left a comment

Choose a reason for hiding this comment

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

please base this pr vs master (or open a new one)

@orenyodfat orenyodfat changed the base branch from schemeconstraint to master October 15, 2020 13:20
@nicoelzer nicoelzer requested a review from orenyodfat October 15, 2020 15:07
@orenyodfat orenyodfat merged commit 7d6ce3a into daostack:master Oct 16, 2020
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