Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix caching #985

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/blockchain/compact_filters/peer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl Mempool {

/// Look-up a transaction in the mempool given an [`Inventory`] request
pub fn get_tx(&self, inventory: &Inventory) -> Option<Transaction> {
let identifer = match inventory {
let identifier = match inventory {
Inventory::Error
| Inventory::Block(_)
| Inventory::WitnessBlock(_)
Expand All @@ -92,7 +92,7 @@ impl Mempool {
}
};

let txid = match identifer {
let txid = match identifier {
TxIdentifier::Txid(txid) => Some(txid),
TxIdentifier::Wtxid(wtxid) => self.0.read().unwrap().wtxids.get(&wtxid).cloned(),
};
Expand Down
2 changes: 1 addition & 1 deletion src/blockchain/electrum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl WalletSync for ElectrumBlockchain {

// The electrum server has been inconsistent somehow in its responses during sync. For
// example, we do a batch request of transactions and the response contains less
// tranascations than in the request. This should never happen but we don't want to panic.
// transactions than in the request. This should never happen but we don't want to panic.
let electrum_goof = || Error::Generic("electrum server misbehaving".to_string());

let batch_update = loop {
Expand Down
2 changes: 1 addition & 1 deletion src/blockchain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ pub trait ConfigurableBlockchain: Blockchain + Sized {

/// Trait for blockchains that don't contain any state
///
/// Statless blockchains can be used to sync multiple wallets with different descriptors.
/// Stateless blockchains can be used to sync multiple wallets with different descriptors.
///
/// [`BlockchainFactory`] is automatically implemented for `Arc<T>` where `T` is a stateless
/// blockchain.
Expand Down
4 changes: 2 additions & 2 deletions src/blockchain/script_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl<'a, D: BatchDatabase> ScriptReq<'a, D> {

pub fn satisfy(
mut self,
// we want to know the txids assoiciated with the script and their height
// we want to know the txids associated with the script and their height
txids: Vec<Vec<(Txid, Option<u32>)>>,
) -> Result<Request<'a, D>, Error> {
for (txid_list, script) in txids.iter().zip(self.scripts_needed.iter()) {
Expand Down Expand Up @@ -397,7 +397,7 @@ impl<'a, D: BatchDatabase> State<'a, D> {

// set every utxo we observed, unless it's already spent
// we don't do this in the loop above as we want to know all the spent outputs before
// adding the non-spent to the batch in case there are new tranasactions
// adding the non-spent to the batch in case there are new transactions
// that spend form each other.
for finished_tx in &finished_txs {
let tx = finished_tx
Expand Down
21 changes: 21 additions & 0 deletions src/testutils/blockchain_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,27 @@ macro_rules! bdk_blockchain_tests {
assert_eq!(wallet.list_transactions(false).unwrap().len(), 2, "incorrect number of txs");
}

#[test]
fn test_sync_multiple_batches() {
let (wallet, blockchain, descriptors, mut test_client) = init_single_sig();

for index in 1..=310 {
if index % 10 == 0 {
test_client.receive(testutils! {
@tx ( (@external descriptors, index) => 1_000 )
});
}
if index % 50 == 0 {
test_client.generate(1, None);
}
}
wallet.sync(&blockchain, SyncOptions::default()).unwrap();

assert_eq!(wallet.get_balance().unwrap().confirmed, 30_000, "incorrect confirmed balance");
assert_eq!(wallet.get_balance().unwrap().untrusted_pending, 1_000, "incorrect untrusted balance");
assert_eq!(wallet.list_transactions(false).unwrap().len(), 31, "incorrect number of txs");
}

#[test]
fn test_sync_before_and_after_receive() {
let (wallet, blockchain, descriptors, mut test_client) = init_single_sig();
Expand Down
57 changes: 42 additions & 15 deletions src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,37 +368,63 @@ where
/// transaction output scripts.
pub fn ensure_addresses_cached(&self, max_addresses: u32) -> Result<bool, Error> {
let mut new_addresses_cached = false;
let max_address = match self.descriptor.has_wildcard() {
false => 0,
true => max_addresses,
let (last_index, max_addresses) = match self.descriptor.has_wildcard() {
false => (0, 0),
true => {
// last used index
let last_index = self
.database
.borrow()
.get_last_index(KeychainKind::External)?
.unwrap_or_default();

(last_index, max_addresses)
}
};
debug!("max_address {}", max_address);
debug!("external max_address {}", last_index + max_addresses);
if self
.database
.borrow()
.get_script_pubkey_from_path(KeychainKind::External, max_address.saturating_sub(1))?
.get_script_pubkey_from_path(KeychainKind::External, last_index + max_addresses)?
.is_none()
{
debug!("caching external addresses");
new_addresses_cached = true;
self.cache_addresses(KeychainKind::External, 0, max_address)?;
debug!(
"caching external addresses from {} to max_address {}",
last_index,
last_index + max_addresses
);
self.cache_addresses(KeychainKind::External, last_index, max_addresses)?;
}

if let Some(change_descriptor) = &self.change_descriptor {
let max_address = match change_descriptor.has_wildcard() {
false => 0,
true => max_addresses,
let (last_index, max_addresses) = match change_descriptor.has_wildcard() {
false => (0, 0),
true => {
// last used index
let last_index = self
.database
.borrow()
.get_last_index(KeychainKind::Internal)?
.unwrap_or_default();

(last_index, max_addresses)
}
};

if self
.database
.borrow()
.get_script_pubkey_from_path(KeychainKind::Internal, max_address.saturating_sub(1))?
.get_script_pubkey_from_path(KeychainKind::Internal, last_index + max_addresses)?
.is_none()
{
debug!("caching internal addresses");
new_addresses_cached = true;
self.cache_addresses(KeychainKind::Internal, 0, max_address)?;
debug!(
"caching internal addresses from {} to max_address {}",
last_index,
last_index + max_addresses
);
self.cache_addresses(KeychainKind::Internal, last_index, max_addresses)?;
}
}
Ok(new_addresses_cached)
Expand Down Expand Up @@ -1391,9 +1417,10 @@ where
}

info!(
"Derivation of {} addresses from {} took {} ms",
"Derivation of {} addresses from {} to {} took {} ms",
count,
from,
from + count,
start_time.elapsed().as_millis()
);

Expand Down Expand Up @@ -1709,7 +1736,7 @@ where
// TODO: what if i generate an address first and cache some addresses?
// TODO: we should sync if generating an address triggers a new batch to be stored

// We need to ensure descriptor is derivable to fullfil "missing cache", otherwise we will
// We need to ensure descriptor is derivable to fulfill "missing cache", otherwise we will
// end up with an infinite loop
let has_wildcard = self.descriptor.has_wildcard()
&& (self.change_descriptor.is_none()
Expand Down