From 0a395ecdb4bb7a7d575aabbb8e0ffc8dddbb2a81 Mon Sep 17 00:00:00 2001 From: 0xFable <0xfable@protonmail.com> Date: Fri, 2 Sep 2022 11:27:30 +0100 Subject: [PATCH 1/4] feat(vote): Backport fix to allow voting as long as prop is not expired Also updated status on execution --- packages/voting-contract/src/lib.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/voting-contract/src/lib.rs b/packages/voting-contract/src/lib.rs index 29cb211d..a6ee2a75 100644 --- a/packages/voting-contract/src/lib.rs +++ b/packages/voting-contract/src/lib.rs @@ -118,10 +118,15 @@ where // ensure proposal exists and can be voted on let mut prop = proposals().load(deps.storage, proposal_id)?; - if prop.current_status(&env.block) != Status::Open { + if ![Status::Open, Status::Passed, Status::Rejected].contains(&prop.status) { return Err(ContractError::NotOpen {}); } + // if they are not expired + if prop.expires.is_expired(&env.block) { + return Err(ContractError::Expired {}); + } + // use a snapshot of "start of proposal" // Must be a member of voting group and have voting power >= 1 let cfg = CONFIG.load(deps.storage)?; @@ -157,10 +162,11 @@ where P: Serialize + DeserializeOwned, { let mut proposal = proposals::

().load(storage, proposal_id)?; - + // Update Status + proposal.update_status(&env.block); // We allow execution even after the proposal "expiration" as long as all votes come in before // that point. If it was approved on time, it can be executed any time. - if proposal.current_status(&env.block) != Status::Passed { + if proposal.status != Status::Passed { return Err(ContractError::WrongExecuteStatus {}); } From c35028ef1c47a0a0db31b853035d8e35fbc561b4 Mon Sep 17 00:00:00 2001 From: 0xFable <0xfable@protonmail.com> Date: Fri, 2 Sep 2022 15:48:01 +0100 Subject: [PATCH 2/4] feat(test): backport and rework test for multitest --- .../voting-contract/src/multitest/voting.rs | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/voting-contract/src/multitest/voting.rs b/packages/voting-contract/src/multitest/voting.rs index 2cf38b37..05f0f1a5 100644 --- a/packages/voting-contract/src/multitest/voting.rs +++ b/packages/voting-contract/src/multitest/voting.rs @@ -368,5 +368,60 @@ fn expired_proposals_cannot_be_voted_on() { // Bob can't vote on the expired proposal let err = suite.vote("bob", proposal_id, Vote::Yes).unwrap_err(); // proposal that is open and expired is rejected - assert_eq!(ContractError::NotOpen {}, err.downcast().unwrap()); + assert_eq!(ContractError::Expired {}, err.downcast().unwrap()); +} + +#[test] +fn proposal_pass_on_expiration() { + let rules = RulesBuilder::new() + .with_threshold(Decimal::percent(51)) + .with_quorum(Decimal::percent(35)) + .build(); + + let mut suite = SuiteBuilder::new() + .with_member("alice", 1) + .with_member("bob", 2) + .with_rules(rules.clone()) + .build(); + + // Create proposal with 1 voting power + let response = suite.propose("alice", "cool proposal", "proposal").unwrap(); + let proposal_id: u64 = get_proposal_id(&response).unwrap(); + + // Bob can vote on the proposal + let _ = suite.vote("bob", proposal_id, Vote::Yes).unwrap(); + + // Move time forward so voting ends + suite.app.advance_seconds(rules.voting_period_secs()); + + // Verify proposal is now passed + let prop = suite.query_proposal(proposal_id).unwrap(); + assert_eq!(prop.status, Status::Passed); + + // Alice can't vote on the proposal + let err = suite.vote("alice", proposal_id, Vote::Yes).unwrap_err(); + + // proposal that is open and expired is rejected + assert_eq!(ContractError::Expired {}, err.downcast().unwrap()); + + // Move time forward more so proposal expires + suite.app.advance_seconds(prop.expires.time().seconds()); + + // But she can execute the proposal + let _ = suite.execute_proposal("alice", proposal_id).unwrap(); + + // Verify proposal is now 'executed' + let prop = suite.query_proposal(proposal_id).unwrap(); + assert_eq!(prop.status, Status::Executed); + + // Closing should NOT be possible + let err = suite + .close( + "bob", + proposal_id + ) + .unwrap_err(); + assert_eq!(ContractError::WrongCloseStatus {}, err.downcast().unwrap()); + + } From f892a1876dd5159dbfe050b06fa4ac7c6167b35e Mon Sep 17 00:00:00 2001 From: 0xFable <0xfable@protonmail.com> Date: Mon, 5 Sep 2022 08:51:11 +0100 Subject: [PATCH 3/4] fix(fmt): Ran cargo fmt on file --- packages/voting-contract/src/multitest/voting.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/voting-contract/src/multitest/voting.rs b/packages/voting-contract/src/multitest/voting.rs index 05f0f1a5..ec152ad1 100644 --- a/packages/voting-contract/src/multitest/voting.rs +++ b/packages/voting-contract/src/multitest/voting.rs @@ -415,13 +415,6 @@ fn proposal_pass_on_expiration() { assert_eq!(prop.status, Status::Executed); // Closing should NOT be possible - let err = suite - .close( - "bob", - proposal_id - ) - .unwrap_err(); + let err = suite.close("bob", proposal_id).unwrap_err(); assert_eq!(ContractError::WrongCloseStatus {}, err.downcast().unwrap()); - - } From 3975c3c416df518997515ab36b014b6494fdf812 Mon Sep 17 00:00:00 2001 From: 0xFable <0xfable@protonmail.com> Date: Tue, 6 Sep 2022 12:40:59 +0100 Subject: [PATCH 4/4] feat(cr): Codereview comments --- packages/voting-contract/src/lib.rs | 2 +- packages/voting-contract/src/multitest/voting.rs | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/voting-contract/src/lib.rs b/packages/voting-contract/src/lib.rs index a6ee2a75..c81d6a1f 100644 --- a/packages/voting-contract/src/lib.rs +++ b/packages/voting-contract/src/lib.rs @@ -166,7 +166,7 @@ where proposal.update_status(&env.block); // We allow execution even after the proposal "expiration" as long as all votes come in before // that point. If it was approved on time, it can be executed any time. - if proposal.status != Status::Passed { + if proposal.current_status(&env.block) != Status::Passed { return Err(ContractError::WrongExecuteStatus {}); } diff --git a/packages/voting-contract/src/multitest/voting.rs b/packages/voting-contract/src/multitest/voting.rs index ec152ad1..e1998c57 100644 --- a/packages/voting-contract/src/multitest/voting.rs +++ b/packages/voting-contract/src/multitest/voting.rs @@ -389,7 +389,7 @@ fn proposal_pass_on_expiration() { let proposal_id: u64 = get_proposal_id(&response).unwrap(); // Bob can vote on the proposal - let _ = suite.vote("bob", proposal_id, Vote::Yes).unwrap(); + suite.vote("bob", proposal_id, Vote::Yes).unwrap(); // Move time forward so voting ends suite.app.advance_seconds(rules.voting_period_secs()); @@ -401,14 +401,11 @@ fn proposal_pass_on_expiration() { // Alice can't vote on the proposal let err = suite.vote("alice", proposal_id, Vote::Yes).unwrap_err(); - // proposal that is open and expired is rejected + // cannot vote on proposal as it has expired assert_eq!(ContractError::Expired {}, err.downcast().unwrap()); - // Move time forward more so proposal expires - suite.app.advance_seconds(prop.expires.time().seconds()); - // But she can execute the proposal - let _ = suite.execute_proposal("alice", proposal_id).unwrap(); + suite.execute_proposal("alice", proposal_id).unwrap(); // Verify proposal is now 'executed' let prop = suite.query_proposal(proposal_id).unwrap();