Optionally require a deposit to make a proposal #11
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
ben2x4
force-pushed
the
proposal-deposit
branch
from
8be8131 to
375fa57
Compare
JakeHartnell
reviewed
Sep 5, 2021
| .add_messages(prop.msgs) | ||
| .add_attribute("action", "execute") | ||
| .add_attribute("sender", info.sender) | ||
| .add_attribute("proposal_id", proposal_id.to_string())) | ||
| } | ||
|
|
||
| fn get_proposal_deposit_refund_message( | ||
| proposer: &Addr, | ||
| config: &ProposalDeposit, |
There was a problem hiding this comment.
Making a comment for later, but feel this variable could be named a bit better... like why not deposit or proposal_deposit? Not a big deal though... good enough for now.
JakeHartnell
reviewed
Sep 5, 2021
| }, | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_proposal_deposit_works() { |
There was a problem hiding this comment.
Can be a separate ticket, but we should add a test to check the when the proposal deposit is 0, it all works without having to set an allowance. Will make a ticket.
There was a problem hiding this comment.
All the existing tests have the proposal deposit set to zero, so we should be good from a coverage perspective. Thought could make sense to have a explicit test to document the requirement.
JakeHartnell
approved these changes
Sep 5, 2021
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.
This PR closes #1.
The dao can now be configured to require a deposit to make a proposal. The deposit is returned if the proposal passes and becomes property of the dao if the proposal fails.
This change will require future work to prevent the dao from accidentally spending proposal deposit funds. This will most likely be implemented by having a separate contract for managing the deposit funds to make the distinction more clear.
This PR also persists the proposer's address to the proposal state object. This was necessary to return the proposal deposit and seems generally useful.