feat(eth-swap): maker tpu v2 implementation #2211

laruh · 2024-09-05T05:34:12Z

impl MakerCoinSwapOpsV2 trait for ETH
eth taker and maker swap v2 implementations are stored in eth_swap_v2 module
added taker, maker and nft_maker gas limit v2 params and consts
Nft swap v2 methods started to use swap address from "swap_v2_contracts" coin field

In addition, added additional wait_for_maker_payment_spend function to MakerCoinSwapOpsV2 trait. It is needed to cover "taker doesnt wait for spend maker payment tx confirmation" issue which we faced in legacy swap protocol #2175 (comment)

Later we can add wait_for_maker_payment_spend usage in MakerPaymentSpent state before changing it to Completed

komodo-defi-framework/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs

Lines 2195 to 2203 in 8f983ec

    
           impl<MakerCoin: MmCoin + MakerCoinSwapOpsV2, TakerCoin: MmCoin + TakerCoinSwapOpsV2> State 
        
               for MakerPaymentSpent<MakerCoin, TakerCoin> 
        
           { 
        
               type StateMachine = TakerSwapStateMachine<MakerCoin, TakerCoin>; 
        
               async fn on_changed(self: Box<Self>, state_machine: &mut Self::StateMachine) -> StateResult<Self::StateMachine> { 
        
                   Self::change_state(Completed::new(), state_machine).await 
        
               } 
        
           }

Note
Provided two test features to be able to test maker and taker swap v2 contracts separately on sepolia.

sepolia-maker-swap-v2-tests = []
sepolia-taker-swap-v2-tests = []

…wap_v2.rs

# Conflicts: # mm2src/coins/lp_coins.rs

…ap ops, remove `swap_contract_addres` from NFT swap args

mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs

…t functions

borngraced

Great work! first review iteration

mm2src/coins/eth.rs

mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs

mm2src/coins/eth/nft_swap_v2/mod.rs

# Conflicts: # mm2src/coins/utxo/utxo_standard.rs

borngraced

last notes

mm2src/coins/coin_errors.rs

mm2src/coins/eth.rs

mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs

borngraced

welldone!

shamardy · 2024-11-08T14:09:54Z

@laruh I think there is nothing to test or document by @KomodoPlatform/qa for this PR, if that's the case please add To Test and nothing to the opening comment, otherwise add normal To Test and docs.

mariocynicys · 2024-11-11T11:09:44Z

mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs

+#[cfg(any(feature = "sepolia-maker-swap-v2-tests", feature = "sepolia-taker-swap-v2-tests"))]
+lazy_static! {
+    pub static ref SEPOLIA_WEB3: Web3<Http> = Web3::new(Http::new(SEPOLIA_RPC_URL).unwrap());
    pub static ref SEPOLIA_NONCE_LOCK: Mutex<()> = Mutex::new(());


does this use any docker instance? why don't we move such tests outside docker (to test_inner)?

Hmm, yes, the Sepolia code doesn’t use Geth-generated addresses.
We could move the code to the sepolia_tests test crate.
However, there’s still a chance that when we update the Swap V2 smart contracts, for example add burnFee to the taker swap contract, this contract could start working on the Geth node (we faced similar issue during the NFT maker swap V2 implementation and testing). In that case, the code could be moved back to docker_tests.

Let’s plan this when we update the taker swap v2 contract, if it doesn’t work on Geth, we’ll move taker/maker Sepolia code to a separate test crate.

mariocynicys · 2024-11-11T11:10:35Z

mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs

    // Mutex used to prevent nonce re-usage during funding addresses used in tests
    pub static ref GETH_NONCE_LOCK: Mutex<()> = Mutex::new(());
+}
+
+#[cfg(any(feature = "sepolia-maker-swap-v2-tests", feature = "sepolia-taker-swap-v2-tests"))]


why not merge the two features?

maker and taker methods are belong to different smart contracts. its better to have them separately, when you need to test only maker or only taker functionality

tbh i would consider this (and not really much) only if these smart contracts are changed so often to justify having a feature to run them specifically, otherwise i would just go for merging these feature flags (or even ignoring the sepolia tests instead) and run tests by their names or name prefix.
i see features as more complexities if we can do without it.

* dev: fix(foot-shooting): remove leftover code that panics via RPC (#2270) refactor(MarketCoinOps): make `wait_for_htlc_tx_spend` async (#2265) feat(eth-swap): maker tpu v2 implementation (#2211) fix(nft): add token_id field to the tx history primary key, fix balance (#2209) feat(cosmos): support IBC types in tx history implementation (#2245) fix(hd-wallet): use `CoinBalanceMap` for UTXO and QTUM (#2259) fix(tests): add more sepolia endpoints in tests (#2262) fix(legacy-swap): check for confirmations on recover taker (#2242) fix(legacy-swap): remove the need for takers to confirm their payment (#2249) refactor(P2P): types and modules (#2256) fix(evm): correctly display eth addr in iguana v2 activation result (#2254) feat(utxo): prioritize electrum connections (#1966) refactor(SwapOps): make all methods async (#2251) refactor(SwapOps): make `send_maker_payment` async (#2250) remove old p2p implementation (#2248) feat(cosmos-offline-tests): prepare IBC channels inside the container (#2246)

laruh added 10 commits August 30, 2024 18:01

create eth_swap_v2 module, and refactor eth_swap_v2.rs to eth_taker_s…

e485de2

…wap_v2.rs

maker ops: send_maker_payment_v2_impl added

6593dc7

WIP: validate_maker_payment_v2_impl

9eac0fd

remove unused import

4fc3971

maker tpu v2: validate_maker_payment_v2_impl

45d4726

maker tpu v2: refund timelock and secret

619359a

maker tpu v2: wait_for_maker_payment_spend TODO

a6e17b4

maker tpu v2: spend_maker_payment_v2_impl

e16bf49

eth maker tpu v2: wait_for_maker_payment_spend_impl

b35e68f

eth maker v2 docker tests

f73b5de

laruh added the in progress Changes will be made from the author label Sep 5, 2024

laruh added 3 commits September 5, 2024 12:36

remove clone for now

e53c028

Merge remote-tracking branch 'origin/dev' into eth-maker-tpu-v2

3815d08

# Conflicts: # mm2src/coins/lp_coins.rs

maker swap v2 docker tests (on sepolia)

7498eec

laruh force-pushed the eth-maker-tpu-v2 branch from 78f6a09 to 7498eec Compare September 8, 2024 06:45

laruh added 7 commits September 10, 2024 14:14

set taker and maker gas limits V2

b1626f7

taker gas limits v2

ce7903d

move prepare erc20 payment data above allowance, impl handle_allowance

f1063c7

start using nft_maker_swap_v2_contract from EthCoin in Nft maker sw…

60f11f8

…ap ops, remove `swap_contract_addres` from NFT swap args

nft maker gas limits v2

0c96992

update taker time

8f983ec

update gas limit fields names

6961d50

laruh force-pushed the eth-maker-tpu-v2 branch from 24004a2 to f2aee82 Compare September 16, 2024 08:18

add sepolia test features

5d9d1de

laruh force-pushed the eth-maker-tpu-v2 branch from f2aee82 to 5d9d1de Compare September 16, 2024 08:37

laruh added under review and removed in progress Changes will be made from the author labels Sep 16, 2024

laruh added 2 commits September 19, 2024 19:03

Merge remote-tracking branch 'origin/dev' into eth-maker-tpu-v2

d57abdb

provide SEPOLIA_TESTS_LOCK

1ef0f43

dimxy reviewed Sep 23, 2024

View reviewed changes

mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs Outdated Show resolved Hide resolved

laruh added in progress Changes will be made from the author and removed under review labels Nov 1, 2024

laruh dismissed shamardy’s stale review via 428480f November 1, 2024 14:48

laruh added 4 commits November 1, 2024 21:54

remove wait_for_maker_payment_spend

db6b2e7

increase v2 ERC20_PAYMENT consts to 150_000

da12b97

review: consts for TAKER/MAKER state indexes

2b81855

review: create separate Refund Args structures for Timelock and Secre…

72ed306

…t functions

laruh force-pushed the eth-maker-tpu-v2 branch from 428480f to 72ed306 Compare November 1, 2024 14:54

laruh added 3 commits November 3, 2024 15:06

review: remove unreachable

a9a5c47

review: split token_address and gas_limit funcs logic

1ccfc77

review: adjust gas limit consts

6e42e34

laruh added under review and removed in progress Changes will be made from the author labels Nov 3, 2024

borngraced requested changes Nov 4, 2024

View reviewed changes

laruh added 6 commits November 4, 2024 17:17

Merge remote-tracking branch 'origin/dev' into eth-maker-tpu-v2

1e6dacb

review: prioritize errors

ce15cd8

review: remove unnecessary as_ref, map functions

57e75ed

review: use ref to a fixed-size array in Validation Args

1e6ea8e

Merge remote-tracking branch 'origin/dev' into eth-maker-tpu-v2

77b7500

# Conflicts: # mm2src/coins/utxo/utxo_standard.rs

review: update payment state index consts names

ed02392

laruh mentioned this pull request Nov 5, 2024

fix(tpu-v2): fix tpu-v2 wait for payment spend and extract secret #2261

Open

borngraced requested changes Nov 6, 2024

View reviewed changes

mm2src/coins/coin_errors.rs Show resolved Hide resolved

mm2src/coins/eth.rs Show resolved Hide resolved

mm2src/coins/eth.rs Show resolved Hide resolved

mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs Outdated Show resolved Hide resolved

review: remove redundant validation

1301084

borngraced approved these changes Nov 8, 2024

View reviewed changes

shamardy approved these changes Nov 8, 2024

View reviewed changes

shamardy merged commit 99cb87a into dev Nov 8, 2024
19 of 23 checks passed

shamardy deleted the eth-maker-tpu-v2 branch November 8, 2024 14:10

mariocynicys reviewed Nov 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eth-swap): maker tpu v2 implementation #2211

feat(eth-swap): maker tpu v2 implementation #2211

laruh commented Sep 5, 2024 •

edited

Loading

borngraced left a comment

borngraced left a comment

borngraced left a comment

shamardy commented Nov 8, 2024

mariocynicys Nov 11, 2024

laruh Nov 11, 2024

mariocynicys Nov 11, 2024

laruh Nov 11, 2024

mariocynicys Nov 11, 2024

	impl<MakerCoin: MmCoin + MakerCoinSwapOpsV2, TakerCoin: MmCoin + TakerCoinSwapOpsV2> State
	for MakerPaymentSpent<MakerCoin, TakerCoin>
	{
	type StateMachine = TakerSwapStateMachine<MakerCoin, TakerCoin>;

	async fn on_changed(self: Box<Self>, state_machine: &mut Self::StateMachine) -> StateResult<Self::StateMachine> {
	Self::change_state(Completed::new(), state_machine).await
	}
	}

feat(eth-swap): maker tpu v2 implementation #2211

feat(eth-swap): maker tpu v2 implementation #2211

Conversation

laruh commented Sep 5, 2024 • edited Loading

borngraced left a comment

Choose a reason for hiding this comment

borngraced left a comment

Choose a reason for hiding this comment

borngraced left a comment

Choose a reason for hiding this comment

shamardy commented Nov 8, 2024

mariocynicys Nov 11, 2024

Choose a reason for hiding this comment

laruh Nov 11, 2024

Choose a reason for hiding this comment

mariocynicys Nov 11, 2024

Choose a reason for hiding this comment

laruh Nov 11, 2024

Choose a reason for hiding this comment

mariocynicys Nov 11, 2024

Choose a reason for hiding this comment

laruh commented Sep 5, 2024 •

edited

Loading