-
Notifications
You must be signed in to change notification settings - Fork 355
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 ics20 denom #244
Fix ics20 denom #244
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.
Very nice. Please see the comment about protocol/app level errors
@@ -130,6 +131,17 @@ pub fn execute_transfer( | |||
Ok(res) | |||
} | |||
|
|||
#[cfg_attr(not(feature = "library"), entry_point)] | |||
pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result<Response, ContractError> { |
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.
🤗 Makes debugging for me so much easier
if split_denom.len() != 3 { | ||
return Err(ContractError::NoForeignTokens {}); | ||
} | ||
// a few more sanity checks |
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.
It makes sense to check all part. 👍
Ok(msg) => { | ||
// build attributes first so we don't have to clone msg below | ||
// similar event messages like ibctransfer module | ||
|
||
// This cannot fail as we parse it in do_ibc_packet_receive. Best to pass the data somehow? | ||
let denom = parse_voucher_denom(&msg.denom, &packet.src).unwrap(); |
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.
Parsing errors should be app level and stored with the event attribute. I may have run into this with an old state and could not read the event.
Properly handle parsing the voucher's denom when it is returned to us.
Thanks @alpe for finding this
port/channel/orig_denom
Also add a migrate entry point that accepts any older cw20-ics20 contract as a migration target.