Skip to content

Commit 5ab2473

Browse files
committed
Fix syncing of wallet during taker initialization
Signed-off-by: Abhay349 <pandeyabhay967@gmail.com>
1 parent 878cf12 commit 5ab2473

File tree

4 files changed

+138
-10
lines changed

4 files changed

+138
-10
lines changed

src/taker/api.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ impl Taker {
247247

248248
let watch_service = WatchService::new(tx_requests, rx_responses);
249249

250-
let mut wallet = Wallet::load_or_init_wallet(&wallet_path, &rpc_config, password)?;
250+
let wallet = Wallet::load_or_init_wallet(&wallet_path, &rpc_config, password)?;
251251

252252
// If config file doesn't exist, default config will be loaded.
253253
let mut config = TakerConfig::new(Some(&data_dir.join("config.toml")))?;
@@ -276,7 +276,7 @@ impl Taker {
276276
)
277277
.start();
278278

279-
wallet.sync_and_save()?;
279+
log::info!("Wallet sync skipped during initialization. Call sync_wallet() manually.");
280280

281281
Ok(Self {
282282
wallet,
@@ -290,6 +290,12 @@ impl Taker {
290290
})
291291
}
292292

293+
/// Sync the wallet and save to disk
294+
pub fn sync_wallet(&mut self) -> Result<(), TakerError> {
295+
self.wallet.sync_and_save()?;
296+
Ok(())
297+
}
298+
293299
/// Restores a wallet from a backup file using the given configuration.
294300
///
295301
/// This is a convenience wrapper around [`Wallet::restore_interactive`] that constructs the

src/taker/api2.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ impl Taker {
280280

281281
let watch_service = WatchService::new(tx_requests, rx_responses);
282282

283-
let mut wallet = if wallet_path.exists() {
283+
let wallet = if wallet_path.exists() {
284284
let wallet = Wallet::load(&wallet_path, &rpc_config, password)?;
285285
log::info!("Loaded wallet from {}", wallet_path.display());
286286
wallet
@@ -312,9 +312,7 @@ impl Taker {
312312
)
313313
.start();
314314

315-
log::info!("Initializing wallet sync...");
316-
wallet.sync_and_save()?;
317-
log::info!("Completed wallet sync");
315+
log::info!("Wallet sync skipped during initialization. Call sync_wallet() manually.");
318316

319317
Ok(Self {
320318
wallet,
@@ -328,6 +326,12 @@ impl Taker {
328326
})
329327
}
330328

329+
/// Sync the wallet and save to disk
330+
pub fn sync_wallet(&mut self) -> Result<(), TakerError> {
331+
self.wallet.sync_and_save()?;
332+
Ok(())
333+
}
334+
331335
/// Get a reference to the wallet
332336
pub fn get_wallet(&self) -> &Wallet {
333337
&self.wallet

tests/test_framework/mod.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,7 @@ pub fn verify_maker_pre_swap_balance_taproot(taproot_makers: &[Arc<TaprootMaker>
537537
pub struct TestFramework {
538538
pub(super) bitcoind: BitcoinD,
539539
pub(super) temp_dir: PathBuf,
540+
pub(super) zmq_addr: String,
540541
shutdown: AtomicBool,
541542
nostr_relay_shutdown: mpsc::Sender<()>,
542543
nostr_relay_handle: Option<JoinHandle<()>>,
@@ -583,6 +584,7 @@ impl TestFramework {
583584
let test_framework = Arc::new(Self {
584585
bitcoind,
585586
temp_dir: temp_dir.clone(),
587+
zmq_addr: zmq_addr.clone(),
586588
shutdown,
587589
nostr_relay_shutdown,
588590
nostr_relay_handle: Some(nostr_relay_handle),
@@ -598,7 +600,7 @@ impl TestFramework {
598600
.enumerate()
599601
.map(|(i, behavior)| {
600602
let taker_id = format!("taker{}", i + 1); // ex: "taker1"
601-
Taker::init(
603+
let mut taker = Taker::init(
602604
Some(temp_dir.join(&taker_id)),
603605
Some(taker_id),
604606
Some(taker_rpc_config.clone()),
@@ -608,7 +610,9 @@ impl TestFramework {
608610
zmq_addr.clone(),
609611
None,
610612
)
611-
.unwrap()
613+
.unwrap();
614+
taker.sync_wallet().unwrap();
615+
taker
612616
})
613617
.collect::<Vec<_>>();
614618
let mut base_rpc_port = 3500; // Random port for RPC connection in tests. (Not used)
@@ -708,6 +712,7 @@ impl TestFramework {
708712
let test_framework = Arc::new(Self {
709713
bitcoind,
710714
temp_dir: temp_dir.clone(),
715+
zmq_addr: zmq_addr.clone(),
711716
shutdown,
712717
nostr_relay_shutdown,
713718
nostr_relay_handle: Some(nostr_relay_handle),
@@ -723,7 +728,7 @@ impl TestFramework {
723728
.enumerate()
724729
.map(|(i, behavior)| {
725730
let taker_id = format!("taker{}", i + 1); // ex: "taker1"
726-
TaprootTaker::init(
731+
let mut taker = TaprootTaker::init(
727732
Some(temp_dir.join(&taker_id)),
728733
Some(taker_id),
729734
Some(taker_rpc_config.clone()),
@@ -733,7 +738,9 @@ impl TestFramework {
733738
None,
734739
behavior,
735740
)
736-
.unwrap()
741+
.unwrap();
742+
taker.sync_wallet().unwrap();
743+
taker
737744
})
738745
.collect::<Vec<_>>();
739746
let mut base_rpc_port = 3500; // Random port for RPC connection in tests. (Not used)

tests/verify_non_blocking_init.rs

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
#![cfg(feature = "integration-test")]
2+
use bitcoin::Amount;
3+
use coinswap::{
4+
taker::{Taker, TakerBehavior},
5+
wallet::{AddressType, RPCConfig},
6+
};
7+
use std::sync::Arc;
8+
9+
mod test_framework;
10+
use test_framework::*;
11+
12+
use log::{info, warn};
13+
14+
#[test]
15+
fn test_non_blocking_wallet_init() {
16+
// ---- Setup ----
17+
// Initialize framework with no makers/takers to just get bitcoind
18+
let (test_framework, _, _, block_generation_handle) =
19+
TestFramework::init(vec![], vec![]);
20+
21+
warn!("🧪 Running Test: Verify Non-Blocking Wallet Init");
22+
let bitcoind = &test_framework.bitcoind;
23+
let temp_dir = &test_framework.temp_dir;
24+
let zmq_addr = test_framework.zmq_addr.clone();
25+
26+
// Init Taker manually (without auto-sync)
27+
let taker_id = "manual_taker".to_string();
28+
let rpc_config = RPCConfig::from(test_framework.as_ref());
29+
let mut taker = Taker::init(
30+
Some(temp_dir.join(&taker_id)),
31+
Some(taker_id),
32+
Some(rpc_config),
33+
TakerBehavior::Normal,
34+
None,
35+
None,
36+
zmq_addr,
37+
None,
38+
)
39+
.unwrap();
40+
41+
// At this point, Taker is initialized but wallet should NOT be synced.
42+
// However, since it's a new wallet, it has no history.
43+
// To verify non-syncing behavior properly, we need to:
44+
// 1. Send coins to the taker's address.
45+
// 2. Generate blocks.
46+
// 3. Check balance (should be 0 because it hasn't scanned/synced).
47+
// 4. Connect/Sync wallet.
48+
// 5. Check balance (should be > 0).
49+
50+
let wallet = taker.get_wallet_mut();
51+
// We can call sync_and_save here to init the wallet file properly if needed, BUT Taker::init already loads/creates it.
52+
// But Taker::init skipped the sync.
53+
// If we get an address now, it should work (derives from seed).
54+
let taker_address = wallet
55+
.get_next_external_address(AddressType::P2WPKH)
56+
.unwrap();
57+
58+
// Send coins
59+
let utxo_value = Amount::from_btc(0.1).unwrap();
60+
send_to_address(bitcoind, &taker_address, utxo_value);
61+
generate_blocks(bitcoind, 6); // Generate confirmations
62+
63+
// Check balance BEFORE sync.
64+
// Since we skipped sync in init, and haven't called it since,
65+
// the wallet should not know about these coins yet.
66+
// Note: Watcher runs in background, but it listens to ZMQ for *new* transactions.
67+
// However, we just sent a tx and mined blocks. Watcher might pick it up?
68+
// Watcher mainly handles swap protocol messages and certain txs.
69+
// Does it update wallet balance?
70+
// `Watcher` updates `FileRegistry`.
71+
// Use `wallet.get_balances()` which queries core RPC for UTXOs *known to the wallet*.
72+
// If wallet hasn't scanned/imported descriptors, it might not know them?
73+
// Actually, `wallet.get_balances` calls `list_descriptor_utxo_spend_info` which iterates over descriptors.
74+
// `list_descriptor_utxo_spend_info` uses `list_unspent` from RPC.
75+
// BUT `list_unspent` only returns UTXOs for addresses *imported* into the wallet.
76+
// When are addresses imported?
77+
// `Wallet::sync_and_save` calls `import_descriptors`.
78+
// If we skipped `sync_and_save`, `import_descriptors` was likely NOT called, or at least not updated.
79+
// `Wallet::load_or_init_wallet` -> `Wallet::init` or `Wallet::load`.
80+
// Neither `init` nor `load` calls `import_descriptors`.
81+
// `import_descriptors` is called inside `sync()`.
82+
// So if `sync()` was skipped, the descriptors are NOT imported in Bitcoin Core wallet.
83+
// Therefore `list_unspent` returns empty.
84+
85+
// Verify balance is ZERO
86+
let balances_pre_sync = wallet.get_balances().unwrap();
87+
info!("Helper check: Pre-sync regular balance: {}", balances_pre_sync.regular);
88+
// If imports didn't happen, balance should be 0.
89+
assert_eq!(balances_pre_sync.regular, Amount::ZERO, "Balance should be 0 before sync because descriptors are not imported yet");
90+
91+
// Drop mutable borrow of wallet to allow borrowing taker for sync
92+
// The borrow checker considers 'wallet' alive until its last use.
93+
// Since we used it above, we can just allow it to end scope or shadow it?
94+
// Actually, simply not using the 'wallet' variable from L50 anymore matches scope end if we re-bind.
95+
// But 'wallet' variable is holding the borrow.
96+
// We must ensure the compiler knows we are done with it.
97+
98+
// Now Sync Manual
99+
info!("🔄 Manually syncing wallet...");
100+
taker.sync_wallet().unwrap();
101+
102+
// Re-borrow wallet to check balance
103+
let wallet = taker.get_wallet_mut();
104+
// Verify balance is updated
105+
let balances_post_sync = wallet.get_balances().unwrap();
106+
info!("Helper check: Post-sync regular balance: {}", balances_post_sync.regular);
107+
assert_eq!(balances_post_sync.regular, utxo_value, "Balance should be 0.1 BTC after sync");
108+
109+
test_framework.stop();
110+
block_generation_handle.join().unwrap();
111+
}

0 commit comments

Comments
 (0)