Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go-based e2e testing for callbacks and cw721 backward compatibility #101

Open
taitruong opened this issue Jun 10, 2024 · 1 comment
Open

Comments

@taitruong
Copy link
Collaborator

taitruong commented Jun 10, 2024

cw-ics721 supports NFT transfer for cw721 v16 contracts and higher - up to latest version v18. Rn, e2e tests covers only v18: https://github.com/CosmWasm/cw-nfts/releases/tag/v0.18.0

e2e should cover tests for handling different

  1. cw721 contract versions: v16, v17 and latest v18 version
  2. cw721 code ids: when transferring an NFT, on target chain cw-ics721 creates a new collection (and NFT). For this CW721_CODE_ID store is used. Ideally all cw-ics721 contracts are using same version across all chains. But in reality this might not be the case. Like for Stargaze there is a custom sg721-base.

Search in go files where cw721_base_v0.18.0.wasm is used and uploaded.

Check in TestSendBetweenThreeIdenticalChains:

// 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. :)
func TestSendBetweenThreeIdenticalChains(t *testing.T) {
...
}

Here it uses always cw721 v18 for transfering to chains A -> B -> C -> A -> C -> B -> A.

In instantiateCw721() and in instantiateBridge() it always uses cw721 v18.

A test case could be changing this:

  • 3 tests cases for TestSendBetweenThreeIdenticalChains for transferring NFT v16, v17 and v18
  • different CW721_CODE_IDs for `cw-ics721 on chain A, B, and C:
    • chain A and B: cw721-base v18
    • chain C: sg721-base using v3.14.0

wasm files are store in ./external-wasm, containing:

  • cw721_base_v0.18.0.wasm
  • cw721_incoming_proxy.wasm
  • cw721_outgoing_proxy_rate_limit.wasm

please note that for SG we also need sg_ics721.wasm

@taitruong
Copy link
Collaborator Author

taitruong commented Jun 10, 2024

There is a callback bug fix in PR #98 - so this issue should work on this PR branch class_id_fix.

The callback is as follows:

  • transfer NFT with receive callback
  • in case no callback addr is provided, nft contract address on target chain (escrowed by ics721) is used
  • before fix it was using instantiate2 for retrieving address
  • now in fix it looks up in CLASS_ID_AND_NFT_CONTRACT_INFO store for address
  • if - and only if - there's is None in CLASS_ID_AND_NFT_CONTRACT_INFO, then it uses instantiate2

So for receive callback there are 3 test cases:

  1. initial nft transfer, where no nft contract is created yet by ics721 on target chain
  2. 2nd NFT transfer, with exsiting nft contract - instantiated and escrowed by ics721
  3. CW721_CODE_ID has changed to another code id in ics721, 3rd NFT transfer should also work

For all 3 test cases, callbacks are ALWAYS executed on the same, escrowed nft contract on target chain!

Receive callback should work on both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant