-
Notifications
You must be signed in to change notification settings - Fork 45
IBC implementation review #234
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
base: main
Are you sure you want to change the base?
Conversation
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.
Some comments about two of the issues. Have to look deeper into the other one.
self.process_ibc_response(api, contract_addr, storage, router, block, res) | ||
} | ||
|
||
fn ibc_packet_receive( |
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.
In order to handle #197, you need to make sure that if contract.ibc_packet_receive
returns an Err
variant, this should not result in an error of this whole ibc_packet_receive
function. It should instead return a json-encoded StdAck::Error(err_msg)
where err_msg
is the stringified error from the contract.
as documented in the CosmWasm IBC docs and implemented in Wasmd.
// No acknowledgment needed | ||
Ok(AppIbcReceiveResponse::default()) |
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.
For #194, this should be the success ack (don't know about events or if we care to emulate the correct values there):
// No acknowledgment needed | |
Ok(AppIbcReceiveResponse::default()) | |
Ok(AppIbcReceiveResponse { | |
events: vec![], | |
acknowledgement: Some(StdAck::success(b"\x01").to_binary()), | |
}) |
No description provided.