feat(ARRR): dockerize zombie/pirate tests #2374

borngraced · 2025-02-26T18:59:26Z

need review here https://github.com/KomodoPlatform/ZOMBIE_test_docker cc @dimxy

shamardy · 2025-03-03T11:57:47Z

@borngraced what is left to make this ready for review. @onur-ozkan @dimxy please ignore reviewing this until it's ready.

borngraced · 2025-03-03T15:01:02Z

@borngraced what is left to make this ready for review. @onur-ozkan @dimxy please ignore reviewing this until it's ready.

Some minor adjustments to the zombie docker image still ongoing

dimxy · 2025-03-05T11:48:59Z

mm2src/mm2_test_helpers/src/for_tests.rs

        "mm2":1,
-        "avg_blocktime": 60,
+        "avg_blocktime": 20,


why is it set to 20 sec?
(I think it should be 60 for ZOMBIE as well)

I wanted fast block production via docker image and I think it might have side effect with this which is why I updated it to the same average blocktime from the docker image https://github.com/KomodoPlatform/ZOMBIE_test_docker/blob/6a06dcf1cf08c52f95a18466b5adf33ad387396e/start_blockchain.py#L30(should be 10 tho.)

I can revert if you think the changes in the script above doesn't really matter.

Ah I see you changed ac_blocktime to 10.
avg_blocktime is used to estimate number of blocks from a duration, so I guess it's better to have it equal to ac_blocktime.

dimxy · 2025-03-05T13:26:25Z

mm2src/mm2_main/tests/docker_tests/z_coin_docker_tests.rs

+
+#[tokio::test(flavor = "multi_thread")]
+async fn zombie_coin_send_and_refund_maker_payment() {
+    let _lock = TEST_MUTEX.lock().await;


I think we should set this lock on the API level (not in tests only)
(to prevent this issue with parallel calls, right?

no sure how we can approach this btw.. since we need to wait from start to finish of a pirate swap.

I think we just should prevent a note to be selected the second time from the received_notes table.
It should not be selected after we update its spent status with the store_sent_tx fn.
So I guess, we need to set a lock before where we add notes and release it after the store_sent_tx fn called. The places in code where we add notes are: init_withdraw, z_send_dex_fee, z_send_htlc

I made a sample code with Z_NOTE_LOCK in the send_outputs fn (it's not ready for prod, just to prove the idea of a lock to prevent access to the available notes)

I also had to add a simple wait_for_z_balance fn, otherwise I had insufficient balance error. So probably the spendable_notes_required_for_tx fn does work properly when different ZCoin {} created in concurrent zombie tests or its confirmation logic is not sufficient: I noticed that transactions may stay for few blocks in mempool so I wait for 5 blocks in wait_for_z_balance.

onur-ozkan · 2025-03-06T08:19:11Z

mm2src/coins/z_coin.rs

+/// Initialize `ZCoin` with a forced `z_spending_key`.
+#[allow(clippy::too_many_arguments)]
+pub async fn z_coin_from_conf_and_params_with_docker(


This can be feature gated with run-docker-tests

onur-ozkan · 2025-03-06T08:19:59Z

mm2src/coins/z_coin/z_htlc.rs

+#[cfg(not(target_arch = "wasm32"))]
+use zcash_client_sqlite::error::SqliteClientError;


You can move this into cfg_native block.

onur-ozkan · 2025-03-06T08:20:32Z

mm2src/coins/z_coin/z_htlc.rs

+    // SAFETY: htlc_output is sending to a t_addr.
+    let to = htlc_output.script_pubkey.address().unwrap();


Suggested change

// SAFETY: htlc_output is sending to a t_addr.

let to = htlc_output.script_pubkey.address().unwrap();

let to = htlc_output.script_pubkey.address().expect("htlc_output is sending to a t_addr");

onur-ozkan · 2025-03-06T08:23:24Z

mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs

+pub const ZOMBIE_ASSET_DOCKER_IMAGE: &str = "docker.io/borngraced/zombietestrunner";
+pub const ZOMBIE_ASSET_DOCKER_IMAGE_WITH_TAG: &str = "docker.io/borngraced/zombietestrunner:multiarch";


Could you please upload those images to komodoofficial org? We should not rely on personal accounts (I know we already do for certain and we should migrate from them).

Created a tracking issue for this: #2381

the docker image needs to be reviewed first https://github.com/KomodoPlatform/ZOMBIE_test_docker

onur-ozkan · 2025-03-06T08:36:53Z

mm2src/mm2_main/tests/docker_tests/z_coin_docker_tests.rs

+// TODO: fix test
+#[ignore]


It's worth to put some context about the failure here, so we don't need to run the test in order to remember what was going wrong.

Yeah, I'm absolutely going to attend to test in this PR

dimxy · 2025-03-06T13:12:08Z

mm2src/coins/z_coin.rs

@@ -1866,7 +1934,7 @@ impl InitWithdrawCoin for ZCoin {
            memo,
        };

-        let (tx, data, _sync_guard) = self.gen_tx(vec![], vec![z_output]).await?;
+        let (tx, _, data, _sync_guard) = self.gen_tx(vec![], vec![z_output]).await?;


Why is there no store_sent_tx() call for withdraw?

this PR is slowly diverting from it's main purpose hance, I have remove additional fixes that shouldn't have been done here...removed fixes will be reimplemented `fix-zombie-tests` PR

borngraced · 2025-03-10T11:28:23Z

this PR(#2374) is slowly diverting from it's main purpose. Hence, I have removed additional fixes that shouldn't have been done here...removed fixes will
be re-implemented in `improve shielded transactions change notes handling in swaps PR.

dimxy · 2025-03-11T17:58:40Z

Do I understand it right that we are going to solve the problems with concurrent zombie tests failure?
If so I am okay with this PR

borngraced added 11 commits February 26, 2025 18:47

save dev state

d789c36

save dev state

3050594

save dev state

c6e8100

save dev state - docker image working!

f68cd01

reduce timeout, add log

bf9312b

uncomment tendermint docker and cleanups

ea42ae7

fix tx_streaming_tests mod

da82da1

use multiarch for zombie docker

0377162

fix post rebase

83133de

Merge branch 'dev' into dockerize_pirate

588ee43

nits

6d423a6

borngraced added the status: in progress label Feb 26, 2025

borngraced self-assigned this Feb 26, 2025

borngraced changed the title ~~feat(ARRR): dockerize ZOMBIE tests~~ feat(ARRR): dockerize zombie/pirate tests Feb 26, 2025

borngraced added 4 commits February 26, 2025 20:01

remove mm2_test_helpers lock file

9950305

rename zombie to pirate

46d59ea

add internal Lockfile to gitignore

2c662ce

revert z_coin save change output and fix docker image init arrangement

d2e09d0

dimxy mentioned this pull request Feb 27, 2025

fix(tests): make zombie tests working #2369

Closed

borngraced added 2 commits February 27, 2025 15:38

fix zcoin cache update test, improve z docker, and nits

ec4845c

fix clippy

300943e

shamardy added the 2.5.0-beta label Mar 3, 2025

shamardy requested review from dimxy and onur-ozkan March 3, 2025 11:56

borngraced added 3 commits March 3, 2025 16:20

move fund shield to docker start script

6e6d37c

fix clippy

5344fa6

remove z_shieldcoinbase rpc

f36a3a1

borngraced changed the base branch from dev to main March 4, 2025 22:06

borngraced changed the base branch from main to dev March 4, 2025 22:07

borngraced force-pushed the dockerize_pirate branch from 75a7e7a to fef5f2a Compare March 4, 2025 22:09

dimxy reviewed Mar 5, 2025

View reviewed changes

onur-ozkan reviewed Mar 6, 2025

View reviewed changes

dimxy reviewed Mar 6, 2025

View reviewed changes

revert all changes not related to dockerizing zombie tests

5d6bc86

this PR is slowly diverting from it's main purpose hance, I have remove additional fixes that shouldn't have been done here...removed fixes will be reimplemented `fix-zombie-tests` PR

borngraced force-pushed the dockerize_pirate branch from b795d88 to 5d6bc86 Compare March 10, 2025 11:23

fix clippy and remove unused

8184e29

borngraced added status: pending review and removed status: in progress labels Mar 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ARRR): dockerize zombie/pirate tests #2374

feat(ARRR): dockerize zombie/pirate tests #2374

borngraced commented Feb 26, 2025 •

edited

Loading

shamardy commented Mar 3, 2025

borngraced commented Mar 3, 2025

dimxy Mar 5, 2025

borngraced Mar 6, 2025 •

edited

Loading

dimxy Mar 11, 2025

dimxy Mar 5, 2025

borngraced Mar 6, 2025

dimxy Mar 6, 2025

dimxy Mar 6, 2025

onur-ozkan Mar 6, 2025

borngraced Mar 10, 2025

onur-ozkan Mar 6, 2025

borngraced Mar 10, 2025

onur-ozkan Mar 6, 2025

onur-ozkan Mar 6, 2025

onur-ozkan Mar 6, 2025

borngraced Mar 6, 2025

onur-ozkan Mar 6, 2025

borngraced Mar 6, 2025

dimxy Mar 6, 2025

borngraced commented Mar 10, 2025

dimxy commented Mar 11, 2025

		#[cfg(not(target_arch = "wasm32"))]
		use zcash_client_sqlite::error::SqliteClientError;

		// SAFETY: htlc_output is sending to a t_addr.
		let to = htlc_output.script_pubkey.address().unwrap();

		pub const ZOMBIE_ASSET_DOCKER_IMAGE: &str = "docker.io/borngraced/zombietestrunner";
		pub const ZOMBIE_ASSET_DOCKER_IMAGE_WITH_TAG: &str = "docker.io/borngraced/zombietestrunner:multiarch";

feat(ARRR): dockerize zombie/pirate tests #2374

Are you sure you want to change the base?

feat(ARRR): dockerize zombie/pirate tests #2374

Conversation

borngraced commented Feb 26, 2025 • edited Loading

shamardy commented Mar 3, 2025

borngraced commented Mar 3, 2025

Choose a reason for hiding this comment

borngraced Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borngraced commented Mar 10, 2025

dimxy commented Mar 11, 2025

borngraced commented Feb 26, 2025 •

edited

Loading

borngraced Mar 6, 2025 •

edited

Loading