Skip to content

Commit

Permalink
Merge pull request #96 from neutron-org/fix/explicitly-take-one-messa…
Browse files Browse the repository at this point in the history 
…ge-in-timelock

fix: explicitly expect one message in timelock execute_execute_proposal #ntrn-129
  • Loading branch information
@pr0n00gler
pr0n00gler authored Nov 3, 2023
2 parents 7b9c15e + 6be87c2 commit 09d3f67
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 35 deletions.
54 changes: 41 additions & 13 deletions contracts/subdaos/cwd-subdao-timelock-single/src/contract.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#[cfg(not(feature = "library"))]
use cosmwasm_std::entry_point;
use cosmwasm_std::{
to_binary, Addr, Binary, CosmosMsg, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdError,
StdResult, SubMsg, WasmMsg,
from_binary, to_binary, Addr, Binary, CosmosMsg, Deps, DepsMut, Env, MessageInfo, Reply,
Response, StdError, StdResult, SubMsg, WasmMsg,
};
use cw2::set_contract_version;
use cw_storage_plus::Bound;
Expand All @@ -16,6 +16,7 @@ use neutron_dao_pre_propose_overrule::msg::{
QueryExt as OverruleQueryExt, QueryMsg as OverruleQueryMsg,
};
use neutron_sdk::bindings::msg::NeutronMsg;
use neutron_subdao_core::msg::ExecuteMsg as CoreExecuteMsg;
use neutron_subdao_core::msg::QueryMsg as SubdaoQuery;
use neutron_subdao_pre_propose_single::msg::QueryMsg as PreProposeQuery;
use neutron_subdao_proposal_single::msg::QueryMsg as ProposalQueryMsg;
Expand Down Expand Up @@ -109,9 +110,12 @@ pub fn execute_timelock_proposal(
return Err(ContractError::Unauthorized {});
}

// We expect only one specific message `ExecuteMsg::ExecuteTimelockedMsgs` inside
let msg = verify_msg(msgs)?;

let proposal = SingleChoiceProposal {
id: proposal_id,
msgs,
msgs: vec![msg],
status: ProposalStatus::Timelocked,
};

Expand Down Expand Up @@ -176,17 +180,12 @@ pub fn execute_execute_proposal(
.querier
.query_wasm_smart(proposal_module, &ProposalQueryMsg::Config {})?;

// We expect only one specific message `ExecuteMsg::ExecuteTimelockedMsgs` inside
let msg = verify_msg(proposal.msgs)?;

match proposal_config.close_proposal_on_execution_failure {
true => {
let msgs: Vec<SubMsg<NeutronMsg>> = proposal
.msgs
.iter()
.map(|msg| SubMsg::reply_on_error(msg.clone(), proposal_id))
.collect();

Response::default().add_submessages(msgs)
}
false => Response::default().add_messages(proposal.msgs),
true => Response::default().add_submessage(SubMsg::reply_on_error(msg, proposal_id)),
false => Response::default().add_message(msg),
}
};

Expand All @@ -198,6 +197,35 @@ pub fn execute_execute_proposal(
.add_attribute("proposal_id", proposal_id.to_string()))
}

/// `verify_msg` checks that there is only one message inside `msgs`
/// and verifies type inside of `CoreExecuteMsg::ExecuteTimelockedMsgs`
fn verify_msg(msgs: Vec<CosmosMsg<NeutronMsg>>) -> Result<CosmosMsg<NeutronMsg>, ContractError> {
let msgs_len = msgs.len();
// Expect exactly one message
if msgs_len != 1 {
return Err(ContractError::CanOnlyExecuteOneMsg { len: msgs_len });
}
let msg: CosmosMsg<NeutronMsg> = msgs
.into_iter()
.next()
.ok_or(ContractError::CanOnlyExecuteOneMsg { len: msgs_len })?;

// Expect only `ExecuteMsg::ExecuteTimelockedMsgs`
match &msg {
&CosmosMsg::Wasm(WasmMsg::Execute {
msg: ref core_execute_msg,
contract_addr: _,
funds: _,
}) => match from_binary::<CoreExecuteMsg>(core_execute_msg) {
Ok(CoreExecuteMsg::ExecuteTimelockedMsgs { msgs: _ }) => {}
_ => return Err(ContractError::CanOnlyExecuteExecuteTimelockedMsgs {}),
},
_ => return Err(ContractError::CanOnlyExecuteExecuteTimelockedMsgs {}),
}

Ok(msg)
}

pub fn execute_overrule_proposal(
deps: DepsMut,
info: MessageInfo,
Expand Down
8 changes: 7 additions & 1 deletion contracts/subdaos/cwd-subdao-timelock-single/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,15 @@ pub enum ContractError {
#[error("Wrong proposal status ({status})")]
WrongStatus { status: String },

#[error("no such proposal ({id})")]
#[error("No such proposal ({id})")]
NoSuchProposal { id: u64 },

#[error("Can not create overrule proposal for main DAO")]
CantCreateOverrule {},

#[error("Can only execute proposals with exactly one message that of ExecuteTimelockedMsgs type. Got {len} messages.")]
CanOnlyExecuteOneMsg { len: usize },

#[error("Can only execute msg of ExecuteTimelockedMsgs type")]
CanOnlyExecuteExecuteTimelockedMsgs {},
}
138 changes: 117 additions & 21 deletions contracts/subdaos/cwd-subdao-timelock-single/src/testing/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use cosmwasm_std::{
};
use cwd_voting::status::Status;
use neutron_sdk::bindings::msg::NeutronMsg;
use neutron_subdao_core::msg::ExecuteMsg as CoreExecuteMsg;
use neutron_subdao_timelock_single::{
msg::{ExecuteMsg, InstantiateMsg, QueryMsg},
types::{Config, ProposalListResponse, ProposalStatus, SingleChoiceProposal},
Expand Down Expand Up @@ -90,12 +91,17 @@ fn test_execute_timelock_proposal() {
let env = mock_env();
let info = mock_info("neutron1unknownsender", &[]);

let msg = ExecuteMsg::TimelockProposal {
// No config set case
let correct_msg = ExecuteMsg::TimelockProposal {
proposal_id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
msgs: vec![correct_proposal_msg()],
};

let res = execute(deps.as_mut(), env.clone(), info.clone(), msg.clone());
let res = execute(
deps.as_mut(),
env.clone(),
info.clone(),
correct_msg.clone(),
);
assert_eq!(
"neutron_subdao_timelock_single::types::Config not found",
res.unwrap_err().to_string()
Expand All @@ -107,11 +113,48 @@ fn test_execute_timelock_proposal() {
subdao: Addr::unchecked(MOCK_SUBDAO_CORE_ADDR),
};
CONFIG.save(deps.as_mut().storage, &config).unwrap();
let res = execute(deps.as_mut(), env.clone(), info, msg.clone());

// Unauthorized case
let res = execute(deps.as_mut(), env.clone(), info, correct_msg.clone());
assert_eq!("Unauthorized", res.unwrap_err().to_string());

let info = mock_info(MOCK_SUBDAO_CORE_ADDR, &[]);
let res_ok = execute(deps.as_mut(), env, info, msg).unwrap();

// check that execution fails when there is a wrong type of message inside
let incorrect_type_msg = ExecuteMsg::TimelockProposal {
proposal_id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
};
let res = execute(deps.as_mut(), env.clone(), info.clone(), incorrect_type_msg);
assert_eq!(
"Can only execute msg of ExecuteTimelockedMsgs type",
res.unwrap_err().to_string()
);

// check that execution fails when there are no messages inside
let empty_msgs_msg = ExecuteMsg::TimelockProposal {
proposal_id: 10,
msgs: vec![],
};
let res = execute(deps.as_mut(), env.clone(), info.clone(), empty_msgs_msg);
assert_eq!(
"Can only execute proposals with exactly one message that of ExecuteTimelockedMsgs type. Got 0 messages.",
res.unwrap_err().to_string()
);

// check that execution fails when there are 2 messages inside
let too_many_msgs_msg = ExecuteMsg::TimelockProposal {
proposal_id: 10,
msgs: vec![correct_proposal_msg(), correct_proposal_msg()],
};
let res = execute(deps.as_mut(), env.clone(), info.clone(), too_many_msgs_msg);
assert_eq!(
"Can only execute proposals with exactly one message that of ExecuteTimelockedMsgs type. Got 2 messages.",
res.unwrap_err().to_string()
);

// successful case
let res_ok = execute(deps.as_mut(), env, info, correct_msg).unwrap();
let expected_attributes = vec![
Attribute::new("action", "timelock_proposal"),
Attribute::new("sender", MOCK_SUBDAO_CORE_ADDR),
Expand All @@ -138,7 +181,7 @@ fn test_execute_timelock_proposal() {

let expected_proposal = SingleChoiceProposal {
id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
let prop = PROPOSALS.load(deps.as_mut().storage, 10u64).unwrap();
Expand Down Expand Up @@ -180,7 +223,7 @@ fn test_execute_proposal() {
for s in wrong_prop_statuses {
let proposal = SingleChoiceProposal {
id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
msgs: vec![correct_proposal_msg()],
status: s,
};
PROPOSALS
Expand All @@ -197,7 +240,7 @@ fn test_execute_proposal() {
deps.querier.set_close_proposal_on_execution_failure(true);
let proposal = SingleChoiceProposal {
id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
PROPOSALS
Expand Down Expand Up @@ -227,11 +270,56 @@ fn test_execute_proposal() {
let updated_prop = PROPOSALS.load(deps.as_mut().storage, 10).unwrap();
assert_eq!(ProposalStatus::Executed, updated_prop.status);

// check that execution fails when there not exactly one message in proposal
let proposal = SingleChoiceProposal {
id: 10,
msgs: vec![correct_proposal_msg(), correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
PROPOSALS
.save(deps.as_mut().storage, proposal.id, &proposal)
.unwrap();
let res = execute(deps.as_mut(), env.clone(), info.clone(), msg.clone());
assert_eq!(
"Can only execute proposals with exactly one message that of ExecuteTimelockedMsgs type. Got 2 messages.",
res.unwrap_err().to_string()
);

// check that execution fails when there is a wrong type of message inside
let proposal = SingleChoiceProposal {
id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
status: ProposalStatus::Timelocked,
};
PROPOSALS
.save(deps.as_mut().storage, proposal.id, &proposal)
.unwrap();
let res = execute(deps.as_mut(), env.clone(), info.clone(), msg.clone());
assert_eq!(
"Can only execute msg of ExecuteTimelockedMsgs type",
res.unwrap_err().to_string()
);

// check that execution fails when there are no messages inside
let proposal = SingleChoiceProposal {
id: 10,
msgs: vec![],
status: ProposalStatus::Timelocked,
};
PROPOSALS
.save(deps.as_mut().storage, proposal.id, &proposal)
.unwrap();
let res = execute(deps.as_mut(), env.clone(), info.clone(), msg.clone());
assert_eq!(
"Can only execute proposals with exactly one message that of ExecuteTimelockedMsgs type. Got 0 messages.",
res.unwrap_err().to_string()
);

// check proposal execution close_proposal_on_execution_failure = false
deps.querier.set_close_proposal_on_execution_failure(false);
let proposal2 = SingleChoiceProposal {
id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
PROPOSALS
Expand Down Expand Up @@ -289,7 +377,7 @@ fn test_overrule_proposal() {
for s in wrong_prop_statuses {
let proposal = SingleChoiceProposal {
id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
msgs: vec![correct_proposal_msg()],
status: s,
};
PROPOSALS
Expand All @@ -304,7 +392,7 @@ fn test_overrule_proposal() {

let proposal = SingleChoiceProposal {
id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
PROPOSALS
Expand Down Expand Up @@ -435,7 +523,7 @@ fn test_query_proposals() {
for i in 1..=100 {
let prop = SingleChoiceProposal {
id: i,
msgs: vec![NeutronMsg::remove_interchain_query(i).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
PROPOSALS.save(deps.as_mut().storage, i, &prop).unwrap();
Expand All @@ -446,7 +534,7 @@ fn test_query_proposals() {
let queried_prop: SingleChoiceProposal = from_binary(&res).unwrap();
let expected_prop = SingleChoiceProposal {
id: i,
msgs: vec![NeutronMsg::remove_interchain_query(i).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
assert_eq!(expected_prop, queried_prop)
Expand All @@ -461,7 +549,7 @@ fn test_query_proposals() {
for (p, i) in queried_props.proposals.iter().zip(1..) {
let expected_prop = SingleChoiceProposal {
id: i,
msgs: vec![NeutronMsg::remove_interchain_query(i).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
assert_eq!(expected_prop, *p);
Expand All @@ -477,7 +565,7 @@ fn test_query_proposals() {
for (p, i) in queried_props.proposals.iter().zip(1..) {
let expected_prop = SingleChoiceProposal {
id: i,
msgs: vec![NeutronMsg::remove_interchain_query(i).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
assert_eq!(expected_prop, *p);
Expand All @@ -493,7 +581,7 @@ fn test_query_proposals() {
for (p, i) in queried_props.proposals.iter().zip(1..) {
let expected_prop = SingleChoiceProposal {
id: i,
msgs: vec![NeutronMsg::remove_interchain_query(i).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
assert_eq!(expected_prop, *p);
Expand All @@ -509,7 +597,7 @@ fn test_query_proposals() {
for (p, i) in queried_props.proposals.iter().zip(51..) {
let expected_prop = SingleChoiceProposal {
id: i,
msgs: vec![NeutronMsg::remove_interchain_query(i).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
assert_eq!(expected_prop, *p);
Expand All @@ -525,7 +613,7 @@ fn test_query_proposals() {
for (p, i) in queried_props.proposals.iter().zip(91..) {
let expected_prop = SingleChoiceProposal {
id: i,
msgs: vec![NeutronMsg::remove_interchain_query(i).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
assert_eq!(expected_prop, *p);
Expand All @@ -542,11 +630,11 @@ fn test_reply() {
result: SubMsgResult::Err("error".to_string()),
};
let err = reply(deps.as_mut(), mock_env(), msg.clone()).unwrap_err();
assert_eq!("no such proposal (10)", err.to_string());
assert_eq!("No such proposal (10)", err.to_string());

let prop = SingleChoiceProposal {
id: 10,
msgs: vec![NeutronMsg::remove_interchain_query(1).into()],
msgs: vec![correct_proposal_msg()],
status: ProposalStatus::Timelocked,
};
let env = mock_env();
Expand All @@ -560,3 +648,11 @@ fn test_reply() {
let error: Option<String> = from_binary(&query_res).unwrap();
assert_eq!(error, Some("error".to_string()));
}

fn correct_proposal_msg() -> CosmosMsg<NeutronMsg> {
CosmosMsg::Wasm(WasmMsg::Execute {
contract_addr: "".to_string(),
msg: to_binary(&CoreExecuteMsg::ExecuteTimelockedMsgs { msgs: vec![] }).unwrap(),
funds: vec![],
})
}

0 comments on commit 09d3f67

Please sign in to comment.