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.

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

Expose Response from contract in cw-multi-test execute #763

Closed
kaimen-sano opened this issue Jul 22, 2022 · 3 comments
Closed

Expose Response from contract in cw-multi-test execute #763

kaimen-sano opened this issue Jul 22, 2022 · 3 comments

Comments

@kaimen-sano
Copy link

kaimen-sano commented Jul 22, 2022

Currently when a contract uses app.execute_contract in cw-multi-test, there is no way to get the resulting Response returned by the contract. cw-multi-test parses the responding messages and only returns the events and data from the result of the execution.

This is the relevant code that needs to be exposed somehow:

let res = self.call_execute(
api,
storage,
contract_addr.clone(),
router,
block,
info,
msg.to_vec(),
)?;

Currently process_response will lose the reference to the Response, as can be seen here:

// recurse in all messages
let data = messages.into_iter().try_fold(data, |data, resend| {
let subres =
self.execute_submsg(api, router, storage, block, contract.clone(), resend)?;
events.extend_from_slice(&subres.events);
Ok::<_, anyhow::Error>(subres.data.or(data))
})?;

It is beneficial to expose the Response returned by the executing function as it allows developers to test the SubMsgs (currently not testable) and attributes that is returned by the execute function in an easier manner than parsing the events vec.

@hashedone
Copy link
Contributor

This is not the purpose of multitest.

First of all - SubMsgs are easily testable with mutli-test, it is the main purpose of it. MT framework doesn't give you them back, instead, it directly executes them. So after execution, you got all the events that happened through all sub-messages execution, it also performed any bank operations and so. The only problem is handling chain-specific messages, which is still possible, but tricky - you need to add the custom module to create App, however, it's not documented anywhere right now. I am working on CosmWasm documentation and it would be included at some point, but right now I am writing down more basic stuff.

Attributes are even easier testable, you just need to understand how they work. There is no notion of arbitrary attributes on contract response. When you call add_attribute, in fact, you add the attribute to default wasm even which is always emitted on execution. Just check wasm event on the returned type and inspect its attributes - your stuff should be there.

If you want to test just your function calls without blockchain simulation, you don't need multiitest - just unit test your functions (you can find unit tests utilities for creating deps/env/msg_info in cosmwasm_std::testing I believe) and then you will be able to directly inspect your Response you returned.

@kaimen-sano
Copy link
Author

kaimen-sano commented Aug 1, 2022

MT framework doesn't give you them back, instead, it directly executes them.

Sure, it's good to execute them - it gives a deeper layer of understanding in tests. However, it's still appropriate to want the original Response back from the original message.
Can you explain why this is "not the purpose of multitest"? The problem with the current setup is that:

  • Attributes aren't good things to test explicitly, they are arbitrary attributes assigned by the contract, and do not necessarily reflect the actions taken in the execution of the contract
  • Reading the events is difficult to do correctly (reading the events to check that my contract returned 5000 uusd is difficult)

The whole point of multi-test is that it can do those deeper tests where your contract needs to do external queries, external contract executions, etc - that's why "function calls without blockchain simulation" to get the Response is not good enough (because it's actually impossible, unless you use a mock querier, which is not an appropriate solution as it's easy to mess-up, and also arbitrary).

Here's an example use-case:
My contract has an execute function which will perform a query to a cw20 token, and then add a response message which increases the allowance.
I cannot test this with simple unit testing by calling the function with mock_dependencies because it will error on the cw20 query. Making a mock querier is difficult, and easy to do incorrectly.
I cannot test this with multitest (presently), because I cannot get the resulting Response to check that my contract correctly attaches a message to increase allowance. The only way I can test this is to perform a query to the cw20 token after calling app.execute_contract, which opens another can of worms when my code may add more messages than I intended.

Adding the feature I describe will allow the best of both worlds - we get to execute the submessages and check for success, and we also get to test that our contract returned a Response that was valid.

@kaimen-sano
Copy link
Author

Hi @hashedone, looking forward to your response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants