From 3991e8531cb14b51253e823661d48628d1f08bf7 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Tue, 24 Dec 2024 22:31:26 +0000 Subject: [PATCH 1/5] transaction_disappears_before_mempool test is set up but doesnt actually disappear the transaction --- darkside-tests/tests/tests.rs | 180 ++++++++++++++++---------- zingolib/src/testutils/lightclient.rs | 1 + 2 files changed, 114 insertions(+), 67 deletions(-) diff --git a/darkside-tests/tests/tests.rs b/darkside-tests/tests/tests.rs index 9dddbd90a..e2bb1d54f 100644 --- a/darkside-tests/tests/tests.rs +++ b/darkside-tests/tests/tests.rs @@ -5,20 +5,21 @@ use darkside_tests::utils::update_tree_states_for_transaction; use darkside_tests::utils::DarksideHandler; use std::future::Future; use std::pin::Pin; -use std::sync::Arc; use testvectors::seeds::DARKSIDE_SEED; use tokio::time::sleep; use zcash_client_backend::PoolType::Shielded; use zcash_client_backend::ShieldedProtocol::Orchard; +use zcash_primitives::consensus::BlockHeight; use zingo_status::confirmation_status::ConfirmationStatus; use zingolib::config::RegtestNetwork; use zingolib::get_base_address_macro; +use zingolib::grpc_connector; use zingolib::lightclient::LightClient; use zingolib::lightclient::PoolBalances; -use zingolib::testutils::chain_generics::conduct_chain::ConductChain as _; -use zingolib::testutils::chain_generics::with_assertions::to_clients_proposal; -use zingolib::testutils::lightclient::from_inputs; +use zingolib::testutils; +use zingolib::testutils::chain_generics::conduct_chain::ConductChain; use zingolib::testutils::scenarios::setup::ClientBuilder; +use zingolib::wallet::notes::query::OutputQuery; #[tokio::test] async fn simple_sync() { @@ -178,7 +179,7 @@ async fn sent_transaction_reorged_into_mempool() { transparent_balance: Some(0) } ); - let one_txid = from_inputs::quick_send( + let one_txid = testutils::lightclient::from_inputs::quick_send( &light_client, vec![(&get_base_address_macro!(recipient, "unified"), 10_000, None)], ) @@ -246,91 +247,136 @@ async fn sent_transaction_reorged_into_mempool() { } #[tokio::test] -#[ignore = "incomplete"] -async fn evicted_transaction_is_rebroadcast() { +async fn transaction_disappears_before_mempool() { std::env::set_var("RUST_BACKTRACE", "1"); - let mut environment = DarksideEnvironment::setup().await; - environment.bump_chain().await; + let mut environment = ::setup().await; + ::bump_chain(&mut environment).await; let primary = environment.fund_client_orchard(1_000_000).await; let secondary = environment.create_client().await; primary.do_sync(false).await.unwrap(); - let proposal = - to_clients_proposal(&primary, &[(&secondary, Shielded(Orchard), 100_000, None)]).await; - - let mut send_height = 0; + let proposal = testutils::chain_generics::with_assertions::to_clients_proposal( + &primary, + &[(&secondary, Shielded(Orchard), 100_000, None)], + ) + .await; + println!("following proposal, preparing to unwind if an assertion fails."); - let txids = &primary + let txids = primary .complete_and_broadcast_stored_proposal() .await .unwrap(); - println!( - "{:?}", - zingolib::testutils::lightclient::list_txids(&primary).await - ); - - let _recorded_fee = *zingolib::testutils::assertions::lookup_fees_with_proposal_check( - &primary, &proposal, txids, - ) - .await - .first() - .expect("one transaction must have been proposed") - .as_ref() - .expect("record must exist"); - - zingolib::testutils::lightclient::lookup_statuses(&primary, txids.clone()) - .await - .map(|status| { - assert_eq!( - status, - Some(ConfirmationStatus::Transmitted(send_height.into())) - ); - }); - - zingolib::testutils::lightclient::lookup_statuses(&secondary, txids.clone()) - .await - .map(|status| { - assert!(status.is_none()); - }); + // a modified zingolib::testutils::chain_generics::with_assertions::follow_proposal block + { + let sender = &primary; + let proposal = &proposal; + let recipients = vec![&secondary]; - environment - .get_connector() - .clear_incoming_transactions() + let server_height_at_send = BlockHeight::from( + grpc_connector::get_latest_block(environment.lightserver_uri().unwrap()) + .await + .unwrap() + .height as u32, + ); + + // check that each record has the expected fee and status, returning the fee + let (sender_recorded_fees, (sender_recorded_outputs, sender_recorded_statuses)): ( + Vec, + (Vec, Vec), + ) = testutils::assertions::for_each_proposed_record( + sender, + proposal, + &txids, + |records, record, step| { + ( + testutils::assertions::compare_fee(records, record, step), + (record.query_sum_value(OutputQuery::any()), record.status), + ) + }, + ) .await - .unwrap(); - environment.bump_chain().await; + .into_iter() + .map(|stepwise_result| { + stepwise_result + .map(|(fee_comparison_result, others)| (fee_comparison_result.unwrap(), others)) + .unwrap() + }) + .unzip(); - zingolib::testutils::lightclient::lookup_statuses(&primary, txids.clone()) - .await - .map(|status| { + for status in sender_recorded_statuses { assert_eq!( status, - Some(ConfirmationStatus::Transmitted(send_height.into())) + ConfirmationStatus::Transmitted(server_height_at_send + 1) ); - }); + } - zingolib::testutils::lightclient::lookup_statuses(&secondary, txids.clone()) + environment.bump_chain().await; + // chain scan shows the same + sender.do_sync(false).await.unwrap(); + + // check that each record has the expected fee and status, returning the fee and outputs + let (sender_confirmed_fees, (sender_confirmed_outputs, sender_confirmed_statuses)): ( + Vec, + (Vec, Vec), + ) = testutils::assertions::for_each_proposed_record( + sender, + proposal, + &txids, + |records, record, step| { + ( + testutils::assertions::compare_fee(records, record, step), + (record.query_sum_value(OutputQuery::any()), record.status), + ) + }, + ) .await - .map(|status| { - assert!(status.is_none()); - }); - - send_height = 0; - - primary.do_sync(false).await.unwrap(); + .into_iter() + .map(|stepwise_result| { + stepwise_result + .map(|(fee_comparison_result, others)| (fee_comparison_result.unwrap(), others)) + .unwrap() + }) + .unzip(); - zingolib::testutils::lightclient::lookup_statuses(&primary, txids.clone()) - .await - .map(|status| { + assert_eq!(sender_confirmed_fees, sender_recorded_fees); + assert_eq!(sender_confirmed_outputs, sender_recorded_outputs); + for status in sender_confirmed_statuses { assert_eq!( status, - Some(ConfirmationStatus::Transmitted(send_height.into())) + ConfirmationStatus::Confirmed(server_height_at_send + 1) ); - }); + } - let ref_primary: Arc = Arc::new(primary); - LightClient::start_mempool_monitor(ref_primary).unwrap(); + let mut recipients_confirmed_outputs = vec![]; + for recipient in recipients { + recipient.do_sync(false).await.unwrap(); + + // check that each record has the status, returning the output value + let (recipient_confirmed_outputs, recipient_confirmed_statuses): ( + Vec, + Vec, + ) = testutils::assertions::for_each_proposed_record( + recipient, + proposal, + &txids, + |_records, record, _step| { + (record.query_sum_value(OutputQuery::any()), record.status) + }, + ) + .await + .into_iter() + .map(|stepwise_result| stepwise_result.unwrap()) + .collect(); + for status in recipient_confirmed_statuses { + assert_eq!( + status, + ConfirmationStatus::Confirmed(server_height_at_send + 1) + ); + } + recipients_confirmed_outputs.push(recipient_confirmed_outputs); + } + } } diff --git a/zingolib/src/testutils/lightclient.rs b/zingolib/src/testutils/lightclient.rs index fc8fb1995..f78df7065 100644 --- a/zingolib/src/testutils/lightclient.rs +++ b/zingolib/src/testutils/lightclient.rs @@ -98,6 +98,7 @@ pub mod from_inputs { } /// gets stati for a vec of txids +#[deprecated(note = "use for_each_proposed_record")] pub async fn lookup_statuses( client: &LightClient, txids: nonempty::NonEmpty, From 7662f5235cd5cea125321c92f0d9c937a664c7b8 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Wed, 25 Dec 2024 02:32:34 +0000 Subject: [PATCH 2/5] test shows that banished transactions stay in Transmitted state --- darkside-tests/tests/tests.rs | 39 +++++++---------------------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/darkside-tests/tests/tests.rs b/darkside-tests/tests/tests.rs index e2bb1d54f..2b3b25361 100644 --- a/darkside-tests/tests/tests.rs +++ b/darkside-tests/tests/tests.rs @@ -273,7 +273,7 @@ async fn transaction_disappears_before_mempool() { { let sender = &primary; let proposal = &proposal; - let recipients = vec![&secondary]; + let _recipients = vec![&secondary]; let server_height_at_send = BlockHeight::from( grpc_connector::get_latest_block(environment.lightserver_uri().unwrap()) @@ -313,6 +313,12 @@ async fn transaction_disappears_before_mempool() { ); } + environment + .get_connector() + .clear_incoming_transactions() + .await + .unwrap(); + environment.bump_chain().await; // chain scan shows the same sender.do_sync(false).await.unwrap(); @@ -346,37 +352,8 @@ async fn transaction_disappears_before_mempool() { for status in sender_confirmed_statuses { assert_eq!( status, - ConfirmationStatus::Confirmed(server_height_at_send + 1) + ConfirmationStatus::Transmitted(server_height_at_send + 1) ); } - - let mut recipients_confirmed_outputs = vec![]; - for recipient in recipients { - recipient.do_sync(false).await.unwrap(); - - // check that each record has the status, returning the output value - let (recipient_confirmed_outputs, recipient_confirmed_statuses): ( - Vec, - Vec, - ) = testutils::assertions::for_each_proposed_record( - recipient, - proposal, - &txids, - |_records, record, _step| { - (record.query_sum_value(OutputQuery::any()), record.status) - }, - ) - .await - .into_iter() - .map(|stepwise_result| stepwise_result.unwrap()) - .collect(); - for status in recipient_confirmed_statuses { - assert_eq!( - status, - ConfirmationStatus::Confirmed(server_height_at_send + 1) - ); - } - recipients_confirmed_outputs.push(recipient_confirmed_outputs); - } } } From 16777a6759939248da44492d69f28c4725d861a3 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Wed, 25 Dec 2024 03:28:12 +0000 Subject: [PATCH 3/5] added timer, giving the wallet a changce to rebroadcast --- darkside-tests/tests/tests.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/darkside-tests/tests/tests.rs b/darkside-tests/tests/tests.rs index 2b3b25361..381389f58 100644 --- a/darkside-tests/tests/tests.rs +++ b/darkside-tests/tests/tests.rs @@ -323,6 +323,12 @@ async fn transaction_disappears_before_mempool() { // chain scan shows the same sender.do_sync(false).await.unwrap(); + sleep(std::time::Duration::from_millis(10_000)).await; + + environment.bump_chain().await; + // chain scan shows the same + sender.do_sync(false).await.unwrap(); + // check that each record has the expected fee and status, returning the fee and outputs let (sender_confirmed_fees, (sender_confirmed_outputs, sender_confirmed_statuses)): ( Vec, From 2b11df3834b338b7295272112410cc9e85b14630 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Wed, 25 Dec 2024 03:39:53 +0000 Subject: [PATCH 4/5] add comments --- darkside-tests/tests/tests.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/darkside-tests/tests/tests.rs b/darkside-tests/tests/tests.rs index 381389f58..83d0a2ac8 100644 --- a/darkside-tests/tests/tests.rs +++ b/darkside-tests/tests/tests.rs @@ -246,6 +246,11 @@ async fn sent_transaction_reorged_into_mempool() { ); } +/// this test demonstrates that +/// when a transaction is dropped by lightwallet, zingolib does not proactively respond +/// the test is set up to have a different result if proactive rebroadcast happens +/// there is a further detail to understand: zingolib currently considers all recorded transactions to be un-contradictable. it cant imagine an alternate history without a transaction until the transaction is deleted. this can cause it to become deeply confused until it forgets about the contradicted transaction VIA a reboot. +/// despite this test, there may be unexpected failure modes in network conditions #[tokio::test] async fn transaction_disappears_before_mempool() { std::env::set_var("RUST_BACKTRACE", "1"); @@ -273,7 +278,6 @@ async fn transaction_disappears_before_mempool() { { let sender = &primary; let proposal = &proposal; - let _recipients = vec![&secondary]; let server_height_at_send = BlockHeight::from( grpc_connector::get_latest_block(environment.lightserver_uri().unwrap()) From b37b45c38e1cb44473ceb66dac0c839243220bd3 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Wed, 25 Dec 2024 03:42:08 +0000 Subject: [PATCH 5/5] cargo clippy --fix --tests --all-features --- zingolib/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zingolib/src/config.rs b/zingolib/src/config.rs index e1ec4455b..3dde3bdcc 100644 --- a/zingolib/src/config.rs +++ b/zingolib/src/config.rs @@ -759,7 +759,7 @@ mod tests { true, ); - assert_eq!(valid_config.is_ok(), true); + assert!(valid_config.is_ok()); } #[tokio::test]