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

Add stargate msg #706

Merged
merged 3 commits into from
Jan 12, 2021
Merged

Add stargate msg #706

merged 3 commits into from
Jan 12, 2021

Conversation

ethanfrey
Copy link
Member

Closes #694

Adds the message and the query types under a feature flag. And also add the proper requires export.

Contracts have it off by default and cargo test --features stargate passes in cosmwasm-std.

This is really just to pass info up to wasmd, and doesn't need test mocks (they would be too hard to build)

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Looks good.

But if those messages don't go through signature verification, how is authorization checked (e.g. the MsgSend sender address)?

packages/std/src/results/cosmos_msg.rs Outdated Show resolved Hide resolved
packages/std/src/query.rs Show resolved Hide resolved
path: String,
/// this is the expected protobuf message type (not any), binary encoded
data: Binary,
},
Copy link
Member

Choose a reason for hiding this comment

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

This is analogue to this call in CosmJS, right?

  public async queryUnverified(path: string, request: Uint8Array): Promise<Uint8Array> {
    const response = await this.tmClient.abciQuery({
      path: path,
      data: request,
      prove: false,
    });

    if (response.code) {
      throw new Error(`Query failed with (${response.code}): ${response.log}`);
    }

    return response.value;
  }

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes 💯 We can provide a link to that code which may help make it more clear

Copy link
Member

Choose a reason for hiding this comment

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

Good, thanks. Then we can avoid the term "message" here, right? It is just "request data" of any kind.

@ethanfrey
Copy link
Member Author

ethanfrey commented Jan 12, 2021

But if those messages don't go through signature verification, how is authorization checked (e.g. the MsgSend sender address)?

This code handles redispatching the CosmosMsg returned from contracts.

Since we know who the caller is (the contract returning the message), we can easily enforce that is a valid signer, the same way they handle this in the sdk.

(This is done for all the messages, which is why we don't require sender fields on them - or auto-enforced the from_address of the BankMsg::Send)

@ethanfrey
Copy link
Member Author

I think 8b7bc41 addresses all the comments.

@ethanfrey
Copy link
Member Author

Any more changes required?

I will make another small PR on top of this now.

@webmaster128 webmaster128 added the automerge See mergify.io label Jan 12, 2021
@mergify mergify bot merged commit 969bb8b into master Jan 12, 2021
@mergify mergify bot deleted the add-stargate-msg branch January 12, 2021 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge See mergify.io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new StargateMsg to return arbitrary protobuf messages and handle arbitrary queries
2 participants