From 2a077f21c77953937adb04dcc5fd2e258502076d Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Tue, 17 Oct 2023 12:56:00 +1000
Subject: [PATCH] fix(ci): Skip mempool checks if it wasn't queried by
 lightwalletd (#7744)

* Correctly skip checks in lightwalletd send transactions test

* Wait longer for transaction query

* Wait longer before re-checking the mempool

* Skip mempool checks if it wasn't queried by lightwalletd

* rustfmt

* clippy and rustfmt

* Fix attribute issue
---
 .../lightwalletd/send_transaction_test.rs     | 37 +++++++++++++++----
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/zebrad/tests/common/lightwalletd/send_transaction_test.rs b/zebrad/tests/common/lightwalletd/send_transaction_test.rs
index 5cfb1c68..870d3992 100644
--- a/zebrad/tests/common/lightwalletd/send_transaction_test.rs
+++ b/zebrad/tests/common/lightwalletd/send_transaction_test.rs
@@ -16,7 +16,7 @@
 //! were obtained. This is to ensure that zebra does not reject the transactions because they have
 //! already been seen in a block.
 
-use std::{cmp::min, sync::Arc};
+use std::{cmp::min, sync::Arc, time::Duration};
 
 use color_eyre::eyre::Result;
 
@@ -36,6 +36,7 @@ use crate::common::{
         sync::wait_for_zebrad_and_lightwalletd_sync,
         wallet_grpc::{self, connect_to_lightwalletd, Empty, Exclude},
     },
+    sync::LARGE_CHECKPOINT_TIMEOUT,
     test_type::TestType::{self, *},
 };
 
@@ -159,10 +160,12 @@ pub async fn run() -> Result<()> {
         .await?
         .into_inner();
 
-    // Lightwalletd won't call `get_raw_mempool` again until 2 seconds after the last call
+    // Lightwalletd won't call `get_raw_mempool` again until 2 seconds after the last call:
     // <https://github.com/zcash/lightwalletd/blob/master/frontend/service.go#L482>
+    //
+    // So we need to wait much longer than that here.
     let sleep_until_lwd_last_mempool_refresh =
-        tokio::time::sleep(std::time::Duration::from_secs(2));
+        tokio::time::sleep(std::time::Duration::from_secs(4));
 
     let transaction_hashes: Vec<transaction::Hash> =
         transactions.iter().map(|tx| tx.hash()).collect();
@@ -201,7 +204,7 @@ pub async fn run() -> Result<()> {
     zebrad.expect_stdout_line_matches("sending mempool transaction broadcast")?;
     // Wait for more transactions to verify, `GetMempoolTx` only returns txs where tx.HasShieldedElements()
     // <https://github.com/zcash/lightwalletd/blob/master/frontend/service.go#L537>
-    tokio::time::sleep(std::time::Duration::from_secs(1)).await;
+    tokio::time::sleep(std::time::Duration::from_secs(2)).await;
     sleep_until_lwd_last_mempool_refresh.await;
 
     tracing::info!("calling GetMempoolTx gRPC to fetch transactions...");
@@ -210,10 +213,26 @@ pub async fn run() -> Result<()> {
         .await?
         .into_inner();
 
-    // check that lightwalletd queries the mempool.
-    zebrad.expect_stdout_line_matches("answered mempool request .*req.*=.*TransactionIds")?;
+    // Sometimes lightwalletd doesn't check the mempool, and waits for the next block instead.
+    // If that happens, we skip the rest of the test.
+    tracing::info!("checking if lightwalletd has queried the mempool...");
 
-    // GetMempoolTx: make sure at least one of the transactions were inserted into the mempool.
+    // We need a short timeout here, because sometimes this message is not logged.
+    zebrad = zebrad.with_timeout(Duration::from_secs(60));
+    let tx_log =
+        zebrad.expect_stdout_line_matches("answered mempool request .*req.*=.*TransactionIds");
+    // Reset the failed timeout and give the rest of the test enough time to finish.
+    #[allow(unused_assignments)]
+    {
+        zebrad = zebrad.with_timeout(LARGE_CHECKPOINT_TIMEOUT);
+    }
+
+    if tx_log.is_err() {
+        tracing::info!("lightwalletd didn't query the mempool, skipping mempool contents checks");
+        return Ok(());
+    }
+
+    tracing::info!("checking the mempool contains some of the sent transactions...");
     let mut counter = 0;
     while let Some(tx) = transactions_stream.message().await? {
         let hash: [u8; 32] = tx.hash.clone().try_into().expect("hash is correct length");
@@ -228,6 +247,8 @@ pub async fn run() -> Result<()> {
         counter += 1;
     }
 
+    // GetMempoolTx: make sure at least one of the transactions were inserted into the mempool.
+    //
     // TODO: Update `load_transactions_from_future_blocks()` to return block height offsets and,
     //       only check if a transaction from the first block has shielded elements
     assert!(
@@ -235,7 +256,7 @@ pub async fn run() -> Result<()> {
         "failed to read v4+ transactions with shielded elements from future blocks in mempool via lightwalletd"
     );
 
-    // GetMempoolTx: make sure at least one of the transactions were inserted into the mempool.
+    // TODO: GetMempoolStream: make sure at least one of the transactions were inserted into the mempool.
     tracing::info!("calling GetMempoolStream gRPC to fetch transactions...");
     let mut transaction_stream = rpc_client.get_mempool_stream(Empty {}).await?.into_inner();