fix(tpu-v2): provide ethcoin support in tpu kickstart process #2300

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

komodo-defi-framework/mm2src/mm2_main/tests/docker_tests/swap_proto_v2_tests.rs

Lines 722 to 723 in ea5f307

    
           #[test] 
        
           fn test_v2_swap_utxo_utxo_kickstart() {

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

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

komodo-defi-framework/mm2src/mm2_main/tests/docker_tests/swap_proto_v2_tests.rs

Lines 722 to 723 in ea5f307

#[test]

fn test_v2_swap_utxo_utxo_kickstart() {

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

komodo-defi-framework/mm2src/mm2_main/tests/docker_tests/swaps_file_lock_tests.rs

Lines 149 to 163 in 5750415

// mm_bob using same DB dir that should kick start the swap

bob_conf["dbdir"] = mm_bob.folder.join("DB").to_str().unwrap().into();

bob_conf["log"] = mm_bob.folder.join("mm2_dup.log").to_str().unwrap().into();

// dropping instead of graceful stop to retain swap file locks

drop(mm_bob);

let mut mm_bob_dup = MarketMakerIt::start(bob_conf, "pass".to_string(), None).unwrap();

let (_bob_dup_dump_log, _bob_dup_dump_dashboard) = mm_dump(&mm_bob_dup.log_path);

log!("{:?}", block_on(enable_native(&mm_bob_dup, "MYCOIN", &[], None)));

log!("{:?}", block_on(enable_native(&mm_bob_dup, "MYCOIN1", &[], None)));

block_on(mm_bob_dup.wait_for_log(50., |log| log.contains(&format!("Swap {} kick started.", uuid)))).unwrap();

// mm_alice using same DB dir that should kick start the swap

alice_conf["dbdir"] = mm_alice.folder.join("DB").to_str().unwrap().into();

alice_conf["log"] = mm_alice.folder.join("mm2_dup.log").to_str().unwrap().into();

RaiiDump is used to avoid the issue rust-lang/rust#42474

komodo-defi-framework/mm2src/mm2_test_helpers/src/for_tests.rs

Lines 1205 to 1214 in 5750415

/// When `drop`ped, dumps the given file to the stdout.

///

/// Used in the tests, copying the MM log to the test output.

///

/// Note that because of https://github.com/rust-lang/rust/issues/42474 it's currently impossible to share the MM log interactively,

/// hence we're doing it in the `drop`.

pub struct RaiiDump {

#[cfg(not(target_arch = "wasm32"))]

pub log_path: PathBuf,

}

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.

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 requested 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tpu-v2): provide ethcoin support in tpu kickstart process #2300

fix(tpu-v2): provide ethcoin support in tpu kickstart process #2300

laruh commented Dec 22, 2024 •

edited

Loading

borngraced left a comment

mariocynicys left a comment

onur-ozkan left a comment

borngraced left a comment

onur-ozkan commented Dec 24, 2024

laruh commented Dec 24, 2024

onur-ozkan commented Dec 24, 2024

laruh commented Dec 27, 2024 •

edited

Loading

borngraced left a comment

mariocynicys Dec 27, 2024

laruh Jan 9, 2025

mariocynicys Jan 9, 2025

mariocynicys Dec 27, 2024

laruh Jan 9, 2025

mariocynicys Dec 27, 2024

mariocynicys Dec 27, 2024

	// mm_bob using same DB dir that should kick start the swap
	bob_conf["dbdir"] = mm_bob.folder.join("DB").to_str().unwrap().into();
	bob_conf["log"] = mm_bob.folder.join("mm2_dup.log").to_str().unwrap().into();
	// dropping instead of graceful stop to retain swap file locks
	drop(mm_bob);
	let mut mm_bob_dup = MarketMakerIt::start(bob_conf, "pass".to_string(), None).unwrap();
	let (_bob_dup_dump_log, _bob_dup_dump_dashboard) = mm_dump(&mm_bob_dup.log_path);
	log!("{:?}", block_on(enable_native(&mm_bob_dup, "MYCOIN", &[], None)));
	log!("{:?}", block_on(enable_native(&mm_bob_dup, "MYCOIN1", &[], None)));

	block_on(mm_bob_dup.wait_for_log(50., \|log\| log.contains(&format!("Swap {} kick started.", uuid)))).unwrap();

	// mm_alice using same DB dir that should kick start the swap
	alice_conf["dbdir"] = mm_alice.folder.join("DB").to_str().unwrap().into();
	alice_conf["log"] = mm_alice.folder.join("mm2_dup.log").to_str().unwrap().into();

	/// When `drop`ped, dumps the given file to the stdout.
	///
	/// Used in the tests, copying the MM log to the test output.
	///
	/// Note that because of https://github.com/rust-lang/rust/issues/42474 it's currently impossible to share the MM log interactively,
	/// hence we're doing it in the `drop`.
	pub struct RaiiDump {
	#[cfg(not(target_arch = "wasm32"))]
	pub log_path: PathBuf,
	}

fix(tpu-v2): provide ethcoin support in tpu kickstart process #2300

Are you sure you want to change the base?

fix(tpu-v2): provide ethcoin support in tpu kickstart process #2300

Conversation

laruh commented Dec 22, 2024 • edited Loading

borngraced left a comment

Choose a reason for hiding this comment

mariocynicys left a comment

Choose a reason for hiding this comment

onur-ozkan left a comment

Choose a reason for hiding this comment

borngraced left a comment

Choose a reason for hiding this comment

onur-ozkan commented Dec 24, 2024

laruh commented Dec 24, 2024

onur-ozkan commented Dec 24, 2024

laruh commented Dec 27, 2024 • edited Loading

borngraced left a comment

Choose a reason for hiding this comment

mariocynicys Dec 27, 2024

Choose a reason for hiding this comment

laruh Jan 9, 2025

Choose a reason for hiding this comment

mariocynicys Jan 9, 2025

Choose a reason for hiding this comment

mariocynicys Dec 27, 2024

Choose a reason for hiding this comment

laruh Jan 9, 2025

Choose a reason for hiding this comment

mariocynicys Dec 27, 2024

Choose a reason for hiding this comment

mariocynicys Dec 27, 2024

Choose a reason for hiding this comment

laruh commented Dec 22, 2024 •

edited

Loading

laruh commented Dec 27, 2024 •

edited

Loading