Repository: DA0-DA0/dao-contracts
Pull request #166 (2022-02-04 01:22:11)

Vt only signers execute
Only allow signers / members to execute proposals in cw3-dao and cw3-multisig. Addresses issue #147 


Patch:
diff --git a/contracts/cw3-dao/src/contract.rs b/contracts/cw3-dao/src/contract.rs
index 1c6044cd..7f420ce4 100644
--- a/contracts/cw3-dao/src/contract.rs
+++ b/contracts/cw3-dao/src/contract.rs
@@ -59,6 +59,7 @@ pub fn instantiate(
         proposal_deposit: msg.proposal_deposit_amount,
         refund_failed_proposals: msg.refund_failed_proposals,
         image_url: msg.image_url,
+        only_members_execute: msg.only_members_execute,
     };
     CONFIG.save(deps.storage, &cfg)?;
 
@@ -325,6 +326,14 @@ pub fn execute_execute(
         }
     }
 
+    let cfg = CONFIG.load(deps.storage)?;
+    if cfg.only_members_execute {
+        let balance = get_staked_balance(deps.as_ref(), info.sender.clone())?;
+        if balance == Uint128::zero() {
+            return Err(ContractError::Unauthorized {});
+        }
+    }
+
     let gov_token = GOV_TOKEN.load(deps.storage)?;
 
     // Anyone can trigger this if the vote passed
diff --git a/contracts/cw3-dao/src/msg.rs b/contracts/cw3-dao/src/msg.rs
index 3a98e7be..05533c27 100644
--- a/contracts/cw3-dao/src/msg.rs
+++ b/contracts/cw3-dao/src/msg.rs
@@ -27,6 +27,7 @@ pub struct InstantiateMsg {
     pub refund_failed_proposals: Option<bool>,
     /// Optional Image URL that is used by the contract
     pub image_url: Option<String>,
+    pub only_members_execute: bool,
 }
 
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
diff --git a/contracts/cw3-dao/src/state.rs b/contracts/cw3-dao/src/state.rs
index 4df171f1..3985ba4b 100644
--- a/contracts/cw3-dao/src/state.rs
+++ b/contracts/cw3-dao/src/state.rs
@@ -22,6 +22,7 @@ pub struct Config {
     pub proposal_deposit: Uint128,
     pub refund_failed_proposals: Option<bool>,
     pub image_url: Option<String>,
+    pub only_members_execute: bool,
 }
 
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
diff --git a/contracts/cw3-multisig/schema/config.json b/contracts/cw3-multisig/schema/config.json
index 415b4e33..2558423e 100644
--- a/contracts/cw3-multisig/schema/config.json
+++ b/contracts/cw3-multisig/schema/config.json
@@ -6,6 +6,7 @@
     "description",
     "max_voting_period",
     "name",
+    "only_members_execute",
     "threshold"
   ],
   "properties": {
@@ -32,6 +33,9 @@
       "description": "The name of the multisig.",
       "type": "string"
     },
+    "only_members_execute": {
+      "type": "boolean"
+    },
     "threshold": {
       "description": "The threshold for a proposal to pass.",
       "allOf": [
diff --git a/contracts/cw3-multisig/schema/config_response.json b/contracts/cw3-multisig/schema/config_response.json
index ec82b197..fca2ae9f 100644
--- a/contracts/cw3-multisig/schema/config_response.json
+++ b/contracts/cw3-multisig/schema/config_response.json
@@ -25,6 +25,7 @@
         "description",
         "max_voting_period",
         "name",
+        "only_members_execute",
         "threshold"
       ],
       "properties": {
@@ -51,6 +52,9 @@
           "description": "The name of the multisig.",
           "type": "string"
         },
+        "only_members_execute": {
+          "type": "boolean"
+        },
         "threshold": {
           "description": "The threshold for a proposal to pass.",
           "allOf": [
diff --git a/contracts/cw3-multisig/schema/execute_msg.json b/contracts/cw3-multisig/schema/execute_msg.json
index e11cf8c8..8c70bde5 100644
--- a/contracts/cw3-multisig/schema/execute_msg.json
+++ b/contracts/cw3-multisig/schema/execute_msg.json
@@ -258,6 +258,7 @@
         "description",
         "max_voting_period",
         "name",
+        "only_members_execute",
         "threshold"
       ],
       "properties": {
@@ -284,6 +285,9 @@
           "description": "The name of the multisig.",
           "type": "string"
         },
+        "only_members_execute": {
+          "type": "boolean"
+        },
         "threshold": {
           "description": "The threshold for a proposal to pass.",
           "allOf": [
diff --git a/contracts/cw3-multisig/src/contract.rs b/contracts/cw3-multisig/src/contract.rs
index 4493ad25..68688dee 100644
--- a/contracts/cw3-multisig/src/contract.rs
+++ b/contracts/cw3-multisig/src/contract.rs
@@ -57,6 +57,7 @@ pub fn instantiate(
         threshold: msg.threshold.clone(),
         max_voting_period: msg.max_voting_period,
         image_url: msg.image_url,
+        only_members_execute: msg.only_members_execute,
     };
     CONFIG.save(deps.storage, &cfg)?;
 
@@ -260,7 +261,15 @@ pub fn execute_execute(
     info: MessageInfo,
     proposal_id: u64,
 ) -> Result<Response, ContractError> {
-    // anyone can trigger this if the vote passed
+    // only members can trigger this when the vote has passed
+    let group_addr = GROUP_ADDRESS.load(deps.storage)?;
+
+    let cfg = CONFIG.load(deps.storage)?;
+    if cfg.only_members_execute {
+        group_addr
+            .is_member(&deps.querier, &info.sender, None)?
+            .ok_or(ContractError::Unauthorized {})?;
+    }
 
     let mut prop = PROPOSALS.load(deps.storage, proposal_id)?;
     // we allow execution even after the proposal "expiration" as long as all vote come in before
diff --git a/contracts/cw3-multisig/src/msg.rs b/contracts/cw3-multisig/src/msg.rs
index 070096c5..c742dc11 100644
--- a/contracts/cw3-multisig/src/msg.rs
+++ b/contracts/cw3-multisig/src/msg.rs
@@ -18,6 +18,7 @@ pub struct InstantiateMsg {
     pub threshold: Threshold,
     pub max_voting_period: Duration,
     pub image_url: Option<String>,
+    pub only_members_execute: bool,
 }
 
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
diff --git a/contracts/cw3-multisig/src/state.rs b/contracts/cw3-multisig/src/state.rs
index 2075cd67..d329e5d6 100644
--- a/contracts/cw3-multisig/src/state.rs
+++ b/contracts/cw3-multisig/src/state.rs
@@ -27,6 +27,7 @@ pub struct Config {
     pub max_voting_period: Duration,
     /// Optional Image URL that is used by the contract
     pub image_url: Option<String>,
+    pub only_members_execute: bool,
 }
 
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]


Test patch:
diff --git a/contracts/cw3-dao/src/tests.rs b/contracts/cw3-dao/src/tests.rs
index 5b43747f..3d743cbb 100644
--- a/contracts/cw3-dao/src/tests.rs
+++ b/contracts/cw3-dao/src/tests.rs
@@ -113,6 +113,7 @@ fn instantiate_dao(
         proposal_deposit_amount: proposal_deposit_amount.unwrap_or_else(Uint128::zero),
         refund_failed_proposals,
         image_url: None,
+        only_members_execute: true,
     };
     app.instantiate_contract(
         dao_code_id,
@@ -273,6 +274,7 @@ fn test_instantiate_works() {
         proposal_deposit_amount: Uint128::zero(),
         refund_failed_proposals: None,
         image_url: None,
+        only_members_execute: true,
     };
     let err = app
         .instantiate_contract(
@@ -333,6 +335,7 @@ fn instantiate_new_gov_token() {
         proposal_deposit_amount: Uint128::zero(),
         refund_failed_proposals: None,
         image_url: None,
+        only_members_execute: true,
     };
     let res = app.instantiate_contract(
         dao_code_id,
@@ -392,6 +395,7 @@ fn instantiate_new_gov_token() {
         proposal_deposit_amount: Uint128::zero(),
         refund_failed_proposals: None,
         image_url: None,
+        only_members_execute: true,
     };
     let res = app.instantiate_contract(
         dao_code_id,
@@ -477,13 +481,13 @@ fn instantiate_new_gov_token() {
     let execution = ExecuteMsg::Execute { proposal_id };
     // Execute works. Anybody can execute Passed proposals
     let res = app
-        .execute_contract(Addr::unchecked(SOMEBODY), dao_addr.clone(), &execution, &[])
+        .execute_contract(Addr::unchecked(VOTER3), dao_addr.clone(), &execution, &[])
         .unwrap();
     assert_eq!(
         res.custom_attrs(1),
         [
             ("action", "execute"),
-            ("sender", SOMEBODY),
+            ("sender", VOTER3),
             ("proposal_id", proposal_id.to_string().as_str()),
         ],
     );
@@ -531,6 +535,7 @@ fn instantiate_new_gov_token() {
         proposal_deposit_amount: Uint128::zero(),
         refund_failed_proposals: None,
         image_url: None,
+        only_members_execute: true,
     };
     let res = app.instantiate_contract(
         dao_code_id,
@@ -607,6 +612,7 @@ fn test_propose_works() {
             &[],
         )
         .unwrap_err();
+
     assert_eq!(ContractError::WrongExpiration {}, err.downcast().unwrap());
 
     // Proposal from voter works
@@ -1179,15 +1185,21 @@ fn test_execute_works() {
         .unwrap_err();
     assert_eq!(ContractError::WrongCloseStatus {}, err.downcast().unwrap());
 
-    // Execute works. Anybody can execute Passed proposals
-    let res = app
+    // Accounts without tokens can't execute
+    let err = app
         .execute_contract(Addr::unchecked(SOMEBODY), dao_addr.clone(), &execution, &[])
+        .unwrap_err();
+    assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap());
+
+    // Execute works. Token holder can execute Passed proposals
+    let res = app
+        .execute_contract(Addr::unchecked(VOTER3), dao_addr.clone(), &execution, &[])
         .unwrap();
     assert_eq!(
         res.custom_attrs(1),
         [
             ("action", "execute"),
-            ("sender", SOMEBODY),
+            ("sender", VOTER3),
             ("proposal_id", proposal_id.to_string().as_str()),
         ],
     );
@@ -1212,7 +1224,7 @@ fn test_execute_works() {
 
     // Trying to execute something that was already executed fails
     let err = app
-        .execute_contract(Addr::unchecked(SOMEBODY), dao_addr, &execution, &[])
+        .execute_contract(Addr::unchecked(OWNER), dao_addr, &execution, &[])
         .unwrap_err();
     assert_eq!(
         ContractError::WrongExecuteStatus {},
@@ -1220,6 +1232,81 @@ fn test_execute_works() {
     )
 }
 
+#[test]
+fn test_execute_works_for_anyone() {
+    let mut app = mock_app();
+
+    let voting_period = Duration::Time(2000000);
+    let threshold = Threshold::ThresholdQuorum {
+        threshold: Decimal::percent(10),
+        quorum: Decimal::percent(10),
+    };
+    let (dao_addr, _cw20_addr, _) = setup_test_case(
+        &mut app,
+        threshold.clone(),
+        voting_period,
+        coins(10, NATIVE_TOKEN_DENOM),
+        None,
+        None,
+    );
+
+    // ensure we have cash to cover the proposal
+    let contract_bal = app
+        .wrap()
+        .query_balance(&dao_addr, NATIVE_TOKEN_DENOM)
+        .unwrap();
+    assert_eq!(contract_bal, coin(10, NATIVE_TOKEN_DENOM));
+
+    // Update config to allow anyone to execute
+    let update_config_msg = ExecuteMsg::UpdateConfig(Config {
+        name: "dao-dao".to_string(),
+        description: "a great DAO!".to_string(),
+        threshold,
+        max_voting_period: voting_period,
+        proposal_deposit: Uint128::zero(),
+        refund_failed_proposals: Some(false),
+        image_url: None,
+        only_members_execute: false,
+    });
+
+    let res = app.execute_contract(dao_addr.clone(), dao_addr.clone(), &update_config_msg, &[]);
+    assert!(res.is_ok());
+
+    // create proposal with 0 vote power
+    let proposal = pay_somebody_proposal();
+    let res = app
+        .execute_contract(Addr::unchecked(OWNER), dao_addr.clone(), &proposal, &[])
+        .unwrap();
+
+    // Get the proposal id from the logs
+    let proposal_id: u64 = res.custom_attrs(1)[2].value.parse().unwrap();
+
+    // Only Passed can be executed
+    let execution = ExecuteMsg::Execute { proposal_id };
+
+    // Vote sot hat it passes
+    let vote = ExecuteMsg::Vote(VoteMsg {
+        proposal_id,
+        vote: Vote::Yes,
+    });
+    let _res = app
+        .execute_contract(Addr::unchecked(VOTER3), dao_addr.clone(), &vote, &[])
+        .unwrap();
+
+    // Execute from account that's not a member
+    let res = app
+        .execute_contract(Addr::unchecked(SOMEBODY), dao_addr, &execution, &[])
+        .unwrap();
+    assert_eq!(
+        res.custom_attrs(1),
+        [
+            ("action", "execute"),
+            ("sender", SOMEBODY),
+            ("proposal_id", proposal_id.to_string().as_str()),
+        ],
+    );
+}
+
 #[test]
 fn proposal_pass_on_expiration() {
     let mut app = mock_app();
@@ -1287,7 +1374,7 @@ fn proposal_pass_on_expiration() {
     // Now execute
     let res = app
         .execute_contract(
-            Addr::unchecked(SOMEBODY),
+            Addr::unchecked(VOTER3),
             dao_addr,
             &ExecuteMsg::Execute { proposal_id },
             &[],
@@ -1297,7 +1384,7 @@ fn proposal_pass_on_expiration() {
         res.custom_attrs(1),
         [
             ("action", "execute"),
-            ("sender", SOMEBODY),
+            ("sender", VOTER3),
             ("proposal_id", proposal_id.to_string().as_str())
         ]
     );
@@ -1425,6 +1512,7 @@ fn test_close_works_with_refund() {
         proposal_deposit: proposal_deposit_amount,
         refund_failed_proposals: Some(false),
         image_url: None,
+        only_members_execute: true,
     });
 
     let res = app.execute_contract(dao_addr.clone(), dao_addr.clone(), &update_config_msg, &[]);
@@ -1790,6 +1878,7 @@ fn test_update_config() {
         proposal_deposit: new_proposal_deposit_amount,
         refund_failed_proposals: None,
         image_url: Some("https://imghostingwebsite.com/fqfpw.jpg".to_string()),
+        only_members_execute: true,
     });
     let res = app.execute_contract(
         Addr::unchecked(VOTER1),
@@ -1855,6 +1944,7 @@ fn test_update_config() {
                 proposal_deposit: new_proposal_deposit_amount,
                 refund_failed_proposals: None,
                 image_url: Some("https://imghostingwebsite.com/fqfpw.jpg".to_string()),
+                only_members_execute: true
             },
             gov_token: cw20_addr,
             staking_contract: staking_addr,
@@ -1947,6 +2037,7 @@ fn test_update_staking_contract() {
                 proposal_deposit: Uint128::zero(),
                 refund_failed_proposals: None,
                 image_url: None,
+                only_members_execute: true
             },
             gov_token: cw20_addr,
             staking_contract: Addr::unchecked("Better_Staking_Contract"),
@@ -2126,6 +2217,7 @@ fn test_config_query() {
                 proposal_deposit: Uint128::zero(),
                 refund_failed_proposals: None,
                 image_url: None,
+                only_members_execute: true
             },
             gov_token: cw20_addr,
             staking_contract: staking_addr,
@@ -2171,6 +2263,7 @@ fn test_proposal_deposit_works() {
         proposal_deposit: proposal_deposit_amount,
         refund_failed_proposals: None,
         image_url: None,
+        only_members_execute: true,
     });
     let res = app.execute_contract(dao_addr.clone(), dao_addr.clone(), &update_config_msg, &[]);
     assert!(res.is_ok());
@@ -2230,13 +2323,13 @@ fn test_proposal_deposit_works() {
 
     // Execute works. Anybody can execute Passed proposals
     let res = app
-        .execute_contract(Addr::unchecked(SOMEBODY), dao_addr, &execution, &[])
+        .execute_contract(Addr::unchecked(VOTER3), dao_addr, &execution, &[])
         .unwrap();
     assert_eq!(
         res.custom_attrs(1),
         [
             ("action", "execute"),
-            ("sender", SOMEBODY),
+            ("sender", VOTER3),
             ("proposal_id", proposal_id.to_string().as_str()),
         ],
     );
diff --git a/contracts/cw3-multisig/src/tests.rs b/contracts/cw3-multisig/src/tests.rs
index 1bfadf90..fe5e09f6 100644
--- a/contracts/cw3-multisig/src/tests.rs
+++ b/contracts/cw3-multisig/src/tests.rs
@@ -105,6 +105,7 @@ fn instantiate_multisig(
         threshold,
         max_voting_period,
         image_url: None,
+        only_members_execute: true,
     };
     app.instantiate_contract(
         multisig_id,
@@ -243,6 +244,7 @@ fn test_instantiate_works() {
         },
         max_voting_period,
         image_url: None,
+        only_members_execute: true,
     };
     let err = app
         .instantiate_contract(
@@ -266,6 +268,7 @@ fn test_instantiate_works() {
         threshold: Threshold::AbsoluteCount { weight: 100 },
         max_voting_period,
         image_url: None,
+        only_members_execute: true,
     };
     let err = app
         .instantiate_contract(
@@ -289,6 +292,7 @@ fn test_instantiate_works() {
         threshold: Threshold::AbsoluteCount { weight: 1 },
         max_voting_period,
         image_url: None,
+        only_members_execute: true,
     };
     let multisig_addr = app
         .instantiate_contract(
@@ -326,6 +330,7 @@ fn test_instantiate_works() {
                 threshold: Threshold::AbsoluteCount { weight: 1 },
                 max_voting_period,
                 image_url: None,
+                only_members_execute: true
             },
             group_address: Cw4Contract::new(Addr::unchecked(group_addr)),
         }
@@ -364,6 +369,7 @@ fn test_instantiate_works() {
         threshold: Threshold::AbsoluteCount { weight: 1 },
         max_voting_period,
         image_url: Some("https://imgur.com/someElmo.png".to_string()),
+        only_members_execute: true,
     };
     let res = app.instantiate_contract(
         multisig_id,
@@ -472,6 +478,7 @@ fn test_update_config() {
             },
             max_voting_period: voting_period,
             image_url: Some("https://someUrl.com/image.png".to_string()),
+            only_members_execute: true,
         },
         multisig_addr.to_string(),
     );
@@ -512,6 +519,7 @@ fn test_update_config() {
                 },
                 max_voting_period: voting_period,
                 image_url: Some("https://someUrl.com/image.png".to_string()),
+                only_members_execute: true
             },
             group_address: Cw4Contract::new(Addr::unchecked(group_addr)),
         }
@@ -525,6 +533,7 @@ fn test_update_config() {
             threshold: Threshold::AbsoluteCount { weight: 10000 },
             max_voting_period: voting_period,
             image_url: None,
+            only_members_execute: true,
         },
         multisig_addr.to_string(),
     );
@@ -1202,11 +1211,22 @@ fn test_execute_works() {
         .unwrap_err();
     assert_eq!(ContractError::WrongCloseStatus {}, err.downcast().unwrap());
 
-    // Execute works. Anybody can execute Passed proposals
-    let res = app
+    // Rejects if non-member attempts to execute
+    let err = app
         .execute_contract(
             Addr::unchecked(SOMEBODY),
             multisig_addr.clone(),
+            &ExecuteMsg::Execute { proposal_id },
+            &[],
+        )
+        .unwrap_err();
+    assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap());
+
+    // Execute works. Members can execute Passed proposals
+    let res = app
+        .execute_contract(
+            Addr::unchecked(VOTER3),
+            multisig_addr.clone(),
             &execution,
             &[],
         )
@@ -1215,7 +1235,7 @@ fn test_execute_works() {
         res.custom_attrs(1),
         [
             ("action", "execute"),
-            ("sender", SOMEBODY),
+            ("sender", VOTER3),
             ("proposal_id", proposal_id.to_string().as_str()),
         ],
     );
@@ -1234,7 +1254,7 @@ fn test_execute_works() {
 
     // Trying to execute something that was already executed fails
     let err = app
-        .execute_contract(Addr::unchecked(SOMEBODY), multisig_addr, &execution, &[])
+        .execute_contract(Addr::unchecked(VOTER3), multisig_addr, &execution, &[])
         .unwrap_err();
     assert_eq!(
         ContractError::WrongExecuteStatus {},
@@ -1242,6 +1262,94 @@ fn test_execute_works() {
     )
 }
 
+#[test]
+fn test_execute_works_for_anyone() {
+    let init_funds = coins(10, "BTC");
+    let mut app = mock_app(&init_funds);
+
+    let threshold = Threshold::ThresholdQuorum {
+        threshold: Decimal::percent(51),
+        quorum: Decimal::percent(1),
+    };
+    let voting_period = Duration::Time(2000000);
+    let (multisig_addr, _) =
+        setup_test_case(&mut app, threshold.clone(), voting_period, init_funds, true);
+
+    // ensure we have cash to cover the proposal
+    let contract_bal = app.wrap().query_balance(&multisig_addr, "BTC").unwrap();
+    assert_eq!(contract_bal, coin(10, "BTC"));
+
+    // Update config so that anyone can execute
+
+    let proposal = update_config_proposal(
+        Config {
+            name: "dogsig".to_string(),
+            description: "🐶".to_string(),
+            threshold,
+            max_voting_period: voting_period,
+            image_url: Some("https://someUrl.com/image.png".to_string()),
+            only_members_execute: false,
+        },
+        multisig_addr.to_string(),
+    );
+
+    let res = app
+        .execute_contract(
+            Addr::unchecked(VOTER4),
+            multisig_addr.clone(),
+            &proposal,
+            &[],
+        )
+        .unwrap();
+
+    let proposal_id: u64 = res.custom_attrs(1)[2].value.parse().unwrap();
+
+    app.execute_contract(
+        Addr::unchecked(VOTER4),
+        multisig_addr.clone(),
+        &ExecuteMsg::Execute { proposal_id },
+        &[],
+    )
+    .unwrap();
+
+    // create new proposal with 0 vote power
+    let proposal = pay_somebody_proposal();
+    let res = app
+        .execute_contract(
+            Addr::unchecked(OWNER),
+            multisig_addr.clone(),
+            &proposal,
+            &[],
+        )
+        .unwrap();
+
+    // Get the proposal id from the logs
+    let proposal_id: u64 = res.custom_attrs(1)[2].value.parse().unwrap();
+
+    // Vote it, so it passes
+    let vote = ExecuteMsg::Vote {
+        proposal_id,
+        vote: Vote::Yes,
+    };
+    let _res = app
+        .execute_contract(Addr::unchecked(VOTER4), multisig_addr.clone(), &vote, &[])
+        .unwrap();
+
+    // Execute proposal from account that's not a member
+    let execution = ExecuteMsg::Execute { proposal_id };
+    let res = app
+        .execute_contract(Addr::unchecked(SOMEBODY), multisig_addr, &execution, &[])
+        .unwrap();
+    assert_eq!(
+        res.custom_attrs(1),
+        [
+            ("action", "execute"),
+            ("sender", SOMEBODY),
+            ("proposal_id", proposal_id.to_string().as_str()),
+        ],
+    );
+}
+
 #[test]
 fn proposal_pass_on_expiration() {
     let init_funds = coins(10, "BTC");
@@ -1303,10 +1411,10 @@ fn proposal_pass_on_expiration() {
         .unwrap();
     assert_eq!(prop.status, Status::Passed);
 
-    // Now execute
+    // Now execute from member address
     let res = app
         .execute_contract(
-            Addr::unchecked(SOMEBODY),
+            Addr::unchecked(VOTER3),
             multsig_addr,
             &ExecuteMsg::Execute { proposal_id },
             &[],
@@ -1316,7 +1424,7 @@ fn proposal_pass_on_expiration() {
         res.custom_attrs(1),
         [
             ("action", "execute"),
-            ("sender", SOMEBODY),
+            ("sender", VOTER3),
             ("proposal_id", proposal_id.to_string().as_str())
         ]
     );