-
Notifications
You must be signed in to change notification settings - Fork 346
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 Ibc sample contract #714
Conversation
df092b4
to
ebb09be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice stuff
35d07ab
to
911187c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool 🎸
|
||
#[entry_point] | ||
/// we do nothing | ||
/// TODO: remove the account from the lookup? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you would propagated this event to the "reflect" contract then it would be able to do some "cleanup" as well. For example if the contract owns tokens for whatever reasons it could return them and would not lock them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would never actually be called.
The flow I have is "to-be-defined-sender-contract on chain A" sends a packet to "ibc-reflect on chain B", which dispatches it.
ibc-reflect
never sends a packet itself, thus should never get these two callbacks (we could error here I guess?)
// and we are golden | ||
Ok(IbcReceiveResponse { | ||
acknowledgement, | ||
messages: vec![wasm_msg.into()], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can be ensure that only "authorized" messages are passed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we create a channel, there is exactly one contract/module on the other side that can send packets on it, correct? (Please double-check this - this is key to all my authorization reasoning).
When they connect, we give them a fresh address that is theirs. Any messages they send get reflected by that custom address. Thus the channel -> address lookup.
If sending packets on the channel is not as limited as I believe, I need to revisit this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: We really need local (port_id, channel_id)
pairs for uniqueness.
However, the receiving contract only has one port, so by fixing the channel_id, we fix the pair.
5ce3369
to
b87385f
Compare
Okay, I think this is done (and a bit of refactoring for future contracts as well). Happy for a final review, then I can merge it and use as a demo contract for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
4983720
to
9cbc21d
Compare
This should be ready for a merge now. I will start integrating the last commit (62b9fc7) into CosmWasm/wasmvm#167 |
Addresses #692
It builds on some other items:
#711 to define the entry points we use (MERGED)
#716 to add stargate support and ibc entry points to the vm (MERGED)
ibc-reflect
contractNote: a unit test brought up CosmWasm/serde-json-wasm#27 but this is not blocked on that (it parses correctly in the vm code, and we never parse this type in contracts).