Client integration tests against framework modules

[andygolay]

Summary

• Update client tests so they run against the Movement framework in our fork of aptos-core

Changelog

• Added client_l2move_l1move and client_l1move_l2move files with tests against framework.
• Fix get_bridge_transfer_details_counterparty which was returning a struct with initiator and recipient address types reversed. Implemented a BridgeTransferDetailsCounterparty struct to achieve this.

Testing

To run client tests:

1. Navigate to the root of the movement repo, checkout the andygolay/framework-client-tests branch, and start a local Suzuka node:

CELESTIA_LOG_LEVEL=FATAL CARGO_PROFILE=release CARGO_PROFILE_FLAGS=--release nix develop --extra-experimental-features nix-command --extra-experimental-features flakes --command bash  -c "just bridge native build.setup.eth-local.celestia-local --keep-tui"

Note, if you have any .movement directories present, e.g. when re-running tests, the directories must be deleted before starting the node.

2. In the generated .movement directory, there will be a config.yaml. In that file, change f90391c81027f03cdea491ed8b36ffaced26b6df208a9b569e5baf2590eb9b16 to 0xA550C18 so the file looks like:

---
profiles:
  default:
    network: Custom
    private_key: "0x0000000000000000000000000000000000000000000000000000000000000001"
    public_key: "0x4cb5abf6ad79fbf5abbccafcc269d85cd2651ed4b885b5869f241aedf0a5ba29"
    account: "0xA550C18"
    rest_url: "http://0.0.0.0:30731/"
    faucet_url: "http://0.0.0.0:30732/"

3. Run each set of Movement client tests:

L1 Move to L2 Move:

rust_backtrace=1 cargo test --test client_l1move_l2move -- --nocapture --test-threads=1

L2 Move to L1 Move:

rust_backtrace=1 cargo test --test client_l2move_l1move -- --nocapture --test-threads=1

Outstanding issues

• The refund and abort tests have a sleep of 20 seconds before calling refund or abort. Just a heads up.
• If your node has been running for awhile, L1 --> L2 tests are sometimes flaky at the extract_bridge_transfer_id_framework call, where it won't find a matching transaction. I don't know why that happens but restarting the node and re-running steps 1 and 2 of the above test directions appears to fix it so tests pass.
• There was an issue with not finding a coin store for the recipient in client_l1move_l2_move test_movement_client_complete_transfer. The fix I implemented for it is rather hacky. That test should be cleaned up but perhaps in a later iteration.

[0xmovses]

This looks good, I will do a thorough review once we have the governance on testnet devnet issue resolved for local client tests.

[0xmovses]

Thank you for this!

Main thing here is that the client_l1move_l2move.rs tests are testing with the old AtomicBridgeInitiator/ Counterparty contract I believe. The abis used in the client handler fns are those contracts not AtomicBridgeInitiatorMOVE.

However, this successfully proves that clients are able to to call the framework functions as expected.

The lack of configurability of ABI on ETH side could cause issues. We assume right now they are the same, which is why this works, but if there were updates to the AtomicBridgeInitiatorMOVE contract, the Clients would break, becase they are always using the sol type AtomicBridgeInitiator.

Before testing e2e we should add this configurability. I'm happy to add this onto this PR if you like while we switch.

[0xmovses]

Where does this address come from?

[andygolay]

It's the dummy valid EIP-55 address used in the framework modules.

[andygolay]

I'll add a comment.

[0xmovses]

Dup line here.

[0xmovses]

These should be testing the MOVE.sol contracts seems like they are initialised currently with the old Initiator contracts. In other words this it AtomicBridgeInitiator -> COIN. Right?

[andygolay]

I didn't touch the Eth client tests, I think I had gotten it in my mind that we were using the E2E tests to tests the .sol contracts; I was just using the client tests to check the Movement client against the framework.

One thing I notice on main is that there aren't Eth tests in client_movement_eth` anymore. Not sure why or how they disappeared.

[0xmovses]

Don't worry about this, I'm going to make sure the .sol MOVE contrats are supported by the relayer in a seperate PR. Right now it only uses the old ABIs for WETH contracts.

[0xmovses]

On second thought, let me add the configurability in a follow up PR, to reduce the scope creep on this. I will add a fast followup PR after this goes in.

[andygolay]

On second thought, let me add the configurability in a follow up PR, to reduce the scope creep on this. I will add a fast followup PR after this goes in.

Sounds good, looking into the diff to see how to fix the E2E test

[0xmovses]

I followed the testing steps written above and all the client tests are passing 🥳

[0xmovses]

I've triggered all the CI to ensure that bumping aptos doesn't introduce any failure, it shouldn't as its only the framework code. After those pass will be good to approve this.