Skip to content

Commit

Permalink
Merge pull request #633 from CosmWasm/ics20-contract-rollback
Browse files Browse the repository at this point in the history 
Ics20 contract rollback
  • Loading branch information
@ethanfrey
ethanfrey authored Jan 27, 2022
2 parents b9bf6b5 + a0ca1dc commit 7523406
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 61 deletions.
142 changes: 82 additions & 60 deletions contracts/cw20-ics20/src/ibc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@ use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use cosmwasm_std::{
attr, entry_point, from_binary, to_binary, BankMsg, Binary, ContractResult, Deps, DepsMut, Env,
IbcBasicResponse, IbcChannel, IbcChannelCloseMsg, IbcChannelConnectMsg, IbcChannelOpenMsg,
IbcEndpoint, IbcOrder, IbcPacket, IbcPacketAckMsg, IbcPacketReceiveMsg, IbcPacketTimeoutMsg,
IbcReceiveResponse, Reply, Response, StdResult, SubMsg, Uint128, WasmMsg,
attr, entry_point, from_binary, to_binary, BankMsg, Binary, ContractResult, CosmosMsg, Deps,
DepsMut, Env, IbcBasicResponse, IbcChannel, IbcChannelCloseMsg, IbcChannelConnectMsg,
IbcChannelOpenMsg, IbcEndpoint, IbcOrder, IbcPacket, IbcPacketAckMsg, IbcPacketReceiveMsg,
IbcPacketTimeoutMsg, IbcReceiveResponse, Reply, Response, SubMsg, Uint128, WasmMsg,
};

use crate::amount::Amount;
use crate::error::{ContractError, Never};
use crate::state::{ChannelInfo, ALLOW_LIST, CHANNEL_INFO, CHANNEL_STATE};
use crate::state::{
increase_channel_balance, reduce_channel_balance, undo_reduce_channel_balance, ChannelInfo,
ReplyArgs, ALLOW_LIST, CHANNEL_INFO, REPLY_ARGS,
};
use cw20::Cw20ExecuteMsg;

pub const ICS20_VERSION: &str = "ics20-1";
Expand Down Expand Up @@ -72,21 +75,44 @@ fn ack_fail(err: String) -> Binary {
to_binary(&res).unwrap()
}

const SEND_TOKEN_ID: u64 = 1337;
const RECEIVE_ID: u64 = 1337;
const ACK_FAILURE_ID: u64 = 0xfa17;

#[cfg_attr(not(feature = "library"), entry_point)]
pub fn reply(_deps: DepsMut, _env: Env, reply: Reply) -> Result<Response, ContractError> {
if reply.id != SEND_TOKEN_ID {
return Err(ContractError::UnknownReplyId { id: reply.id });
pub fn reply(deps: DepsMut, _env: Env, reply: Reply) -> Result<Response, ContractError> {
match reply.id {
RECEIVE_ID => match reply.result {
ContractResult::Ok(_) => Ok(Response::new()),
ContractResult::Err(err) => {
// Important design note: with ibcv2 and wasmd 0.22 we can implement this all much easier.
// No reply needed... the receive function and submessage should return error on failure and all
// state gets reverted with a proper app-level message auto-generated

// Since we need compatibility with Juno (Jan 2022), we need to ensure that optimisitic
// state updates in ibc_packet_receive get reverted in the (unlikely) chance of an
// error while sending the token

// However, this requires passing some state between the ibc_packet_receive function and
// the reply handler. We do this with a singleton, with is "okay" for IBC as there is no
// reentrancy on these functions (cannot be called by another contract). This pattern
// should not be used for ExecuteMsg handlers
let reply_args = REPLY_ARGS.load(deps.storage)?;
undo_reduce_channel_balance(
deps.storage,
&reply_args.channel,
&reply_args.denom,
reply_args.amount,
)?;

Ok(Response::new().set_data(ack_fail(err)))
}
},
ACK_FAILURE_ID => match reply.result {
ContractResult::Ok(_) => Ok(Response::new()),
ContractResult::Err(err) => Ok(Response::new().set_data(ack_fail(err))),
},
_ => Err(ContractError::UnknownReplyId { id: reply.id }),
}
let res = match reply.result {
ContractResult::Ok(_) => Response::new(),
ContractResult::Err(err) => {
// encode an acknowledgement error
Response::new().set_data(ack_fail(err))
}
};
Ok(res)
}

#[cfg_attr(not(feature = "library"), entry_point)]
Expand Down Expand Up @@ -212,27 +238,26 @@ fn do_ibc_packet_receive(
// If it originated on our chain, it looks like "port/channel/ucosm".
let denom = parse_voucher_denom(&msg.denom, &packet.src)?;

CHANNEL_STATE.update(
deps.storage,
(&channel, denom),
|orig| -> Result<_, ContractError> {
// this will return error if we don't have the funds there to cover the request (or no denom registered)
let mut cur = orig.ok_or(ContractError::InsufficientFunds {})?;
cur.outstanding = cur
.outstanding
.checked_sub(msg.amount)
.or(Err(ContractError::InsufficientFunds {}))?;
Ok(cur)
},
)?;
// make sure we have enough balance for this
reduce_channel_balance(deps.storage, &channel, denom, msg.amount)?;

// we need to save the data to update the balances in reply
let reply_args = ReplyArgs {
channel,
denom: denom.to_string(),
amount: msg.amount,
};
REPLY_ARGS.save(deps.storage, &reply_args)?;

let to_send = Amount::from_parts(denom.to_string(), msg.amount);
let gas_limit = check_gas_limit(deps.as_ref(), &to_send)?;
let send = send_amount(to_send, msg.receiver.clone(), gas_limit);
let send = send_amount(to_send, msg.receiver.clone());
let mut submsg = SubMsg::reply_on_error(send, RECEIVE_ID);
submsg.gas_limit = gas_limit;

let res = IbcReceiveResponse::new()
.set_ack(ack_success())
.add_submessage(send)
.add_submessage(submsg)
.add_attribute("action", "receive")
.add_attribute("sender", msg.sender)
.add_attribute("receiver", msg.receiver)
Expand Down Expand Up @@ -264,7 +289,9 @@ pub fn ibc_packet_ack(
_env: Env,
msg: IbcPacketAckMsg,
) -> Result<IbcBasicResponse, ContractError> {
// TODO: trap error like in receive?
// Design decision: should we trap error like in receive?
// TODO: unsure... as it is now a failed ack handling would revert the tx and would be
// retried again and again. is that good?
let ics20msg: Ics20Ack = from_binary(&msg.acknowledgement.data)?;
match ics20msg {
Ics20Ack::Result(_) => on_packet_success(deps, msg.original_packet),
Expand All @@ -279,7 +306,7 @@ pub fn ibc_packet_timeout(
_env: Env,
msg: IbcPacketTimeoutMsg,
) -> Result<IbcBasicResponse, ContractError> {
// TODO: trap error like in receive?
// TODO: trap error like in receive? (same question as ack above)
let packet = msg.packet;
on_packet_failure(deps, packet, "timeout".to_string())
}
Expand All @@ -297,15 +324,8 @@ fn on_packet_success(deps: DepsMut, packet: IbcPacket) -> Result<IbcBasicRespons
attr("success", "true"),
];

let channel = packet.src.channel_id;
let denom = msg.denom;
let amount = msg.amount;
CHANNEL_STATE.update(deps.storage, (&channel, &denom), |orig| -> StdResult<_> {
let mut state = orig.unwrap_or_default();
state.outstanding += amount;
state.total_sent += amount;
Ok(state)
})?;
// we have made a proper transfer, record this
increase_channel_balance(deps.storage, &packet.src.channel_id, &msg.denom, msg.amount)?;

Ok(IbcBasicResponse::new().add_attributes(attributes))
}
Expand All @@ -320,11 +340,13 @@ fn on_packet_failure(

let to_send = Amount::from_parts(msg.denom.clone(), msg.amount);
let gas_limit = check_gas_limit(deps.as_ref(), &to_send)?;
let send = send_amount(to_send, msg.sender.clone(), gas_limit);
let send = send_amount(to_send, msg.sender.clone());
let mut submsg = SubMsg::reply_on_error(send, ACK_FAILURE_ID);
submsg.gas_limit = gas_limit;

// similar event messages like ibctransfer module
let res = IbcBasicResponse::new()
.add_submessage(send)
.add_submessage(submsg)
.add_attribute("action", "acknowledge")
.add_attribute("sender", msg.sender)
.add_attribute("receiver", msg.receiver)
Expand All @@ -336,28 +358,24 @@ fn on_packet_failure(
Ok(res)
}

fn send_amount(amount: Amount, recipient: String, gas_limit: Option<u64>) -> SubMsg {
fn send_amount(amount: Amount, recipient: String) -> CosmosMsg {
match amount {
Amount::Native(coin) => SubMsg::reply_on_error(
BankMsg::Send {
to_address: recipient,
amount: vec![coin],
},
SEND_TOKEN_ID,
),
Amount::Native(coin) => BankMsg::Send {
to_address: recipient,
amount: vec![coin],
}
.into(),
Amount::Cw20(coin) => {
let msg = Cw20ExecuteMsg::Transfer {
recipient,
amount: coin.amount,
};
let exec = WasmMsg::Execute {
WasmMsg::Execute {
contract_addr: coin.address,
msg: to_binary(&msg).unwrap(),
funds: vec![],
};
let mut sub = SubMsg::reply_on_error(exec, SEND_TOKEN_ID);
sub.gas_limit = gas_limit;
sub
}
.into()
}
}
}
Expand Down Expand Up @@ -413,7 +431,7 @@ mod test {
msg: to_binary(&msg).unwrap(),
funds: vec![],
};
let mut msg = SubMsg::reply_on_error(exec, SEND_TOKEN_ID);
let mut msg = SubMsg::reply_on_error(exec, RECEIVE_ID);
msg.gas_limit = gas_limit;
msg
}
Expand All @@ -424,7 +442,7 @@ mod test {
to_address: recipient.into(),
amount: coins(amount, denom),
},
SEND_TOKEN_ID,
RECEIVE_ID,
)
}

Expand Down Expand Up @@ -531,6 +549,8 @@ mod test {
let ack: Ics20Ack = from_binary(&res.acknowledgement).unwrap();
matches!(ack, Ics20Ack::Result(_));

// TODO: we need to call the reply block

// query channel state
let state = query_channel(deps.as_ref(), send_channel.to_string()).unwrap();
assert_eq!(state.balances, vec![Amount::cw20(111111111, cw20_addr)]);
Expand Down Expand Up @@ -585,6 +605,8 @@ mod test {
let ack: Ics20Ack = from_binary(&res.acknowledgement).unwrap();
matches!(ack, Ics20Ack::Result(_));

// TODO: we must call the reply block

// query channel state
let state = query_channel(deps.as_ref(), send_channel.to_string()).unwrap();
assert_eq!(state.balances, vec![Amount::native(111111111, denom)]);
Expand Down
66 changes: 65 additions & 1 deletion contracts/cw20-ics20/src/state.rs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use cosmwasm_std::{Addr, IbcEndpoint, Uint128};
use crate::ContractError;
use cosmwasm_std::{Addr, IbcEndpoint, StdResult, Storage, Uint128};
use cw_storage_plus::{Item, Map};

pub const CONFIG: Item<Config> = Item::new("ics20_config");

// Used to pass info from the ibc_packet_receive to the reply handler
pub const REPLY_ARGS: Item<ReplyArgs> = Item::new("reply_args");

/// static info on one channel that doesn't change
pub const CHANNEL_INFO: Map<&str, ChannelInfo> = Map::new("channel_info");

Expand Down Expand Up @@ -41,3 +45,63 @@ pub struct ChannelInfo {
pub struct AllowInfo {
pub gas_limit: Option<u64>,
}

#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
pub struct ReplyArgs {
pub channel: String,
pub denom: String,
pub amount: Uint128,
}

pub fn increase_channel_balance(
storage: &mut dyn Storage,
channel: &str,
denom: &str,
amount: Uint128,
) -> Result<(), ContractError> {
CHANNEL_STATE.update(storage, (channel, denom), |orig| -> StdResult<_> {
let mut state = orig.unwrap_or_default();
state.outstanding += amount;
state.total_sent += amount;
Ok(state)
})?;
Ok(())
}

pub fn reduce_channel_balance(
storage: &mut dyn Storage,
channel: &str,
denom: &str,
amount: Uint128,
) -> Result<(), ContractError> {
CHANNEL_STATE.update(
storage,
(channel, denom),
|orig| -> Result<_, ContractError> {
// this will return error if we don't have the funds there to cover the request (or no denom registered)
let mut cur = orig.ok_or(ContractError::InsufficientFunds {})?;
cur.outstanding = cur
.outstanding
.checked_sub(amount)
.or(Err(ContractError::InsufficientFunds {}))?;
Ok(cur)
},
)?;
Ok(())
}

// this is like increase, but it only "un-subtracts" (= adds) outstanding, not total_sent
// calling `reduce_channel_balance` and then `undo_reduce_channel_balance` should leave state unchanged.
pub fn undo_reduce_channel_balance(
storage: &mut dyn Storage,
channel: &str,
denom: &str,
amount: Uint128,
) -> Result<(), ContractError> {
CHANNEL_STATE.update(storage, (channel, denom), |orig| -> StdResult<_> {
let mut state = orig.unwrap_or_default();
state.outstanding += amount;
Ok(state)
})?;
Ok(())
}

0 comments on commit 7523406

Please sign in to comment.