fix: make wallet tests wait consensus

douglaz · Aug 22, 2023 · a4f45c2 · a4f45c2
1 parent 71e4d2c
commit a4f45c2
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 27 deletions.
diff --git a/devimint/src/federation.rs b/devimint/src/federation.rs
@@ -178,6 +178,8 @@ impl Federation {
 
     pub async fn pegin(&self, amount: u64) -> Result<()> {
         info!(amount, "Peg-in");
+        // Some blocks must always be mined first before pegin
+        self.await_minimum_blocks().await?;
         let deposit = cmd!(self, "deposit-address").out_json().await?;
         let deposit_address = deposit["address"].as_str().unwrap();
         let deposit_operation_id = deposit["operation_id"].as_str().unwrap();
@@ -195,6 +197,8 @@ impl Federation {
 
     pub async fn pegin_gateway(&self, amount: u64, gw: &Gatewayd) -> Result<()> {
         info!(amount, "Pegging-in gateway funds");
+        // Some blocks must always be mined first before pegin
+        self.await_minimum_blocks().await?;
         let fed_id = self.federation_id().await;
         let pegin_addr = cmd!(gw, "address", "--federation-id={fed_id}")
             .out_json()
@@ -227,21 +231,32 @@ impl Federation {
             .to_string()
     }
 
-    pub async fn await_block_sync(&self) -> Result<()> {
-        let client = self.client().await?;
-        let wallet_cfg: &WalletClientConfig = client
-            .config_ref()
-            .0
-            .get_module(LEGACY_HARDCODED_INSTANCE_ID_WALLET)?;
-        let finality_delay = wallet_cfg.finality_delay;
+    pub async fn await_block_sync(&self) -> Result<u64> {
+        let finality_delay = self.get_finality_delay().await?;
         let bitcoind_block_count = self.bitcoind.client().get_blockchain_info()?.blocks;
-        let expected = bitcoind_block_count - (finality_delay as u64);
+        let expected = bitcoind_block_count.saturating_sub(finality_delay.into());
         cmd!(self, "dev", "wait-block-count", expected)
             .run()
             .await?;
+        Ok(expected)
+    }
+
+    pub async fn await_minimum_blocks(&self) -> Result<()> {
+        // TODO: optimize this
+        self.generate_epochs(1).await?;
         Ok(())
     }
 
+    async fn get_finality_delay(&self) -> Result<u32, anyhow::Error> {
+        let client: fedimint_client_legacy::Client<UserClientConfig> = self.client().await?;
+        let wallet_cfg: &WalletClientConfig = client
+            .config_ref()
+            .0
+            .get_module(LEGACY_HARDCODED_INSTANCE_ID_WALLET)?;
+        let finality_delay = wallet_cfg.finality_delay;
+        Ok(finality_delay)
+    }
+
     pub async fn await_gateways_registered(&self) -> Result<()> {
         poll("gateways registered", || async {
             Ok(cmd!(self, "list-gateways")
@@ -279,8 +294,9 @@ impl Federation {
     }
 
     pub async fn generate_epochs(&self, epochs: usize) -> Result<()> {
+        let finality_delay = self.get_finality_delay().await?;
         for _ in 0..epochs {
-            self.bitcoind.mine_blocks(10).await?;
+            self.bitcoind.mine_blocks(finality_delay.into()).await?;
             self.await_block_sync().await?;
         }
         Ok(())

diff --git a/fedimint-client/src/lib.rs b/fedimint-client/src/lib.rs
@@ -391,7 +391,7 @@ impl IGlobalClientContext for ModuleGlobalClientContext {
                 TransactionBuilder::new().with_input(instance_input),
             )
             .await
-            .expect("Can obly fail if additional funding is needed")
+            .expect("Can only fail if additional funding is needed")
     }
 
     async fn fund_output_dyn(

diff --git a/fedimint-testing/src/btc/mock.rs b/fedimint-testing/src/btc/mock.rs
@@ -22,6 +22,7 @@ use fedimint_core::task::{sleep, TaskHandle};
 use fedimint_core::txoproof::TxOutProof;
 use fedimint_core::{Amount, Feerate};
 use rand::rngs::OsRng;
+use tracing::info;
 use url::Url;
 
 use super::BitcoinTest;
@@ -102,6 +103,11 @@ impl FakeBitcoinTest {
     }
 
     fn mine_block(blocks: &mut Vec<Block>, pending: &mut Vec<Transaction>) {
+        info!(
+            "Mining block: {} transactions, {} blocks",
+            pending.len(),
+            blocks.len()
+        );
         let root = BlockHash::hash(&[0]);
         // all blocks need at least one transaction
         if pending.is_empty() {

diff --git a/modules/fedimint-wallet-server/src/lib.rs b/modules/fedimint-wallet-server/src/lib.rs
@@ -326,7 +326,7 @@ impl ServerModule for Wallet {
             .await
             .saturating_sub(self.cfg.consensus.finality_delay);
 
-        if block_count_proposal != self.consensus_block_count(dbtx).await {
+        if Some(block_count_proposal) != self.consensus_block_count(dbtx).await {
             items.push(WalletConsensusItem::BlockCount(block_count_proposal));
         }
 
@@ -369,14 +369,12 @@ impl ServerModule for Wallet {
 
                 let new_consensus_block_count = self.consensus_block_count(dbtx).await;
 
-                // only sync from the first non-default consensus block count
-                if new_consensus_block_count > old_consensus_block_count {
-                    self.sync_up_to_consensus_height(
-                        dbtx,
-                        old_consensus_block_count,
-                        new_consensus_block_count,
-                    )
-                    .await;
+                // only sync when we have a consensus block count
+                match (old_consensus_block_count, new_consensus_block_count) {
+                    (Some(old), Some(new)) if new > old => {
+                        self.sync_up_to_consensus_height(dbtx, old, new).await;
+                    }
+                    _ => {}
                 }
             }
             WalletConsensusItem::Feerate(feerate) => {
@@ -574,7 +572,8 @@ impl ServerModule for Wallet {
             api_endpoint! {
                 "block_count",
                 async |module: &Wallet, context, _params: ()| -> u32 {
-                    Ok(module.consensus_block_count(&mut context.dbtx()).await)
+                    // TODO: perhaps change this to an Option
+                    Ok(module.consensus_block_count(&mut context.dbtx()).await.unwrap_or_default())
                 }
             },
             api_endpoint! {
@@ -795,7 +794,10 @@ impl Wallet {
             .unwrap_or(self.cfg.consensus.default_fee)
     }
 
-    pub async fn consensus_block_count(&self, dbtx: &mut ModuleDatabaseTransaction<'_>) -> u32 {
+    pub async fn consensus_block_count(
+        &self,
+        dbtx: &mut ModuleDatabaseTransaction<'_>,
+    ) -> Option<u32> {
         let peer_count = self.cfg.consensus.peer_peg_in_keys.total();
 
         let mut counts = dbtx
@@ -806,14 +808,13 @@ impl Wallet {
             .await;
 
         assert!(counts.len() <= peer_count);
-
-        while counts.len() < peer_count {
-            counts.push(0);
+        if counts.len() < peer_count {
+            return None;
         }
 
         counts.sort_unstable();
 
-        counts[peer_count / 2]
+        Some(counts[peer_count / 2])
     }
 
     pub async fn consensus_fee_rate(&self, dbtx: &mut ModuleDatabaseTransaction<'_>) -> Feerate {

diff --git a/modules/fedimint-wallet-tests/tests/tests.rs b/modules/fedimint-wallet-tests/tests/tests.rs
@@ -2,18 +2,23 @@ use std::time::{Duration, SystemTime};
 
 use anyhow::bail;
 use fedimint_client::Client;
+use fedimint_core::task::sleep;
 use fedimint_core::util::{BoxStream, NextOrPending};
 use fedimint_core::{sats, Amount, Feerate};
 use fedimint_dummy_client::DummyClientGen;
 use fedimint_dummy_common::config::DummyGenParams;
 use fedimint_dummy_server::DummyGen;
 use fedimint_testing::btc::BitcoinTest;
 use fedimint_testing::fixtures::Fixtures;
-use fedimint_wallet_client::{DepositState, WalletClientExt, WalletClientGen, WithdrawState};
+use fedimint_wallet_client::api::WalletFederationApi;
+use fedimint_wallet_client::{
+    DepositState, WalletClientExt, WalletClientGen, WalletClientModule, WithdrawState,
+};
 use fedimint_wallet_common::config::WalletGenParams;
 use fedimint_wallet_common::{PegOutFees, Rbf};
 use fedimint_wallet_server::WalletGen;
 use futures::stream::StreamExt;
+use tracing::info;
 
 fn fixtures() -> Fixtures {
     let fixtures = Fixtures::new_primary(DummyClientGen, DummyGen, DummyGenParams::default());
@@ -60,15 +65,36 @@ async fn peg_in<'a>(
     Ok(balance_sub)
 }
 
+async fn await_consensus_to_catch_up(client: &Client, block_count: u64) -> anyhow::Result<()> {
+    let (_, instance) =
+        client.get_first_module::<WalletClientModule>(&fedimint_wallet_client::KIND);
+    loop {
+        let current_consensus = client
+            .api()
+            .with_module(instance.id)
+            .fetch_consensus_block_count()
+            .await?;
+        if current_consensus < block_count {
+            info!("Current consensus block count is {current_consensus}, waiting for consensus to reach block count {block_count}");
+            sleep(Duration::from_secs(1)).await;
+        } else {
+            break;
+        }
+    }
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn on_chain_peg_in_and_peg_out_happy_case() -> anyhow::Result<()> {
     let fixtures = fixtures();
     let fed = fixtures.new_fed().await;
     let client = fed.new_client().await;
     let bitcoin = fixtures.bitcoin();
+    let bitcoin = bitcoin.lock_exclusive().await;
 
     let finality_delay = 10;
     bitcoin.mine_blocks(finality_delay).await;
+    await_consensus_to_catch_up(&client, 1).await?;
 
     let mut balance_sub = peg_in(&client, bitcoin.as_ref(), finality_delay).await?;
 
@@ -103,8 +129,10 @@ async fn peg_out_fail_refund() -> anyhow::Result<()> {
     let fed = fixtures.new_fed().await;
     let client = fed.new_client().await;
     let bitcoin = fixtures.bitcoin();
+    let bitcoin = bitcoin.lock_exclusive().await;
     let finality_delay = 10;
     bitcoin.mine_blocks(finality_delay).await;
+    await_consensus_to_catch_up(&client, 1).await?;
 
     let mut balance_sub = peg_in(&client, bitcoin.as_ref(), finality_delay).await?;
 
@@ -146,6 +174,7 @@ async fn peg_outs_support_rbf() -> anyhow::Result<()> {
 
     let finality_delay = 10;
     bitcoin.mine_blocks(finality_delay).await;
+    await_consensus_to_catch_up(&client, 1).await?;
 
     let mut balance_sub = peg_in(&client, bitcoin.as_ref(), finality_delay).await?;
 

diff --git a/nix/craneBuild.nix b/nix/craneBuild.nix
@@ -111,7 +111,7 @@ craneLib.overrideScope' (self: prev: {
     pnameSuffix = "-lcov";
     version = "0.0.1";
     cargoArtifacts = self.workspaceDepsCov;
-    buildPhaseCargoCommand = "mkdir -p $out ; env RUST_BACKTRACE=1 RUST_LOG=info,timing=debug cargo llvm-cov --locked --workspace --all-targets --profile $CARGO_PROFILE --lcov --tests --output-path $out/lcov.info --  --test-threads=$(($(nproc) * 2))";
+    buildPhaseCargoCommand = "mkdir -p $out ; env RUST_BACKTRACE=1 RUST_LOG=debug,timing=debug cargo llvm-cov --locked  --profile $CARGO_PROFILE --lcov --test fedimint_wallet_tests --output-path $out/lcov.info --  --test-threads=$(($(nproc) * 2))";
     installPhaseCommand = "true";
     nativeBuildInputs = self.commonArgs.nativeBuildInputs ++ [ self.cargo-llvm-cov ];
     doCheck = false;