Skip to content

Commit

Permalink
update from audit
Browse files Browse the repository at this point in the history
  • Loading branch information
tubackkhoa committed Jun 25, 2024
2 parents 630c7f4 + b66a998 commit 2cfbcf7
Show file tree
Hide file tree
Showing 23 changed files with 1,062 additions and 1,655 deletions.
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,7 @@ libwasmvm_muslc.a
tests/test-data.json
**/artifacts/*
!**/artifacts/*.wasm
.env
.env

.scannerwork/
clippy-report.json
65 changes: 32 additions & 33 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,17 @@ resolver = "2"

[workspace.dependencies]
cosmwasm-testing-util = { git = "https://github.com/oraichain/cosmwasm-testing-util.git", rev = "24c138c" }
oraiswap = { git = "https://github.com/oraichain/oraiswap.git", rev = "03f4955b" }
oraiswap = { git = "https://github.com/oraichain/oraiswap.git", rev = "29decac" }
cw2 = { version = "1.0.1" }
cw20 = { version = "1.0.1" }
cw20-base = { version = "1.0.1" }
cw-storage-plus = { version = "1.0.1" }
cw-controllers = { version = "1.0.1" }
cw-utils = "0.16.0"
cw20-ics20-msg = { path = "./packages/cw20-ics20-msg" }
cosmwasm-schema = { version = "1.2.8" }
cosmwasm-std = { version = "1.2.8", default-features = false }
cosmwasm-vm = { version = "1.2.8" }
# osmosis-test-tube = { git = "https://github.com/oraichain/test-tube.git", rev = "354d580" }

[profile.release]
Expand Down
16 changes: 5 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Let's assume the network that has the contract cw20-ics20 deployed is network B,

In the source code, we call the network having cw20-ics20 deployed local chain, other networks are remote chains.

As of now, we only support A bridging to B first, not the other way around.

In the cw-ics20-latest contract, there are couple transfer flows in the code below:

## Network A transfers native tokens to B first (A->B, where native token is not IBC token)
Expand All @@ -14,20 +16,12 @@ Here, if any line returns an error, then the cw-ics20 contract will send a fail

## Network A transfers IBC tokens to B (A->B, where IBC tokens are tokens that were previously sent from B to A)

In this case, the packet is also caught in the `do_ibc_packet_receive`, but we parse the ibc denom to get the original denom instead, and send the same amount that is stored in the cw-ics20 contract to the receiver using SubMsg (with reply on error). This is the logic that the CosmWasm developers wrote.
Currently not supported.

## Network B transfers tokens to A (B->A, where tokens are either native or cw20 that originated from B)

In this case, the user will deposit his tokens into the cw-ics20 contract, then the contract will create a new IBCPacket and transfer it to the network A. The cw-ics20 contract then invokes the `ibc_packet_ack` function, which has the acknowledgement packet sent from A. If the ack is a failure, then we refund (using SubMsg with reply on error) the original sender by using the tokens stored on the cw-ics20 contract. This is also the logic that CosmWasm developers wrote.
Currently not supported.

## Network B transfers tokens to A (B->A, where tokens are native tokens from A)

In this case, the user will deposit tokens to the `allow_contract`, and the allow contract will call the `execute_transfer_back_to_remote_chain` function. Here, the cw-ics20 contract will not hold any tokens. Instead, it only forwards the logic to the chain A. The cw-ics20 contract then invokes the `ibc_packet_ack` function, which has the acknowledgement packet sent from A. If the ack is a failure, then we refund by calling a message to the `allow_contract` requesting for a refund. We use the `CosmosMsg` instead of the `SubMsg`.

This is really important because by using the CosmosMsg, we force the `allow_contract` to actually refunds successfully. If we use SubMsg, then it could be that the `allow_contract` fails somewhere, and only its state gets reverted, aka the sender receives no refunds.

If we use CosmosMsg, then the acknowledgement packet will fail entirely, and it will be retried by the relayer as long as we fix the `allow_contract`.

Normally, if it is a `ibctransfer` application developed as a submodule in Cosmos SDK, then the refund part must not fail, and we can trust that it will not fail. However, the `allow_contract` can be developed by anyone, and can be replaced => cannot be trusted.

# build packages
In this case, the user will deposit tokens and invoke the function `execute_transfer_back_to_remote_chain` function. It will create an IBC Send packet and lock the cw20 or native token in the cw20-ics20 contract. If there's a failed acknowledgement, the contract will try to automatically refund the locked tokens back to the sender. If the refund msg is failed, then the token will be locked in the contract, and the team will refund these tokens manually for security purposes.
19 changes: 9 additions & 10 deletions contracts/cw-ics20-latest/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,20 @@ backtraces = ["cosmwasm-std/backtraces"]
library = []

[dependencies]
cosmwasm-schema = "=1.2"
cw-utils = "0.16.0"
cw2 = "1.0.1"
cw20 = "1.0.1"
cw20-ics20-msg = { path = "../../packages/cw20-ics20-msg" }
cosmwasm-schema = { workspace = true }
cw-utils = { workspace = true }
cw2 = { workspace = true }
cw20 = { workspace = true }
cw20-ics20-msg = { workspace = true }
oraiswap = { workspace = true }
cosmwasm-std = { version = "=1.2", features = ["ibc3"] }
cw-storage-plus = "1.0.1"
cw-controllers = "1.0.1"
cosmwasm-std = { workspace = true, features = ["ibc3"] }
cw-storage-plus = { workspace = true }
cw-controllers = { workspace = true }
thiserror = { version = "1.0.23" }
sha256 = "=1.1.0"

[dev-dependencies]
cosmwasm-vm = { version = "=1.2" }
cosmwasm-vm = { workspace = true }
# osmosis-test-tube = { workspace = true }
cosmwasm-testing-util = { workspace = true }
anybuf = "0.3.0"
cw-multi-test = "0.16.0"
22 changes: 7 additions & 15 deletions contracts/cw-ics20-latest/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use cosmwasm_std::{
WasmMsg,
};
use cw2::set_contract_version;
use cw20::{Cw20Coin, Cw20ExecuteMsg, Cw20ReceiveMsg};
use cw20::{Cw20ExecuteMsg, Cw20ReceiveMsg};
use cw20_ics20_msg::converter::ConverterController;
use cw20_ics20_msg::helper::parse_ibc_wasm_port_id;
use cw_storage_plus::Bound;
Expand Down Expand Up @@ -239,7 +239,7 @@ pub fn handle_increase_channel_balance_ibc_receive(
let reply_args = ReplyArgs {
channel: dst_channel_id.clone(),
denom: ibc_denom.clone(),
amount: remote_amount.clone(),
amount: remote_amount,
local_receiver: local_receiver.clone(),
};
REPLY_ARGS.save(deps.storage, &reply_args)?;
Expand Down Expand Up @@ -373,10 +373,7 @@ pub fn execute_receive(
) -> Result<Response, ContractError> {
nonpayable(&info)?;

let amount = Amount::Cw20(Cw20Coin {
address: info.sender.to_string(),
amount: wrapper.amount,
});
let amount = Amount::cw20(wrapper.amount, info.sender);
let api = deps.api;

let msg: TransferBackMsg = from_binary(&wrapper.msg)?;
Expand Down Expand Up @@ -483,21 +480,16 @@ pub fn execute_transfer_back_to_remote_chain(
let mapping = mappings
.into_iter()
.find(|pair| -> bool {
let (denom, is_native) = parse_voucher_denom(
match parse_voucher_denom(
pair.key.as_str(),
&IbcEndpoint {
port_id: parse_ibc_wasm_port_id(env.contract.address.as_str()),
channel_id: msg.local_channel_id.clone(), // also verify local channel id
},
)
.unwrap_or_else(|_| ("", true)); // if there's an error, change is_native to true so it automatically returns false
if is_native {
return false;
) {
Ok((denom, false)) => msg.remote_denom.eq(denom),
_ => false,
}
if denom.eq(&msg.remote_denom) {
return true;
}
return false;
})
.ok_or(ContractError::MappingPairNotFound {})?;

Expand Down
Loading

0 comments on commit 2cfbcf7

Please sign in to comment.