Skip to content

feat: ckb <-> axon transfer test#332

Merged
Flouse merged 12 commits intomainfrom
ckb-axon-transfer-test
Oct 8, 2023
Merged

feat: ckb <-> axon transfer test#332
Flouse merged 12 commits intomainfrom
ckb-axon-transfer-test

Conversation

@blckngm
Copy link
Copy Markdown
Contributor

@blckngm blckngm commented Sep 25, 2023

Description

Added ckb <-> axon, SUDT <-> ERC20 transfer test.


PR author checklist:

  • Added tests: integration (for Forcerelay) or unit/mock tests (for modules).
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.

@blckngm blckngm force-pushed the ckb-axon-transfer-test branch from 1496f9e to 13051ca Compare September 25, 2023 11:13
@Flouse Flouse requested review from ashuralyk and jjyr September 26, 2023 01:55
Comment thread tools/ibc-test/contracts/version
@blckngm blckngm force-pushed the ckb-axon-transfer-test branch from e9a7836 to 2b3e86b Compare September 26, 2023 05:45
@blckngm blckngm marked this pull request as ready for review September 27, 2023 03:10
Comment thread tools/ibc-test/src/framework/utils/axon.rs
Comment thread tools/ibc-test/src/tests/ibc/sudt_erc20_transfer.rs
Comment thread tools/ibc-test/src/tests/ibc/sudt_erc20_transfer.rs
Comment thread tools/ibc-test/src/tests/ibc/sudt_erc20_transfer.rs
log::info!("Received SUDT");

// Sleep some time so the ack can be written to axon.
std::thread::sleep(Duration::from_secs(60));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can here also run a Js script to listen and check the write-back of ack packet ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's certainly possible. What function/event should be checked? Is it AcknowledgePacket?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like using this method to emit a custom event which takes as a symbol of operation finished

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think AcknowledgePacket is better because it doesn't require modifying the contract to emit an event just for testing. I'll try this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it work, for listening AcknowledgePacket event from Axon?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the acknowledge_packet function is never called on axon.

@ashuralyk
Copy link
Copy Markdown
Contributor

ashuralyk commented Sep 27, 2023

please check the CI output and figure out the new added test code was not impacted by the errors from Axon event monitor

https://github.com/synapseweb3/forcerelay/actions/runs/6310730564/job/17133270135?pr=332 (move into Run IBC tests)

@Flouse Flouse requested a review from 15168316096 September 27, 2023 04:39
@blckngm
Copy link
Copy Markdown
Contributor Author

blckngm commented Sep 27, 2023

The errors are probably from previous tests.

Copy link
Copy Markdown
Collaborator

@15168316096 15168316096 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See if it is necessary to add lld dependency installation instructions in the readme to prevent errors when compiling axon,
ref:

Comment thread crates/relayer/src/chain/axon.rs
#[async_trait]
pub trait AxonRpc {
async fn get_block_by_id(&self, block_id: BlockId) -> Response<AxonBlock>;
async fn get_block_by_id(&self, block_id: BlockId) -> Response<Option<AxonBlock>>;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of Option is to increase the robustness of rpc, but considering that the return type of the interface has also changed, compatibility needs to be considered.

Comment thread crates/relayer/src/light_client/axon.rs
Copy link
Copy Markdown
Collaborator

@15168316096 15168316096 Sep 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you want to verify the integration test locally, you need to pay attention to the environment variables involved in the workflows and whether you need to supplement the local readme instructions. For example, in ubuntu, you can use the following command to carry out the integration test

$ export PATH=$PATH:/tmp/ckb_v0.110.0_x86_64-unknown-linux-gnu
@nikaJames95 ➜ /workspaces/forcerelay (ckb-axon-transfer-test) $ export PATH=$PATH:${SRC_DIR}/axon/target/release/
@nikaJames95 ➜ /workspaces/forcerelay (ckb-axon-transfer-test) $ export PATH=$PATH:${SRC_DIR}/ibc-solidity-contract
export CHAIN_COMMAND_PATHS="value1,value2"
export ACCOUNT_PREFIXES="prefix1,prefix2"
export AXON_SRC_PATH="/path/to/axon"
export IBC_CONTRACTS_SRC_PATH="/path/to/ibc-solidity-contract"
export RUST_LOG="info"
export RUST_BACKTRACE="1"

Comment thread tools/ibc-test/src/tests/ibc/sudt_erc20_transfer.rs
@blckngm blckngm force-pushed the ckb-axon-transfer-test branch from b255da8 to a20e0be Compare October 7, 2023 09:19

impl TestOverrides for SudtErc20TransferTest {}

impl BinaryConnectionTest for SudtErc20TransferTest {
Copy link
Copy Markdown
Collaborator

@jjyr jjyr Oct 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to encapsule these code into this trait?

We can use this contract to replace the default transfer module - the 'Mock' contract

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can refactor this later for a generic transfer test. For now ckb as sink zone is not supported so the only practically supported scenario is CKB source SUDT to/from axon voucher ERC20.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related discussion: #332 (comment)

@Flouse Flouse linked an issue Oct 7, 2023 that may be closed by this pull request
1 task
@Flouse Flouse merged commit cebfec8 into main Oct 8, 2023
@Flouse Flouse deleted the ckb-axon-transfer-test branch October 8, 2023 03:38
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

Successfully merging this pull request may close these issues.

axon_event_monitor Error: Websocket closed unexpectedly

5 participants