Incoming proxy #76

taitruong · 2023-12-27T22:23:57Z

Similar to outgoing proxy (for source chain), this PR adds an optional incoming proxy for target chain. In case an incoming proxy is defined, incoming IbcPacket and NonFungibleTokenPacketData is passed to proxy.

A default implementation can be found here: https://github.com/arkprotocol/cw721-proxy/tree/main/contracts/cw721-incoming-proxy

The incoming proxy validates from which channel (=chain and ics721) IbcPacket is coming from. In case channel is not whitelisted an UnauthorizedSourceChannel is thrown.

An incoming proxy is needed in case chain or ics721 on source/other chain has been compromised.

taitruong · 2023-12-27T22:51:43Z

packages/ics721-types/src/types.rs

+    /// Being called on receiving the NFT before transfer is completed. (destination side)
+    /// `on_recieve` hook
+    /// Note - Failing this message will fail the transfer.
+    Ics721ReceivePacketMsg {
+        packet: IbcPacket,
+        data: NonFungibleTokenPacketData,
+    },


In case optional incoming proxy is set, during ibc_packet_receive this msg is called on proxy.

packages/ics721/src/execute.rs

taitruong · 2023-12-27T22:59:31Z

packages/ics721/src/ibc_packet_receive.rs

    let data: NonFungibleTokenPacketData = from_json(&packet.data)?;
    data.validate()?;

+    // If there is an incoming proxy, let proxy validate the packet, in case it fails, we fail the transfer


Here's where incoming proxy is getting called on target chain. In case proxy throws an error, NFT is returned back to soure chain. For details check here: https://github.com/arkprotocol/cw721-proxy/blob/main/packages/cw-incoming-proxy/src/lib.rs#L44-L56

packages/ics721/src/testing/contract.rs

packages/ics721/src/testing/integration_tests.rs

taitruong · 2023-12-27T23:05:44Z

packages/ics721/src/testing/integration_tests.rs

@@ -2025,7 +2073,8 @@ fn test_migration() {
                new_code_id: test.ics721_id,
                msg: to_json_binary(&MigrateMsg::WithUpdate {
                    pauser: None,
-                    proxy: None,
+                    incoming_proxy: None,


test migration

taitruong · 2023-12-27T23:07:31Z

ts-relayer-tests/src/ics721.spec.ts

@@ -186,6 +244,115 @@ test.serial("transfer NFT", async (t) => {
  t.is(osmoAddr, tokenOwner.owner);
 });

+test.serial("transfer NFT with incoming proxy: osmo -> wasmd", async (t) => {


relayer test for incoming proxy with 2 scenarios:

nft send to WLed channel

nft send to unknown channel

taitruong · 2023-12-27T23:08:25Z

ts-relayer-tests/src/ics721.spec.ts

+    nft_contract: { class_id: wasmClassId },
+  });
+
+  tokenOwner = await ownerOf(wasmClient, wasmCw721, tokenId);


scenario 1: NFT received on target chain

taitruong · 2023-12-27T23:08:55Z

ts-relayer-tests/src/ics721.spec.ts

+  // assert NFT on chain A is returned to owner
+  tokenOwner = await ownerOf(osmoClient, osmoCw721, tokenId);
+  t.is(osmoClient.senderAddress, tokenOwner.owner);
+  t.log(`NFT #${tokenId} returned to owner`);


scenario 2: NFT returned to source chain.

taitruong · 2023-12-28T15:12:43Z

packages/ics721/src/execute.rs

        // use by default ClassId, in case there's no class data with name and symbol
        let mut instantiate_msg = cw721_base::msg::InstantiateMsg {
            name: class.id.clone().into(),
            symbol: class.id.clone().into(),
            minter: env.contract.address.to_string(),
+            withdraw_address: Some(creator),


Allow creator of ics721 for setting withdraw address on nft contract (allowing to send and withdraw funds). This affected only ics721-base for permissionless chains.
sg-ics721 is unchanged.
More on withdraw address, see here: public-awesome/cw-nfts#149

packages/ics721-types/src/ibc.rs

humanalgorithm · 2023-12-28T22:40:09Z

packages/ics721-types/src/token_types.rs

+}
+
+impl<'a> Bounder<'a> for ClassId {
+    fn inclusive_bound(self) -> Option<cw_storage_plus::Bound<'a, Self>> {


What exactly is this doing?

This is needed in query_nft_contracts() since paginated query requires a bounder. Simple need trick for inferring a bounder: https://github.com/arkprotocol/ark-cw-ics721/blob/incoming_proxy/packages/ics721/src/query.rs#L138
Bounder defined here: https://github.com/DA0-DA0/dao-contracts/blob/v2.4.0/packages/cw-paginate-storage/src/lib.rs#L18

Well my question is, it seems like the code is just copy paste something that already exists? Doesn't it already get this functionality through an import? What specifically is changing in the override?

No, it doesn't exist yet. If I dont implement bounder, then I have to explicitely provide bounder in query.

packages/ics721-types/src/token_types.rs

packages/ics721/src/execute.rs

packages/ics721/src/ibc.rs

taitruong · 2023-12-30T23:11:39Z

Can a high-level description be added to this PR?

Done - description added above.

taitruong · 2023-12-30T23:18:24Z

ts-relayer-tests/src/ics721.spec.ts

+test.serial(
+  "transfer NFT with osmo outgoing and wasm incoming proxy",
+  async (t) => {


NEW: added ts-relayer integration test for:

ics721 handles incoming msg from outgoing proxy properly

ics721 passes ibc packet to incoming proxy

success: in case packet is coming from WLed channel

fail: in case channel is not WLed -> NFT returned back to onwer on source chain

taitruong · 2023-12-31T10:58:45Z

packages/ics721/src/execute.rs

            }) => self.execute_receive_nft(deps, env, info, token_id, sender, msg),
-            ExecuteMsg::ReceiveProxyNft { eyeball, msg } => {
-                self.execute_receive_proxy_nft(deps, env, info, eyeball, msg)
-            }


ReceiveProxyNft has been removed, instead ReceiveNft handles both: msgs from collection and outgoing proxy contract

Art3miX

looks good

packages/ics721/src/execute.rs

use creator for withdraw address

0ebc458

taitruong commented Dec 27, 2023

View reviewed changes

taitruong requested review from Art3miX, shanev, jhernandezb and humanalgorithm December 27, 2023 23:28

taitruong added 17 commits December 28, 2023 14:49

update branch

659c87e

use cw721-base with withdraw address

d74b39d

fix tests

3a06220

rename proxy > outgoing proxy

3b6aac1

fix test

de167a3

update libs

26ee914

optional incoming proxy

8b808ee

cargo clippy and fmt

e8f22b4

move types to dedicated package

7c5e9c3

cleanup

f5bba4c

integration test for incoming proxy

b8fcdca

cleanup

61a96cb

test sending to non-WLed channel in incoming proxy

0921f66

cleanup

a488a23

docs

4e2f400

use double quote to prevent globbing and word splitting

b1f5c07

fix CI

60277c0

taitruong force-pushed the incoming_proxy branch from f112d26 to 60277c0 Compare December 28, 2023 15:05

taitruong commented Dec 28, 2023

View reviewed changes

humanalgorithm reviewed Dec 28, 2023

View reviewed changes

packages/ics721-types/src/ibc.rs Outdated Show resolved Hide resolved

humanalgorithm reviewed Dec 28, 2023

View reviewed changes

packages/ics721-types/src/token_types.rs Show resolved Hide resolved

humanalgorithm reviewed Dec 28, 2023

View reviewed changes

packages/ics721/src/execute.rs Outdated Show resolved Hide resolved

humanalgorithm reviewed Dec 28, 2023

View reviewed changes

packages/ics721/src/ibc.rs Outdated Show resolved Hide resolved

taitruong added 8 commits December 30, 2023 19:26

cleanup

549e9a0

ts-relayer test for receive nft via outgoing proxy

10d64ed

fix test

0cdc119

cleanup

28d28fe

cleanup

fee2aaa

moved to helper

03091a0

increase capacity to 3

88ae912

update versions

62e118e

taitruong commented Dec 30, 2023

View reviewed changes

docs

a7a4778

taitruong requested a review from Art3miX December 31, 2023 08:17

taitruong commented Dec 31, 2023

View reviewed changes

Art3miX approved these changes Dec 31, 2023

View reviewed changes

packages/ics721/src/execute.rs Show resolved Hide resolved

taitruong added 7 commits December 31, 2023 13:23

detailed docs on receive nft

b930ded

update libs

f8c42c0

update lib

84ed066

update wasm files for tests

565f56e

update cw721-proxy

d796fc9

update incoming wasm

8bcfc38

update incoming proxy

3ec3ed5

humanalgorithm approved these changes Jan 2, 2024

View reviewed changes

taitruong added 3 commits January 2, 2024 12:14

use tag cw-721proxy v0.1.0

a2d4965

rename cw721 -> cw-ics721

1fadd3b

cw-ics721-proxy: update lib

0b4fd68

shanev approved these changes Jan 3, 2024

View reviewed changes

taitruong added this pull request to the merge queue Jan 3, 2024

Merged via the queue into public-awesome:main with commit f1dfefc Jan 3, 2024
1 check passed

taitruong deleted the incoming_proxy branch June 10, 2024 17:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incoming proxy #76

Incoming proxy #76

taitruong commented Dec 27, 2023 •

edited

Loading

taitruong Dec 27, 2023 •

edited

Loading

taitruong Dec 27, 2023

taitruong Dec 27, 2023

taitruong Dec 27, 2023

taitruong Dec 27, 2023

taitruong Dec 27, 2023

taitruong Dec 28, 2023

humanalgorithm Dec 28, 2023

taitruong Dec 28, 2023

humanalgorithm Dec 30, 2023

taitruong Dec 30, 2023

taitruong commented Dec 30, 2023

taitruong Dec 30, 2023

taitruong Dec 31, 2023

Art3miX left a comment

Incoming proxy #76

Incoming proxy #76

Conversation

taitruong commented Dec 27, 2023 • edited Loading

taitruong Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taitruong commented Dec 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Art3miX left a comment

Choose a reason for hiding this comment

taitruong commented Dec 27, 2023 •

edited

Loading

taitruong Dec 27, 2023 •

edited

Loading