From c154dc3c4a43bc82db747b0ab606ac20c5b233dd Mon Sep 17 00:00:00 2001 From: Jakub Bogucki Date: Sat, 18 Jun 2022 12:11:04 +0200 Subject: [PATCH 1/5] cw3-flex: Add implementation for optional executor --- contracts/cw3-flex-multisig/src/contract.rs | 24 +++++++++++++++++++-- contracts/cw3-flex-multisig/src/msg.rs | 5 +++++ contracts/cw3-flex-multisig/src/state.rs | 13 +++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/contracts/cw3-flex-multisig/src/contract.rs b/contracts/cw3-flex-multisig/src/contract.rs index 5616cba7d..7bc0978c1 100644 --- a/contracts/cw3-flex-multisig/src/contract.rs +++ b/contracts/cw3-flex-multisig/src/contract.rs @@ -19,7 +19,7 @@ use cw_utils::{maybe_addr, Expiration, ThresholdResponse}; use crate::error::ContractError; use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg}; -use crate::state::{Config, CONFIG}; +use crate::state::{Config, Executor, CONFIG}; // version info for migration info const CONTRACT_NAME: &str = "crates.io:cw3-flex-multisig"; @@ -46,6 +46,7 @@ pub fn instantiate( threshold: msg.threshold, max_voting_period: msg.max_voting_period, group_addr, + executor: msg.executor, }; CONFIG.save(deps.storage, &cfg)?; @@ -192,7 +193,26 @@ pub fn execute_execute( info: MessageInfo, proposal_id: u64, ) -> Result { - // anyone can trigger this if the vote passed + let cfg = CONFIG.load(deps.storage)?; + + // Executor can be set in 3 ways: + // - Member: any member of the voting group can execute + // - Only: only passed address is able to execute + // - None: Anyone can execute message + if let Some(executor) = cfg.executor { + match executor { + Executor::Member => { + cfg.group_addr + .is_member(&deps.querier, &info.sender, None)? + .ok_or(ContractError::Unauthorized {})?; + } + Executor::Only(addr) => { + if addr != info.sender { + return Err(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-flex-multisig/src/msg.rs b/contracts/cw3-flex-multisig/src/msg.rs index 8d72cd37b..99437b26b 100644 --- a/contracts/cw3-flex-multisig/src/msg.rs +++ b/contracts/cw3-flex-multisig/src/msg.rs @@ -6,12 +6,17 @@ use cw3::Vote; use cw4::MemberChangedHookMsg; use cw_utils::{Duration, Expiration, Threshold}; +use crate::state::Executor; + #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] pub struct InstantiateMsg { // this is the group contract that contains the member list pub group_addr: String, pub threshold: Threshold, pub max_voting_period: Duration, + // who is able to execute passed proposals + // None means that anyone can execute + pub executor: Option, } // TODO: add some T variants? Maybe good enough as fixed Empty for now diff --git a/contracts/cw3-flex-multisig/src/state.rs b/contracts/cw3-flex-multisig/src/state.rs index 023662b8b..bdbc90456 100644 --- a/contracts/cw3-flex-multisig/src/state.rs +++ b/contracts/cw3-flex-multisig/src/state.rs @@ -1,16 +1,29 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use cosmwasm_std::Addr; use cw4::Cw4Contract; use cw_storage_plus::Item; use cw_utils::{Duration, Threshold}; +/// Defines who is able to execute proposals once passed +#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] +pub enum Executor { + /// Any member of the voting group, even with 0 points + Member, + /// Only the given address + Only(Addr), +} + #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] pub struct Config { pub threshold: Threshold, pub max_voting_period: Duration, // Total weight and voters are queried from this contract pub group_addr: Cw4Contract, + // who is able to execute passed proposals + // None means that anyone can execute + pub executor: Option, } // unique items From 57c11d65f692b4566a6bd01a7aba14ddd8a9efd3 Mon Sep 17 00:00:00 2001 From: Jakub Bogucki Date: Sat, 18 Jun 2022 12:22:20 +0200 Subject: [PATCH 2/5] cw3-flex: Adjust existing test to new executor parameter --- contracts/cw3-flex-multisig/src/contract.rs | 49 ++++++++++++++------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/contracts/cw3-flex-multisig/src/contract.rs b/contracts/cw3-flex-multisig/src/contract.rs index 7bc0978c1..111b8f2ff 100644 --- a/contracts/cw3-flex-multisig/src/contract.rs +++ b/contracts/cw3-flex-multisig/src/contract.rs @@ -193,12 +193,18 @@ pub fn execute_execute( info: MessageInfo, proposal_id: u64, ) -> Result { - let cfg = CONFIG.load(deps.storage)?; + 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 + // that point. If it was approved on time, it can be executed any time. + if prop.current_status(&env.block) != Status::Passed { + return Err(ContractError::WrongExecuteStatus {}); + } // Executor can be set in 3 ways: // - Member: any member of the voting group can execute // - Only: only passed address is able to execute // - None: Anyone can execute message + let cfg = CONFIG.load(deps.storage)?; if let Some(executor) = cfg.executor { match executor { Executor::Member => { @@ -214,13 +220,6 @@ pub fn execute_execute( } } - 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 - // that point. If it was approved on time, it can be executed any time. - if prop.current_status(&env.block) != Status::Passed { - return Err(ContractError::WrongExecuteStatus {}); - } - // set it to executed prop.status = Status::Executed; PROPOSALS.save(deps.storage, proposal_id, &prop)?; @@ -517,12 +516,14 @@ mod tests { group: Addr, threshold: Threshold, max_voting_period: Duration, + executor: Option, ) -> Addr { let flex_id = app.store_code(contract_flex()); let msg = crate::msg::InstantiateMsg { group_addr: group.to_string(), threshold, max_voting_period, + executor, }; app.instantiate_contract(flex_id, Addr::unchecked(OWNER), &msg, &[], "flex", None) .unwrap() @@ -547,6 +548,7 @@ mod tests { max_voting_period, init_funds, multisig_as_group_admin, + None, ) } @@ -557,6 +559,7 @@ mod tests { max_voting_period: Duration, init_funds: Vec, multisig_as_group_admin: bool, + executor: Option, ) -> (Addr, Addr) { // 1. Instantiate group contract with members (and OWNER as admin) let members = vec![ @@ -571,7 +574,13 @@ mod tests { app.update_block(next_block); // 2. Set up Multisig backed by this group - let flex_addr = instantiate_flex(app, group_addr.clone(), threshold, max_voting_period); + let flex_addr = instantiate_flex( + app, + group_addr.clone(), + threshold, + max_voting_period, + executor, + ); app.update_block(next_block); // 3. (Optional) Set the multisig as the group owner @@ -636,6 +645,7 @@ mod tests { quorum: Decimal::percent(1), }, max_voting_period, + executor: None, }; let err = app .instantiate_contract( @@ -657,6 +667,7 @@ mod tests { group_addr: group_addr.to_string(), threshold: Threshold::AbsoluteCount { weight: 100 }, max_voting_period, + executor: None, }; let err = app .instantiate_contract( @@ -678,6 +689,7 @@ mod tests { group_addr: group_addr.to_string(), threshold: Threshold::AbsoluteCount { weight: 1 }, max_voting_period, + executor: None, }; let flex_addr = app .instantiate_contract( @@ -835,7 +847,8 @@ mod tests { threshold: Decimal::percent(80), quorum: Decimal::percent(20), }; - let (flex_addr, _) = setup_test_case(&mut app, threshold, voting_period, init_funds, false); + let (flex_addr, _) = + setup_test_case(&mut app, threshold, voting_period, init_funds, false, None); // create proposal with 1 vote power let proposal = pay_somebody_proposal(); @@ -930,7 +943,8 @@ mod tests { quorum: Decimal::percent(1), }; let voting_period = Duration::Time(2000000); - let (flex_addr, _) = setup_test_case(&mut app, threshold, voting_period, init_funds, false); + let (flex_addr, _) = + setup_test_case(&mut app, threshold, voting_period, init_funds, false, None); // create proposal with 0 vote power let proposal = pay_somebody_proposal(); @@ -1121,7 +1135,8 @@ mod tests { quorum: Decimal::percent(1), }; let voting_period = Duration::Time(2000000); - let (flex_addr, _) = setup_test_case(&mut app, threshold, voting_period, init_funds, true); + let (flex_addr, _) = + setup_test_case(&mut app, threshold, voting_period, init_funds, true, None); // ensure we have cash to cover the proposal let contract_bal = app.wrap().query_balance(&flex_addr, "BTC").unwrap(); @@ -1227,6 +1242,7 @@ mod tests { Duration::Time(voting_period), init_funds, true, + None, ); // ensure we have cash to cover the proposal @@ -1302,7 +1318,8 @@ mod tests { quorum: Decimal::percent(1), }; let voting_period = Duration::Height(2000000); - let (flex_addr, _) = setup_test_case(&mut app, threshold, voting_period, init_funds, true); + let (flex_addr, _) = + setup_test_case(&mut app, threshold, voting_period, init_funds, true, None); // create proposal with 0 vote power let proposal = pay_somebody_proposal(); @@ -1354,7 +1371,7 @@ mod tests { }; let voting_period = Duration::Time(20000); let (flex_addr, group_addr) = - setup_test_case(&mut app, threshold, voting_period, init_funds, false); + setup_test_case(&mut app, threshold, voting_period, init_funds, false, None); // VOTER1 starts a proposal to send some tokens (1/4 votes) let proposal = pay_somebody_proposal(); @@ -1600,7 +1617,7 @@ mod tests { }; let voting_period = Duration::Time(20000); let (flex_addr, group_addr) = - setup_test_case(&mut app, threshold, voting_period, init_funds, false); + setup_test_case(&mut app, threshold, voting_period, init_funds, false, None); // VOTER3 starts a proposal to send some tokens (3/12 votes) let proposal = pay_somebody_proposal(); @@ -1683,6 +1700,7 @@ mod tests { voting_period, init_funds, false, + None, ); // VOTER3 starts a proposal to send some tokens (3 votes) @@ -1753,6 +1771,7 @@ mod tests { voting_period, init_funds, false, + None, ); // create proposal From 467c4725200ab4c766c9569e6fce675fecb400b3 Mon Sep 17 00:00:00 2001 From: Jakub Bogucki Date: Sat, 18 Jun 2022 13:49:14 +0200 Subject: [PATCH 3/5] cw3-flex: Add testcases that confirms executor works --- contracts/cw3-flex-multisig/src/contract.rs | 122 ++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/contracts/cw3-flex-multisig/src/contract.rs b/contracts/cw3-flex-multisig/src/contract.rs index 111b8f2ff..53e8032fd 100644 --- a/contracts/cw3-flex-multisig/src/contract.rs +++ b/contracts/cw3-flex-multisig/src/contract.rs @@ -1226,6 +1226,128 @@ mod tests { ); } + #[test] + fn execute_with_executor_member() { + 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 (flex_addr, _) = setup_test_case( + &mut app, + threshold, + voting_period, + init_funds, + true, + Some(crate::state::Executor::Member), + ); + + // create proposal with 0 vote power + let proposal = pay_somebody_proposal(); + let res = app + .execute_contract(Addr::unchecked(OWNER), flex_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, + }; + app.execute_contract(Addr::unchecked(VOTER4), flex_addr.clone(), &vote, &[]) + .unwrap(); + + let execution = ExecuteMsg::Execute { proposal_id }; + let err = app + .execute_contract( + Addr::unchecked(Addr::unchecked("anyone")), + flex_addr.clone(), + &execution, + &[], + ) + .unwrap_err(); + assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap()); + + app.execute_contract( + Addr::unchecked(Addr::unchecked(VOTER2)), + flex_addr, + &execution, + &[], + ) + .unwrap(); + } + + #[test] + fn execute_with_executor_only() { + 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 (flex_addr, _) = setup_test_case( + &mut app, + threshold, + voting_period, + init_funds, + true, + Some(crate::state::Executor::Only(Addr::unchecked(VOTER3))), + ); + + // create proposal with 0 vote power + let proposal = pay_somebody_proposal(); + let res = app + .execute_contract(Addr::unchecked(OWNER), flex_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, + }; + app.execute_contract(Addr::unchecked(VOTER4), flex_addr.clone(), &vote, &[]) + .unwrap(); + + let execution = ExecuteMsg::Execute { proposal_id }; + let err = app + .execute_contract( + Addr::unchecked(Addr::unchecked("anyone")), + flex_addr.clone(), + &execution, + &[], + ) + .unwrap_err(); + assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap()); + + let err = app + .execute_contract( + Addr::unchecked(Addr::unchecked(VOTER1)), + flex_addr.clone(), + &execution, + &[], + ) + .unwrap_err(); + assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap()); + + app.execute_contract( + Addr::unchecked(Addr::unchecked(VOTER3)), + flex_addr, + &execution, + &[], + ) + .unwrap(); + } + #[test] fn proposal_pass_on_expiration() { let init_funds = coins(10, "BTC"); From ad6a88930fb873675214cb83b31162cd1136078f Mon Sep 17 00:00:00 2001 From: Jakub Bogucki Date: Sun, 19 Jun 2022 12:17:18 +0200 Subject: [PATCH 4/5] cw3-flex: Create authorize method for Config structure --- contracts/cw3-flex-multisig/src/contract.rs | 21 ++-------------- contracts/cw3-flex-multisig/src/state.rs | 28 ++++++++++++++++++++- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/contracts/cw3-flex-multisig/src/contract.rs b/contracts/cw3-flex-multisig/src/contract.rs index 53e8032fd..4157a83c3 100644 --- a/contracts/cw3-flex-multisig/src/contract.rs +++ b/contracts/cw3-flex-multisig/src/contract.rs @@ -19,7 +19,7 @@ use cw_utils::{maybe_addr, Expiration, ThresholdResponse}; use crate::error::ContractError; use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg}; -use crate::state::{Config, Executor, CONFIG}; +use crate::state::{Config, CONFIG}; // version info for migration info const CONTRACT_NAME: &str = "crates.io:cw3-flex-multisig"; @@ -200,25 +200,8 @@ pub fn execute_execute( return Err(ContractError::WrongExecuteStatus {}); } - // Executor can be set in 3 ways: - // - Member: any member of the voting group can execute - // - Only: only passed address is able to execute - // - None: Anyone can execute message let cfg = CONFIG.load(deps.storage)?; - if let Some(executor) = cfg.executor { - match executor { - Executor::Member => { - cfg.group_addr - .is_member(&deps.querier, &info.sender, None)? - .ok_or(ContractError::Unauthorized {})?; - } - Executor::Only(addr) => { - if addr != info.sender { - return Err(ContractError::Unauthorized {}); - } - } - } - } + cfg.authorize(&deps.querier, &info.sender)?; // set it to executed prop.status = Status::Executed; diff --git a/contracts/cw3-flex-multisig/src/state.rs b/contracts/cw3-flex-multisig/src/state.rs index bdbc90456..721c43882 100644 --- a/contracts/cw3-flex-multisig/src/state.rs +++ b/contracts/cw3-flex-multisig/src/state.rs @@ -1,11 +1,13 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use cosmwasm_std::Addr; +use cosmwasm_std::{Addr, QuerierWrapper}; use cw4::Cw4Contract; use cw_storage_plus::Item; use cw_utils::{Duration, Threshold}; +use crate::error::ContractError; + /// Defines who is able to execute proposals once passed #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] pub enum Executor { @@ -26,5 +28,29 @@ pub struct Config { pub executor: Option, } +impl Config { + // Executor can be set in 3 ways: + // - Member: any member of the voting group can execute + // - Only: only passed address is able to execute + // - None: Anyone can execute message + pub fn authorize(&self, querier: &QuerierWrapper, sender: &Addr) -> Result<(), ContractError> { + if let Some(executor) = &self.executor { + match executor { + Executor::Member => { + self.group_addr + .is_member(querier, sender, None)? + .ok_or(ContractError::Unauthorized {})?; + } + Executor::Only(addr) => { + if addr != sender { + return Err(ContractError::Unauthorized {}); + } + } + } + } + Ok(()) + } +} + // unique items pub const CONFIG: Item = Item::new("config"); From 1e1f6e582da162b9304fd02f975ca5f7420a6fbb Mon Sep 17 00:00:00 2001 From: Jakub Bogucki Date: Sun, 19 Jun 2022 12:23:42 +0200 Subject: [PATCH 5/5] cw3-flex: Adjust comments to match authorize methods better --- contracts/cw3-flex-multisig/src/contract.rs | 14 +++++++------- contracts/cw3-flex-multisig/src/state.rs | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/contracts/cw3-flex-multisig/src/contract.rs b/contracts/cw3-flex-multisig/src/contract.rs index 4157a83c3..79ac0fc27 100644 --- a/contracts/cw3-flex-multisig/src/contract.rs +++ b/contracts/cw3-flex-multisig/src/contract.rs @@ -1225,7 +1225,7 @@ mod tests { voting_period, init_funds, true, - Some(crate::state::Executor::Member), + Some(crate::state::Executor::Member), // set executor as Member of voting group ); // create proposal with 0 vote power @@ -1248,7 +1248,7 @@ mod tests { let execution = ExecuteMsg::Execute { proposal_id }; let err = app .execute_contract( - Addr::unchecked(Addr::unchecked("anyone")), + Addr::unchecked(Addr::unchecked("anyone")), // anyone is not allowed to execute flex_addr.clone(), &execution, &[], @@ -1257,7 +1257,7 @@ mod tests { assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap()); app.execute_contract( - Addr::unchecked(Addr::unchecked(VOTER2)), + Addr::unchecked(Addr::unchecked(VOTER2)), // member of voting group is allowed to execute flex_addr, &execution, &[], @@ -1281,7 +1281,7 @@ mod tests { voting_period, init_funds, true, - Some(crate::state::Executor::Only(Addr::unchecked(VOTER3))), + Some(crate::state::Executor::Only(Addr::unchecked(VOTER3))), // only VOTER3 can execute proposal ); // create proposal with 0 vote power @@ -1304,7 +1304,7 @@ mod tests { let execution = ExecuteMsg::Execute { proposal_id }; let err = app .execute_contract( - Addr::unchecked(Addr::unchecked("anyone")), + Addr::unchecked(Addr::unchecked("anyone")), // anyone is not allowed to execute flex_addr.clone(), &execution, &[], @@ -1314,7 +1314,7 @@ mod tests { let err = app .execute_contract( - Addr::unchecked(Addr::unchecked(VOTER1)), + Addr::unchecked(Addr::unchecked(VOTER1)), // VOTER1 is not allowed to execute flex_addr.clone(), &execution, &[], @@ -1323,7 +1323,7 @@ mod tests { assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap()); app.execute_contract( - Addr::unchecked(Addr::unchecked(VOTER3)), + Addr::unchecked(Addr::unchecked(VOTER3)), // VOTER3 is allowed to execute flex_addr, &execution, &[], diff --git a/contracts/cw3-flex-multisig/src/state.rs b/contracts/cw3-flex-multisig/src/state.rs index 721c43882..a1e388777 100644 --- a/contracts/cw3-flex-multisig/src/state.rs +++ b/contracts/cw3-flex-multisig/src/state.rs @@ -30,9 +30,9 @@ pub struct Config { impl Config { // Executor can be set in 3 ways: - // - Member: any member of the voting group can execute - // - Only: only passed address is able to execute - // - None: Anyone can execute message + // - Member: any member of the voting group is authorized + // - Only: only passed address is authorized + // - None: Everyone are authorized pub fn authorize(&self, querier: &QuerierWrapper, sender: &Addr) -> Result<(), ContractError> { if let Some(executor) = &self.executor { match executor {