Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Document submsg data failures and fix them #383

Merged
merged 4 commits into from
Aug 10, 2021
Merged

Document submsg data failures and fix them #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a6b216
Cherry-pick test changes from 2440816ce
ethanfrey Aug 9, 2021
84bd7e3
Review and extend tests
ethanfrey Aug 9, 2021
b34093b
Corrected submessage data response handling in multi-test
hashedone Aug 10, 2021
09e9e0f
Merge pull request #385 from CosmWasm/data-response-handling
ethanfrey Aug 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
226 changes: 212 additions & 14 deletions packages/multi-test/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,13 +965,35 @@ mod test {
assert_eq!(state.beneficiary, random);
}

mod replay_data_overwrite {
mod reply_data_overwrite {
use super::*;

fn make_echo_submsg(
contract: Addr,
data: impl Into<Option<&'static str>>,
sub_msg: Vec<SubMsg>,
id: u64,
) -> SubMsg {
let data = data.into().map(|s| s.to_owned());
SubMsg::reply_always(
CosmosMsg::Wasm(WasmMsg::Execute {
contract_addr: contract.into(),
msg: to_binary(&echo::Message {
data,
sub_msg,
..echo::Message::default()
})
.unwrap(),
funds: vec![],
}),
id,
)
}

fn make_echo_submsg_no_reply(
contract: Addr,
data: impl Into<Option<&'static str>>,
sub_msg: Vec<SubMsg>,
) -> SubMsg {
let data = data.into().map(|s| s.to_owned());
SubMsg::new(CosmosMsg::Wasm(WasmMsg::Execute {
Expand Down Expand Up @@ -1009,7 +1031,7 @@ mod test {
)
.unwrap();

assert_eq!(response.data, Some("Data".as_bytes().into()));
assert_eq!(response.data, Some(b"Data".into()));
}

#[test]
Expand All @@ -1029,18 +1051,45 @@ mod test {
contract.clone(),
&echo::Message {
data: Some("First".to_owned()),
sub_msg: vec![make_echo_submsg(contract, "Second", vec![])],
sub_msg: vec![make_echo_submsg(contract, "Second", vec![], 1)],
..echo::Message::default()
},
&[],
)
.unwrap();

assert_eq!(response.data, Some(b"Second".into()));
}

#[test]
fn single_submsg_no_reply() {
let mut app = mock_app();

let owner = Addr::unchecked("owner");

let contract_id = app.store_code(echo::contract());
let contract = app
.instantiate_contract(contract_id, owner.clone(), &EmptyMsg {}, &[], "Echo", None)
.unwrap();

let response = app
.execute_contract(
owner,
contract.clone(),
&echo::Message {
data: Some("First".to_owned()),
sub_msg: vec![make_echo_submsg_no_reply(contract, "Second", vec![])],
..echo::Message::default()
},
&[],
)
.unwrap();

assert_eq!(response.data, Some("Second".as_bytes().into()));
assert_eq!(response.data, Some(b"First".into()));
}

#[test]
fn single_no_data() {
fn single_no_submsg_data() {
let mut app = mock_app();

let owner = Addr::unchecked("owner");
Expand All @@ -1056,14 +1105,96 @@ mod test {
contract.clone(),
&echo::Message {
data: Some("First".to_owned()),
sub_msg: vec![make_echo_submsg(contract, None, vec![])],
sub_msg: vec![make_echo_submsg(contract, None, vec![], 1)],
..echo::Message::default()
},
&[],
)
.unwrap();

assert_eq!(response.data, Some("First".as_bytes().into()));
assert_eq!(response.data, Some(b"First".into()));
}

#[test]
fn single_no_top_level_data() {
let mut app = mock_app();

let owner = Addr::unchecked("owner");

let contract_id = app.store_code(echo::contract());
let contract = app
.instantiate_contract(contract_id, owner.clone(), &EmptyMsg {}, &[], "Echo", None)
.unwrap();

let response = app
.execute_contract(
owner,
contract.clone(),
&echo::Message {
sub_msg: vec![make_echo_submsg(contract, "Second", vec![], 1)],
..echo::Message::default()
},
&[],
)
.unwrap();

assert_eq!(response.data, Some(b"Second".into()));
}

#[test]
fn single_submsg_reply_returns_none() {
let mut app = custom_app();

// set personal balance
let owner = Addr::unchecked("owner");
app.init_bank_balance(&owner, coins(100, "tgd")).unwrap();

// set up reflect contract
let reflect_id = app.store_code(reflect::contract());
let reflect_addr = app
.instantiate_contract(
reflect_id,
owner.clone(),
&EmptyMsg {},
&[],
"Reflect",
None,
)
.unwrap();

// set up echo contract
let echo_id = app.store_code(echo::custom_contract());
let echo_addr = app
.instantiate_contract(echo_id, owner.clone(), &EmptyMsg {}, &[], "Echo", None)
.unwrap();

// reflect will call echo
// echo will set the data
// top-level app will not display the data
let echo_msg = echo::Message {
data: Some("my echo".into()),
events: vec![Event::new("echo").add_attribute("called", "true")],
..echo::Message::default()
};
let reflect_msg = reflect::Message {
messages: vec![SubMsg::new(WasmMsg::Execute {
contract_addr: echo_addr.to_string(),
msg: to_binary(&echo_msg).unwrap(),
funds: vec![],
})],
};

let res = app
.execute_contract(owner, reflect_addr, &reflect_msg, &[])
.unwrap();

// ensure data is empty
assert_eq!(res.data, None);
// ensure expected events
assert_eq!(res.events.len(), 3, "{:?}", res.events);
assert_eq!("execute", &res.events[0].ty);
assert_eq!("execute", &res.events[1].ty);
assert_eq!("wasm-echo", &res.events[2].ty);
}

#[test]
Expand All @@ -1084,18 +1215,82 @@ mod test {
&echo::Message {
data: Some("Orig".to_owned()),
sub_msg: vec![
make_echo_submsg(contract.clone(), None, vec![]),
make_echo_submsg(contract.clone(), "First", vec![]),
make_echo_submsg(contract.clone(), "Second", vec![]),
make_echo_submsg(contract, None, vec![]),
make_echo_submsg(contract.clone(), None, vec![], 1),
make_echo_submsg(contract.clone(), "First", vec![], 2),
make_echo_submsg(contract.clone(), "Second", vec![], 3),
make_echo_submsg(contract, None, vec![], 4),
],
..echo::Message::default()
},
&[],
)
.unwrap();

assert_eq!(response.data, Some(b"Second".into()));
}

#[test]
fn multiple_submsg_no_reply() {
let mut app = mock_app();

let owner = Addr::unchecked("owner");

let contract_id = app.store_code(echo::contract());
let contract = app
.instantiate_contract(contract_id, owner.clone(), &EmptyMsg {}, &[], "Echo", None)
.unwrap();

let response = app
.execute_contract(
owner,
contract.clone(),
&echo::Message {
data: Some("Orig".to_owned()),
sub_msg: vec![
make_echo_submsg_no_reply(contract.clone(), None, vec![]),
make_echo_submsg_no_reply(contract.clone(), "First", vec![]),
make_echo_submsg_no_reply(contract.clone(), "Second", vec![]),
make_echo_submsg_no_reply(contract, None, vec![]),
],
..echo::Message::default()
},
&[],
)
.unwrap();

assert_eq!(response.data, Some(b"Orig".into()));
}

#[test]
fn multiple_submsg_mixed() {
let mut app = mock_app();

let owner = Addr::unchecked("owner");

let contract_id = app.store_code(echo::contract());
let contract = app
.instantiate_contract(contract_id, owner.clone(), &EmptyMsg {}, &[], "Echo", None)
.unwrap();

let response = app
.execute_contract(
owner,
contract.clone(),
&echo::Message {
sub_msg: vec![
make_echo_submsg(contract.clone(), None, vec![], 1),
make_echo_submsg_no_reply(contract.clone(), "Hidden", vec![]),
make_echo_submsg(contract.clone(), "Shown", vec![], 2),
make_echo_submsg(contract.clone(), None, vec![], 3),
make_echo_submsg_no_reply(contract, "Lost", vec![]),
],
..echo::Message::default()
},
&[],
)
.unwrap();

assert_eq!(response.data, Some("Second".as_bytes().into()));
assert_eq!(response.data, Some(b"Shown".into()));
}

#[test]
Expand Down Expand Up @@ -1124,17 +1319,20 @@ mod test {
vec![make_echo_submsg(
contract.clone(),
"Second",
vec![make_echo_submsg(contract, None, vec![])],
vec![make_echo_submsg(contract, None, vec![], 4)],
3,
)],
2,
)],
1,
)],
..echo::Message::default()
},
&[],
)
.unwrap();

assert_eq!(response.data, Some("Second".as_bytes().into()));
assert_eq!(response.data, Some(b"Second".into()));
}
}

Expand Down
24 changes: 13 additions & 11 deletions packages/multi-test/src/wasm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,9 @@ where
///
/// For normal use cases, you can use Router::execute() or Router::execute_multi().
/// This is designed to be handled internally as part of larger process flows.
///
/// The `data` on `AppResponse` is data returned from `reply` call, not from execution of
/// submessage itself. In case if `reply` is not called, no `data` is set.
fn execute_submsg(
&self,
api: &dyn Api,
Expand All @@ -389,28 +392,27 @@ where
});

// call reply if meaningful
if let Ok(r) = res {
if let Ok(mut r) = res {
if matches!(reply_on, ReplyOn::Always | ReplyOn::Success) {
let mut orig = r.clone();
let reply = Reply {
id,
result: ContractResult::Ok(SubMsgExecutionResponse {
events: r.events,
events: r.events.clone(),
data: r.data,
}),
};
// do reply and combine it with the original response
let res2 = self._reply(api, router, storage, block, contract, reply)?;
// override data if set
if let Some(data) = res2.data {
orig.data = Some(data);
}
let reply_res = self._reply(api, router, storage, block, contract, reply)?;
// override data
r.data = reply_res.data;
// append the events
orig.events.extend_from_slice(&res2.events);
Ok(orig)
r.events.extend_from_slice(&reply_res.events);
} else {
Ok(r)
// reply is not called, no data should be rerturned
r.data = None;
}

Ok(r)
} else if let Err(e) = res {
if matches!(reply_on, ReplyOn::Always | ReplyOn::Error) {
let reply = Reply {
Expand Down