atomicDEX-API icon indicating copy to clipboard operation
atomicDEX-API copied to clipboard
verified publisher icon KomodoPlatform

feat(eth-swap): eth tpu v2 methods, eth docker test enhancements

Open laruh opened this issue 1 year ago • 1 comments

Note: This PR contains two ignore tendermint tests commits, which should be removed after PR approve

https://github.com/KomodoPlatform/komodo-defi-framework/pull/2169/commits/c26c7a51154c46dcdbc808378b59e356d72459cc https://github.com/KomodoPlatform/komodo-defi-framework/pull/2169/commits/dc215ec1b19e4e120d502c1b02365ae829fe33e4

eth tpu v2 changes

TakerCoinSwapOpsV2 trait implemented methods for EthCoin

  • [x] send_taker_funding
  • [x] validate_taker_funding
  • [x] refund_taker_funding_timelock
  • [x] refund_taker_funding_secret
  • [ ] search_for_taker_funding_spend (check that taker payment state is TakerApproved)
  • [x] gen_taker_funding_spend_preimage
  • [x] validate_taker_funding_spend_preimage
  • [x] sign_and_send_taker_funding_spend
  • [ ] refund_combined_taker_payment (lets call refund_taker_funding_timelock inside this func?)
  • [x] gen_taker_payment_spend_preimage
  • [x] validate_taker_payment_spend_preimage
  • [x] sign_and_broadcast_taker_payment_spend
  • [ ] wait_for_taker_payment_spend

nft tpu v2 changes

  • global_nft_with_random_privkey in eth_docker_tests.rs started to use eth_coin_from_conf_and_request_v2 for nft actiavtion https://github.com/KomodoPlatform/komodo-defi-framework/blob/747e51ef63c24079bd261458caf3151ff7420089/mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs#L405
  • nft tests code was refactored to reduce code duplication

laruh avatar Jul 19 '24 09:07 laruh

@shamardy I suppose you was right that txs on Geth node fails the confirmation due to reaching gas limits. In separate branch Im going to add new gas_limit v2 params with higher gas limit prices and test it on Geth node.

The below is gas usage difference description:

Here is the log from new branch https://github.com/KomodoPlatform/komodo-defi-framework/commit/ef1a0911cd770d0af381541c819b7c6dfaef576b which I pushed for geth tests https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/10247108896/job/28345630770

---- docker_tests::eth_docker_tests::geth_send_approve_and_spend_erc20 stdout ----
05 10:37:35, eth_docker_tests:2080] taker address: 0x4808c9ad67241d4d992c7a85a5e72d067eb7906f
05 10:37:45, eth_docker_tests:2083] maker address: 0x988d93da7c0c0563eb256d54b6bb88c5897a4f7b
05 10:37:45, eth_docker_tests:2108] Taker sent ERC20 funding, tx hash: e983d6b0dc5546f50ef410183344487d80dd432021b5850be8f86a36675e525d
· 2024-08-05 10:37:45 +0000 [ERC20DEV] Waiting for confirmations… Failed.
thread 'docker_tests::eth_docker_tests::geth_send_approve_and_spend_erc20' panicked at 'called `Result::unwrap()` on an `Err` value: "eth:5287] Internal: eth:5287] Tx receipt Receipt { transaction_hash: 0xe983d6b0dc5546f50ef410183344487d80dd432021b5850be8f86a36675e525d, transaction_index: 0, block_hash: Some(0x8b678309c8ef4cd9ce7e18af85102008b4abf905f814c2de3d65adef8ba9f175), block_number: Some(93), from: 0x4808c9ad67241d4d992c7a85a5e72d067eb7906f, to: Some(0x3cf6249ecbba17f85272bd9bc19e70635f3a99ea), cumulative_gas_used: 148752, gas_used: Some(148752), contract_address: None, logs: [], status: Some(0), root: None, logs_bloom: 0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, transaction_type: Some(0), effective_gas_price: Some(1000009330) } status of ERC20DEV tx 0xe983d6b0dc5546f50ef410183344487d80dd432021b5850be8f86a36675e525d is failed"', mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs:1502:55
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- docker_tests::eth_docker_tests::geth_send_approve_and_spend_eth stdout ----
05 10:37:39, eth_docker_tests:1989] taker address: 0xc33eefa6f7e9d0084bf15c4bbb8cfc197381acb3
05 10:37:49, eth_docker_tests:1992] maker address: 0x3468ac597c379f739a6d9683cffd255d49ae429f
05 10:37:49, eth_docker_tests:2017] Taker sent ETH funding, tx hash: 80fa824d00432bf142413c955daadae28a197298779d1a8952763813545097fe
· 2024-08-05 10:37:49 +0000 [ETH] Waiting for confirmations… Confirmed.
05 10:37:49, eth_docker_tests:2050] Taker approved ETH payment, tx hash: 468d72ec266e5aff2baf008446569f9fb0c5181a2d1f86f4d809f5a3a5cb02b8
· 2024-08-05 10:37:49 +0000 [ETH] Waiting for confirmations… Confirmed.
05 10:37:49, eth_docker_tests:2071] Maker spent ETH payment, tx hash: e888683d05ee9711499834597175523947bf70f72b7eef1957aba60f1b13d511
· 2024-08-05 10:37:49 +0000 [ETH] Waiting for confirmations… Failed.
thread 'docker_tests::eth_docker_tests::geth_send_approve_and_spend_eth' panicked at 'called `Result::unwrap()` on an `Err` value: "eth:5287] Internal: eth:5287] Tx receipt Receipt { transaction_hash: 0xe888683d05ee9711499834597175523947bf70f72b7eef1957aba60f1b13d511, transaction_index: 0, block_hash: Some(0xe48ce43550da57de28f558b794e6eb9971eb6d6870ec93a55fdfaa4afccc178f), block_number: Some(98), from: 0x3468ac597c379f739a6d9683cffd255d49ae429f, to: Some(0x3cf6249ecbba17f85272bd9bc19e70635f3a99ea), cumulative_gas_used: 65000, gas_used: Some(65000), contract_address: None, logs: [], status: Some(0), root: None, logs_bloom: 0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, transaction_type: Some(0), effective_gas_price: Some(1000004822) } status of ETH tx 0xe888683d05ee9711499834597175523947bf70f72b7eef1957aba60f1b13d511 is failed"', mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs:1502:55


failures:
    docker_tests::eth_docker_tests::geth_send_approve_and_spend_erc20
    docker_tests::eth_docker_tests::geth_send_approve_and_spend_eth

test result: FAILED. 182 passed; 2 failed; 33 ignored; 0 measured; 0 filtered out; finished in 950.22s

But I can see that sepolia tests (current PR) are working, as on sepolia testnet tx are executed with less gas usage. The issue is that the Geth dev node and Sepolia testnet should both provide gas usage similar to that of the mainnet (in theory). However for example erc20TakerPayment method gas usage on sepolia is twice smaller then gas usage on Geth node: https://sepolia.etherscan.io/tx/0x7535caf1aa5d301d0184fcc239fb532334bc35027183b31f96cf36e8a9d91c47 Gas Limit & Usage by Txn: 150,000 | 75,008 (50.01%) - Sepolia log from Geth nodegas_used: Some(148752)

Note: Tests on Sepolia can fail with the error replacement transaction underpriced, even when using the wait_pending_transactions function. As a result, some tests pass while others fail randomly. But checking several iterations/commits, we can see that all tests pass

fix code after merge https://github.com/KomodoPlatform/komodo-defi-framework/commit/747e51ef63c24079bd261458caf3151ff7420089 CI CD https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/10237292202/job/28320411640#step:6:1182

failures:
    docker_tests::eth_docker_tests::send_and_refund_taker_funding_by_secret_eth
    docker_tests::eth_docker_tests::send_and_refund_taker_funding_exceed_pre_approve_timelock_erc20

eth docker tests: remove geth_send_approve_and_spend_eth https://github.com/KomodoPlatform/komodo-defi-framework/commit/ff00bf0bab4465de05dc620ce78d7126a6ca755b CI CD https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/10244130226/job/28336579092#step:6:1014 test docker_tests::eth_docker_tests::send_and_refund_taker_funding_by_secret_eth ... ok https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/10244130226/job/28336579092#step:6:1047 test docker_tests::eth_docker_tests::send_and_refund_taker_funding_exceed_pre_approve_timelock_erc20 ... ok

laruh avatar Aug 05 '24 13:08 laruh

made pr r2r, next release i suggest to add new default gas limit v2 consts based on Sepolia txs info and test both environments: Seplia and Geth node, providing EthGasLimit with higher gas limits in geth coin config.

laruh avatar Aug 06 '24 10:08 laruh

Some notes for the next steps after this PR is merged:

  • send_and_refund_taker_funding_by_secret_erc20 test is failing, we should begin adding v2 gas limits after this PR is merged.
  • We should start working on enabling fallback to legacy swap for non supported protocols to start testing the TPU, I don't think we will need the protocol version work being worked on here https://github.com/KomodoPlatform/komodo-defi-framework/pull/2112 since it's related to v1 messages.
  • We should also work on the premium part before starting testing the TPU.

These are the 3 points I think are needed to start testing then launching the TPU, I might have missed some other points, we can discuss these in details next week.

shamardy avatar Aug 16 '24 22:08 shamardy

refund_taker_funding_timelock will never be used unless the taker lost his secret somehow. It's for the case where we recreate the taker swap data from maker swap data since the taker lost the swap data, and then the taker can recover the funding amount after thetimelock. We don't need this implemented now so you can add this to the checklist to not forget it about it. We can have only one function and reuse it for both cases where the passed args are different, but let's leave it as is for now.

@shamardy could you clarify please what is he logic of refund_combined_taker_payment method? as I can see in utxo implementation we use utxo_common::refund_htlc_payment for refund_combined_taker_payment and for refund_maker_payment_v2_timelock

laruh avatar Aug 19 '24 09:08 laruh

send_and_refund_taker_funding_by_secret_erc20 test is failing, we should begin adding v2 gas limits after this PR is merged.

its failing due to race condition with nonces (as tests are using Sepolia and the same private keys for now. I hope if we increase gas limits only for Geth tests, they will start working) please check this note https://github.com/KomodoPlatform/komodo-defi-framework/pull/2169#issuecomment-2269060070 tests can fail randomly, I had moments, when send_and_refund_taker_funding_by_secret_erc20 passed

I had a lot of times of failing send_and_refund_taker_funding_exceed_pre_approve_timelock_erc20test, until I added sleep duration. I suppose I had to do the same with send_and_refund_taker_funding_by_secret_erc20

laruh avatar Aug 19 '24 10:08 laruh

In this commit https://github.com/KomodoPlatform/komodo-defi-framework/pull/2169/commits/f7668cef9591c010114344f68fd49d24dd29ac04 I provided eth_coin_from_conf_and_request_v2_for_test function which doesn't use p2p context, so dead locks dont occur in nft/eth swap v2 tests from eth_docker_tests.rs. solves this issue https://github.com/KomodoPlatform/komodo-defi-framework/pull/2169#discussion_r1724410056

laruh avatar Aug 22 '24 12:08 laruh

@laruh there is an unstable docker test, we shouldn't have this in dev to avoid regressions as docker tests are green there.

shamardy avatar Aug 27 '24 14:08 shamardy

@laruh there is an unstable docker test, we shouldn't have this in dev to avoid regressions as docker tests are green there.

They are all unstable due to race conditions. I mean different tests can fail with replacement transaction underpriced. https://github.com/KomodoPlatform/komodo-defi-framework/pull/2169#issuecomment-2269060070

Btw I moved taker swapv2 tests to Geth node using higher gas limits, but confirmations still fail nft-refund-payment-geth-use-v2-activation-increase-gas-limit branch commits

CI https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/10589881445/job/29344726554#step:6:1637 example

---- docker_tests::eth_docker_tests::send_and_refund_taker_funding_by_secret_erc20 stdout ----
28 04:04:38, eth_docker_tests:1426] Taker sent ERC20 funding, tx hash: b83a9e1b47b47be644d47feae879cd13f4484bfd9d9151eed42f4b428336d735
· 2024-08-28 04:04:38 +0000 [ERC20DEV] Waiting for confirmations… Failed.
thread 'docker_tests::eth_docker_tests::send_and_refund_taker_funding_by_secret_erc20' panicked at 'called `Result::unwrap()` on an `Err` value: "eth:5292] Internal: eth:5292] Tx receipt Receipt { transaction_hash: 0xb83a9e1b47b47be644d47feae879cd13f4484bfd9d9151eed42f4b428336d735, transaction_index: 0, block_hash: Some(0x03e12c460467c5f5934260b1dcf1097f0ce9183c3ca33d214bf0622d819520f8), block_number: Some(128), from: 0xdbf79d75cb3b0b942500dfb01e1e8327582e941b, to: Some(0x844bf2c9ecca714f2ba60c3e694c776700bbee82), cumulative_gas_used: 24309, gas_used: Some(24309), contract_address: None, logs: [], status: Some(0), root: None, logs_bloom: 0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, transaction_type: Some(0), effective_gas_price: Some(1000000078) } status of ERC20DEV tx 0xb83a9e1b47b47be644d47feae879cd13f4484bfd9d9151eed42f4b428336d735 is failed"', mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs:1201:55

gas_used: Some(24309). gas limit in send erc20 was 210_000, which is enough, but confirmation failed.

I suggest to leave Sepolia tests and ignore them when we merge it to dev. And run them again when you work on the swap v2 feature.

laruh avatar Aug 28 '24 05:08 laruh