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

Fix instaniate reply data #517

Merged
merged 8 commits into from
Oct 28, 2021
Merged

Fix instaniate reply data #517

merged 8 commits into from
Oct 28, 2021

Conversation

ethanfrey
Copy link
Member

@ethanfrey ethanfrey commented Oct 27, 2021

Par one of #516

This properly handles the instantiate wrapping, even when they set data in reply.

I started working on the envelope for WasmMsg::Execute and WasmMsg::Migrate, but that broke quite some tests. Let's merge this first (as it caused issues testing some PRs) and then do the exec ones in a follow up PR.

@ethanfrey ethanfrey force-pushed the 516-demo-reply-errors branch from 8ef0e6d to 8c09265 Compare October 28, 2021 10:21
@ethanfrey ethanfrey changed the title Demo reply errors Fix instaniate reply data Oct 28, 2021
Copy link
Contributor

@maurolacy maurolacy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the main changes, in the sequence of operations; approving on your behalf.

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"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what about the top level data?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contract returned "remove-me" on the message, it was overwritten with "babble" via the reply.

This is what I wanted to test. I guess the comment could be improved

@maurolacy maurolacy force-pushed the 516-demo-reply-errors branch from 8c09265 to 7b3b1cf Compare October 28, 2021 13:53
@ethanfrey ethanfrey merged commit 9a4d36d into main Oct 28, 2021
@ethanfrey ethanfrey deleted the 516-demo-reply-errors branch October 28, 2021 14:13
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

Successfully merging this pull request may close these issues.

2 participants