From 4fe62cc0d8594ddbe9ef1fa6e30aeb959eeb5d35 Mon Sep 17 00:00:00 2001 From: ducphamle2 Date: Tue, 1 Aug 2023 21:40:28 +0700 Subject: [PATCH] removed auto refund when handle_ibc_packet_receive_native_remote_chain --- contracts/cw-ics20-latest/src/ibc.rs | 58 ++++++++--------- contracts/cw-ics20-latest/src/ibc_tests.rs | 74 ++++++++++++---------- 2 files changed, 67 insertions(+), 65 deletions(-) diff --git a/contracts/cw-ics20-latest/src/ibc.rs b/contracts/cw-ics20-latest/src/ibc.rs index a8a5db0..0802f3b 100644 --- a/contracts/cw-ics20-latest/src/ibc.rs +++ b/contracts/cw-ics20-latest/src/ibc.rs @@ -670,7 +670,7 @@ pub fn build_ibc_msg( .eq(&destination.destination_channel) }) .ok_or(StdError::generic_err("cannot find pair mappings"))?; - let msg: CosmosMsg = process_ibc_msg( + return process_ibc_msg( storage, mapping, receiver_asset_info, @@ -681,9 +681,7 @@ pub fn build_ibc_msg( amount, timeout, reply_args, - )? - .into(); - return Ok(SubMsg::reply_on_error(msg, NATIVE_RECEIVE_ID)); + ); } // 2nd case, where destination network is not evm, but it is still supported on our channel (eg: cw20 ATOM mapped with native ATOM on Cosmos), then we call let is_cosmos_based = destination.is_receiver_cosmos_based(); @@ -696,7 +694,7 @@ pub fn build_ibc_msg( .eq(&destination.destination_channel) }); if let Some(mapping) = mapping { - let msg: CosmosMsg = process_ibc_msg( + return process_ibc_msg( storage, mapping, receiver_asset_info, @@ -707,9 +705,7 @@ pub fn build_ibc_msg( amount, timeout, reply_args, - )? - .into(); - return Ok(SubMsg::reply_on_error(msg, NATIVE_RECEIVE_ID)); + ); } // final case, where the destination token is from a remote chain that we dont have a pair mapping with. @@ -745,7 +741,7 @@ pub fn process_ibc_msg( amount: Uint128, timeout: Timestamp, mut reply_args: SingleStepReplyArgs, -) -> StdResult { +) -> StdResult { let (new_deducted_amount, _) = deduct_token_fee( storage, parse_ibc_denom_without_sanity_checks(&pair_mapping.0)?, // denom mapping in the form port/channel/denom @@ -769,7 +765,7 @@ pub fn process_ibc_msg( .map_err(|err| StdError::generic_err(err.to_string()))?; // prepare ibc message - let msg = build_ibc_send_packet( + let msg: CosmosMsg = build_ibc_send_packet( remote_amount, &pair_mapping.0, ibc_msg_sender, @@ -777,7 +773,8 @@ pub fn process_ibc_msg( memo, src_channel, timeout.into(), - )?; + )? + .into(); reply_args.channel = src_channel.to_string(); reply_args.ibc_data = Some(IbcSingleStepData { @@ -786,13 +783,13 @@ pub fn process_ibc_msg( }); // keep track of the reply. We need to keep a seperate value because if using REPLY, it could be overriden by the channel increase later on in reply SINGLE_STEP_REPLY_ARGS.save(storage, &reply_args)?; - Ok(msg) + Ok(SubMsg::reply_on_error(msg, FOLLOW_UP_ERROR_ID)) } pub fn handle_follow_up_failure( storage: &mut dyn Storage, reply_args: SingleStepReplyArgs, - err: String, + _err: String, ) -> Result { // if there's an error but no ibc msg aka no channel balance reduce => wont undo reduce let mut response: Response = Response::new(); @@ -810,23 +807,24 @@ pub fn handle_follow_up_failure( attr("undo_reduce_channel_balance", ibc_data.remote_amount), ]); } - let refund_amount = Amount::from_parts( - parse_asset_info_denom(reply_args.refund_asset_info.clone()), - reply_args.local_amount, - ); - // we send refund to the local receiver of the single-step tx because the funds are currently in this contract - let send = refund_amount.send_amount(reply_args.receiver, None); - response = response - .add_submessage(SubMsg::reply_on_error(send, REFUND_FAILURE_ID)) - .set_data(ack_fail(err.clone())) - .add_attributes(vec![ - attr("error_follow_up_msgs", err), - attr( - "attempt_refund_denom", - reply_args.refund_asset_info.to_string(), - ), - attr("attempt_refund_amount", reply_args.local_amount), - ]); + // remove refunding because there can be unexpected risks. Will refund manually if this happens + // let refund_amount = Amount::from_parts( + // parse_asset_info_denom(reply_args.refund_asset_info.clone()), + // reply_args.local_amount, + // ); + // // we send refund to the local receiver of the single-step tx because the funds are currently in this contract + // let send = refund_amount.send_amount(reply_args.receiver, None); + // response = response + // .add_submessage(SubMsg::reply_on_error(send, REFUND_FAILURE_ID)) + // .set_data(ack_fail(err.clone())) + // .add_attributes(vec![ + // attr("error_follow_up_msgs", err), + // attr( + // "attempt_refund_denom", + // reply_args.refund_asset_info.to_string(), + // ), + // attr("attempt_refund_amount", reply_args.local_amount), + // ]); Ok(response) } diff --git a/contracts/cw-ics20-latest/src/ibc_tests.rs b/contracts/cw-ics20-latest/src/ibc_tests.rs index 09579d5..b41c24b 100644 --- a/contracts/cw-ics20-latest/src/ibc_tests.rs +++ b/contracts/cw-ics20-latest/src/ibc_tests.rs @@ -10,8 +10,8 @@ mod test { convert_remote_denom_to_evm_prefix, deduct_fee, deduct_relayer_fee, deduct_token_fee, handle_follow_up_failure, ibc_packet_receive, is_follow_up_msgs_only_send_amount, parse_ibc_channel_without_sanity_checks, parse_ibc_denom_without_sanity_checks, - parse_voucher_denom, process_ibc_msg, Ics20Ack, Ics20Packet, IBC_TRANSFER_NATIVE_ERROR_ID, - NATIVE_RECEIVE_ID, REFUND_FAILURE_ID, + parse_voucher_denom, process_ibc_msg, Ics20Ack, Ics20Packet, FOLLOW_UP_ERROR_ID, + IBC_TRANSFER_NATIVE_ERROR_ID, NATIVE_RECEIVE_ID, REFUND_FAILURE_ID, }; use crate::ibc::{build_swap_operations, get_follow_up_msgs}; use crate::test_helpers::*; @@ -830,7 +830,7 @@ mod test { .unwrap(), timeout: env.block.time.plus_seconds(timeout).into() }), - NATIVE_RECEIVE_ID + FOLLOW_UP_ERROR_ID ) ); let reply_args = SINGLE_STEP_REPLY_ARGS.load(deps.as_mut().storage).unwrap(); @@ -977,7 +977,7 @@ mod test { .unwrap(), timeout: env.block.time.plus_seconds(timeout).into() }), - NATIVE_RECEIVE_ID + FOLLOW_UP_ERROR_ID ) ); } @@ -1169,24 +1169,25 @@ mod test { err.to_string(), ) .unwrap(); - assert_eq!( - result, - Response::new() - .add_submessage(SubMsg::reply_on_error( - Amount::from_parts(native_denom.to_string(), amount.clone()) - .send_amount(single_step_reply_args.receiver.clone(), None), - REFUND_FAILURE_ID - )) - .set_data(ack_fail(err.to_string())) - .add_attributes(vec![ - attr("error_follow_up_msgs", err), - attr( - "attempt_refund_denom", - single_step_reply_args.refund_asset_info.to_string(), - ), - attr("attempt_refund_amount", single_step_reply_args.local_amount), - ]) - ); + // assert_eq!( + // result, + // Response::new() + // .add_submessage(SubMsg::reply_on_error( + // Amount::from_parts(native_denom.to_string(), amount.clone()) + // .send_amount(single_step_reply_args.receiver.clone(), None), + // REFUND_FAILURE_ID + // )) + // .set_data(ack_fail(err.to_string())) + // .add_attributes(vec![ + // attr("error_follow_up_msgs", err), + // attr( + // "attempt_refund_denom", + // single_step_reply_args.refund_asset_info.to_string(), + // ), + // attr("attempt_refund_amount", single_step_reply_args.local_amount), + // ]) + // ); + assert_eq!(result, Response::new()); let ibc_denom = "ibc_denom"; let remote_amount = convert_local_to_remote(amount, 18, 6).unwrap(); single_step_reply_args.ibc_data = Some(IbcSingleStepData { @@ -1525,18 +1526,21 @@ mod test { ); assert_eq!( result, - IbcMsg::SendPacket { - channel_id: local_channel_id.to_string(), - data: to_binary(&Ics20Packet { - amount: remote_amount.clone(), - denom: ibc_denom.to_string(), - receiver: ibc_msg_receiver.to_string(), - sender: ibc_msg_sender.to_string(), - memo: None - }) - .unwrap(), - timeout: IbcTimeout::with_timestamp(timeout) - } - ); + SubMsg::reply_on_error( + IbcMsg::SendPacket { + channel_id: local_channel_id.to_string(), + data: to_binary(&Ics20Packet { + amount: remote_amount.clone(), + denom: ibc_denom.to_string(), + receiver: ibc_msg_receiver.to_string(), + sender: ibc_msg_sender.to_string(), + memo: None + }) + .unwrap(), + timeout: IbcTimeout::with_timestamp(timeout) + }, + FOLLOW_UP_ERROR_ID + ) + ) } }