Responses validation in multi-test #373
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hashedone
force-pushed
the
341-returned-items-validity
branch
from
b3c59f5
to
c2fb2e5
Compare
ethanfrey
reviewed
Aug 3, 2021
| @@ -720,6 +724,43 @@ where | |||
| let storage = ReadonlyPrefixedStorage::multilevel(storage, &[NAMESPACE_WASM, &namespace]); | |||
| Box::new(storage) | |||
| } | |||
|
|
|||
| fn verify_attributes(attributes: &[Attribute]) -> Result<(), String> { | |||
| @@ -268,6 +268,10 @@ where | |||
| funds, | |||
| label, | |||
| } => { | |||
| if label.is_empty() { | |||
| }), | ||
| .. | ||
| } = msg | ||
| { | ||
| Ok(Response::new().set_data(data)) | ||
| Ok(Response::new().set_data(data).add_events(events)) |
There was a problem hiding this comment.
I would actually not add these to the reply call. These should be appended by the runtime when there is a SubMsg executed without the reply blob doing anything
packages/multi-test/src/wasm.rs
Outdated
|
|
||
| fn verify_attributes(attributes: &[Attribute]) -> Result<(), String> { | ||
| for attr in attributes { | ||
| if attr.key.is_empty() { |
There was a problem hiding this comment.
Note in the real wasmd implementation, we trim off whitespace from the ends before checking empty, so " " is also invalid.
The relevant Go code changes are are https://github.com/CosmWasm/wasmd/pull/562/files maybe you can use them as reference
There was a problem hiding this comment.
Missed it, corrected.
packages/multi-test/src/wasm.rs
Outdated
|
|
||
| for event in &response.events { | ||
| Self::verify_attributes(&event.attributes)?; | ||
| if event.ty.len() < 2 { |
There was a problem hiding this comment.
Same thing, I think we trim whitespace before checking length
hashedone
force-pushed
the
341-returned-items-validity
branch
from
c2fb2e5
to
4fbb283
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #341
I don't like checking the errors in test by comparing strings - I would really prefer to switch to proper error types (we have
thiserroranyway), and if needed converting them to string at the end. If for simplicity we want to unify error types, then we could just go with handling them withanyhowso if needed, error types can be retrieved by downcasting.