From bfb9b46fb2fc6aae1c04f32c71bb8dff2bd1a05c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 7 Sep 2018 11:56:41 -0400 Subject: [PATCH] Refactor/dont re-enter block_conencted on duplicate watch calls Previously we'd hit an infinite loop if a block_connected call always resulted in the same ChainWatchInterface registrations. While we're at it, we also split ChainWatchUtil in two to make things a bit more flexible for users, though not sure if that actually matters, and make the matching more aggressive in testing, even if we pick the more performant option at runtime. --- src/chain/chaininterface.rs | 137 ++++++++++++++++++++++++++++-------- 1 file changed, 107 insertions(+), 30 deletions(-) diff --git a/src/chain/chaininterface.rs b/src/chain/chaininterface.rs index 3a0b2d735..5ff720c32 100644 --- a/src/chain/chaininterface.rs +++ b/src/chain/chaininterface.rs @@ -5,9 +5,12 @@ use bitcoin::blockdata::constants::genesis_block; use bitcoin::util::hash::Sha256dHash; use bitcoin::network::constants::Network; use bitcoin::network::serialize::BitcoinHash; + use util::logger::Logger; + use std::sync::{Mutex,Weak,MutexGuard,Arc}; use std::sync::atomic::{AtomicUsize, Ordering}; +use std::collections::HashSet; /// Used to give chain error details upstream pub enum ChainError { @@ -57,6 +60,8 @@ pub trait ChainListener: Sync + Send { /// Note that if a new transaction/outpoint is watched during a block_connected call, the block /// *must* be re-scanned with the new transaction/outpoints and block_connected should be /// called again with the same header and (at least) the new transactions. + /// Note that if non-new transaction/outpoints may be registered during a call, a second call + /// *must not* happen. /// This also means those counting confirmations using block_connected callbacks should watch /// for duplicate headers and not count them towards confirmations! fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]); @@ -85,11 +90,98 @@ pub trait FeeEstimator: Sync + Send { fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u64; } +/// Utility for tracking registered txn/outpoints and checking for matches +pub struct ChainWatchedUtil { + watch_all: bool, + + // We are more conservative in matching during testing to ensure everything matches *exactly*, + // even though during normal runtime we take more optimized match approaches... + #[cfg(test)] + watched_txn: HashSet<(Sha256dHash, Script)>, + #[cfg(not(test))] + watched_txn: HashSet