1Hive / conviction-voting-app

Aragon app used to collectively allocate funds from a shared treasury 💧
93 stars 32 forks source link

Add pause function and role #133

Closed lkngtn closed 3 years ago

lkngtn commented 3 years ago

Since conviction voting is used to allocate funds, if there is an issue with the mechanism that is disclosed responsibly it would be useful to be able to pause proposal execution until the issue can be resolved. Currently two seed members have the permission to alter the configuration params for this purpose, as it enables them to set the parameters such that it becomes impossible to execute proposals but this has two major downsides:

  1. the permission gives them the ability to change conviction params, this is way more permission than is necessary to achieve the goal of simply being able to pause the system in the event a bug is reported.
  2. if a bug were to be discovered in the conviction logic, its possible that changing the parameters would be ineffective in preventing execution.

I propose we add a simple function that when called would pause proposal execution with a simple check. This would enable a group of people to be granted permission to pause and unpause execution. The permission would be managed via voting, so if the permission was abused it could be revoked through the longer voting process, but would allow taking immediate action to prevent a bug in conviction voting from being exploited if it was responsibly disclosed.

Additionally, as part of this improvement we would revoke the permission that has been granted to devs to alter the conviction parameters so that they must be modified via the voting process instead.

willjgriff commented 3 years ago

There's many ways in which we can interact with disputable conviction voting that we may or may not want to pause.

There's the standard new proposal, stake, withdraw, execute and cancel functions which will all be pausable.

Then there's withdrawals that happen when a user transfers a token, I will not pause these, because doing so will prevent transferring the staked tokens. However, there's a chance there could be an issue in this code, like there was before, so redeployment of the contract may be necessary if a pause is required but there is an issue in this code.

Finally there are challenged proposals. I propose preventing challenges from being made once the contract is paused. However, I'm unsure about preventing currently open challenges/disputes from being resolved. Not allowing them to be resolved locks up creator and or challenge collateral. Allowing them to be resolved means the proposals could change status, although this only sets a status for the proposal, no other processing is done (eg executing), so any damage that could do will be negligible if any.

fabriziovigevani commented 3 years ago

I will not pause these, because doing so will prevent transferring the staked tokens

As commented here, wondering if we should also not pause withdrawal from all inactive proposals.

However, I'm unsure about preventing currently open challenges/disputes from being finalised. Not allowing them to be finalised locks up creator and or challenge collateral. Allowing them to be finalised means the proposals could change status, although this only sets a status for the proposal, no other processing is done, so any damage that could do will be negligible if any.

I agree with your proposed PR allowing challenges to be finalised. I think disputes should be able to be ruled regardless of the disputable app state.