fix(tpu-v2): provide ethcoin support in tpu kickstart process by laruh · Pull Request #2300 · GLEECBTC/komodo-defi-framework

laruh · 2024-12-22T11:26:26Z

This pr provides EthCoin support in kickstart handler for taker and maker swaps

added test_v2_eth_eth_kickstart

This reverts commit df7778c.

…swap_kickstart_handler_for_taker in swap_kick_starts func

borngraced

welldone, 3 minor notes

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs

mariocynicys

Thanks
LGTM

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs

onur-ozkan

Please ping me once you are done with reviews @borngraced

borngraced

great work!

onur-ozkan · 2024-12-24T08:41:37Z

Can you add test coverage for ETH ? Seems like we have one for UTXOs https://github.com/KomodoPlatform/komodo-defi-framework/blob/ea5f3076084e568f528e6927f5aa9f6fb0b03435/mm2src/mm2_main/tests/docker_tests/swap_proto_v2_tests.rs#L722-L723

laruh · 2024-12-24T08:45:01Z

Can you add test coverage for ETH ? Seems like we have one for UTXOs

https://github.com/KomodoPlatform/komodo-defi-framework/blob/ea5f3076084e568f528e6927f5aa9f6fb0b03435/mm2src/mm2_main/tests/docker_tests/swap_proto_v2_tests.rs#L722-L723

I can, but it will be on sepolia and use feature flag

onur-ozkan · 2024-12-24T09:03:56Z

I can, but it will be on sepolia and use feature flag

No problem I guess, better than nothing

…-for-payment-spend-kickstart

…d' into fix-tpu-v2-wait-for-payment-spend-kickstart

laruh · 2024-12-27T08:46:17Z

Added test_v2_eth_eth_kickstart test.
It uses Geth node, checks that taker and maker restarted their tpu eth-eth swap after stop (active swap uuid should be the same as before stop).

borngraced

re-approve

mariocynicys · 2024-12-27T13:18:31Z

mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs

+    let uuids = block_on(start_swaps(&mut mm_bob, &mut mm_alice, &[(ETH, ETH1)], 1.0, 1.0, 77.));
+    log!("{:?}", uuids);
+    let parsed_uuids: Vec<Uuid> = uuids.iter().map(|u| u.parse().unwrap()).collect();
+
+    for uuid in uuids.iter() {
+        log_swap_status_before_stop(&mm_bob, uuid, "Maker");
+        log_swap_status_before_stop(&mm_alice, uuid, "Taker");
+    }


nit: uuids is just a single item in a vector. you might want to extract it.

The loop handles unexpected cases where there might be more than one swap.
Logging all uuids (even if vec has one element) ensures we don’t miss anything if something goes wrong, making the test more reliable.

The loop handles unexpected cases where there might be more than one swap.

tbh, i don't think this should be our way to approach things, this will leave us with a lot of dept.
and i don't think there should even be any unexpected cases in a test.

that's my opinion about the reasoning, but the thing is still a nit.

mariocynicys · 2024-12-27T13:19:40Z

mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs

+    // Restart Bob and Alice
+    bob_conf.conf["dbdir"] = mm_bob.folder.join("DB").to_str().unwrap().into();
+    bob_conf.conf["log"] = mm_bob.folder.join("mm2_dup.log").to_str().unwrap().into();


nit: or you might just use MarketMakerIt::seednode_trade_v2

As I see we do this in every kickstart test
https://github.com/KomodoPlatform/komodo-defi-framework/blob/5750415eb12e6207c49da77d53ba60894967c612/mm2src/mm2_main/tests/docker_tests/swaps_file_lock_tests.rs#L149-L163

RaiiDump is used to avoid the issue rust-lang/rust#42474
https://github.com/KomodoPlatform/komodo-defi-framework/blob/5750415eb12e6207c49da77d53ba60894967c612/mm2src/mm2_test_helpers/src/for_tests.rs#L1205-L1214
According the last comments some problems still occur, so its better to follow current approach in tests

mariocynicys · 2024-12-27T13:20:11Z

mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs

+    alice_conf.conf["dbdir"] = mm_alice.folder.join("DB").to_str().unwrap().into();
+    alice_conf.conf["log"] = mm_alice.folder.join("mm2_dup.log").to_str().unwrap().into();
+    alice_conf.conf["seednodes"] = vec![mm_bob.ip.to_string()].into();


nit: or MarketMakerIt::light_node_trade_v2

btw, why create a different mm2_dup.log? u could not call mm_dump if u want.

shamardy

Thanks for the PR, LGTM! I will not merge this until #2261 is approved first, but I remember this should be merged first.

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs

shamardy · 2025-02-03T10:01:49Z

Merge blocked by #2261

…d' into fix-tpu-v2-wait-for-payment-spend-kickstart # Conflicts: # mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs

…d' into fix-tpu-v2-wait-for-payment-spend-kickstart

) This commit also provides EthCoin support in kickstart handler for taker and maker swaps #2300

impl swap kickstart for eth coin in taker and maker swaps

df7778c

laruh force-pushed the fix-tpu-v2-wait-for-payment-spend-kickstart branch from d08fa86 to df7778c Compare December 22, 2024 11:36

laruh added 2 commits December 23, 2024 14:07

Revert "impl swap kickstart for eth coin in taker and maker swaps"

f65d6a2

This reverts commit df7778c.

refactor and spawn futures from swap_kickstart_handler_for_maker and …

676d09c

…swap_kickstart_handler_for_taker in swap_kick_starts func

laruh added the status: pending review label Dec 23, 2024

mariocynicys self-assigned this Dec 23, 2024

mariocynicys self-requested a review December 23, 2024 13:00

mariocynicys removed their assignment Dec 23, 2024

borngraced suggested changes Dec 23, 2024

View reviewed changes

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs Outdated Show resolved Hide resolved

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs Outdated Show resolved Hide resolved

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs Outdated Show resolved Hide resolved

mariocynicys approved these changes Dec 23, 2024

View reviewed changes

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs Outdated Show resolved Hide resolved

onur-ozkan reviewed Dec 24, 2024

View reviewed changes

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs Outdated Show resolved Hide resolved

onur-ozkan reviewed Dec 24, 2024

View reviewed changes

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs Outdated Show resolved Hide resolved

review: update doc comment

09b4b6b

onur-ozkan approved these changes Dec 24, 2024

View reviewed changes

laruh added 2 commits December 24, 2024 14:23

review: omit types

1065997

review: use if let Some

c214726

borngraced approved these changes Dec 24, 2024

View reviewed changes

onur-ozkan added status: in progress and removed status: pending review labels Dec 25, 2024

laruh added 2 commits December 26, 2024 16:42

Merge branch 'fix-tpu-v2-wait-for-payment-spend' into fix-tpu-v2-wait…

3c625be

…-for-payment-spend-kickstart

Merge branch 'fix-tpu-v2-wait-for-payment-spend' into fix-tpu-v2-wait…

bbe2f5c

…-for-payment-spend-kickstart

laruh force-pushed the fix-tpu-v2-wait-for-payment-spend-kickstart branch from 6c332f2 to 1e10d83 Compare December 26, 2024 14:05

eth-eth kickstart test

d48cc25

laruh force-pushed the fix-tpu-v2-wait-for-payment-spend-kickstart branch from 1e10d83 to d48cc25 Compare December 26, 2024 14:09

laruh added status: pending review and removed status: in progress labels Dec 27, 2024

laruh requested a review from mariocynicys December 27, 2024 08:33

laruh requested review from borngraced and onur-ozkan December 27, 2024 08:33

Merge remote-tracking branch 'origin/fix-tpu-v2-wait-for-payment-spen…

5750415

…d' into fix-tpu-v2-wait-for-payment-spend-kickstart

borngraced approved these changes Dec 27, 2024

View reviewed changes

mariocynicys approved these changes Dec 27, 2024

View reviewed changes

laruh added the priority: medium Moderately important tasks that should be completed but are not urgent. label Jan 15, 2025

This was referenced Jan 17, 2025

Trading protocol upgrade v2. #1895

Open

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

Merged

shamardy self-requested a review January 22, 2025 12:55

shamardy approved these changes Jan 22, 2025

View reviewed changes

dimxy reviewed Jan 23, 2025

View reviewed changes

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs Show resolved Hide resolved

dimxy self-requested a review January 27, 2025 07:12

dimxy approved these changes Jan 27, 2025

View reviewed changes

shamardy added 2.4.0-beta status: blocked and removed status: pending review labels Feb 3, 2025

laruh added 2 commits February 6, 2025 17:45

Merge remote-tracking branch 'origin/fix-tpu-v2-wait-for-payment-spen…

e114b94

…d' into fix-tpu-v2-wait-for-payment-spend-kickstart # Conflicts: # mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs

Merge remote-tracking branch 'origin/fix-tpu-v2-wait-for-payment-spen…

4bfe410

…d' into fix-tpu-v2-wait-for-payment-spend-kickstart

shamardy removed the status: blocked label Feb 8, 2025

shamardy merged commit 91fea90 into fix-tpu-v2-wait-for-payment-spend Feb 8, 2025

shamardy deleted the fix-tpu-v2-wait-for-payment-spend-kickstart branch February 8, 2025 12:06

shamardy pushed a commit that referenced this pull request Feb 8, 2025

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

39515a9

) This commit also provides EthCoin support in kickstart handler for taker and maker swaps #2300

Conversation

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

Uh oh!

Uh oh!

borngraced left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mariocynicys left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

onur-ozkan left a comment

Choose a reason for hiding this comment

Uh oh!

borngraced left a comment

Choose a reason for hiding this comment

Uh oh!

onur-ozkan commented Dec 24, 2024

Uh oh!

laruh commented Dec 24, 2024

Uh oh!

onur-ozkan commented Dec 24, 2024

Uh oh!

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

Uh oh!

Uh oh!

borngraced left a comment

Choose a reason for hiding this comment

Uh oh!

mariocynicys Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

laruh Jan 9, 2025

Choose a reason for hiding this comment

Uh oh!

mariocynicys Jan 9, 2025

Choose a reason for hiding this comment

Uh oh!

mariocynicys Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

laruh Jan 9, 2025

Choose a reason for hiding this comment

Uh oh!

mariocynicys Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

mariocynicys Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shamardy commented Feb 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

laruh commented Dec 22, 2024 •

edited

Loading

laruh commented Dec 27, 2024 •

edited

Loading