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

Proper execute responses #519

Merged
merged 5 commits into from
Oct 28, 2021
Merged

Proper execute responses #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f310e7f
Rename init_response to instantiate_response
maurolacy Oct 28, 2021
5d8fa4f
Try to clarify reply message flows (for the echo contract)
maurolacy Oct 28, 2021
1838eaa
Wrap on WasmMsg::Execute,Migrate, remove wrapper in top-level execute…
ethanfrey Oct 28, 2021
c20f112
Unwrap envelope in echo contract reply
ethanfrey Oct 28, 2021
c0d4c01
Use the reply id to indicate Instantiate vs. Execute messages (echo c…
maurolacy Oct 28, 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
92 changes: 69 additions & 23 deletions packages/multi-test/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1824,6 +1824,8 @@ mod test {
mod reply_data_overwrite {
use super::*;

use echo::EXECUTE_REPLY_BASE_ID;

fn make_echo_submsg(
contract: Addr,
data: impl Into<Option<&'static str>>,
Expand Down Expand Up @@ -1907,7 +1909,12 @@ mod test {
contract.clone(),
&echo::Message {
data: Some("First".to_owned()),
sub_msg: vec![make_echo_submsg(contract, "Second", vec![], 1)],
sub_msg: vec![make_echo_submsg(
contract,
"Second",
vec![],
EXECUTE_REPLY_BASE_ID,
)],
..echo::Message::default()
},
&[],
Expand Down Expand Up @@ -1987,7 +1994,12 @@ mod test {
owner,
contract.clone(),
&echo::Message {
sub_msg: vec![make_echo_submsg(contract, "Second", vec![], 1)],
sub_msg: vec![make_echo_submsg(
contract,
"Second",
vec![],
EXECUTE_REPLY_BASE_ID,
)],
..echo::Message::default()
},
&[],
Expand Down Expand Up @@ -2076,10 +2088,25 @@ mod test {
&echo::Message {
data: Some("Orig".to_owned()),
sub_msg: 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),
make_echo_submsg(
contract.clone(),
None,
vec![],
EXECUTE_REPLY_BASE_ID + 1,
),
make_echo_submsg(
contract.clone(),
"First",
vec![],
EXECUTE_REPLY_BASE_ID + 2,
),
make_echo_submsg(
contract.clone(),
"Second",
vec![],
EXECUTE_REPLY_BASE_ID + 3,
),
make_echo_submsg(contract, None, vec![], EXECUTE_REPLY_BASE_ID + 4),
],
..echo::Message::default()
},
Expand Down Expand Up @@ -2139,10 +2166,25 @@ mod test {
contract.clone(),
&echo::Message {
sub_msg: vec![
make_echo_submsg(contract.clone(), None, vec![], 1),
make_echo_submsg(
contract.clone(),
None,
vec![],
EXECUTE_REPLY_BASE_ID + 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(
contract.clone(),
"Shown",
vec![],
EXECUTE_REPLY_BASE_ID + 2,
),
make_echo_submsg(
contract.clone(),
None,
vec![],
EXECUTE_REPLY_BASE_ID + 3,
),
make_echo_submsg_no_reply(contract, "Lost", vec![]),
],
..echo::Message::default()
Expand Down Expand Up @@ -2180,12 +2222,17 @@ mod test {
vec![make_echo_submsg(
contract.clone(),
"Second",
vec![make_echo_submsg(contract, None, vec![], 4)],
3,
vec![make_echo_submsg(
contract,
None,
vec![],
EXECUTE_REPLY_BASE_ID + 4,
)],
EXECUTE_REPLY_BASE_ID + 3,
)],
2,
EXECUTE_REPLY_BASE_ID + 2,
)],
1,
EXECUTE_REPLY_BASE_ID + 1,
)],
..echo::Message::default()
},
Expand Down Expand Up @@ -2393,7 +2440,8 @@ mod test {

mod protobuf_wrapped_data {
use super::*;
use cw0::{parse_execute_response_data, parse_instantiate_response_data};
use crate::test_helpers::contracts::echo::EXECUTE_REPLY_BASE_ID;
use cw0::parse_instantiate_response_data;

#[test]
fn instantiate_wrapped_properly() {
Expand Down Expand Up @@ -2477,7 +2525,7 @@ mod test {

// another echo contract
let msg = echo::Message::<Empty> {
data: Some("babble".into()),
data: Some("Passed to contract instantiation, returned as reply, and then returned as response".into()),
maurolacy marked this conversation as resolved.
Show resolved Hide resolved
..Default::default()
};
let sub_msg = SubMsg::reply_on_success(
Expand All @@ -2486,10 +2534,10 @@ mod test {
msg: to_binary(&msg).unwrap(),
funds: vec![],
},
1234,
EXECUTE_REPLY_BASE_ID,
);
let init_msg = echo::InitMessage::<Empty> {
data: Some("remove_me".into()),
data: Some("Overwrite me".into()),
sub_msg: Some(vec![sub_msg]),
};
let init_msg = to_binary(&init_msg).unwrap();
Expand All @@ -2505,13 +2553,12 @@ mod test {
// assert we have a proper instantiate result
let parsed = parse_instantiate_response_data(res.data.unwrap().as_slice()).unwrap();
assert!(parsed.data.is_some());
// from the reply, not the top level
assert_eq!(parsed.data.unwrap(), Binary::from(b"babble"));
// Result is from the reply, not the original one
assert_eq!(parsed.data.unwrap(), Binary::from(b"Passed to contract instantiation, returned as reply, and then returned as response"));
assert!(!parsed.contract_address.is_empty());
assert_ne!(parsed.contract_address, addr1.to_string());
}

#[ignore]
#[test]
fn execute_wrapped_properly() {
let owner = Addr::unchecked("owner");
Expand All @@ -2528,10 +2575,9 @@ mod test {
data: Some("hello".into()),
..echo::Message::default()
};
// execute_contract now decodes a protobuf wrapper, so we get the top-level response
let exec_res = app.execute_contract(owner, echo_addr, &msg, &[]).unwrap();
assert!(exec_res.data.is_some());
let parsed = parse_execute_response_data(&exec_res.data.unwrap()).unwrap();
assert_eq!(parsed.data, Some(Binary::from(b"hello")));
assert_eq!(exec_res.data, Some(Binary::from(b"hello")));
}
}
}
11 changes: 8 additions & 3 deletions packages/multi-test/src/executor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use cosmwasm_std::{
to_binary, Addr, Attribute, BankMsg, Binary, Coin, CosmosMsg, Event, SubMsgExecutionResponse,
WasmMsg,
};
use cw0::parse_instantiate_response_data;
use cw0::{parse_execute_response_data, parse_instantiate_response_data};
use schemars::JsonSchema;
use serde::Serialize;

Expand Down Expand Up @@ -96,7 +96,8 @@ where
}

/// Execute a contract and process all returned messages.
/// This is just a helper around execute()
/// This is just a helper around execute(),
/// but we parse out the data field to that what is returned by the contract (not the protobuf wrapper)
fn execute_contract<T: Serialize>(
&mut self,
sender: Addr,
Expand All @@ -110,7 +111,11 @@ where
msg,
funds: send_funds.to_vec(),
};
self.execute(sender, msg.into())
let mut res = self.execute(sender, msg.into())?;
res.data = res
.data
.and_then(|d| parse_execute_response_data(d.as_slice()).unwrap().data);
Ok(res)
}

/// Migrate a contract. Sender must be registered admin.
Expand Down
29 changes: 26 additions & 3 deletions packages/multi-test/src/test_helpers/contracts/echo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,13 @@ use crate::{test_helpers::EmptyMsg, Contract, ContractWrapper};
use schemars::JsonSchema;
use std::fmt::Debug;

use cw0::{parse_execute_response_data, parse_instantiate_response_data};
use derivative::Derivative;

// Choosing a reply id less than ECHO_EXECUTE_BASE_ID indicates an Instantiate message reply by convention.
// An Execute message reply otherwise.
pub(crate) const EXECUTE_REPLY_BASE_ID: u64 = i64::MAX as u64;

#[derive(Debug, Clone, Serialize, Deserialize, Derivative)]
#[derivative(Default(bound = "", new = "true"))]
pub struct Message<ExecC>
Expand Down Expand Up @@ -89,17 +94,35 @@ fn reply<ExecC>(_deps: DepsMut, _env: Env, msg: Reply) -> Result<Response<ExecC>
where
ExecC: Debug + PartialEq + Clone + JsonSchema + 'static,
{
let res = Response::new();
if let Reply {
id,
result:
ContractResult::Ok(SubMsgExecutionResponse {
data: Some(data), ..
}),
..
} = msg
{
Ok(Response::new().set_data(data))
// We parse out the WasmMsg::Execute wrapper...
// TODO: Handle all of Execute, Instantiate, and BankMsg replies differently.
let parsed_data;
if id < EXECUTE_REPLY_BASE_ID {
parsed_data = parse_instantiate_response_data(data.as_slice())
.map_err(|e| StdError::generic_err(e.to_string()))?
.data;
} else {
parsed_data = parse_execute_response_data(data.as_slice())
.map_err(|e| StdError::generic_err(e.to_string()))?
.data;
}

if let Some(data) = parsed_data {
Ok(res.set_data(data))
} else {
Ok(res)
}
} else {
Ok(Response::new())
Ok(res)
}
}

Expand Down
13 changes: 6 additions & 7 deletions packages/multi-test/src/wasm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@ where
Event::new("execute").add_attribute(CONTRACT_ATTR, &contract_addr);

let (res, msgs) = self.build_app_response(&contract_addr, custom_event, res);
let res =
let mut res =
self.process_response(api, router, storage, block, contract_addr, res, msgs)?;
// res.data = execute_response(res.data);
res.data = execute_response(res.data);
Ok(res)
}
WasmMsg::Instantiate {
Expand Down Expand Up @@ -348,7 +348,7 @@ where
res,
msgs,
)?;
res.data = Some(init_response(res.data, &contract_addr));
res.data = Some(instantiate_response(res.data, &contract_addr));
Ok(res)
}
WasmMsg::Migrate {
Expand Down Expand Up @@ -384,9 +384,9 @@ where
.add_attribute(CONTRACT_ATTR, &contract_addr)
.add_attribute("code_id", new_code_id.to_string());
let (res, msgs) = self.build_app_response(&contract_addr, custom_event, res);
let res =
let mut res =
self.process_response(api, router, storage, block, contract_addr, res, msgs)?;
// res.data = execute_response(res.data);
res.data = execute_response(res.data);
Ok(res)
}
msg => bail!(Error::UnsupportedWasmMsg(msg)),
Expand Down Expand Up @@ -852,7 +852,7 @@ struct InstantiateResponse {
}

// TODO: encode helpers in cw0
fn init_response(data: Option<Binary>, contact_address: &Addr) -> Binary {
fn instantiate_response(data: Option<Binary>, contact_address: &Addr) -> Binary {
let data = data.unwrap_or_default().to_vec();
let init_data = InstantiateResponse {
address: contact_address.into(),
Expand All @@ -871,7 +871,6 @@ struct ExecuteResponse {
}

// empty return if no data present in original
#[allow(dead_code)]
fn execute_response(data: Option<Binary>) -> Option<Binary> {
data.map(|d| {
let exec_data = ExecuteResponse { data: d.to_vec() };
Expand Down