diff --git a/Cargo.lock b/Cargo.lock index 18ae0fd8..49c4a4a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -181,9 +181,9 @@ dependencies = [ [[package]] name = "crypto-bigint" -version = "0.5.3" +version = "0.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "740fe28e594155f10cfc383984cbefd529d7396050557148f79cb0f621204124" +checksum = "0dc92fb57ca44df6db8059111ab3af99a63d5d0f8375d9972e319a379c6bab76" dependencies = [ "generic-array", "rand_core 0.6.4", @@ -239,7 +239,7 @@ checksum = "579e2c2f2c0877b839c5cad85e67811074e854a50c1ff3085eb8290b1c27809c" dependencies = [ "anyhow", "cosmwasm-std", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw-utils 1.0.2", "derivative", "itertools", @@ -260,7 +260,7 @@ dependencies = [ "cosmwasm-std", "cw-address-like", "cw-ownable-derive", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw-utils 1.0.2", "thiserror", ] @@ -279,11 +279,11 @@ dependencies = [ [[package]] name = "cw-paginate-storage" version = "2.3.0" -source = "git+https://github.com/DA0-DA0/dao-contracts.git#db42d45c097174572b55f789aa3908af591e7a29" +source = "git+https://github.com/DA0-DA0/dao-contracts.git#1187dbac2729bc0504a3d39b7f617773a8a5b6ad" dependencies = [ "cosmwasm-std", "cosmwasm-storage", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "serde", ] @@ -293,7 +293,7 @@ version = "0.1.0" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw-utils 1.0.2", "thiserror", ] @@ -305,7 +305,7 @@ source = "git+https://github.com/arkprotocol/cw721-proxy.git?tag=v0.0.7#a433953f dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "schemars", "serde", "thiserror", @@ -324,9 +324,9 @@ dependencies = [ [[package]] name = "cw-storage-plus" -version = "1.1.0" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f0e92a069d62067f3472c62e30adedb4cab1754725c0f2a682b3128d2bf3c79" +checksum = "d5ff29294ee99373e2cd5fd21786a3c0ced99a52fec2ca347d565489c61b723c" dependencies = [ "cosmwasm-std", "schemars", @@ -384,7 +384,7 @@ checksum = "9431d14f64f49e41c6ef5561ed11a5391c417d0cb16455dea8cdcb9037a8d197" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "schemars", "serde", "thiserror", @@ -442,7 +442,7 @@ dependencies = [ "cosmwasm-schema", "cosmwasm-std", "cw-ownable", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw-utils 1.0.2", "cw2 1.1.1", "cw721 0.18.0", @@ -459,7 +459,7 @@ source = "git+https://github.com/arkprotocol/cw721-proxy.git?tag=v0.0.7#a433953f dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw721 0.18.0", "cw721-proxy-derive", "thiserror", @@ -484,7 +484,7 @@ dependencies = [ "cosmwasm-schema", "cosmwasm-std", "cw-rate-limiter", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw2 1.1.1", "cw721 0.18.0", "cw721-proxy", @@ -499,7 +499,7 @@ version = "0.1.0" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw2 1.1.1", "cw721-base 0.18.0", "thiserror", @@ -555,9 +555,9 @@ checksum = "545b22097d44f8a9581187cdf93de7a71e4722bf51200cfaba810865b49a495d" [[package]] name = "ecdsa" -version = "0.16.8" +version = "0.16.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4b1e0c257a9e9f25f90ff76d7a68360ed497ee519c8e428d1825ef0000799d4" +checksum = "ee27f32b5c5292967d2d4a9d7f1e0b0aed2c15daded5a60300e4abb9d8020bca" dependencies = [ "der", "digest 0.10.7", @@ -590,9 +590,9 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "elliptic-curve" -version = "0.13.6" +version = "0.13.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d97ca172ae9dc9f9b779a6e3a65d308f2af74e5b8c921299075bdb4a0370e914" +checksum = "e9775b22bc152ad86a0cf23f0f348b884b26add12bf741e7ffc4d4ab2ab4d205" dependencies = [ "base16ct", "crypto-bigint", @@ -702,7 +702,7 @@ dependencies = [ "cw-ownable", "cw-paginate-storage", "cw-pause-once", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw-utils 1.0.2", "cw2 1.1.1", "cw721 0.16.0", @@ -733,8 +733,9 @@ dependencies = [ "cosmwasm-schema", "cosmwasm-std", "cosmwasm-storage", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw2 1.1.1", + "cw721 0.18.0", "ics721", "thiserror", ] @@ -766,9 +767,9 @@ checksum = "af150ab688ff2122fcef229be89cb50dd66af9e01a4ff320cc137eecc9bacc38" [[package]] name = "k256" -version = "0.13.1" +version = "0.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cadb76004ed8e97623117f3df85b17aaa6626ab0b0831e6573f104df16cd1bcc" +checksum = "3f01b677d82ef7a676aa37e099defd83a28e15687112cafdd112d60236b6115b" dependencies = [ "cfg-if", "ecdsa", @@ -823,9 +824,9 @@ dependencies = [ [[package]] name = "prost" -version = "0.12.1" +version = "0.12.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4fdd22f3b9c31b53c060df4a0613a1c7f062d4115a2b984dd15b1858f7e340d" +checksum = "5a5a410fc7882af66deb8d01d01737353cf3ad6204c408177ba494291a626312" dependencies = [ "bytes", "prost-derive", @@ -833,9 +834,9 @@ dependencies = [ [[package]] name = "prost-derive" -version = "0.12.1" +version = "0.12.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "265baba7fabd416cf5078179f7d2cbeca4ce7a9041111900675ea7c4cb8a4c32" +checksum = "065717a5dfaca4a83d2fe57db3487b311365200000551d7a364e715dbf4346bc" dependencies = [ "anyhow", "itertools", @@ -886,9 +887,9 @@ checksum = "1ad4cc8da4ef723ed60bced201181d83791ad433213d8c24efffda1eec85d741" [[package]] name = "schemars" -version = "0.8.15" +version = "0.8.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1f7b0ce13155372a76ee2e1c5ffba1fe61ede73fbea5630d61eee6fac4929c0c" +checksum = "45a28f4c49489add4ce10783f7911893516f15afe45d015608d41faca6bc4d29" dependencies = [ "dyn-clone", "schemars_derive", @@ -898,9 +899,9 @@ dependencies = [ [[package]] name = "schemars_derive" -version = "0.8.15" +version = "0.8.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e85e2a16b12bdb763244c69ab79363d71db2b4b918a2def53f80b02e0574b13c" +checksum = "c767fd6fa65d9ccf9cf026122c1b555f2ef9a4f0cea69da4d7dbc3e258d30967" dependencies = [ "proc-macro2", "quote", @@ -990,7 +991,7 @@ dependencies = [ "cw-cii", "cw-multi-test", "cw-pause-once", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw2 1.1.1", "cw721 0.18.0", "cw721-base 0.18.0", @@ -1041,7 +1042,7 @@ dependencies = [ "cosmwasm-schema", "cosmwasm-std", "cw-ownable", - "cw-storage-plus 1.1.0", + "cw-storage-plus 1.2.0", "cw-utils 1.0.2", "cw2 1.1.1", "cw721 0.18.0", @@ -1079,9 +1080,9 @@ dependencies = [ [[package]] name = "signature" -version = "2.1.0" +version = "2.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e1788eed21689f9cf370582dfc467ef36ed9c707f073528ddafa8d83e3b8500" +checksum = "77549399552de45a898a580c1b41d445bf730df867cc44e6c0233bbc4b8329de" dependencies = [ "digest 0.10.7", "rand_core 0.6.4", @@ -1218,9 +1219,9 @@ checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" [[package]] name = "zeroize" -version = "1.6.0" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a0956f1ba7c7909bfb66c2e9e4124ab6f6482560f6628b5aaeba39207c9aad9" +checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" [[package]] name = "zip-optional" diff --git a/README.md b/README.md index c4ed2b6b..b51ce829 100644 --- a/README.md +++ b/README.md @@ -1,19 +1,12 @@ # CW-ICS721 -This is an implementation of the [ICS 721 -specification](https://github.com/cosmos/ibc/tree/master/spec/app/ics-721-nft-transfer) -written in CosmWasm. It allows NFTs to be moved between IBC compatible -blockchains. +This is an implementation of the [ICS 721 specification](https://github.com/cosmos/ibc/tree/master/spec/app/ics-721-nft-transfer) written in CosmWasm. It allows NFTs to be moved between IBC compatible blockchains. This implementation -1. is entirely compatible with the cw721 NFT standard, the standard - used by most NFT marketplaces in the IBC ecosystem; -2. has a minimal, but powerful governance system that can quickly - pause the system in an emergency, without ceding any of the - governance module's control over the ICS721 contract; -3. supports a proxy system that allows for arbitrary filtering and - rate limiting of outgoing NFTs; +1. is entirely compatible with the cw721 NFT standard, the standard used by most NFT marketplaces in the IBC ecosystem; +2. has a minimal, but powerful governance system that can quickly pause the system in an emergency, without ceding any of the governance module's control over the ICS721 contract; +3. supports a proxy system that allows for arbitrary filtering and rate limiting of outgoing NFTs; 4. is well tested. To enable ICS721 contracts to function correctly, the app chain needs to have at least `wasmd v0.31.0` installed, with the `cosmwasm_1_2` feature enabled. This requirement arises from the fact that the ICS721 contract uses `instantiate2` for creating predicted cw721 addresses. For more detailed information, please refer to the [CHANGELOG.md](https://github.com/CosmWasm/wasmd/blob/main/CHANGELOG.md#v0310-2023-03-13) in the `wasmd` repository. @@ -45,85 +38,104 @@ This contract deals in debt-vouchers. To send a NFT from chain A to chain B: 1. The NFT is locked on chain A. -2. A message is delivered over IBC to the destination chain describing - the NFT that has been locked. -3. A debt-voucher, which is conveniently an exact replica of the NFT - locked on chain A, is minted on chain B. +2. A message is delivered over IBC to the destination chain describing the NFT that has been locked. +3. A debt-voucher, which is conveniently an exact replica of the NFT locked on chain A, is minted on chain B. -The duplicate NFT on the receiving chain is a debt-voucher. Possession -of that debt-voucher on the receiving chain gives the holder the right -to redeem it for the original NFT on chain A. +The duplicate NFT on the receiving chain is a debt-voucher. Possession of that debt-voucher on the receiving chain gives the holder the right to redeem it for the original NFT on chain A. To return the transferred NFT: 1. The debt-voucher is returned to the ICS721 contract. -2. A message is sent to the source chain informing it that the debt - voucher has been returned. +2. A message is sent to the source chain informing it that the debt voucher has been returned. 3. The original NFT is unlocked and sent to the receiver of the NFT. 4. The debt-voucher is burned on chain B. -The failure handling logic for this contract is also reasonably simple -to explain: if the receiver does not process the packet correctly, the -NFT sent to the ICS721 contract is returned to the sender as if the transfer -had never happened. +The failure handling logic for this contract is also reasonably simple to explain: if the receiver does not process the packet correctly, the NFT sent to the ICS721 contract is returned to the sender as if the transfer had never happened. ## From closer to the ground -The complete process for an ICS-721 NFT transfer is described in this -flowchart: +The complete process for an ICS-721 NFT transfer is described in this flowchart: ![ics721-flowchart](https://user-images.githubusercontent.com/30676292/195717720-8d0629c1-dcdb-4f99-8ffd-b828dc1a216d.png) ## Quick pauses and filtering -This implementation can be quickly paused by a subDAO and supports -rich filtering and rate limiting for the NFTs allowed to traverse it. - -Pause functionality is designed to allow for quick pauses by a trusted -group, without conceding the ability to lock the contract to that -group. To this end, the admin of this contract may appoint a subDAO -which may pause the contract a _single time_. In pausing the contract, -the subDAO loses the ability to pause again until it is reauthorized -by governance. - -After a pause, the ICS721 contract will remain paused until governance chooses -to unpause it. During the unpause process governance may appoint a new -subDAO or reappoint the existing one as pause manager. It is imagined -that the admin of this contract will be a chain's community pool, and -the pause manager will be a small, active subDAO. This process means -that the subDAO may pause the contract in the event of a problem, but -may not lock the contract, as in pausing the contract the subDAO burns -its ability to do so again. - -Filtering is enabled by an optional proxy that the ICS721 contract may be -configured to use. If a proxy is configured, the ICS721 contract will only -accept NFTs delivered by the proxy address. This proxy interface is -very minimal and enables very flexible rate limiting and -filtering. Currently, per-collection rate limiting is -implemented. Users of this ICS721 contract are encouraged to implement their -own filtering regimes and may add them to the [proxy -repository](https://github.com/arkprotocol/cw721-proxy) so that others may -use them. +This implementation can be quickly paused by a subDAO and supports rich filtering and rate limiting for the NFTs allowed to traverse it. + +Pause functionality is designed to allow for quick pauses by a trusted group, without conceding the ability to lock the contract to that group. To this end, the admin of this contract may appoint a subDAO which may pause the contract a _single time_. In pausing the contract, the subDAO loses the ability to pause again until it is reauthorized by governance. + +After a pause, the ICS721 contract will remain paused until governance chooses to unpause it. During the unpause process governance may appoint a new subDAO or reappoint the existing one as pause manager. It is imagined that the admin of this contract will be a chain's community pool, and the pause manager will be a small, active subDAO. This process means that the subDAO may pause the contract in the event of a problem, but may not lock the contract, as in pausing the contract the subDAO burns its ability to do so again. + +Filtering is enabled by an optional proxy that the ICS721 contract may be configured to use. If a proxy is configured, the ICS721 contract will only accept NFTs delivered by the proxy address. This proxy interface is very minimal and enables very flexible rate limiting and filtering. Currently, per-collection rate limiting is implemented. Users of this ICS721 contract are encouraged to implement their own filtering regimes and may add them to the [proxy repository](https://github.com/arkprotocol/cw721-proxy) so that others may use them. ## Failure handling errata -This contract will never close an IBC channel between itself and -another ICS721 contract or module. If the other side of a channel closes the connection, -the ICS721 contract assumes this has happened due to a catastrophic bug in its -counterparty or a malicious action. As such, if a channel closes NFTs -will not be removable from it until governance intervention sets the -policy for what to do. - -Depending on what kind of filtering is applied to this contract, -permissionless chains where anyone can instantiate a NFT contract may -allow the transfer of a buggy cw721 implementation that causes -transfers to fail. - -These sorts of issues can cause trouble with relayer -implementations. The inability to collect fees for relaying is a -limitation of the IBC protocol and this ICS721 contract can not hope -to address that. To this end, it is strongly recommended that users of -this ICS721 contract and all other IBC bridges have users [relay their own -packets](https://github.com/DA0-DA0/dao-dao-ui/issues/885). We will be -working on an implementation of this that other front ends can easily -integrate as part of this work. +This contract will never close an IBC channel between itself and another ICS721 contract or module. If the other side of a channel closes the connection, the ICS721 contract assumes this has happened due to a catastrophic bug in its counterparty or a malicious action. As such, if a channel closes NFTs will not be removable from it until governance intervention sets the policy for what to do. + +Depending on what kind of filtering is applied to this contract, permissionless chains where anyone can instantiate a NFT contract may allow the transfer of a buggy cw721 implementation that causes transfers to fail. + +These sorts of issues can cause trouble with relayer implementations. The inability to collect fees for relaying is a limitation of the IBC protocol and this ICS721 contract can not hope to address that. To this end, it is strongly recommended that users of this ICS721 contract and all other IBC bridges have users [relay their own packets](https://github.com/DA0-DA0/dao-dao-ui/issues/885). We will be working on an implementation of this that other front ends can easily integrate as part of this work. + +## Callbacks + +There are 2 types of callbacks users can use when transfering an NFT: + +1. Receive callback - Callback that is being called on the receiving chain when the NFT was succesfully transferred. +2. Ack callback - Callback that is being called on the sending chain notifying about the status of the transfer. + +Callbacks are optional and can be added in the memo field of the transfer message: + +```json +{ + "callbacks": { + "ack_callback_data": "custom data to pass with the callback", + "ack_callback_addr": "cosmos1...", + "receive_callback_data": "custom data to pass with the callback", + "receive_callback_addr": "cosmos1..." + } +} +``` + +In order to execute an ack callback, `ack_callback_data` must not be empty. In order to execute a receive callback, `receive_callback_data` must not be empty. + +### Contract to accept callbacks + +In order for a contract to accept callbacks, it must implement the next messages: + +```rust +pub enum ReceiverExecuteMsg { + Ics721ReceiveCallback(Ics721ReceiveCallbackMsg), + Ics721AckCallback(Ics721AckCallbackMsg), +} +``` + +`Ics721ReceiveCallback` is used for receive callbacks and gets the next data: + +```rust +pub struct Ics721ReceiveCallbackMsg { + /// The nft contract address that received the NFT + pub nft_contract: String, + /// The original packet that was sent + pub original_packet: NonFungibleTokenPacketData, + /// The provided custom msg by the sender + pub msg: Binary, +} +``` + +`Ics721AckCallback` - is used for ack callback and gets the next data: + +```rust +pub struct Ics721AckCallbackMsg { + /// The status of the transfer (succeeded or failed) + pub status: Ics721Status, + /// The nft contract address that sent the NFT + pub nft_contract: String, + /// The original packet that was sent + pub original_packet: NonFungibleTokenPacketData, + /// The provided custom msg by the sender + pub msg: Binary, +} +``` + +**IMPORTANT** - Those messages are permission-less and can be called by anyone with any data. It is the responsibility of the contract to validate the sender and make sure the sender is a trusted ICS721 contract. +Its also a good practice to confirm the owner of the transferred NFT by querying the nft contract. diff --git a/contracts/ics721-base-tester/Cargo.toml b/contracts/ics721-base-tester/Cargo.toml index 7f736234..1c266320 100644 --- a/contracts/ics721-base-tester/Cargo.toml +++ b/contracts/ics721-base-tester/Cargo.toml @@ -20,3 +20,4 @@ cw-storage-plus = { workspace = true } cw2 = { workspace = true } ics721 = { workspace = true } thiserror = { workspace = true } +cw721 = { workspace = true } diff --git a/contracts/ics721-base-tester/src/contract.rs b/contracts/ics721-base-tester/src/contract.rs index e5b7c023..94410133 100644 --- a/contracts/ics721-base-tester/src/contract.rs +++ b/contracts/ics721-base-tester/src/contract.rs @@ -2,7 +2,7 @@ use cosmwasm_std::entry_point; use cosmwasm_std::{ to_json_binary, Binary, Deps, DepsMut, Env, IbcMsg, IbcTimeout, MessageInfo, Response, - StdResult, + StdResult, WasmMsg, }; use cw2::set_contract_version; use ics721::ibc::NonFungibleTokenPacketData; @@ -10,7 +10,7 @@ use ics721::ibc::NonFungibleTokenPacketData; use crate::{ error::ContractError, msg::{AckMode, ExecuteMsg, InstantiateMsg, QueryMsg}, - state::{ACK_MODE, LAST_ACK}, + state::{ACK_MODE, ICS721, LAST_ACK, NFT_CONTRACT, RECEIVED_CALLBACK, SENT_CALLBACK}, }; const CONTRACT_NAME: &str = "crates.io:ics721-base-tester"; @@ -25,17 +25,33 @@ pub fn instantiate( ) -> Result { set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; ACK_MODE.save(deps.storage, &msg.ack_mode)?; + ICS721.save(deps.storage, &deps.api.addr_validate(&msg.ics721)?)?; Ok(Response::new().add_attribute("method", "instantiate")) } #[cfg_attr(not(feature = "library"), entry_point)] pub fn execute( deps: DepsMut, - _env: Env, - _info: MessageInfo, + env: Env, + info: MessageInfo, msg: ExecuteMsg, ) -> Result { match msg { + ExecuteMsg::ReceiveNft(msg) => receive_callbacks::handle_receive_cw_callback(deps, msg), + ExecuteMsg::Ics721ReceiveCallback(msg) => { + receive_callbacks::handle_receive_callback(deps, &info, msg) + } + ExecuteMsg::Ics721AckCallback(msg) => { + receive_callbacks::handle_ack_callback(deps, &info, msg) + } + ExecuteMsg::SendNft { + cw721, + ics721, + token_id, + recipient, + channel_id, + memo, + } => execute_send_nft(env, cw721, ics721, token_id, recipient, channel_id, memo), ExecuteMsg::SendPacket { channel_id, timeout, @@ -46,6 +62,177 @@ pub fn execute( } } +mod receive_callbacks { + use cosmwasm_std::{ensure_eq, from_json, DepsMut, MessageInfo, Response}; + use ics721::{ + ibc::NonFungibleTokenPacketData, + types::{Ics721AckCallbackMsg, Ics721ReceiveCallbackMsg, Ics721Status}, + }; + + use crate::{ + msg::Ics721Callbacks, + state::{CW721_RECEIVE, ICS721, NFT_CONTRACT, RECEIVED_CALLBACK, SENT_CALLBACK}, + ContractError, + }; + + pub(crate) fn handle_receive_cw_callback( + deps: DepsMut, + _msg: cw721::Cw721ReceiveMsg, + ) -> Result { + // We got the callback, so its working + CW721_RECEIVE.save(deps.storage, &"success".to_string())?; + Ok(Response::new()) + } + + pub(crate) fn handle_receive_callback( + deps: DepsMut, + info: &MessageInfo, + msg: Ics721ReceiveCallbackMsg, + ) -> Result { + match from_json::(&msg.msg)? { + Ics721Callbacks::NftReceived {} => { + nft_received(deps, info, msg.original_packet, msg.nft_contract) + } + Ics721Callbacks::FailCallback {} => fail_callback(), + _ => Err(ContractError::InvalidCallback {}), + } + } + + pub(crate) fn handle_ack_callback( + deps: DepsMut, + info: &MessageInfo, + msg: Ics721AckCallbackMsg, + ) -> Result { + match from_json::(&msg.msg)? { + Ics721Callbacks::NftSent {} => nft_sent( + deps, + info, + msg.status, + msg.original_packet, + msg.nft_contract, + ), + Ics721Callbacks::FailCallback {} => fail_callback(), + _ => Err(ContractError::InvalidCallback {}), + } + } + + fn nft_sent( + deps: DepsMut, + info: &MessageInfo, + status: Ics721Status, + packet: NonFungibleTokenPacketData, + nft_contract: String, + ) -> Result { + let ics_addr = ICS721.load(deps.storage)?; + ensure_eq!(ics_addr, info.sender, ContractError::SenderIsNotIcs721); + + NFT_CONTRACT.save(deps.storage, &deps.api.addr_validate(&nft_contract)?)?; + + let owner: Option = deps + .querier + .query_wasm_smart::( + nft_contract, + &cw721::Cw721QueryMsg::OwnerOf { + token_id: packet.token_ids[0].clone().into(), + include_expired: None, + }, + ) + .ok(); + + SENT_CALLBACK.save(deps.storage, &owner)?; + + match status { + Ics721Status::Success => { + // Transfer completed, the owner should either be None + // or ics721 if we on source chain, + // the owner should be ics721 if we on + // dest chain, the owner should be None + } + Ics721Status::Failed(..) => { + // Transfer failed, the NFT owner should be the sender + } + } + + Ok(Response::new()) + } + + /// We don't care about the status on receive callback because if + /// the transfer failed the callback wont be called anyway, so + /// we assume the transfer is always successful.` + fn nft_received( + deps: DepsMut, + info: &MessageInfo, + packet: NonFungibleTokenPacketData, + nft_contract: String, + ) -> Result { + // Owner should be the receiver. + let ics_addr = ICS721.load(deps.storage)?; + ensure_eq!(ics_addr, info.sender, ContractError::SenderIsNotIcs721); + + NFT_CONTRACT.save(deps.storage, &deps.api.addr_validate(&nft_contract)?)?; + + let owner = deps + .querier + .query_wasm_smart::( + nft_contract, + &cw721::Cw721QueryMsg::OwnerOf { + token_id: packet.token_ids[0].clone().into(), + include_expired: None, + }, + ) + .ok(); + + RECEIVED_CALLBACK.save(deps.storage, &owner)?; + + Ok(Response::new()) + } + + fn fail_callback() -> Result { + // we want to test what happens when an callback is failed + + // On ACK callback nothing should happen, ack callback is just a + // notifier to the sending contract that the NFT was + // transferred successfully or not. + + // On Receive callback it is important if the callback fails or not, + // because we send the NFT with a purpose to this contract, so if the + // callback fails it means we didn't get what we wanted, so we + // should send the NFT back to the sender. but the callback + // was successful, everything is fine. Ex: marketplaces can + // accept NFTs and place them on sale, if the `put on sale` process + // fails the NFT should be sent back to the sender. + Err(ContractError::RandomError) + } +} + +fn execute_send_nft( + env: Env, + cw721: String, + ics721: String, + token_id: String, + recipient: String, + channel_id: String, + memo: Option, +) -> Result { + // Send send msg to cw721, send it to ics721 with the correct msg. + let msg = WasmMsg::Execute { + contract_addr: cw721, + msg: to_json_binary(&cw721::Cw721ExecuteMsg::SendNft { + contract: ics721, + token_id, + msg: to_json_binary(&ics721::msg::IbcOutgoingMsg { + receiver: recipient, + channel_id, + timeout: IbcTimeout::with_timestamp(env.block.time.plus_seconds(1000)), + memo, + })?, + })?, + funds: vec![], + }; + + Ok(Response::default().add_message(msg)) +} + fn execute_send_packet( channel_id: String, timeout: IbcTimeout, @@ -74,5 +261,8 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { QueryMsg::AckMode {} => to_json_binary(&ACK_MODE.load(deps.storage)?), QueryMsg::LastAck {} => to_json_binary(&LAST_ACK.load(deps.storage)?), + QueryMsg::GetReceivedCallback {} => to_json_binary(&RECEIVED_CALLBACK.load(deps.storage)?), + QueryMsg::GetNftContract {} => to_json_binary(&NFT_CONTRACT.load(deps.storage)?), + QueryMsg::GetSentCallback {} => to_json_binary(&SENT_CALLBACK.load(deps.storage)?), } } diff --git a/contracts/ics721-base-tester/src/error.rs b/contracts/ics721-base-tester/src/error.rs index 9c05fd83..e0d51e9d 100644 --- a/contracts/ics721-base-tester/src/error.rs +++ b/contracts/ics721-base-tester/src/error.rs @@ -17,4 +17,13 @@ pub enum ContractError { #[error("{what}")] Debug { what: String }, + + #[error("Just some random error")] + RandomError, + + #[error("Invalid callback")] + InvalidCallback, + + #[error("The callback sender is not the ics721")] + SenderIsNotIcs721, } diff --git a/contracts/ics721-base-tester/src/msg.rs b/contracts/ics721-base-tester/src/msg.rs index f5730e80..3038279c 100644 --- a/contracts/ics721-base-tester/src/msg.rs +++ b/contracts/ics721-base-tester/src/msg.rs @@ -1,6 +1,29 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::IbcTimeout; +/// A struct to handle callbacks of ICS721 transfers. +/// +/// If you have a contract that does sending and receiving +/// you can have a simple structure of callbacks like this: +/// ```rust +/// pub enum Ics721Callbacks { +/// NftSent {}, +/// NftReceived {}, +/// } +/// ``` +/// `NftSent` is called after nft was transfered on the `sender` +/// +/// `NftReceived` is called after nft was transfered on the `receiver` +#[cw_serde] +pub enum Ics721Callbacks { + /// We notify the sender that the NFT was sent successfuly. + NftSent {}, + /// Do something on the receiving chain once the NFT was sent. + NftReceived {}, + /// NFT was sent successfuly, but we fail the callback for tests. + FailCallback {}, +} + #[cw_serde] pub enum AckMode { // Messages should respond with an error ACK. @@ -12,12 +35,24 @@ pub enum AckMode { #[cw_serde] pub struct InstantiateMsg { pub ack_mode: AckMode, + pub ics721: String, } #[cw_serde] #[allow(clippy::large_enum_variant)] // `data` field is a bit large // for clippy's taste. pub enum ExecuteMsg { + ReceiveNft(cw721::Cw721ReceiveMsg), + Ics721ReceiveCallback(ics721::types::Ics721ReceiveCallbackMsg), + Ics721AckCallback(ics721::types::Ics721AckCallbackMsg), + SendNft { + cw721: String, + ics721: String, + token_id: String, + recipient: String, + channel_id: String, + memo: Option, + }, CloseChannel { channel_id: String, }, @@ -39,4 +74,7 @@ pub enum QueryMsg { /// Gets the mode of the last ack this contract received. Errors /// if no ACK has ever been received. Returns `AckMode`. LastAck {}, + GetReceivedCallback {}, + GetNftContract {}, + GetSentCallback {}, } diff --git a/contracts/ics721-base-tester/src/state.rs b/contracts/ics721-base-tester/src/state.rs index 9de2f030..4ad7df15 100644 --- a/contracts/ics721-base-tester/src/state.rs +++ b/contracts/ics721-base-tester/src/state.rs @@ -1,6 +1,13 @@ +use cosmwasm_std::Addr; use cw_storage_plus::Item; use crate::msg::AckMode; pub const ACK_MODE: Item = Item::new("ack_mode"); pub const LAST_ACK: Item = Item::new("ack_mode"); + +pub const ICS721: Item = Item::new("ics721"); +pub const SENT_CALLBACK: Item> = Item::new("sent"); +pub const RECEIVED_CALLBACK: Item> = Item::new("received"); +pub const NFT_CONTRACT: Item = Item::new("nft_contract"); +pub const CW721_RECEIVE: Item = Item::new("cw721_received"); diff --git a/contracts/sg-ics721/src/testing/integration_tests.rs b/contracts/sg-ics721/src/testing/integration_tests.rs index 409729aa..f77f1094 100644 --- a/contracts/sg-ics721/src/testing/integration_tests.rs +++ b/contracts/sg-ics721/src/testing/integration_tests.rs @@ -183,7 +183,7 @@ impl Api for MockApiBech32 { } } } - Err(StdError::generic_err(format!("Invalid input: {}", input))) + Err(StdError::generic_err(format!("Invalid input: {input}"))) } fn addr_humanize(&self, canonical: &CanonicalAddr) -> StdResult { @@ -319,16 +319,14 @@ impl Test { app.api().addr_make(ICS721_CREATOR), &InstantiateMsg { cw721_base_code_id: source_cw721_id, - proxy: proxy.clone(), + proxy, pauser: admin_and_pauser .clone() .map(|p| app.api().addr_make(&p).to_string()), }, &[], "sg-ics721", - admin_and_pauser - .clone() - .map(|p| app.api().addr_make(&p).to_string()), + admin_and_pauser.map(|p| app.api().addr_make(&p).to_string()), ) .unwrap(); @@ -634,7 +632,7 @@ fn test_do_instantiate_and_mint() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 1); - assert_eq!(nft_contracts[0].0, class_id.to_string()); + assert_eq!(nft_contracts[0].0, class_id); // Get the address of the instantiated NFT. let nft_contract: Addr = test .app @@ -733,7 +731,7 @@ fn test_do_instantiate_and_mint() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id.to_string(), + class_id, }, ) .unwrap(); @@ -810,7 +808,7 @@ fn test_do_instantiate_and_mint() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 1); - assert_eq!(nft_contracts[0].0, class_id.to_string()); + assert_eq!(nft_contracts[0].0, class_id); // Get the address of the instantiated NFT. let nft_contract: Addr = test .app @@ -909,7 +907,7 @@ fn test_do_instantiate_and_mint() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id.to_string(), + class_id, }, ) .unwrap(); @@ -984,7 +982,7 @@ fn test_do_instantiate_and_mint() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 1); - assert_eq!(nft_contracts[0].0, class_id.to_string()); + assert_eq!(nft_contracts[0].0, class_id); // Get the address of the instantiated NFT. let nft_contract: Addr = test .app @@ -1083,7 +1081,7 @@ fn test_do_instantiate_and_mint() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id.to_string(), + class_id, }, ) .unwrap(); @@ -1162,7 +1160,7 @@ fn test_do_instantiate_and_mint() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 1); - assert_eq!(nft_contracts[0].0, class_id.to_string()); + assert_eq!(nft_contracts[0].0, class_id); // Get the address of the instantiated NFT. let nft_contract: Addr = test .app @@ -1261,11 +1259,189 @@ fn test_do_instantiate_and_mint() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), + class_id, + }, + ) + .unwrap(); + + assert_eq!(owner.owner, nft_contract.to_string()); + + // check cw721 owner query matches ics721 owner query + let base_owner: cw721::OwnerOfResponse = test + .app + .wrap() + .query_wasm_smart( + nft_contract, + &cw721::Cw721QueryMsg::OwnerOf { + token_id: "1".to_string(), + include_expired: None, + }, + ) + .unwrap(); + assert_eq!(base_owner, owner); + } + // test case: instantiate cw721 with PartialCustomCollectionData (includes name and symbol) + // results in nft contract using name and symbol + { + let mut test = Test::new(false, None, sg721_base_contract()); + let collection_contract_source_chain = + ClassId::new(test.app.api().addr_make(COLLECTION_CONTRACT_SOURCE_CHAIN)); + let class_id = format!( + "wasm.{}/{}/{}", + test.ics721, CHANNEL_TARGET_CHAIN, collection_contract_source_chain + ); + test.app + .execute_contract( + test.ics721.clone(), + test.ics721.clone(), + &ExecuteMsg::Callback(CallbackMsg::CreateVouchers { + receiver: test.app.api().addr_make(NFT_OWNER_TARGET_CHAIN).to_string(), + create: VoucherCreation { + class: Class { + id: ClassId::new(class_id.clone()), + uri: Some("https://moonphase.is".to_string()), + data: Some( + // CustomClassData doesn't apply to CollectionData type and won't be considered + // collection name wont be transferred to instantiated nft contract + to_json_binary(&PartialCustomCollectionData { + owner: None, + name: "collection-name".to_string(), + symbol: "collection-symbol".to_string(), + bar: "bar".to_string(), + foo: Some( + test.app + .api() + .addr_make(COLLECTION_OWNER_TARGET_CHAIN) + .to_string(), + ), + }) + .unwrap(), + ), + }, + tokens: vec![ + Token { + id: TokenId::new("1"), + uri: Some("https://moonphase.is/image.svg".to_string()), + data: None, + }, + Token { + id: TokenId::new("2"), + uri: Some("https://foo.bar".to_string()), + data: None, + }, + ], + }, + }), + &[], + ) + .unwrap(); + // Check entry added in CLASS_ID_TO_NFT_CONTRACT + let nft_contracts = test.query_nft_contracts(); + assert_eq!(nft_contracts.len(), 1); + assert_eq!(nft_contracts[0].0, class_id); + // Get the address of the instantiated NFT. + let nft_contract: Addr = test + .app + .wrap() + .query_wasm_smart( + test.ics721.clone(), + &QueryMsg::NftContract { class_id: class_id.to_string(), }, ) .unwrap(); + // check name and symbol contains class id for instantiated nft contract + let contract_info: cw721::ContractInfoResponse = test + .app + .wrap() + .query_wasm_smart( + nft_contract.clone(), + &Cw721QueryMsg::::ContractInfo {}, + ) + .unwrap(); + assert_eq!( + contract_info, + cw721::ContractInfoResponse { + name: "collection-name".to_string(), + symbol: "collection-symbol".to_string() + } + ); + + // check collection info is properly set + let collection_info: CollectionInfoResponse = test + .app + .wrap() + .query_wasm_smart(nft_contract.clone(), &Sg721QueryMsg::CollectionInfo {}) + .unwrap(); + + assert_eq!( + collection_info, + CollectionInfoResponse { + // creator of ics721 contract is creator of nft contract, since no owner in ClassData provided + creator: test.app.api().addr_make(ICS721_CREATOR).to_string(), + description: "".to_string(), + image: "https://arkprotocol.io".to_string(), + external_link: None, + explicit_content: None, + start_trading_time: None, + royalty_info: None, + } + ); + + // Check that token_uri was set properly. + let token_info: cw721::NftInfoResponse = test + .app + .wrap() + .query_wasm_smart( + nft_contract.clone(), + &cw721::Cw721QueryMsg::NftInfo { + token_id: "1".to_string(), + }, + ) + .unwrap(); + assert_eq!( + token_info.token_uri, + Some("https://moonphase.is/image.svg".to_string()) + ); + let token_info: cw721::NftInfoResponse = test + .app + .wrap() + .query_wasm_smart( + nft_contract.clone(), + &cw721::Cw721QueryMsg::NftInfo { + token_id: "2".to_string(), + }, + ) + .unwrap(); + assert_eq!(token_info.token_uri, Some("https://foo.bar".to_string())); + + // After transfer to target, test owner can do any action, like transfer, on collection + test.app + .execute_contract( + test.app.api().addr_make(NFT_OWNER_TARGET_CHAIN), + nft_contract.clone(), + &cw721_base::msg::ExecuteMsg::::TransferNft { + recipient: nft_contract.to_string(), // new owner + token_id: "1".to_string(), + }, + &[], + ) + .unwrap(); + + // ics721 owner query and check nft contract owns it + let owner: cw721::OwnerOfResponse = test + .app + .wrap() + .query_wasm_smart( + test.ics721, + &QueryMsg::Owner { + token_id: "1".to_string(), + class_id, + }, + ) + .unwrap(); + assert_eq!(owner.owner, nft_contract.to_string()); // check cw721 owner query matches ics721 owner query @@ -1364,8 +1540,8 @@ fn test_do_instantiate_and_mint_2_different_collections() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 2); - assert_eq!(nft_contracts[0].0, class_id_1.to_string()); - assert_eq!(nft_contracts[1].0, class_id_2.to_string()); + assert_eq!(nft_contracts[0].0, class_id_1); + assert_eq!(nft_contracts[1].0, class_id_2); // Get the address of the instantiated NFT. let nft_contract_1: Addr = test .app @@ -1545,7 +1721,7 @@ fn test_do_instantiate_and_mint_2_different_collections() { test.ics721.clone(), &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id_1.to_string(), + class_id: class_id_1, }, ) .unwrap(); @@ -1556,7 +1732,7 @@ fn test_do_instantiate_and_mint_2_different_collections() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id_2.to_string(), + class_id: class_id_2, }, ) .unwrap(); @@ -1646,7 +1822,7 @@ fn test_do_instantiate_and_mint_no_instantiate() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let class_id_to_nft_contract = test.query_nft_contracts(); assert_eq!(class_id_to_nft_contract.len(), 1); - assert_eq!(class_id_to_nft_contract[0].0, class_id.to_string()); + assert_eq!(class_id_to_nft_contract[0].0, class_id); // 2nd call will only do a mint as the contract for the class ID has // already been instantiated. @@ -1683,12 +1859,7 @@ fn test_do_instantiate_and_mint_no_instantiate() { let nft_contract: Addr = test .app .wrap() - .query_wasm_smart( - test.ics721, - &QueryMsg::NftContract { - class_id: class_id.to_string(), - }, - ) + .query_wasm_smart(test.ics721, &QueryMsg::NftContract { class_id }) .unwrap(); // check collection info is properly set @@ -1899,46 +2070,44 @@ fn test_proxy_authorized() { #[test] fn test_receive_nft() { - // test case: receive nft from sg721-base - { - let mut test = Test::new(false, None, sg721_base_contract()); - // simplify: mint and escrowed/owned by ics721, as a precondition for receive nft - let token_id = test.execute_cw721_mint(test.ics721.clone()).unwrap(); - // ics721 receives NFT from sender/collection contract, - // if - and only if - nft is escrowed by ics721, ics721 will interchain transfer NFT! - // otherwise proxy is unauthorised to transfer NFT - let res = test - .app - .execute_contract( - test.source_cw721.clone(), - test.ics721.clone(), - &ExecuteMsg::ReceiveNft(cw721::Cw721ReceiveMsg { - sender: test.source_cw721_owner.to_string(), - token_id: token_id.clone(), - msg: to_json_binary(&IbcOutgoingMsg { - receiver: NFT_OWNER_TARGET_CHAIN.to_string(), // nft owner for other chain, on this chain ics721 is owner - channel_id: "channel-0".to_string(), - timeout: IbcTimeout::with_block(IbcTimeoutBlock { - revision: 0, - height: 10, - }), - memo: None, - }) - .unwrap(), - }), - &[], - ) - .unwrap(); + let mut test = Test::new(false, None, sg721_base_contract()); + // simplify: mint and escrowed/owned by ics721, as a precondition for receive nft + let token_id = test.execute_cw721_mint(test.ics721.clone()).unwrap(); + // ics721 receives NFT from sender/collection contract, + // if - and only if - nft is escrowed by ics721, ics721 will interchain transfer NFT! + // otherwise proxy is unauthorised to transfer NFT + let res = test + .app + .execute_contract( + test.source_cw721.clone(), + test.ics721.clone(), + &ExecuteMsg::ReceiveNft(cw721::Cw721ReceiveMsg { + sender: test.source_cw721_owner.to_string(), + token_id, + msg: to_json_binary(&IbcOutgoingMsg { + receiver: "mr-t".to_string(), + channel_id: "channel-0".to_string(), + timeout: IbcTimeout::with_block(IbcTimeoutBlock { + revision: 0, + height: 10, + }), + memo: None, + }) + .unwrap(), + }), + &[], + ) + .unwrap(); - // get class data (containing collection data) from response - let event = res.events.into_iter().find(|e| e.ty == "wasm").unwrap(); - let class_data_attribute = event - .attributes - .into_iter() - .find(|a| a.key == "class_data") - .unwrap(); - // check collection data matches with data from source nft contract - let expected_contract_info: cosmwasm_std::ContractInfoResponse = + // get class data (containing collection data) from response + let event = res.events.into_iter().find(|e| e.ty == "wasm").unwrap(); + let class_data_attribute = event + .attributes + .into_iter() + .find(|a| a.key == "class_data") + .unwrap(); + // check collection data matches with data from source nft contract + let expected_contract_info: cosmwasm_std::ContractInfoResponse = // workaround using from_json/to_json_binary since ContractInfoResponse is non-exhaustive, can't be created directly from_json( to_json_binary(&ContractInfoResponse { @@ -1951,31 +2120,30 @@ fn test_receive_nft() { .unwrap(), ) .unwrap(); - let expected_collection_data = to_json_binary(&SgCollectionData { - owner: Some( - // collection data from source chain - test.source_cw721_owner.to_string(), - ), - contract_info: Some(expected_contract_info), - name: "name".to_string(), - symbol: "symbol".to_string(), - num_tokens: Some(1), - collection_info: Some(CollectionInfoResponse { - creator: test.ics721.to_string(), - description: "".to_string(), - image: "https://arkprotocol.io".to_string(), - external_link: None, - explicit_content: None, - start_trading_time: None, - royalty_info: None, - }), - }) - .unwrap(); - assert_eq!( - class_data_attribute.value, - format!("{:?}", expected_collection_data) - ); - } + let expected_collection_data = to_json_binary(&SgCollectionData { + owner: Some( + // collection data from source chain + test.source_cw721_owner.to_string(), + ), + contract_info: Some(expected_contract_info), + name: "name".to_string(), + symbol: "symbol".to_string(), + num_tokens: Some(1), + collection_info: Some(CollectionInfoResponse { + creator: test.ics721.to_string(), + description: "".to_string(), + image: "https://arkprotocol.io".to_string(), + external_link: None, + explicit_content: None, + start_trading_time: None, + royalty_info: None, + }), + }) + .unwrap(); + assert_eq!( + class_data_attribute.value, + format!("{expected_collection_data:?}") + ); } /// In case proxy for ICS721 is defined, ICS721 only accepts receival from proxy - not from nft contract! diff --git a/e2e/adversarial_test.go b/e2e/adversarial_test.go index 5a813143..c4ce761a 100644 --- a/e2e/adversarial_test.go +++ b/e2e/adversarial_test.go @@ -90,6 +90,7 @@ func (suite *AdversarialTestSuite) SetupTest() { instantiateBridgeTester := InstantiateBridgeTester{ "success", + suite.bridgeC.String(), } instantiateBridgeTesterRaw, err := json.Marshal(instantiateBridgeTester) require.NoError(suite.T(), err) @@ -133,7 +134,7 @@ func (suite *AdversarialTestSuite) SetupTest() { func (suite *AdversarialTestSuite) TestUnexpectedClose() { // Make a pending IBC message across the AC path, but do not // relay it. - msg := getCw721SendIbcAwayMessage(suite.pathAC, suite.coordinator, suite.tokenIdA, suite.bridgeA, suite.chainC.SenderAccount.GetAddress(), suite.coordinator.CurrentTime.Add(time.Second*4).UnixNano()) + msg := getCw721SendIbcAwayMessage(suite.pathAC, suite.coordinator, suite.tokenIdA, suite.bridgeA, suite.chainC.SenderAccount.GetAddress(), suite.coordinator.CurrentTime.Add(time.Second*4).UnixNano(), "") _, err := suite.chainA.SendMsgs(&wasmtypes.MsgExecuteContract{ Sender: suite.chainA.SenderAccount.GetAddress().String(), Contract: suite.cw721A.String(), @@ -156,7 +157,7 @@ func (suite *AdversarialTestSuite) TestUnexpectedClose() { suite.coordinator.TimeoutPendingPackets(suite.pathAC) suite.pathAC.EndpointA.ChanCloseConfirm() - owner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String()) + owner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "bad kid 1") require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), owner) require.Equal(suite.T(), channeltypes.CLOSED, suite.pathAC.Invert().EndpointA.GetChannel().State) @@ -170,7 +171,7 @@ func (suite *AdversarialTestSuite) TestUnexpectedClose() { newAcc := CreateAndFundAccount(suite.T(), suite.chainA, 10) mintNFT(suite.T(), suite.chainA, suite.cw721A.String(), "bad kid 2", newAcc.Address) - msg = getCw721SendIbcAwayMessage(suite.pathAC, suite.coordinator, "bad kid 2", suite.bridgeA, suite.chainC.SenderAccount.GetAddress(), suite.coordinator.CurrentTime.Add(time.Second*4).UnixNano()) + msg = getCw721SendIbcAwayMessage(suite.pathAC, suite.coordinator, "bad kid 2", suite.bridgeA, suite.chainC.SenderAccount.GetAddress(), suite.coordinator.CurrentTime.Add(time.Second*4).UnixNano(), "") _, err = SendMsgsFromAccount(suite.T(), suite.chainA, newAcc, &wasmtypes.MsgExecuteContract{ Sender: newAcc.Address.String(), Contract: suite.cw721A.String(), @@ -194,13 +195,13 @@ func (suite *AdversarialTestSuite) TestUnexpectedClose() { // metadata when a new packet comes in with conflicting information. func (suite *AdversarialTestSuite) TestInvalidOnMineValidOnTheirs() { // Send a NFT to chain B from A. - ics721Nft(suite.T(), suite.chainA, suite.pathAB, suite.coordinator, suite.cw721A.String(), suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress()) + ics721Nft(suite.T(), suite.chainA, suite.pathAB, suite.coordinator, suite.cw721A.String(), "bad kid 1", suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress(), "") chainBClassId := fmt.Sprintf("%s/%s/%s", suite.pathAB.EndpointB.ChannelConfig.PortID, suite.pathAB.EndpointB.ChannelID, suite.cw721A.String()) // Check that the NFT has been received on chain B. chainBCw721 := queryGetNftForClass(suite.T(), suite.chainB, suite.bridgeB.String(), chainBClassId) - chainBOwner := queryGetOwnerOf(suite.T(), suite.chainB, chainBCw721) + chainBOwner := queryGetOwnerOf(suite.T(), suite.chainB, chainBCw721, "bad kid 1") require.Equal(suite.T(), suite.chainB.SenderAccount.GetAddress().String(), chainBOwner) // From chain C send a message using the chain B class ID to @@ -216,13 +217,13 @@ func (suite *AdversarialTestSuite) TestInvalidOnMineValidOnTheirs() { suite.coordinator.RelayAndAckPendingPackets(suite.pathAC.Invert()) // NFT should still be owned by the ICS721 contract on chain A. - chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String()) + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "bad kid 1") require.Equal(suite.T(), suite.bridgeA.String(), chainAOwner) // A new NFT should have been minted on chain A. chainAClassId := fmt.Sprintf("%s/%s/%s", suite.pathAC.EndpointA.ChannelConfig.PortID, suite.pathAC.EndpointA.ChannelID, chainBClassId) chainACw721 := queryGetNftForClass(suite.T(), suite.chainA, suite.bridgeA.String(), chainAClassId) - chainAOwner = queryGetOwnerOf(suite.T(), suite.chainA, chainACw721) + chainAOwner = queryGetOwnerOf(suite.T(), suite.chainA, chainACw721, "bad kid 1") require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), chainAOwner) // Metadata should be set. @@ -240,7 +241,7 @@ func (suite *AdversarialTestSuite) TestInvalidOnMineValidOnTheirs() { // The newly minted NFT should be returnable to the source // chain and cause a burn when returned. - ics721Nft(suite.T(), suite.chainA, suite.pathAC, suite.coordinator, chainACw721, suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainC.SenderAccount.GetAddress()) + ics721Nft(suite.T(), suite.chainA, suite.pathAC, suite.coordinator, chainACw721, "bad kid 1", suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainC.SenderAccount.GetAddress(), "") err = suite.chainA.SmartQuery(chainACw721, OwnerOfQuery{OwnerOf: OwnerOfQueryData{TokenID: suite.tokenIdA}}, &OwnerOfResponse{}) require.ErrorContains(suite.T(), err, "cw721_base::state::TokenInfo> not found") @@ -287,12 +288,12 @@ func (suite *AdversarialTestSuite) TestEmptyClassId() { _, err := suite.chainC.SendMsgs(&wasmtypes.MsgExecuteContract{ Sender: suite.chainC.SenderAccount.GetAddress().String(), Contract: suite.bridgeC.String(), - Msg: []byte(fmt.Sprintf(`{ "send_packet": { "channel_id": "%s", "timeout": { "timestamp": "%d" }, "data": {"classId":"","classUri":"https://metadata-url.com/my-metadata","tokenIds":["%s"],"tokenUris":["https://metadata-url.com/my-metadata1"],"sender":"%s","receiver":"%s"} }}`, - suite.pathAC.Invert().EndpointA.ChannelID, - suite.coordinator.CurrentTime.Add(time.Hour*100).UnixNano(), - suite.tokenIdA, suite.chainC.SenderAccount.GetAddress().String(), - suite.chainA.SenderAccount.GetAddress().String())), - Funds: []sdk.Coin{}, + Msg: []byte(fmt.Sprintf(`{ "send_packet": { "channel_id": "%s", "timeout": { "timestamp": "%d" }, "data": {"classId":"","classUri":"https://metadata-url.com/my-metadata","tokenIds":["%s"],"tokenUris":["https://metadata-url.com/my-metadata1"],"sender":"%s","receiver":"%s"} }}`, + suite.pathAC.Invert().EndpointA.ChannelID, + suite.coordinator.CurrentTime.Add(time.Hour*100).UnixNano(), + suite.tokenIdA, suite.chainC.SenderAccount.GetAddress().String(), + suite.chainA.SenderAccount.GetAddress().String())), + Funds: []sdk.Coin{}, }) require.NoError(suite.T(), err) suite.coordinator.UpdateTime() @@ -397,9 +398,11 @@ func (suite *AdversarialTestSuite) TestMetadataForwarding() { suite.pathAB, suite.coordinator, chainAAddress, + "bad kid 1", suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress(), + "", ) // Check that class metadata has been forwarded. @@ -452,9 +455,11 @@ func (suite *AdversarialTestSuite) TestMetadataForwarding() { suite.pathAB.Invert(), suite.coordinator, chainBAddress, + "bad kid 1", suite.bridgeB, suite.chainB.SenderAccount.GetAddress(), suite.chainA.SenderAccount.GetAddress(), + "", ) suite.coordinator.UpdateTime() @@ -635,7 +640,7 @@ func (suite *AdversarialTestSuite) TestSendReplayAttack() { // Make sure the receiver is the owner of the token. chainAClassId := fmt.Sprintf("%s/%s/%s", suite.pathAC.EndpointA.ChannelConfig.PortID, suite.pathAC.EndpointA.ChannelID, "classID") chainACw721 := queryGetNftForClass(suite.T(), suite.chainA, suite.bridgeA.String(), chainAClassId) - chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, chainACw721) + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, chainACw721, "bad kid 1") require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), chainAOwner) } @@ -668,9 +673,67 @@ func (suite *AdversarialTestSuite) TestDoubleSendInSingleMessage() { require.Equal(suite.T(), "", chainACw721) } +// NOTE - we comment original test because this case no longer should be handled successfully. +// please check below test for what should be done in this case. +// func (suite *AdversarialTestSuite) TestReceiveMultipleNtsDifferentActions() { +// // Send a NFT from chain A to the evil chain. +// ics721Nft(suite.T(), suite.chainA, suite.pathAC, suite.coordinator, suite.cw721A.String(), suite.tokenIdA, suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress(), "") + +// chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), suite.tokenIdA) +// require.Equal(suite.T(), suite.bridgeA.String(), chainAOwner) + +// pathCA := suite.pathAC.Invert() +// chainCClassId := fmt.Sprintf("%s/%s/%s", pathCA.EndpointA.ChannelConfig.PortID, pathCA.EndpointA.ChannelID, suite.cw721A) + +// // Evil chain responds with: +// // +// // class ID: class ID of sent NFT +// // token IDs: [chainAToken, chainAToken] +// _, err := suite.chainC.SendMsgs(&wasmtypes.MsgExecuteContract{ +// Sender: suite.chainC.SenderAccount.GetAddress().String(), +// Contract: suite.bridgeC.String(), +// Msg: []byte(fmt.Sprintf(`{ "send_packet": { "channel_id": "%s", "timeout": { "timestamp": "%d" }, "data": {"classId":"%s","classUri":"https://metadata-url.com/my-metadata","tokenIds":["%s", "%s"],"tokenUris":["https://metadata-url.com/my-metadata1", "https://moonphase.is/image.svg"],"sender":"%s","receiver":"%s", "memo": "{'ignore': ''}"} }}`, pathCA.EndpointA.ChannelID, suite.coordinator.CurrentTime.Add(time.Hour*100).UnixNano(), chainCClassId, suite.tokenIdA, suite.tokenIdA, suite.chainC.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())), +// Funds: []sdk.Coin{}, +// }) +// require.NoError(suite.T(), err) +// suite.coordinator.UpdateTime() +// suite.coordinator.RelayAndAckPendingPackets(suite.pathAC.Invert()) + +// // All assumptions have now been violated. +// // +// // 1. Remote chain says it has minted a new version of our +// // local NFT on its chain. +// // 2. Remote chain says that there are two NFTs belonging to +// // the same collection with the same token ID. +// // +// // ICS721 contract is a based and does not care what other +// // chain's NFT rules are. Only rule is that NFTs on ICS721 +// // contract's chain follow bridge contract's chain's NFT +// // rules. ICS721 contract says: +// // +// // > I know one of those tokens is valid and corresponds to the +// // > NFT I previously sent away so I will return that one to +// // > the recipient. For all I know chain C social norms allow +// // > for more than one collection with the same ID, so for +// // > that one I will create a new collection (so that it +// // > follows my chain's social norms) and give a token for +// // > that collection for the receiver. +// chainAOwner = queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), suite.tokenIdA) +// require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), chainAOwner) + +// chainAClassId := fmt.Sprintf("%s/%s/%s/%s/%s", suite.pathAC.EndpointA.ChannelConfig.PortID, suite.pathAC.EndpointA.ChannelID, pathCA.EndpointA.ChannelConfig.PortID, pathCA.EndpointA.ChannelID, suite.cw721A) +// chainANft := queryGetNftForClass(suite.T(), suite.chainA, suite.bridgeA.String(), chainAClassId) +// chainAOwner = queryGetOwnerOf(suite.T(), suite.chainA, chainANft, "bad kid 1") +// require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), chainAOwner) +// } + func (suite *AdversarialTestSuite) TestReceiveMultipleNtsDifferentActions() { // Send a NFT from chain A to the evil chain. - ics721Nft(suite.T(), suite.chainA, suite.pathAC, suite.coordinator, suite.cw721A.String(), suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress()) + ics721Nft(suite.T(), suite.chainA, suite.pathAC, suite.coordinator, suite.cw721A.String(), suite.tokenIdA, suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress(), "") + + // Verify nft is escrowed by the bridge on A + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), suite.tokenIdA) + require.Equal(suite.T(), suite.bridgeA.String(), chainAOwner) pathCA := suite.pathAC.Invert() chainCClassId := fmt.Sprintf("%s/%s/%s", pathCA.EndpointA.ChannelConfig.PortID, pathCA.EndpointA.ChannelID, suite.cw721A) @@ -682,7 +745,7 @@ func (suite *AdversarialTestSuite) TestReceiveMultipleNtsDifferentActions() { _, err := suite.chainC.SendMsgs(&wasmtypes.MsgExecuteContract{ Sender: suite.chainC.SenderAccount.GetAddress().String(), Contract: suite.bridgeC.String(), - Msg: []byte(fmt.Sprintf(`{ "send_packet": { "channel_id": "%s", "timeout": { "timestamp": "%d" }, "data": {"classId":"%s","classUri":"https://metadata-url.com/my-metadata","tokenIds":["%s", "%s"],"tokenUris":["https://metadata-url.com/my-metadata1", "https://moonphase.is/image.svg"],"sender":"%s","receiver":"%s"} }}`, pathCA.EndpointA.ChannelID, suite.coordinator.CurrentTime.Add(time.Hour*100).UnixNano(), chainCClassId, suite.tokenIdA, suite.tokenIdA, suite.chainC.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())), + Msg: []byte(fmt.Sprintf(`{ "send_packet": { "channel_id": "%s", "timeout": { "timestamp": "%d" }, "data": {"classId":"%s","classUri":"https://metadata-url.com/my-metadata","tokenIds":["%s", "%s"],"tokenUris":["https://metadata-url.com/my-metadata1", "https://moonphase.is/image.svg"],"sender":"%s","receiver":"%s", "memo": "{'ignore': ''}"} }}`, pathCA.EndpointA.ChannelID, suite.coordinator.CurrentTime.Add(time.Hour*100).UnixNano(), chainCClassId, suite.tokenIdA, suite.tokenIdA, suite.chainC.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())), Funds: []sdk.Coin{}, }) require.NoError(suite.T(), err) @@ -693,26 +756,20 @@ func (suite *AdversarialTestSuite) TestReceiveMultipleNtsDifferentActions() { // // 1. Remote chain says it has minted a new version of our // local NFT on its chain. - // 2. Remote chian says that there are two NFTs belonging to + // 2. Remote chain says that there are two NFTs belonging to // the same collection with the same token ID. // // ICS721 contract is a based and does not care what other // chain's NFT rules are. Only rule is that NFTs on ICS721 // contract's chain follow bridge contract's chain's NFT - // rules. ICS721 contract says: + // rules. // - // > I know one of those tokens is valid and corresponds to the - // > NFT I previously sent away so I will return that one to - // > the recipient. For all I know chain C social norms allow - // > for more than one collection with the same ID, so for - // > that one I will create a new collection (so that it - // > follows my chain's social norms) and give a token for - // > that collection for the receiver. - chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String()) - require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), chainAOwner) + // BUT I know that 2 different actions in the same transfer is not possible, + // which means it is a malicious attack and I will not accept it. + // While I know that separating that same transfer into 2 separate transfers + // will still work, I catch what I can. - chainAClassId := fmt.Sprintf("%s/%s/%s/%s/%s", suite.pathAC.EndpointA.ChannelConfig.PortID, suite.pathAC.EndpointA.ChannelID, pathCA.EndpointA.ChannelConfig.PortID, pathCA.EndpointA.ChannelID, suite.cw721A) - chainANft := queryGetNftForClass(suite.T(), suite.chainA, suite.bridgeA.String(), chainAClassId) - chainAOwner = queryGetOwnerOf(suite.T(), suite.chainA, chainANft) - require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), chainAOwner) + // The NFT should still be escrowed by the bridge on chain A, tx failed. + chainAOwner = queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), suite.tokenIdA) + require.Equal(suite.T(), suite.bridgeA.String(), chainAOwner) } diff --git a/e2e/callback_test.go b/e2e/callback_test.go new file mode 100644 index 00000000..d0520b55 --- /dev/null +++ b/e2e/callback_test.go @@ -0,0 +1,542 @@ +package e2e_test + +import ( + b64 "encoding/base64" + "encoding/json" + "fmt" + "testing" + "time" + + wasmibctesting "github.com/CosmWasm/wasmd/x/wasm/ibctesting" + wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" + sdk "github.com/cosmos/cosmos-sdk/types" + channeltypes "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types" + ibctesting "github.com/cosmos/ibc-go/v4/testing" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type CbTestSuite struct { + suite.Suite + + coordinator *wasmibctesting.Coordinator + + // testing chains used for convenience and readability + chainA *wasmibctesting.TestChain + chainB *wasmibctesting.TestChain + chainC *wasmibctesting.TestChain + + bridgeA sdk.AccAddress + bridgeB sdk.AccAddress + bridgeC sdk.AccAddress + + pathAB *wasmibctesting.Path + pathAC *wasmibctesting.Path + pathBC *wasmibctesting.Path + + testerA sdk.AccAddress + testerB sdk.AccAddress + testerC sdk.AccAddress + + cw721A sdk.AccAddress + cw721B sdk.AccAddress +} + +func TestCallbacks(t *testing.T) { + suite.Run(t, new(CbTestSuite)) +} + +func (suite *CbTestSuite) SetupTest() { + suite.coordinator = wasmibctesting.NewCoordinator(suite.T(), 3) + suite.chainA = suite.coordinator.GetChain(wasmibctesting.GetChainID(0)) + suite.chainB = suite.coordinator.GetChain(wasmibctesting.GetChainID(1)) + suite.chainC = suite.coordinator.GetChain(wasmibctesting.GetChainID(2)) + // suite.coordinator.CommitBlock(suite.chainA, suite.chainB) + + // Store codes and instantiate contracts + storeCodes := func(chain *wasmibctesting.TestChain, bridge *sdk.AccAddress, tester *sdk.AccAddress, num int) { + resp := chain.StoreCodeFile("../artifacts/ics721_base.wasm") + require.Equal(suite.T(), uint64(1), resp.CodeID) + + resp = chain.StoreCodeFile("../external-wasms/cw721_base_v0.18.0.wasm") + require.Equal(suite.T(), uint64(2), resp.CodeID) + + resp = chain.StoreCodeFile("../artifacts/ics721_base_tester.wasm") + require.Equal(suite.T(), uint64(3), resp.CodeID) + + // init dummy contracts based on how much we need + for i := 0; i < num; i++ { + cw721Instantiate := InstantiateCw721{ + "bad/kids", + "bad/kids", + suite.chainA.SenderAccount.GetAddress().String(), + } + instantiateRaw, err := json.Marshal(cw721Instantiate) + require.NoError(suite.T(), err) + + chain.InstantiateContract(2, instantiateRaw) + } + + // init ics721 + instantiateBridge := InstantiateICS721Bridge{ + 2, + nil, + nil, + } + instantiateBridgeRaw, err := json.Marshal(instantiateBridge) + require.NoError(suite.T(), err) + + *bridge = chain.InstantiateContract(1, instantiateBridgeRaw) + + // init tester + instantiateBridgeTester := InstantiateBridgeTester{ + "success", + bridge.String(), + } + instantiateBridgeTesterRaw, err := json.Marshal(instantiateBridgeTester) + require.NoError(suite.T(), err) + + *tester = chain.InstantiateContract(3, instantiateBridgeTesterRaw) + } + + storeCodes(suite.chainA, &suite.bridgeA, &suite.testerA, 0) + storeCodes(suite.chainB, &suite.bridgeB, &suite.testerB, 3) + storeCodes(suite.chainC, &suite.bridgeC, &suite.testerC, 6) + + // Helper function to init ibc paths + initPath := func(chain1 *wasmibctesting.TestChain, chain2 *wasmibctesting.TestChain, contract1 sdk.AccAddress, contract2 sdk.AccAddress) *wasmibctesting.Path { + sourcePortID := chain1.ContractInfo(contract1).IBCPortID + counterpartPortID := chain2.ContractInfo(contract2).IBCPortID + path := wasmibctesting.NewPath(chain1, chain2) + path.EndpointA.ChannelConfig = &ibctesting.ChannelConfig{ + PortID: sourcePortID, + Version: "ics721-1", + Order: channeltypes.UNORDERED, + } + path.EndpointB.ChannelConfig = &ibctesting.ChannelConfig{ + PortID: counterpartPortID, + Version: "ics721-1", + Order: channeltypes.UNORDERED, + } + suite.coordinator.SetupConnections(path) + suite.coordinator.CreateChannels(path) + return path + } + + // init ibc path between chains + suite.pathAB = initPath(suite.chainA, suite.chainB, suite.bridgeA, suite.bridgeB) + suite.pathAC = initPath(suite.chainA, suite.chainC, suite.bridgeA, suite.bridgeC) + suite.pathBC = initPath(suite.chainB, suite.chainC, suite.bridgeB, suite.bridgeC) + + // init cw721 on chain A + cw721Instantiate := InstantiateCw721{ + "bad/kids", + "bad/kids", + suite.chainA.SenderAccount.GetAddress().String(), + } + instantiateRaw, err := json.Marshal(cw721Instantiate) + require.NoError(suite.T(), err) + + suite.cw721A = suite.chainA.InstantiateContract(2, instantiateRaw) + + _, err = suite.chainA.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainA.SenderAccount.GetAddress().String(), + Contract: suite.cw721A.String(), + Msg: []byte(fmt.Sprintf(`{ "mint": { "token_id": "1", "owner": "%s" } }`, suite.chainA.SenderAccount.GetAddress().String())), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + //Send NFT to chain B + ics721Nft(suite.T(), suite.chainA, suite.pathAB, suite.coordinator, suite.cw721A.String(), "1", suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress(), "") + + classIdChainB := fmt.Sprintf("%s/%s/%s", suite.pathAB.EndpointB.ChannelConfig.PortID, suite.pathAB.EndpointB.ChannelID, suite.cw721A.String()) + addr := queryGetNftForClass(suite.T(), suite.chainB, suite.bridgeB.String(), classIdChainB) + suite.cw721B, err = sdk.AccAddressFromBech32(addr) + require.NoError(suite.T(), err) + + // mint working NFT to tester + _, err = suite.chainA.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainA.SenderAccount.GetAddress().String(), + Contract: suite.cw721A.String(), + Msg: []byte(fmt.Sprintf(`{ "mint": { "token_id": "2", "owner": "%s" } }`, suite.testerA.String())), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + // mint another NFT to tester + _, err = suite.chainA.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainA.SenderAccount.GetAddress().String(), + Contract: suite.cw721A.String(), + Msg: []byte(fmt.Sprintf(`{ "mint": { "token_id": "4", "owner": "%s" } }`, suite.testerA.String())), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + // mint NFT to sender + _, err = suite.chainA.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainA.SenderAccount.GetAddress().String(), + Contract: suite.cw721A.String(), + Msg: []byte(fmt.Sprintf(`{ "mint": { "token_id": "3", "owner": "%s" } }`, suite.chainA.SenderAccount.GetAddress().String())), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + suite.T().Logf("chain A bridge = (%s)", suite.bridgeA.String()) + suite.T().Logf("chain B bridge = (%s)", suite.bridgeB.String()) + suite.T().Logf("chain C bridge = (%s)", suite.bridgeC.String()) + suite.T().Logf("chain A tester = (%s)", suite.testerA.String()) + suite.T().Logf("chain B tester = (%s)", suite.testerB.String()) + suite.T().Logf("chain C tester = (%s)", suite.testerC.String()) + suite.T().Logf("chain A cw721) = (%s)", suite.cw721A.String()) + suite.T().Logf("chain B cw721) = (%s)", suite.cw721B.String()) +} + +func callbackMemo(src_cb, src_receiver, dest_cb, dest_receiver string) string { + src_cb = parseOptional(src_cb) + src_receiver = parseOptional(src_receiver) + dest_cb = parseOptional(dest_cb) + dest_receiver = parseOptional(dest_receiver) + memo := fmt.Sprintf(`{ "callbacks": { "ack_callback_data": %s, "ack_callback_addr": %s, "receive_callback_data": %s, "receive_callback_addr": %s } }`, src_cb, src_receiver, dest_cb, dest_receiver) + return b64.StdEncoding.EncodeToString([]byte(memo)) +} + +func nftSentCb() string { + return b64.StdEncoding.EncodeToString([]byte(`{ "nft_sent": {}}`)) +} + +func nftReceivedCb() string { + return b64.StdEncoding.EncodeToString([]byte(`{ "nft_received": {}}`)) +} + +func failedCb() string { + return b64.StdEncoding.EncodeToString([]byte(`{ "fail_callback": {}}`)) +} + +func sendIcsFromChainAToB(suite *CbTestSuite, nft, token_id, memo string, relay bool) { + msg := fmt.Sprintf(`{ "send_nft": {"cw721": "%s", "ics721": "%s", "token_id": "%s", "recipient":"%s", "channel_id":"%s", "memo":"%s"}}`, nft, suite.bridgeA.String(), token_id, suite.testerB.String(), suite.pathAB.EndpointA.ChannelID, memo) + _, err := suite.chainA.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainA.SenderAccount.GetAddress().String(), + Contract: suite.testerA.String(), + Msg: []byte(msg), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + if relay { + suite.coordinator.UpdateTime() + suite.coordinator.RelayAndAckPendingPackets(suite.pathAB) + suite.coordinator.UpdateTime() + } +} + +func sendIcsFromChainAToC(suite *CbTestSuite, nft, token_id, memo string, relay bool) { + msg := fmt.Sprintf(`{ "send_nft": {"cw721": "%s", "ics721": "%s", "token_id": "%s", "recipient":"%s", "channel_id":"%s", "memo":"%s"}}`, nft, suite.bridgeA.String(), token_id, suite.testerC.String(), suite.pathAC.EndpointA.ChannelID, memo) + _, err := suite.chainA.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainA.SenderAccount.GetAddress().String(), + Contract: suite.testerA.String(), + Msg: []byte(msg), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + if relay { + suite.coordinator.UpdateTime() + suite.coordinator.RelayAndAckPendingPackets(suite.pathAC) + suite.coordinator.UpdateTime() + } +} + +func sendIcsFromChainBToA(suite *CbTestSuite, nft, token_id, memo string, relay bool) { + msg := fmt.Sprintf(`{ "send_nft": {"cw721": "%s", "ics721": "%s", "token_id": "%s", "recipient":"%s", "channel_id":"%s", "memo":"%s"}}`, nft, suite.bridgeB.String(), token_id, suite.testerA.String(), suite.pathAB.EndpointB.ChannelID, memo) + _, err := suite.chainB.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainB.SenderAccount.GetAddress().String(), + Contract: suite.testerB.String(), + Msg: []byte(msg), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + if relay { + suite.coordinator.UpdateTime() + suite.coordinator.RelayAndAckPendingPackets(suite.pathAB.Invert()) + } +} + +func sendIcsFromChainBToC(suite *CbTestSuite, nft, token_id, memo string, relay bool) { + msg := fmt.Sprintf(`{ "send_nft": {"cw721": "%s", "ics721": "%s", "token_id": "%s", "recipient":"%s", "channel_id":"%s", "memo":"%s"}}`, nft, suite.bridgeB.String(), token_id, suite.testerC.String(), suite.pathBC.EndpointA.ChannelID, memo) + _, err := suite.chainB.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainB.SenderAccount.GetAddress().String(), + Contract: suite.testerB.String(), + Msg: []byte(msg), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + if relay { + suite.coordinator.UpdateTime() + suite.coordinator.RelayAndAckPendingPackets(suite.pathBC) + suite.coordinator.UpdateTime() + } +} + +func sendIcsFromChainCToB(suite *CbTestSuite, nft, token_id, memo string, relay bool) { + msg := fmt.Sprintf(`{ "send_nft": {"cw721": "%s", "ics721": "%s", "token_id": "%s", "recipient":"%s", "channel_id":"%s", "memo":"%s"}}`, nft, suite.bridgeC.String(), token_id, suite.testerB.String(), suite.pathBC.EndpointB.ChannelID, memo) + _, err := suite.chainC.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainC.SenderAccount.GetAddress().String(), + Contract: suite.testerC.String(), + Msg: []byte(msg), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + if relay { + suite.coordinator.UpdateTime() + suite.coordinator.RelayAndAckPendingPackets(suite.pathBC.Invert()) + suite.coordinator.UpdateTime() + } +} + +func sendIcsFromChainCToA(suite *CbTestSuite, nft, token_id, memo string, relay bool) { + msg := fmt.Sprintf(`{ "send_nft": {"cw721": "%s", "ics721": "%s", "token_id": "%s", "recipient":"%s", "channel_id":"%s", "memo":"%s"}}`, nft, suite.bridgeC.String(), token_id, suite.testerA.String(), suite.pathAC.EndpointB.ChannelID, memo) + _, err := suite.chainC.SendMsgs(&wasmtypes.MsgExecuteContract{ + Sender: suite.chainC.SenderAccount.GetAddress().String(), + Contract: suite.testerC.String(), + Msg: []byte(msg), + Funds: []sdk.Coin{}, + }) + require.NoError(suite.T(), err) + + if relay { + suite.coordinator.UpdateTime() + suite.coordinator.RelayAndAckPendingPackets(suite.pathAC.Invert()) + } +} + +func queryTesterSent(t *testing.T, chain *wasmibctesting.TestChain, tester string) string { + resp := TesterResponse{} + testerSentQuery := TesterSentQuery{ + GetSentCallback: EmptyData{}, + } + err := chain.SmartQuery(tester, testerSentQuery, &resp) + require.NoError(t, err) + return *resp.Owner +} + +func queryTesterReceived(t *testing.T, chain *wasmibctesting.TestChain, tester string) string { + resp := TesterResponse{} + testerReceivedQuery := TesterReceivedQuery{ + GetReceivedCallback: EmptyData{}, + } + err := chain.SmartQuery(tester, testerReceivedQuery, &resp) + require.NoError(t, err) + return *resp.Owner +} + +func queryTesterNftContract(t *testing.T, chain *wasmibctesting.TestChain, tester string) string { + resp := "" + testerReceivedQuery := TesterNftContractQuery{ + GetNftContract: EmptyData{}, + } + err := chain.SmartQuery(tester, testerReceivedQuery, &resp) + require.NoError(t, err) + return resp +} + +func queryTesterReceivedErr(t *testing.T, chain *wasmibctesting.TestChain, tester string) error { + resp := TesterResponse{} + testerReceivedQuery := TesterReceivedQuery{ + GetReceivedCallback: EmptyData{}, + } + err := chain.SmartQuery(tester, testerReceivedQuery, &resp) + return err +} + +func (suite *CbTestSuite) TestSuccessfulTransfer() { + memo := callbackMemo(nftSentCb(), "", nftReceivedCb(), "") + sendIcsFromChainAToB(suite, suite.cw721A.String(), "2", memo, true) + + // Query the owner of NFT on cw721 + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "2") + require.Equal(suite.T(), chainAOwner, suite.bridgeA.String()) + chainBOwner := queryGetOwnerOf(suite.T(), suite.chainB, suite.cw721B.String(), "2") + require.Equal(suite.T(), chainBOwner, suite.testerB.String()) + + // We query the data we have on the tester contract + // This ensures that the callbacks are called after all the messages was completed + // and the transfer was successful + testerDataOwnerA := queryTesterSent(suite.T(), suite.chainA, suite.testerA.String()) + require.Equal(suite.T(), testerDataOwnerA, suite.bridgeA.String()) + testerNftContract := queryTesterNftContract(suite.T(), suite.chainB, suite.testerB.String()) + require.Equal(suite.T(), testerNftContract, suite.cw721B.String()) + testerDataOwnerB := queryTesterReceived(suite.T(), suite.chainB, suite.testerB.String()) + require.Equal(suite.T(), testerDataOwnerB, suite.testerB.String()) +} + +func (suite *CbTestSuite) TestSuccessfulTransferWithReceivers() { + memo := callbackMemo(nftSentCb(), suite.testerA.String(), nftReceivedCb(), suite.testerB.String()) + + // Send NFT to chain B + ics721Nft(suite.T(), suite.chainA, suite.pathAB, suite.coordinator, suite.cw721A.String(), "3", suite.bridgeA, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress(), memo) + + // Query the owner of NFT on cw721 + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "3") + require.Equal(suite.T(), chainAOwner, suite.bridgeA.String()) + chainBOwner := queryGetOwnerOf(suite.T(), suite.chainB, suite.cw721B.String(), "3") + require.Equal(suite.T(), chainBOwner, suite.chainB.SenderAccount.GetAddress().String()) + + // We query the data we have on the tester contract + // This ensures that the callbacks are called after all the messages was completed + // and the transfer was successful + testerDataOwnerA := queryTesterSent(suite.T(), suite.chainA, suite.testerA.String()) + require.Equal(suite.T(), testerDataOwnerA, suite.bridgeA.String()) + testerNftContract := queryTesterNftContract(suite.T(), suite.chainB, suite.testerB.String()) + require.Equal(suite.T(), testerNftContract, suite.cw721B.String()) + testerDataOwnerB := queryTesterReceived(suite.T(), suite.chainB, suite.testerB.String()) + require.Equal(suite.T(), testerDataOwnerB, suite.chainB.SenderAccount.GetAddress().String()) +} + +func (suite *CbTestSuite) TestTimeoutTransfer() { + memo := callbackMemo(nftSentCb(), "", nftReceivedCb(), "") + sendIcsFromChainAToB(suite, suite.cw721A.String(), "2", memo, false) + suite.coordinator.IncrementTimeBy(time.Second * 2001) + suite.coordinator.UpdateTime() + suite.coordinator.TimeoutPendingPackets(suite.pathAB) + + // Query the owner of NFT on cw721 + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "2") + require.Equal(suite.T(), chainAOwner, suite.testerA.String()) + err := queryGetOwnerOfErr(suite.T(), suite.chainB, suite.cw721B.String(), "2") + require.Error(suite.T(), err) + + // callbacks should update sender contract of the failed transfer + // so we query the contract to see who is the new owner + // if the query is working and owner is correct, we can confirm the callback was called successfully + testerDataOwnerA := queryTesterSent(suite.T(), suite.chainA, suite.testerA.String()) + require.Equal(suite.T(), testerDataOwnerA, suite.testerA.String()) + + // Querying the receving end, should fail because we did not receive the NFT + // so the callback should not have been called. + err = queryTesterReceivedErr(suite.T(), suite.chainB, suite.testerB.String()) + require.Error(suite.T(), err) +} + +func (suite *CbTestSuite) TestFailedCallbackTransfer() { + memo := callbackMemo(nftSentCb(), "", failedCb(), "") + sendIcsFromChainAToB(suite, suite.cw721A.String(), "2", memo, true) + + // Query the owner of NFT on cw721 + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "2") + require.Equal(suite.T(), chainAOwner, suite.testerA.String()) + err := queryGetOwnerOfErr(suite.T(), suite.chainB, suite.cw721B.String(), "2") + require.Error(suite.T(), err) + + // callbacks should update sender contract of the failed transfer + // so we query the contract to see who is the new owner + // if the query is working and owner is correct, we can confirm the callback was called successfully + testerDataOwnerA := queryTesterSent(suite.T(), suite.chainA, suite.testerA.String()) + require.Equal(suite.T(), testerDataOwnerA, suite.testerA.String()) + + // Querying the receving end, should fail because we did not receive the NFT + // so the callback should not have been called. + err = queryTesterReceivedErr(suite.T(), suite.chainB, suite.testerB.String()) + require.Error(suite.T(), err) +} + +func (suite *CbTestSuite) TestFailedCallbackOnAck() { + // Transfer to chain B + memo := callbackMemo("", "", "", "") + sendIcsFromChainAToB(suite, suite.cw721A.String(), "2", memo, true) + + // Transfer from B to chain A, + // We fail the ack callback and see if the NFT was burned or not + // Because the transfer should be successful even if the ack callback is failing + // we make sure that the NFT was burned on chain B, and that the owner is correct on chain A + memo = callbackMemo(failedCb(), "", "", "") + sendIcsFromChainBToA(suite, suite.cw721B.String(), "2", memo, true) + + // Transfer was successful, so the owner on chain A should be the testerA + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "2") + require.Equal(suite.T(), chainAOwner, suite.testerA.String()) + + // Transfer was successful, so nft "2" should be burned and fail the query + err := queryGetOwnerOfErr(suite.T(), suite.chainB, suite.cw721B.String(), "2") + require.Error(suite.T(), err) + + // We don't do any query on tester, because we don't have receive callback set + // and the ack callback should fail, so no data to show. +} + +func (suite *CbTestSuite) TestMultipleChainsTransfers() { + confirmNftContracts := func(ackChain *wasmibctesting.TestChain, receiveChain *wasmibctesting.TestChain, testerAck string, testerReceive string, expectAck string, expectReceive string) { + ackContract := queryTesterNftContract(suite.T(), ackChain, testerAck) + require.Equal(suite.T(), ackContract, expectAck) + + receiveContract := queryTesterNftContract(suite.T(), receiveChain, testerReceive) + require.Equal(suite.T(), receiveContract, expectReceive) + } + + memo := callbackMemo(nftSentCb(), "", nftReceivedCb(), "") + sendIcsFromChainAToB(suite, suite.cw721A.String(), "2", memo, true) + + // Owner should be the bridge on chain A + chainAOwner := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "2") + require.Equal(suite.T(), chainAOwner, suite.bridgeA.String()) + + chainBOwner := queryGetOwnerOf(suite.T(), suite.chainB, suite.cw721B.String(), "2") + require.Equal(suite.T(), chainBOwner, suite.testerB.String()) + + confirmNftContracts(suite.chainA, suite.chainB, suite.testerA.String(), suite.testerB.String(), suite.cw721A.String(), suite.cw721B.String()) + + // Send from ChainB to ChainC + sendIcsFromChainBToC(suite, suite.cw721B.String(), "2", memo, true) + + // Get the cw721 address on ChainC when received from ChainB + BCClassId := fmt.Sprintf("%s/%s/%s/%s/%s", suite.pathBC.EndpointB.ChannelConfig.PortID, suite.pathBC.EndpointB.ChannelID, suite.pathAB.EndpointB.ChannelConfig.PortID, suite.pathAB.EndpointB.ChannelID, suite.cw721A) + BCCw721 := queryGetNftForClass(suite.T(), suite.chainC, suite.bridgeC.String(), BCClassId) + + chainBOwner = queryGetOwnerOf(suite.T(), suite.chainB, suite.cw721B.String(), "2") + require.Equal(suite.T(), chainBOwner, suite.bridgeB.String()) + + // Make sure the transfer was correct and successful + chainCOwner := queryGetOwnerOf(suite.T(), suite.chainC, BCCw721, "2") + require.Equal(suite.T(), chainCOwner, suite.testerC.String()) + + confirmNftContracts(suite.chainB, suite.chainC, suite.testerB.String(), suite.testerC.String(), suite.cw721B.String(), BCCw721) + + // Send from ChainA to ChainC + sendIcsFromChainAToC(suite, suite.cw721A.String(), "4", memo, true) + + // Get the cw721 address on ChainC when received from ChainB + ACClassId := fmt.Sprintf("%s/%s/%s", suite.pathAC.EndpointB.ChannelConfig.PortID, suite.pathAC.EndpointB.ChannelID, suite.cw721A) + ACCw721 := queryGetNftForClass(suite.T(), suite.chainC, suite.bridgeC.String(), ACClassId) + + // Confirm tester is the owner on Chain C of the nft id "4" + chainCOwner = queryGetOwnerOf(suite.T(), suite.chainC, ACCw721, "4") + require.Equal(suite.T(), chainCOwner, suite.testerC.String()) + + confirmNftContracts(suite.chainA, suite.chainC, suite.testerA.String(), suite.testerC.String(), suite.cw721A.String(), ACCw721) + + // Let send back all NFTs to Chain A + sendIcsFromChainCToA(suite, ACCw721, "4", memo, true) + confirmNftContracts(suite.chainC, suite.chainA, suite.testerC.String(), suite.testerA.String(), ACCw721, suite.cw721A.String()) + + sendIcsFromChainCToB(suite, BCCw721, "2", memo, true) + confirmNftContracts(suite.chainC, suite.chainB, suite.testerC.String(), suite.testerB.String(), BCCw721, suite.cw721B.String()) + + sendIcsFromChainBToA(suite, suite.cw721B.String(), "2", memo, true) + confirmNftContracts(suite.chainB, suite.chainA, suite.testerB.String(), suite.testerA.String(), suite.cw721B.String(), suite.cw721A.String()) + + chainAOwner1 := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "2") + require.Equal(suite.T(), chainAOwner1, suite.testerA.String()) + chainAOwner2 := queryGetOwnerOf(suite.T(), suite.chainA, suite.cw721A.String(), "4") + require.Equal(suite.T(), chainAOwner2, suite.testerA.String()) + + // NFTs should no exist on Chain B and Chain C, they should be burned and query for owner should error + err := queryGetOwnerOfErr(suite.T(), suite.chainB, suite.cw721B.String(), "2") + require.Error(suite.T(), err) + err = queryGetOwnerOfErr(suite.T(), suite.chainC, BCCw721, "2") + require.Error(suite.T(), err) + err = queryGetOwnerOfErr(suite.T(), suite.chainC, ACCw721, "4") + require.Error(suite.T(), err) +} diff --git a/e2e/suite_helpers.go b/e2e/suite_helpers.go index 8e37f85d..f13d94c0 100644 --- a/e2e/suite_helpers.go +++ b/e2e/suite_helpers.go @@ -1,6 +1,8 @@ package e2e_test import ( + "encoding/hex" + "fmt" "testing" wasmibctesting "github.com/CosmWasm/wasmd/x/wasm/ibctesting" @@ -13,6 +15,16 @@ import ( "github.com/stretchr/testify/require" ) +func parseOptional(memo string) string { + r := "" + if memo != "" { + r = fmt.Sprintf("\"%s\"", memo) + } else { + r = "null" + } + return r +} + // Creates and funds a new account for CHAIN. ACCOUNT_NUMBER is the // number of accounts that have been previously created on CHAIN. func CreateAndFundAccount(t *testing.T, chain *wasmibctesting.TestChain, accountNumber uint64) Account { @@ -71,3 +83,12 @@ func SendMsgsFromAccount(t *testing.T, chain *wasmibctesting.TestChain, account return r, nil } + +func AccAddressFromHex(address string) (addr sdk.AccAddress, err error) { + bz, err := addressBytesFromHexString(address) + return sdk.AccAddress(bz), err +} + +func addressBytesFromHexString(address string) ([]byte, error) { + return hex.DecodeString(address) +} diff --git a/e2e/suite_test.go b/e2e/suite_test.go index 8fb014f0..32140034 100644 --- a/e2e/suite_test.go +++ b/e2e/suite_test.go @@ -307,22 +307,23 @@ func transferNft(t *testing.T, chain *wasmibctesting.TestChain, nft, token_id st require.NoError(t, err) } -func getCw721SendIbcAwayMessage(path *wasmibctesting.Path, coordinator *wasmibctesting.Coordinator, tokenId string, bridge, receiver sdk.AccAddress, timeout int64) string { - ibcAway := fmt.Sprintf(`{ "receiver": "%s", "channel_id": "%s", "timeout": { "timestamp": "%d" } }`, receiver.String(), path.EndpointA.ChannelID, timeout) +func getCw721SendIbcAwayMessage(path *wasmibctesting.Path, coordinator *wasmibctesting.Coordinator, tokenId string, bridge, receiver sdk.AccAddress, timeout int64, memo string) string { + memo = parseOptional(memo) + ibcAway := fmt.Sprintf(`{ "receiver": "%s", "channel_id": "%s", "timeout": { "timestamp": "%d" }, "memo": %s }`, receiver.String(), path.EndpointA.ChannelID, timeout, memo) ibcAwayEncoded := b64.StdEncoding.EncodeToString([]byte(ibcAway)) return fmt.Sprintf(`{ "send_nft": { "contract": "%s", "token_id": "%s", "msg": "%s" } }`, bridge, tokenId, ibcAwayEncoded) } -func ics721Nft(t *testing.T, chain *wasmibctesting.TestChain, path *wasmibctesting.Path, coordinator *wasmibctesting.Coordinator, nft string, bridge, sender, receiver sdk.AccAddress) { - // Send the NFT away. - _, err := chain.SendMsgs(&wasmtypes.MsgExecuteContract{ +func ics721Nft(t *testing.T, chain *wasmibctesting.TestChain, path *wasmibctesting.Path, coordinator *wasmibctesting.Coordinator, nft string, token_id string, bridge, sender, receiver sdk.AccAddress, memo string) *sdk.Result { // Send the NFT away. + res, err := chain.SendMsgs(&wasmtypes.MsgExecuteContract{ Sender: sender.String(), Contract: nft, - Msg: []byte(getCw721SendIbcAwayMessage(path, coordinator, "bad kid 1", bridge, receiver, coordinator.CurrentTime.UnixNano()+1000000000000)), + Msg: []byte(getCw721SendIbcAwayMessage(path, coordinator, token_id, bridge, receiver, coordinator.CurrentTime.UnixNano()+1000000000000, memo)), Funds: []sdk.Coin{}, }) require.NoError(t, err) coordinator.RelayAndAckPendingPackets(path) + return res } func queryGetNftForClass(t *testing.T, chain *wasmibctesting.TestChain, bridge, classID string) string { @@ -337,11 +338,21 @@ func queryGetNftForClass(t *testing.T, chain *wasmibctesting.TestChain, bridge, return cw721 } -func queryGetOwnerOf(t *testing.T, chain *wasmibctesting.TestChain, nft string) string { +func queryGetNftContracts(t *testing.T, chain *wasmibctesting.TestChain, bridge string) [][]string { + getClassQuery := NftContractsQuery{ + NftContracts: NftContractsQueryData{}, + } + var cw721 [][]string + err := chain.SmartQuery(bridge, getClassQuery, &cw721) + require.NoError(t, err) + return cw721 +} + +func queryGetOwnerOf(t *testing.T, chain *wasmibctesting.TestChain, nft string, token_id string) string { resp := OwnerOfResponse{} ownerOfQuery := OwnerOfQuery{ OwnerOf: OwnerOfQueryData{ - TokenID: "bad kid 1", + TokenID: token_id, }, } err := chain.SmartQuery(nft, ownerOfQuery, &resp) @@ -349,6 +360,17 @@ func queryGetOwnerOf(t *testing.T, chain *wasmibctesting.TestChain, nft string) return resp.Owner } +func queryGetOwnerOfErr(t *testing.T, chain *wasmibctesting.TestChain, nft string, token_id string) error { + resp := OwnerOfResponse{} + ownerOfQuery := OwnerOfQuery{ + OwnerOf: OwnerOfQueryData{ + TokenID: token_id, + }, + } + err := chain.SmartQuery(nft, ownerOfQuery, &resp) + return err +} + // Builds three identical chains A, B, and C then sends along the path // A -> B -> C -> A -> C -> B -> A. If this works, likely most other // things do too. :) @@ -408,56 +430,56 @@ func TestSendBetweenThreeIdenticalChains(t *testing.T) { // A -> B path := getPath(0, 1) - ics721Nft(t, chainA, path, coordinator, chainANft, bridge, chainA.SenderAccount.GetAddress(), chainB.SenderAccount.GetAddress()) + ics721Nft(t, chainA, path, coordinator, chainANft, "bad kid 1", bridge, chainA.SenderAccount.GetAddress(), chainB.SenderAccount.GetAddress(), "") // Check that chain B received the NFT. chainBClassID := fmt.Sprintf(`%s/%s/%s`, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, chainANft) chainBNft := queryGetNftForClass(t, chainB, bridge.String(), chainBClassID) t.Logf("chain B cw721: %s", chainBNft) - ownerB := queryGetOwnerOf(t, chainB, chainBNft) + ownerB := queryGetOwnerOf(t, chainB, chainBNft, "bad kid 1") require.Equal(t, chainB.SenderAccount.GetAddress().String(), ownerB) // Make sure chain A has the NFT in its ICS721 contract. - ownerA := queryGetOwnerOf(t, chainA, chainANft) + ownerA := queryGetOwnerOf(t, chainA, chainANft, "bad kid 1") require.Equal(t, ownerA, bridge.String()) // B -> C path = getPath(1, 2) - ics721Nft(t, chainB, path, coordinator, chainBNft, bridge, chainB.SenderAccount.GetAddress(), chainC.SenderAccount.GetAddress()) + ics721Nft(t, chainB, path, coordinator, chainBNft, "bad kid 1", bridge, chainB.SenderAccount.GetAddress(), chainC.SenderAccount.GetAddress(), "") chainCClassID := fmt.Sprintf(`%s/%s/%s`, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, chainBClassID) chainCNft := queryGetNftForClass(t, chainC, bridge.String(), chainCClassID) t.Logf("chain C cw721: %s", chainCNft) - ownerC := queryGetOwnerOf(t, chainC, chainCNft) + ownerC := queryGetOwnerOf(t, chainC, chainCNft, "bad kid 1") require.Equal(t, chainC.SenderAccount.GetAddress().String(), ownerC) // Make sure the NFT is locked in the ICS721 contract on chain B. - ownerB = queryGetOwnerOf(t, chainB, chainBNft) + ownerB = queryGetOwnerOf(t, chainB, chainBNft, "bad kid 1") require.Equal(t, bridge.String(), ownerB) // C -> A path = getPath(2, 0) - ics721Nft(t, chainC, path, coordinator, chainCNft, bridge, chainC.SenderAccount.GetAddress(), chainA.SenderAccount.GetAddress()) + ics721Nft(t, chainC, path, coordinator, chainCNft, "bad kid 1", bridge, chainC.SenderAccount.GetAddress(), chainA.SenderAccount.GetAddress(), "") chainAClassID := fmt.Sprintf(`%s/%s/%s`, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, chainCClassID) // This is a derivative and not actually the original chain A nft. chainANftDerivative := queryGetNftForClass(t, chainA, bridge.String(), chainAClassID) require.NotEqual(t, chainANft, chainANftDerivative) t.Logf("chain A cw721 derivative: %s", chainANftDerivative) - ownerA = queryGetOwnerOf(t, chainA, chainANftDerivative) + ownerA = queryGetOwnerOf(t, chainA, chainANftDerivative, "bad kid 1") require.Equal(t, chainA.SenderAccount.GetAddress().String(), ownerA) // Make sure that the NFT is held in the ICS721 contract now. - ownerC = queryGetOwnerOf(t, chainC, chainCNft) + ownerC = queryGetOwnerOf(t, chainC, chainCNft, "bad kid 1") require.Equal(t, bridge.String(), ownerC) // Now, lets unwind the stack. // A -> C path = getPath(0, 2) - ics721Nft(t, chainA, path, coordinator, chainANftDerivative, bridge, chainA.SenderAccount.GetAddress(), chainC.SenderAccount.GetAddress()) + ics721Nft(t, chainA, path, coordinator, chainANftDerivative, "bad kid 1", bridge, chainA.SenderAccount.GetAddress(), chainC.SenderAccount.GetAddress(), "") // NFT should now be burned on chain A. We can't ask the // contract "is this burned" so we just query and make sure it @@ -466,15 +488,15 @@ func TestSendBetweenThreeIdenticalChains(t *testing.T) { require.ErrorContains(t, err, "cw721_base::state::TokenInfo> not found") // NFT should belong to chainC sender on chain C. - ownerC = queryGetOwnerOf(t, chainC, chainCNft) + ownerC = queryGetOwnerOf(t, chainC, chainCNft, "bad kid 1") require.Equal(t, chainC.SenderAccount.GetAddress().String(), ownerC) // C -> B path = getPath(2, 1) - ics721Nft(t, chainC, path, coordinator, chainCNft, bridge, chainC.SenderAccount.GetAddress(), chainB.SenderAccount.GetAddress()) + ics721Nft(t, chainC, path, coordinator, chainCNft, "bad kid 1", bridge, chainC.SenderAccount.GetAddress(), chainB.SenderAccount.GetAddress(), "") // Received on B. - ownerB = queryGetOwnerOf(t, chainB, chainBNft) + ownerB = queryGetOwnerOf(t, chainB, chainBNft, "bad kid 1") require.Equal(t, chainB.SenderAccount.GetAddress().String(), ownerB) // Burned on C. @@ -483,10 +505,10 @@ func TestSendBetweenThreeIdenticalChains(t *testing.T) { // B -> A path = getPath(1, 0) - ics721Nft(t, chainB, path, coordinator, chainBNft, bridge, chainB.SenderAccount.GetAddress(), chainA.SenderAccount.GetAddress()) + ics721Nft(t, chainB, path, coordinator, chainBNft, "bad kid 1", bridge, chainB.SenderAccount.GetAddress(), chainA.SenderAccount.GetAddress(), "") // Received on chain A. - ownerA = queryGetOwnerOf(t, chainA, chainANft) + ownerA = queryGetOwnerOf(t, chainA, chainANft, "bad kid 1") require.Equal(t, chainA.SenderAccount.GetAddress().String(), ownerA) // Burned on chain B. @@ -520,7 +542,7 @@ func (suite *TransferTestSuite) TestMultipleAddressesInvolved() { chainANft := instantiateCw721(suite.T(), suite.chainA) mintNFT(suite.T(), suite.chainA, chainANft.String(), "bad kid 1", suite.chainA.SenderAccount.GetAddress()) - ics721Nft(suite.T(), suite.chainA, path, suite.coordinator, chainANft.String(), suite.chainABridge, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress()) + ics721Nft(suite.T(), suite.chainA, path, suite.coordinator, chainANft.String(), "bad kid 1", suite.chainABridge, suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress(), "") chainBClassID := fmt.Sprintf(`%s/%s/%s`, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, chainANft) chainBNft := queryGetNftForClass(suite.T(), suite.chainB, suite.chainBBridge.String(), chainBClassID) @@ -551,7 +573,7 @@ func (suite *TransferTestSuite) TestMultipleAddressesInvolved() { // Make another account on chain B and transfer to the new account. anotherAcount := CreateAndFundAccount(suite.T(), suite.chainB, 19) - ics721Nft(suite.T(), suite.chainA, path, suite.coordinator, chainANft.String(), suite.chainABridge, suite.chainA.SenderAccount.GetAddress(), anotherAcount.Address) + ics721Nft(suite.T(), suite.chainA, path, suite.coordinator, chainANft.String(), "bad kid 1", suite.chainABridge, suite.chainA.SenderAccount.GetAddress(), anotherAcount.Address, "") // Transfer it back to chain A using this new account. _, err = SendMsgsFromAccount(suite.T(), suite.chainB, anotherAcount, &wasmtypes.MsgExecuteContract{ @@ -718,7 +740,7 @@ func (suite *TransferTestSuite) TestPacketTimeoutCausesRefund() { suite.coordinator.TimeoutPendingPackets(path) // NFTs should be returned to sender on packet timeout. - owner := queryGetOwnerOf(suite.T(), suite.chainA, cw721.String()) + owner := queryGetOwnerOf(suite.T(), suite.chainA, cw721.String(), "bad kid 1") require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), owner) } @@ -771,6 +793,6 @@ func (suite *TransferTestSuite) TestRefundOnAckFail() { require.Equal(suite.T(), chainBNft, "") // Check that the NFT was returned to the sender due to the failure. - ownerA := queryGetOwnerOf(suite.T(), suite.chainA, chainANft.String()) + ownerA := queryGetOwnerOf(suite.T(), suite.chainA, chainANft.String(), "bad kid 1") require.Equal(suite.T(), suite.chainA.SenderAccount.GetAddress().String(), ownerA) } diff --git a/e2e/types.go b/e2e/types.go index 152ae016..c69776c6 100644 --- a/e2e/types.go +++ b/e2e/types.go @@ -37,6 +37,7 @@ type InstantiateCw721 struct { type InstantiateBridgeTester struct { AckMode string `json:"ack_mode"` + Ics721 string `json:"ics721"` } type OwnerOfResponse struct { @@ -45,6 +46,10 @@ type OwnerOfResponse struct { // about it so we just don't unmarshal. } +type TesterResponse struct { + Owner *string `json:"owner"` +} + // Owner query for ICS721 contract. type OwnerQueryData struct { TokenID string `json:"token_id"` @@ -58,10 +63,17 @@ type OwnerQuery struct { type NftContractQueryData struct { ClassID string `json:"class_id"` } + +type NftContractsQueryData struct{} + type NftContractQuery struct { NftContractForClassId NftContractQueryData `json:"nft_contract"` } +type NftContractsQuery struct { + NftContracts NftContractsQueryData `json:"nft_contracts"` +} + // Query for getting class ID given NFT contract. type ClassIdQueryData struct { Contract string `json:"contract"` @@ -95,6 +107,20 @@ type OwnerOfQuery struct { OwnerOf OwnerOfQueryData `json:"owner_of"` } +type EmptyData struct{} + +type TesterSentQuery struct { + GetSentCallback EmptyData `json:"get_sent_callback"` +} + +type TesterReceivedQuery struct { + GetReceivedCallback EmptyData `json:"get_received_callback"` +} + +type TesterNftContractQuery struct { + GetNftContract EmptyData `json:"get_nft_contract"` +} + // cw721 contract info query. type ContractInfoQueryData struct{} type ContractInfoQuery struct { diff --git a/justfile b/justfile index faba21f0..cdb51921 100644 --- a/justfile +++ b/justfile @@ -20,10 +20,10 @@ loc: # Generate optimized WASM artifacts optimize: #!/usr/bin/env sh - nohup docker run --rm -v "$(pwd)":/code --platform {{platform}} \ + docker run --rm -v "$(pwd)":/code --platform {{platform}} \ --mount type=volume,source="$(basename "$(pwd)")_cache",target=/code/target \ --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \ - {{image}} > optimize.log & + {{image}} optimize-watch: @tail -f optimize.log | bat --paging=never -l log diff --git a/packages/ics721/src/error.rs b/packages/ics721/src/error.rs index 7dbb11fa..876eda4d 100644 --- a/packages/ics721/src/error.rs +++ b/packages/ics721/src/error.rs @@ -59,4 +59,13 @@ pub enum ContractError { #[error("tokenIds, tokenUris, and tokenData must have the same length")] TokenInfoLenMissmatch {}, + + #[error("Transfer contains both redemption and a creation action")] + InvalidTransferBothActions, + + #[error("Transfer Doesn't contain any action, no redemption or creation")] + InvalidTransferNoAction, + + #[error("Couldn't find nft contract for this class id: {0}")] + NoNftContractForClassId(String), } diff --git a/packages/ics721/src/execute.rs b/packages/ics721/src/execute.rs index 575cf191..990ae8ec 100644 --- a/packages/ics721/src/execute.rs +++ b/packages/ics721/src/execute.rs @@ -1,13 +1,14 @@ use std::fmt::Debug; use cosmwasm_std::{ - from_json, instantiate2_address, to_json_binary, Addr, Binary, CodeInfoResponse, Deps, DepsMut, - Empty, Env, IbcMsg, MessageInfo, Response, StdResult, SubMsg, WasmMsg, + from_json, to_json_binary, Addr, Binary, Deps, DepsMut, Empty, Env, IbcMsg, MessageInfo, + Response, StdResult, SubMsg, WasmMsg, }; use serde::{de::DeserializeOwned, Serialize}; use sha2::{Digest, Sha256}; use crate::{ + helpers::get_instantiate2_address, ibc::{NonFungibleTokenPacketData, INSTANTIATE_CW721_REPLY_ID, INSTANTIATE_PROXY_REPLY_ID}, msg::{CallbackMsg, ExecuteMsg, IbcOutgoingMsg, InstantiateMsg, MigrateMsg}, state::{ @@ -199,7 +200,7 @@ where // so only can output binary here let class_data_string = class .data - .map_or("none".to_string(), |data| format!("{:?}", data)); + .map_or("none".to_string(), |data| format!("{data:?}")); Ok(Response::default() .add_attribute("method", "execute_receive_nft") @@ -264,29 +265,29 @@ where vec![] } else { let class_id = ClassId::new(class.id.clone()); + let cw721_code_id = CW721_CODE_ID.load(deps.storage)?; // for creating a predictable nft contract using, using instantiate2, we need: checksum, creator, and salt: // - using class id as salt for instantiating nft contract guarantees a) predictable address and b) uniqueness // for this salt must be of length 32 bytes, so we use sha256 to hash class id let mut hasher = Sha256::new(); hasher.update(class_id.as_bytes()); let salt = hasher.finalize().to_vec(); - // Get the canonical address of the contract creator - let canonical_creator = deps.api.addr_canonicalize(env.contract.address.as_str())?; - // get the checksum of the contract we're going to instantiate - let CodeInfoResponse { checksum, .. } = deps - .querier - .query_wasm_code_info(CW721_CODE_ID.load(deps.storage)?)?; - let canonical_cw721_addr = instantiate2_address(&checksum, &canonical_creator, &salt)?; - let cw721_addr = deps.api.addr_humanize(&canonical_cw721_addr)?; + + let cw721_addr = get_instantiate2_address( + deps.as_ref(), + env.contract.address.as_str(), + &salt, + cw721_code_id, + )?; // Save classId <-> contract mappings. CLASS_ID_TO_NFT_CONTRACT.save(deps.storage, class_id.clone(), &cw721_addr)?; - NFT_CONTRACT_TO_CLASS_ID.save(deps.storage, cw721_addr.clone(), &class_id)?; + NFT_CONTRACT_TO_CLASS_ID.save(deps.storage, cw721_addr, &class_id)?; let message = SubMsg::::reply_on_success( WasmMsg::Instantiate2 { admin: None, - code_id: CW721_CODE_ID.load(deps.storage)?, + code_id: cw721_code_id, msg: self.init_msg(deps.as_ref(), &env, &class)?, funds: vec![], // Attempting to fit the class ID in the label field diff --git a/packages/ics721/src/helpers.rs b/packages/ics721/src/helpers.rs new file mode 100644 index 00000000..4b912472 --- /dev/null +++ b/packages/ics721/src/helpers.rs @@ -0,0 +1,140 @@ +use cosmwasm_std::{ + from_json, instantiate2_address, to_json_binary, Addr, Binary, CodeInfoResponse, Deps, SubMsg, + WasmMsg, +}; +use serde::Deserialize; + +use crate::{ + ibc::{NonFungibleTokenPacketData, ACK_CALLBACK_REPLY_ID}, + types::{ + Ics721AckCallbackMsg, Ics721Callbacks, Ics721Memo, Ics721ReceiveCallbackMsg, Ics721Status, + ReceiverExecuteMsg, + }, + ContractError, +}; + +/// Parse the memo field into the type we want +/// Ideally it would be `Ics721Memo` type or any type that extends it +fn parse_memo Deserialize<'de>>(memo: Option) -> Option { + let binary = Binary::from_base64(memo?.as_str()).ok()?; + from_json::(&binary).ok() +} + +/// Parse callback from the memo field +fn parse_callback(memo: Option) -> Option { + parse_memo::(memo)?.callbacks +} + +// Create a subMsg that execute the callback on the sender callback +// we use a subMsg on error because we don't want to fail the whole tx +// if the callback fails +// if we were to fail the whole tx, the NFT would have been minted on +// the other chain while the NFT on this chain would not have been +// burned +pub(crate) fn ack_callback_msg( + deps: Deps, + status: Ics721Status, + packet: NonFungibleTokenPacketData, + nft_contract: String, +) -> Option { + // Get the callback object + let callbacks = parse_callback(packet.memo.clone())?; + + // Validate the address + let receiver = callbacks.ack_callback_addr.unwrap_or(packet.sender.clone()); + let contract_addr = deps.api.addr_validate(receiver.as_str()).ok()?.to_string(); + + // Create the message we send to the contract + // The status is the status we want to send back to the contract + // The msg is the msg we forward from the sender + let msg = to_json_binary(&ReceiverExecuteMsg::Ics721AckCallback( + Ics721AckCallbackMsg { + status, + nft_contract, + original_packet: packet, + msg: callbacks.ack_callback_data?, + }, + )) + .ok()?; + + Some(SubMsg::reply_on_error( + WasmMsg::Execute { + contract_addr, + msg, + funds: vec![], + }, + ACK_CALLBACK_REPLY_ID, + )) +} + +/// Get the msg and address from the memo field +/// if there is no receive callback returns None +pub(crate) fn get_receive_callback( + packet: &NonFungibleTokenPacketData, +) -> Option<(Binary, Option)> { + let callbacks = parse_callback(packet.memo.clone())?; + + Some(( + callbacks.receive_callback_data?, + callbacks.receive_callback_addr, + )) +} + +pub(crate) fn generate_receive_callback_msg( + deps: Deps, + packet: &NonFungibleTokenPacketData, + receive_callback_data: Binary, + receive_callback_addr: Option, + nft_contract: String, +) -> Option { + let callback_receiver = receive_callback_addr.unwrap_or(packet.receiver.clone()); + let contract_addr = deps + .api + .addr_validate(callback_receiver.as_str()) + .ok()? + .to_string(); + + // Create the message we send to the contract + // The status is the status we want to send back to the contract + // The msg is the msg we forward from the sender + let msg = to_json_binary(&ReceiverExecuteMsg::Ics721ReceiveCallback( + Ics721ReceiveCallbackMsg { + msg: receive_callback_data, + nft_contract, + original_packet: packet.clone(), + }, + )) + .ok()?; + + Some(WasmMsg::Execute { + contract_addr, + msg, + funds: vec![], + }) +} + +pub fn get_instantiate2_address( + deps: Deps, + creator: &str, + salt: &[u8], + code_id: u64, +) -> Result { + // Get the canonical address of the contract creator + let canonical_creator = deps.api.addr_canonicalize(creator)?; + + // get the checksum of the contract we're going to instantiate + let CodeInfoResponse { checksum, .. } = deps.querier.query_wasm_code_info(code_id)?; + + let canonical_cw721_addr = instantiate2_address(&checksum, &canonical_creator, salt)?; + + Ok(deps.api.addr_humanize(&canonical_cw721_addr)?) +} + +mod test { + #[test] + fn test_parsing() { + let memo = Some("some".to_string()); + let callbacks = super::parse_callback(memo); + println!("{callbacks:?}") + } +} diff --git a/packages/ics721/src/ibc.rs b/packages/ics721/src/ibc.rs index e316cf0b..4365bea5 100644 --- a/packages/ics721/src/ibc.rs +++ b/packages/ics721/src/ibc.rs @@ -9,6 +9,7 @@ use cw_utils::parse_reply_instantiate_data; use serde::{de::DeserializeOwned, Serialize}; use crate::{ + helpers::ack_callback_msg, ibc_helpers::{ack_fail, ack_success, try_get_ack_error, validate_order_and_version}, ibc_packet_receive::receive_ibc_packet, state::{ @@ -16,6 +17,7 @@ use crate::{ OUTGOING_CLASS_TOKEN_TO_CHANNEL, PROXY, TOKEN_METADATA, }, token_types::{ClassId, TokenId}, + types::Ics721Status, ContractError, }; @@ -27,6 +29,8 @@ pub(crate) const INSTANTIATE_PROXY_REPLY_ID: u64 = 1; /// response depending on if the submessage execution succeded or /// failed. pub(crate) const ACK_AND_DO_NOTHING: u64 = 2; +/// Reply on callback +pub(crate) const ACK_CALLBACK_REPLY_ID: u64 = 3; /// The IBC version this contract expects to communicate with. pub const IBC_VERSION: &str = "ics721-1"; @@ -181,8 +185,19 @@ where }, )?; + let callback = match ack_callback_msg( + deps.as_ref(), + Ics721Status::Success, + msg.clone(), + nft_contract.to_string(), + ) { + Some(msg) => vec![msg], + None => vec![], + }; + Ok(IbcBasicResponse::new() .add_messages(burn_notices) + .add_submessages(callback) .add_attribute("method", "acknowledge") .add_attribute("sender", msg.sender) .add_attribute("receiver", msg.receiver) @@ -208,7 +223,7 @@ where error: &str, ) -> Result { let message: NonFungibleTokenPacketData = from_json(&packet.data)?; - let nft_address = CLASS_ID_TO_NFT_CONTRACT.load(deps.storage, message.class_id.clone())?; + let nft_contract = CLASS_ID_TO_NFT_CONTRACT.load(deps.storage, message.class_id.clone())?; let sender = deps.api.addr_validate(&message.sender)?; let messages = message @@ -219,7 +234,7 @@ where OUTGOING_CLASS_TOKEN_TO_CHANNEL .remove(deps.storage, (message.class_id.clone(), token_id.clone())); Ok(WasmMsg::Execute { - contract_addr: nft_address.to_string(), + contract_addr: nft_contract.to_string(), msg: to_json_binary(&cw721::Cw721ExecuteMsg::TransferNft { recipient: sender.to_string(), token_id: token_id.into(), @@ -229,8 +244,19 @@ where }) .collect::>>()?; + let callback = match ack_callback_msg( + deps.as_ref(), + Ics721Status::Failed(error.to_string()), + message.clone(), + nft_contract.to_string(), + ) { + Some(msg) => vec![msg], + None => vec![], + }; + Ok(IbcBasicResponse::new() .add_messages(messages) + .add_submessages(callback) .add_attribute("method", "handle_packet_fail") .add_attribute("token_ids", format!("{:?}", message.token_ids)) .add_attribute("class_id", message.class_id) @@ -282,6 +308,10 @@ where SubMsgResult::Err(err) => Ok(Response::new().set_data(ack_fail(err))), } } + ACK_CALLBACK_REPLY_ID => { + let err = reply.result.unwrap_err(); + Ok(Response::new().add_attribute("error", err)) + } _ => Err(ContractError::UnrecognisedReplyId {}), } } diff --git a/packages/ics721/src/ibc_packet_receive.rs b/packages/ics721/src/ibc_packet_receive.rs index b6acceb4..bdc84f0c 100644 --- a/packages/ics721/src/ibc_packet_receive.rs +++ b/packages/ics721/src/ibc_packet_receive.rs @@ -2,13 +2,18 @@ use cosmwasm_std::{ from_json, to_json_binary, Addr, Binary, DepsMut, Empty, Env, IbcPacket, IbcReceiveResponse, StdResult, SubMsg, WasmMsg, }; +use sha2::{Digest, Sha256}; use zip_optional::Zippable; use crate::{ + helpers::{generate_receive_callback_msg, get_instantiate2_address, get_receive_callback}, ibc::{NonFungibleTokenPacketData, ACK_AND_DO_NOTHING}, ibc_helpers::{get_endpoint_prefix, try_pop_source_prefix}, msg::{CallbackMsg, ExecuteMsg}, - state::{INCOMING_CLASS_TOKEN_TO_CHANNEL, OUTGOING_CLASS_TOKEN_TO_CHANNEL, PO}, + state::{ + CLASS_ID_TO_NFT_CONTRACT, CW721_CODE_ID, INCOMING_CLASS_TOKEN_TO_CHANNEL, + OUTGOING_CLASS_TOKEN_TO_CHANNEL, PO, + }, token_types::{Class, ClassId, Token, TokenId, VoucherCreation, VoucherRedemption}, ContractError, }; @@ -51,11 +56,15 @@ pub(crate) fn receive_ibc_packet( let data: NonFungibleTokenPacketData = from_json(&packet.data)?; data.validate()?; - let local_class_id = try_pop_source_prefix(&packet.src, &data.class_id); + let cloned_data = data.clone(); let receiver = deps.api.addr_validate(&data.receiver)?; let token_count = data.token_ids.len(); - let submessage = data + // Check if NFT is local if not get the local class id + let maybe_local_class_id = try_pop_source_prefix(&packet.src, &data.class_id); + let callback = get_receive_callback(&data); + + let action_aggregator = data .token_ids .into_iter() .zip_optional(data.token_uris) @@ -63,14 +72,18 @@ pub(crate) fn receive_ibc_packet( .try_fold( Vec::::with_capacity(token_count), |mut messages, ((token_id, token_uri), token_data)| -> StdResult<_> { - if let Some(local_class_id) = local_class_id { + // If class is not local, its something new + if let Some(local_class_id) = maybe_local_class_id { let local_class_id = ClassId::new(local_class_id); - let key = (local_class_id.clone(), token_id.clone()); + let key: (ClassId, TokenId) = (local_class_id.clone(), token_id.clone()); let outgoing_channel = OUTGOING_CLASS_TOKEN_TO_CHANNEL.may_load(deps.storage, key.clone())?; + + // Make sure the channel that used for outgoing transfer, is the same you use to transfer back let returning_to_source = outgoing_channel.map_or(false, |outgoing_channel| { outgoing_channel == packet.dest.channel_id }); + if returning_to_source { // We previously sent this NFT out on this // channel. Unlock the local version for the @@ -87,6 +100,7 @@ pub(crate) fn receive_ibc_packet( // new NFT. let local_prefix = get_endpoint_prefix(&packet.dest); let local_class_id = ClassId::new(format!("{}{}", local_prefix, data.class_id)); + INCOMING_CLASS_TOKEN_TO_CHANNEL.save( deps.storage, (local_class_id.clone(), token_id.clone()), @@ -107,8 +121,60 @@ pub(crate) fn receive_ibc_packet( .fold( ActionAggregator::new(data.class_uri, data.class_data), ActionAggregator::add_action, + ); + + // All token ids in the transfer must be either a redeption or creation + // they can't be both, if they are both something is wrong. + if action_aggregator.redemption.is_some() && action_aggregator.creation.is_some() { + return Err(ContractError::InvalidTransferBothActions); + } + + // if there is a callback, generate the callback message + let callback_msg = if let Some((receive_callback_data, receive_callback_addr)) = callback { + // callback require the nft contract, get it using the class id from the action + let nft_contract = if let Some(voucher) = action_aggregator.redemption.clone() { + // If its a redemption, it means we already have the contract address in storage + + CLASS_ID_TO_NFT_CONTRACT + .load(deps.storage, voucher.class.id.clone()) + .map_err(|_| ContractError::NoNftContractForClassId(voucher.class.id.to_string())) + } else if let Some(voucher) = action_aggregator.creation.clone() { + // If its a creation action, we can use the instantiate2 function to get the nft contract + // we don't care of the contract is instantiated yet or not, as later submessage will instantiate it if its not. + // The reason we use instantiate2 here is because we don't know if it was already instantiated or not. + + let cw721_code_id = CW721_CODE_ID.load(deps.storage)?; + // for creating a predictable nft contract using, using instantiate2, we need: checksum, creator, and salt: + // - using class id as salt for instantiating nft contract guarantees a) predictable address and b) uniqueness + // for this salt must be of length 32 bytes, so we use sha256 to hash class id + let mut hasher = Sha256::new(); + hasher.update(voucher.class.id.as_bytes()); + let salt = hasher.finalize().to_vec(); + + get_instantiate2_address( + deps.as_ref(), + env.contract.address.as_str(), + &salt, + cw721_code_id, + ) + } else { + // This should never happen, as we must have at least 1 of the above actions + Err(ContractError::InvalidTransferNoAction) + }?; + + generate_receive_callback_msg( + deps.as_ref(), + &cloned_data, + receive_callback_data, + receive_callback_addr, + nft_contract.to_string(), ) - .into_submessage(env.contract.address, receiver)?; + } else { + None + }; + + let submessage = + action_aggregator.into_submessage(env.contract.address, receiver, callback_msg)?; let response = if let Some(memo) = data.memo { IbcReceiveResponse::default().add_attribute("ics721_memo", memo) @@ -185,6 +251,9 @@ impl ActionAggregator { // // see `TestDoubleSendInSingleMessage` in `/e2e/adversarial_test.go` // for a test demonstrating this. + // + // Having both redemption and creation action in the same transfer + // tells us its a malicious act that we should reject. pub fn add_action(mut self, action: Action) -> Self { match action { Action::Redemption { class_id, token_id } => { @@ -223,7 +292,12 @@ impl ActionAggregator { self } - pub fn into_submessage(self, contract: Addr, receiver: Addr) -> StdResult> { + pub fn into_submessage( + self, + contract: Addr, + receiver: Addr, + callback_msg: Option, + ) -> StdResult> { let mut m = Vec::with_capacity(2); if let Some(redeem) = self.redemption { m.push(redeem.into_wasm_msg(contract.clone(), receiver.to_string())?) @@ -231,6 +305,9 @@ impl ActionAggregator { if let Some(create) = self.creation { m.push(create.into_wasm_msg(contract.clone(), receiver.into_string())?) } + if let Some(callback_msg) = callback_msg { + m.push(callback_msg) + } let message = if m.len() == 1 { m[0].clone() } else { diff --git a/packages/ics721/src/lib.rs b/packages/ics721/src/lib.rs index 80aa05a3..5502942e 100644 --- a/packages/ics721/src/lib.rs +++ b/packages/ics721/src/lib.rs @@ -1,5 +1,6 @@ pub mod error; pub mod execute; +pub mod helpers; pub mod ibc; pub mod ibc_helpers; pub mod ibc_packet_receive; @@ -7,6 +8,7 @@ pub mod msg; pub mod query; pub mod state; pub mod token_types; +pub mod types; pub mod utils; pub use crate::error::ContractError; diff --git a/packages/ics721/src/testing/ibc_tests.rs b/packages/ics721/src/testing/ibc_tests.rs index 364b9756..b8092f48 100644 --- a/packages/ics721/src/testing/ibc_tests.rs +++ b/packages/ics721/src/testing/ibc_tests.rs @@ -1,10 +1,11 @@ +use cosmwasm_schema::cw_serde; use cosmwasm_std::{ - attr, + attr, from_json, testing::{mock_dependencies, mock_env, mock_info}, to_json_binary, to_json_vec, Addr, Attribute, Binary, DepsMut, Empty, Env, IbcAcknowledgement, IbcChannel, IbcChannelConnectMsg, IbcChannelOpenMsg, IbcEndpoint, IbcOrder, IbcPacket, IbcPacketReceiveMsg, IbcTimeout, Order, Reply, Response, StdResult, SubMsgResponse, - SubMsgResult, Timestamp, + SubMsgResult, Timestamp, WasmMsg, }; use crate::{ @@ -14,10 +15,11 @@ use crate::{ INSTANTIATE_CW721_REPLY_ID, }, ibc_helpers::{ack_fail, ack_success, try_get_ack_error}, - msg::{InstantiateMsg, QueryMsg}, + msg::{CallbackMsg, ExecuteMsg, InstantiateMsg, QueryMsg}, query::Ics721Query, state::{CollectionData, INCOMING_CLASS_TOKEN_TO_CHANNEL, NFT_CONTRACT_TO_CLASS_ID, PO}, token_types::{ClassId, TokenId}, + types::Ics721Callbacks, utils::get_collection_data, ContractError, }; @@ -84,7 +86,7 @@ fn add_channel(mut deps: DepsMut, env: Env, channel_id: &str) { .unwrap(); let connect_msg = IbcChannelConnectMsg::new_ack(channel.clone(), IBC_VERSION); let res = Ics721Contract::default() - .ibc_channel_connect(deps.branch(), env.clone(), connect_msg) + .ibc_channel_connect(deps.branch(), env, connect_msg) .unwrap(); // Smoke check our attributes @@ -155,7 +157,7 @@ fn test_reply_cw721() { let cw721_addr = Addr::unchecked("cosmos2contract"); let class_id = ClassId::new("wasm.address1/channel-10/address2"); NFT_CONTRACT_TO_CLASS_ID - .save(deps.as_mut().storage, cw721_addr.clone(), &class_id) + .save(deps.as_mut().storage, cw721_addr, &class_id) .unwrap(); let res = Ics721Contract::default() @@ -651,3 +653,87 @@ fn test_no_receive_when_paused() { .unwrap() .starts_with("contract is paused pending governance intervention")) } + +#[test] +fn test_different_memo_ignored() { + #[cw_serde] + struct DifferentMemo { + different: Option, + extra: Option, + } + + let dest_callback = to_json_binary(&()).unwrap(); + let data = NonFungibleTokenPacketData { + class_id: ClassId::new("id"), + class_uri: None, + class_data: None, + token_ids: vec![TokenId::new("1")], + token_uris: None, + token_data: None, + sender: "violet".to_string(), + receiver: "blue".to_string(), + memo: Some( + to_json_binary(&DifferentMemo { + different: Some(Ics721Callbacks { + ack_callback_data: Some(to_json_binary("some_random").unwrap()), + ack_callback_addr: None, + receive_callback_data: Some(dest_callback), + receive_callback_addr: None, + }), + extra: None, + }) + .unwrap() + .to_string(), + ), + }; + let ibc_packet = mock_packet(to_json_binary(&data).unwrap()); + let packet = IbcPacketReceiveMsg::new(ibc_packet, Addr::unchecked(RELAYER_ADDR)); + let mut deps = mock_dependencies(); + let env = mock_env(); + PO.set_pauser(&mut deps.storage, &deps.api, None).unwrap(); + + // Memo is ignored here, because it's not a valid ICS721Memo + let res = Ics721Contract::default() + .ibc_packet_receive(deps.as_mut(), env, packet) + .unwrap(); + + let memo_callback_msg = match res.messages[0].msg.clone() { + cosmwasm_std::CosmosMsg::Wasm(WasmMsg::Execute { msg, .. }) => { + match from_json::(msg).unwrap() { + ExecuteMsg::Callback(callback_msg) => match callback_msg { + CallbackMsg::Conjunction { operands } => Some(operands), + _ => Some(vec![]), + }, + _ => None, + } + } + _ => None, + }; + assert!(memo_callback_msg.is_some()); + assert!(memo_callback_msg.unwrap().is_empty()); +} + +#[test] +fn test_ibc_packet_not_json_memo() { + let data = NonFungibleTokenPacketData { + class_id: ClassId::new("wasm.address1/channel-1/id"), + class_uri: None, + class_data: None, + token_ids: vec![TokenId::new("1")], + token_uris: None, + token_data: None, + sender: "violet".to_string(), + receiver: "blue".to_string(), + memo: None, + }; + + let ibc_packet = mock_packet(to_json_binary(&data).unwrap()); + let packet = IbcPacketReceiveMsg::new(ibc_packet, Addr::unchecked(RELAYER_ADDR)); + let mut deps = mock_dependencies(); + let env = mock_env(); + PO.set_pauser(&mut deps.storage, &deps.api, None).unwrap(); + + Ics721Contract::default() + .ibc_packet_receive(deps.as_mut(), env, packet) + .unwrap(); +} diff --git a/packages/ics721/src/testing/integration_tests.rs b/packages/ics721/src/testing/integration_tests.rs index 1a4ddecd..c5dcd607 100644 --- a/packages/ics721/src/testing/integration_tests.rs +++ b/packages/ics721/src/testing/integration_tests.rs @@ -183,7 +183,7 @@ impl Api for MockApiBech32 { } } } - Err(StdError::generic_err(format!("Invalid input: {}", input))) + Err(StdError::generic_err(format!("Invalid input: {input}"))) } fn addr_humanize(&self, canonical: &CanonicalAddr) -> StdResult { @@ -319,16 +319,14 @@ impl Test { app.api().addr_make(ICS721_CREATOR), &InstantiateMsg { cw721_base_code_id: source_cw721_id, - proxy: proxy.clone(), + proxy, pauser: admin_and_pauser .clone() .map(|p| app.api().addr_make(&p).to_string()), }, &[], "ics721-base", - admin_and_pauser - .clone() - .map(|p| app.api().addr_make(&p).to_string()), + admin_and_pauser.map(|p| app.api().addr_make(&p).to_string()), ) .unwrap(); @@ -616,7 +614,7 @@ fn test_do_instantiate_and_mint() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 1); - assert_eq!(nft_contracts[0].0, class_id.to_string()); + assert_eq!(nft_contracts[0].0, class_id); // Get the address of the instantiated NFT. let nft_contract: Addr = test .app @@ -694,7 +692,7 @@ fn test_do_instantiate_and_mint() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id.to_string(), + class_id, }, ) .unwrap(); @@ -770,7 +768,7 @@ fn test_do_instantiate_and_mint() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 1); - assert_eq!(nft_contracts[0].0, class_id.to_string()); + assert_eq!(nft_contracts[0].0, class_id); // Get the address of the instantiated NFT. let nft_contract: Addr = test .app @@ -848,7 +846,7 @@ fn test_do_instantiate_and_mint() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id.to_string(), + class_id, }, ) .unwrap(); @@ -923,7 +921,7 @@ fn test_do_instantiate_and_mint() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 1); - assert_eq!(nft_contracts[0].0, class_id.to_string()); + assert_eq!(nft_contracts[0].0, class_id); // Get the address of the instantiated NFT. let nft_contract: Addr = test .app @@ -1001,7 +999,7 @@ fn test_do_instantiate_and_mint() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id.to_string(), + class_id, }, ) .unwrap(); @@ -1080,7 +1078,7 @@ fn test_do_instantiate_and_mint() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 1); - assert_eq!(nft_contracts[0].0, class_id.to_string()); + assert_eq!(nft_contracts[0].0, class_id); // Get the address of the instantiated NFT. let nft_contract: Addr = test .app @@ -1158,7 +1156,7 @@ fn test_do_instantiate_and_mint() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id.to_string(), + class_id, }, ) .unwrap(); @@ -1261,8 +1259,8 @@ fn test_do_instantiate_and_mint_2_different_collections() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let nft_contracts = test.query_nft_contracts(); assert_eq!(nft_contracts.len(), 2); - assert_eq!(nft_contracts[0].0, class_id_1.to_string()); - assert_eq!(nft_contracts[1].0, class_id_2.to_string()); + assert_eq!(nft_contracts[0].0, class_id_1); + assert_eq!(nft_contracts[1].0, class_id_2); // Get the address of the instantiated NFT. let nft_contract_1: Addr = test .app @@ -1404,7 +1402,7 @@ fn test_do_instantiate_and_mint_2_different_collections() { test.ics721.clone(), &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id_1.to_string(), + class_id: class_id_1, }, ) .unwrap(); @@ -1415,7 +1413,7 @@ fn test_do_instantiate_and_mint_2_different_collections() { test.ics721, &QueryMsg::Owner { token_id: "1".to_string(), - class_id: class_id_2.to_string(), + class_id: class_id_2, }, ) .unwrap(); @@ -1503,7 +1501,7 @@ fn test_do_instantiate_and_mint_no_instantiate() { // Check entry added in CLASS_ID_TO_NFT_CONTRACT let class_id_to_nft_contract = test.query_nft_contracts(); assert_eq!(class_id_to_nft_contract.len(), 1); - assert_eq!(class_id_to_nft_contract[0].0, class_id.to_string()); + assert_eq!(class_id_to_nft_contract[0].0, class_id); // 2nd call will only do a mint as the contract for the class ID has // already been instantiated. @@ -1540,12 +1538,7 @@ fn test_do_instantiate_and_mint_no_instantiate() { let nft_contract: Addr = test .app .wrap() - .query_wasm_smart( - test.ics721, - &QueryMsg::NftContract { - class_id: class_id.to_string(), - }, - ) + .query_wasm_smart(test.ics721, &QueryMsg::NftContract { class_id }) .unwrap(); // Make sure we have our tokens. @@ -1739,7 +1732,7 @@ fn test_receive_nft() { test.ics721, &ExecuteMsg::ReceiveNft(cw721::Cw721ReceiveMsg { sender: test.source_cw721_owner.to_string(), - token_id: token_id.clone(), + token_id, msg: to_json_binary(&IbcOutgoingMsg { receiver: NFT_OWNER_TARGET_CHAIN.to_string(), // nft owner for other chain, on this chain ics721 is owner channel_id: "channel-0".to_string(), @@ -1789,7 +1782,7 @@ fn test_receive_nft() { .unwrap(); assert_eq!( class_data_attribute.value, - format!("{:?}", expected_collection_data) + format!("{expected_collection_data:?}") ); } // test case: backward compatibility - receive nft also works for old/v016 cw721-base @@ -1807,7 +1800,7 @@ fn test_receive_nft() { test.ics721, &ExecuteMsg::ReceiveNft(cw721::Cw721ReceiveMsg { sender: test.source_cw721_owner.to_string(), - token_id: token_id.clone(), + token_id, msg: to_json_binary(&IbcOutgoingMsg { receiver: NFT_OWNER_TARGET_CHAIN.to_string(), // nft owner for other chain, on this chain ics721 is owner channel_id: "channel-0".to_string(), @@ -1860,7 +1853,7 @@ fn test_receive_nft() { .unwrap(); assert_eq!( class_data_attribute.value, - format!("{:?}", expected_collection_data) + format!("{expected_collection_data:?}") ); } } diff --git a/packages/ics721/src/types.rs b/packages/ics721/src/types.rs new file mode 100644 index 00000000..e181e04f --- /dev/null +++ b/packages/ics721/src/types.rs @@ -0,0 +1,71 @@ +use cosmwasm_schema::{cw_serde, schemars::JsonSchema}; +use cosmwasm_std::Binary; +use serde::{Deserialize, Serialize}; + +use crate::ibc::NonFungibleTokenPacketData; + +#[derive(Serialize, Deserialize, JsonSchema, Clone, Debug, PartialEq)] +#[allow(clippy::derive_partial_eq_without_eq)] +#[schemars(crate = "cosmwasm_schema::schemars")] +#[serde(crate = "cosmwasm_schema::serde")] +pub struct Ics721Memo { + pub callbacks: Option, +} + +/// The format we expect for the memo field on a send +#[cw_serde] +pub struct Ics721Callbacks { + /// Data to pass with a callback on source side (status update) + /// Note - If this field is empty, no callback will be sent + pub ack_callback_data: Option, + /// The address that will receive the callback message + /// Defaults to the sender address + pub ack_callback_addr: Option, + /// Data to pass with a callback on the destination side (ReceiveNftIcs721) + /// Note - If this field is empty, no callback will be sent + pub receive_callback_data: Option, + /// The address that will receive the callback message + /// Defaults to the receiver address + pub receive_callback_addr: Option, +} + +/// A message is that is being called on receiving the NFT after transfer was completed. +/// Receiving this message means that the NFT was successfully transferred. +/// You must verify this message was called by an approved ICS721 contract, either by code_id or address. +#[cw_serde] +pub struct Ics721ReceiveCallbackMsg { + pub nft_contract: String, + pub original_packet: NonFungibleTokenPacketData, + pub msg: Binary, +} + +/// A message to update your contract of the status of a transfer +/// status = Ics721Status::Success - the transfer was successful and NFT is on the other chain +/// status = Ics721Status::Failed - Transfer failed and contract still owns the NFT +#[cw_serde] +pub struct Ics721AckCallbackMsg { + pub status: Ics721Status, + pub nft_contract: String, + pub original_packet: NonFungibleTokenPacketData, + pub msg: Binary, +} + +/// The status of a transfer on callback +#[cw_serde] +pub enum Ics721Status { + Success, + Failed(String), +} + +/// This is a wrapper for ics721 callbacks +/// so contracts will be able to recieve both status update and on receive hook. +#[cw_serde] +pub enum ReceiverExecuteMsg { + /// Being called on receiving the NFT after transfer was completed. (destination side) + /// `on_recieve` hook + /// Note - Failing this message will fail the transfer. + Ics721ReceiveCallback(Ics721ReceiveCallbackMsg), + /// Being called as a status update of the transfer. (source side) + /// Note - Failing this message will NOT fail the transfer, its just a status update. + Ics721AckCallback(Ics721AckCallbackMsg), +}