feat(eth-swap): maker tpu v2 implementation by laruh · Pull Request #2211 · GLEECBTC/komodo-defi-framework

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
https://github.com/KomodoPlatform/komodo-defi-framework/blob/8f983ec362cb00704b27582b8c0dc3e9934aa912/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs#L2195-L2203

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

    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 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 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 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 labels Nov 3, 2024

borngraced suggested 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

Merged

borngraced suggested 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

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

mariocynicys reviewed Nov 11, 2024

View reviewed changes

shamardy mentioned this pull request Nov 19, 2024

chore(release): v2.2.0-beta #2197

Merged

Alrighttt pushed a commit that referenced this pull request Nov 26, 2024

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

d187519

laruh mentioned this pull request Jan 26, 2025

Trading protocol upgrade v2. #1895

Open

29 tasks

Conversation

laruh commented Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

borngraced left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

borngraced left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

borngraced left a comment

Choose a reason for hiding this comment

Uh oh!

shamardy commented Nov 8, 2024

Uh oh!

mariocynicys Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

laruh Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

mariocynicys Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

laruh Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

mariocynicys Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

laruh commented Sep 5, 2024 •

edited

Loading