Replies: 4 comments 1 reply
-
Should think about if we want to move If the pre-propose module is removed if it fails to handle a hook. This opens an attack vector where someone may attempt to get the pre-propose module into a bad state, causing it to be removed and the active check to be removed as well. If we choose to leave active checks inside of individual modules it doesn't open that path. It may be nice to keep security critical logic out of these deposit modules. |
Beta Was this translation helpful? Give feedback.
-
Also investigate what the intermediate state should be while we're replacing or setting the pre-propose module. If we set it to |
Beta Was this translation helpful? Give feedback.
-
For deposit modules, I think these are going to end up sharing a lot of code so I think we should take the This contract will have the same To start, our goal is that we can use this base contract to implement:
Across all of these, the shared logic is:
My plan is to implement this functionality entirely in the base contract. To support (3) above, that contract will need to override the |
Beta Was this translation helpful? Give feedback.
-
Here is a diagram of the design as of now: Some thoughts:
|
Beta Was this translation helpful? Give feedback.
-
Despite being easy to explain in natural language, an actual implementation of proposal deposits can be quite complex. To name a few things:
Additionally, extending our current system to support more sophisticated pre-proposal requirements is non-trivial because logic needs to be duplicated across modules.
To address these issues, I propose that we move all pre-proposal checks into a new module interface we call
cw-dao-pre-propose
. We can then modify our proposal modules so that an enum describes who may submit a proposal:In the
Anyone {}
case, any address may create a proposal to the DAO. In theModule {}
case, only the specifiedaddress
may do so. Proposal modules will set this globally in theirConfig
struct and return this information in response toGetConfig {}
queries.The
ExecuteMsg::Propose
variant should then be extended with aproposer
field which may only be set if a module is configured to create proposals:Setting a module as the proposer for a module will add that address to the module's proposal hook receivers. If the module ever errors handling a proposal status change hook, the
WhoMayPropose
enum will automatically fall back to theAnyone {}
state to prevent a bad pre-propose module from locking a DAO.With these changes we will remove all proposal deposit logic from proposal modules. Because the pre-propose module receives proposal status change updates, and controls who may create a proposal it can issue refunds when proposals complete, and impose arbitrary constraints on proposal creation.
Building draft proposals with this design.
Draft proposals come pretty easily with this model. We write a pre-propose module that allows new proposals to be created inside of it and allows editing of those proposals. Once the proposal is deemed ready, it is proposed in the parent proposal module.
Accepting native tokens for proposal deposits with this design.
This is just a pre-propose module that checks the
funds
field of a message before passing it along to the main DAO.Accepting cw20 tokens for proposal deposits with this design.
This is functionally the same as our current logic. Proposers will give an allowance to the pre-propose module, and the pre-propose module will transfer that allowance to itself on proposal creation.
Security concerns.
This design fails open meaning that a buggy or compromised deposit module will not lock the DAO. Failing open here is desirable as proposal deposits function primarily as an incentive to create good proposals and not as a critical security feature of DAOs.
Frontend integration.
This will require changes in the DAO DAO frontend on the query side, but for an initial implementation that has feature-parity with our current deposit logic this will not require any cosmetic changes or new designs.
Implementation
Proposal modules need to be instantiatable with a pre-propose module. This will require moving the
ModuleInstantiateInfo
struct to a shared package.Pre propose module needs to be agnostic to the actual structure of the propose execute message. Modules should store that in a
Binary
object submitted by the proposer. This can still support editing for drafts later because theBinary
that gets submitted can be updated.Beta Was this translation helpful? Give feedback.
All reactions