Return wallet events when applying updates (3.0 milestone)

docs/adr/0003_events.md

@@ -0,0 +1,104 @@
+# Return user-facing events when applying updates after syncing
+
+* Status: accepted
+* Authors: @notmandatory
+* Date: 2025-09-21
+* Targeted modules: wallet
+* Associated tickets/PRs: #6, #310
+
+## Context and Problem Statement
+
+When syncing a `Wallet` with new blockchain data using `Wallet::apply_update` it does not return any value on success, 
+only a `CannotConnectError` if it fails. 
+
+Users have asked for a concise list of events that reflect if or how new blockchain data has changed the 
+blockchain tip and the status of transactions relevant to the wallet's bitcoin balance. This information should also
+be useful for on-chain apps who want to notify users of wallet changes after syncing.
+
+If the end user app ends for some reason before handling the wallet events, the same wallet events should be 
+regenerated when the same blockchain sync data is re-downloaded and reapplied to the wallet. 
+
+## Decision Drivers
+
+* Currently `Wallet::apply_update` does not return any value except a `CannotConnectError` if it fails.
+* Downstream users need updates on chain tip, new transactions and transaction status changes.
+* If the app doesn't process all the events before it ends the same events should be returned on a subsequent sync.
+* Current downstream users requesting this feature are: LDK node (@tnull) and Bitkit (@ovitrif).
+* This feature was requested in May 2024, over a year and a half ago.
+
+## Considered Options
+
+#### Option 1: Do nothing
+
+Do not change anything since all the data the users require is available with the current API by comparing the 
+wallet's canonical transaction list before and after applying a sync update.
+
+**Pros:**
+
+* No API changes are needed and user can customize the events to exactly what they need.
+
+**Cons:**
+
+* Users will need to duplicate the work to add this feature on every project.
+* It's easier for the core BDK team to add this feature once in a way that meets most users needs.
+
+#### Option 2: Modify the `Wallet::apply_update` to return a list of `WalletEvent`
+
+Adds `WalletEvent` enum of user facing events that are generated when a sync update is applied to a wallet using the
+existing `Wallet::apply_update` function. The `WalletEvent` enum includes an event for changes in blockchain tip and
+events for changes to the status of transactions that are relevant to the wallet, including:
+
+1. newly seen in the mempool
+2. replaced in the mempool 
+3. dropped from the mempool
+4. confirmed in a block
+5. confirmed in a new block due to a reorg
+6. unconfirmed due to a reorg
+
+Chain tip change events are generated by comparing the wallet's chain tip before and after applying an update. Wallet 
+transaction events are generated by comparing a snapshot of canonical transactions.
+
+As long as updates to the wallet are not persisted until after all events are processed by the caller then if the app
+crashes for some reason and the wallet is re-sync'd a new update will re-return the same events.
+
+The `WalletEvent` enum is non-exhaustive.
+
+**Pros:**
+
+* Events are always generated when a wallet update is applied.
+* The user doesn't need to add this functionality themselves.
+* New events can be added without a breaking change.
+
+**Cons:**
+
+* This can not be rolled out except as a breaking release since it changes the `Wallet::apply_update` function signature.
+* If an app doesn't care about these events they must still generate them.
+
+#### Option 3: Same as option 2 but add a new function
+
+This option is the same as option 2 but adds a new `Wallet::apply_update_events` function to update the wallet and
+return the list of `WalletEvent` enums.
+
+**Pros:**
+
+* Same reasons as above and does not require an API breaking release.
+* Keeps option for users to update the wallet with original `Wallet::apply_update` and not get events.
+
+**Cons:**
+
+* Could be confusing to users which function to use, the original or new one.
+* If in a future breaking release we decide to always return events we'll need to deprecate `Wallet::apply_update_events`.
+
+## Decision Outcome
+
+Chosen option: "Option 3", because it can be delivered to users in the next minor release. This option also lets us 
+get user feedback and see how the events are used before forcing all users to generate them during an update.
+
+### Positive Consequences
+
+* The new wallet events can be used for more responsive on chain wallet UIs.
+
+### Negative Consequences
+
+* The down stream `bdk-ffi` and book of bdk projects will need to be updated for this new feature.
+

wallet/src/test_utils.rs

@@ -4,7 +4,7 @@ use alloc::string::ToString;
 use alloc::sync::Arc;
 use core::str::FromStr;
 
-use bdk_chain::{BlockId, ConfirmationBlockTime, TxUpdate};
+use bdk_chain::{BlockId, CheckPoint, ConfirmationBlockTime, TxUpdate};
 use bitcoin::{
     absolute, hashes::Hash, transaction, Address, Amount, BlockHash, FeeRate, Network, OutPoint,
     Transaction, TxIn, TxOut, Txid,
@@ -22,13 +22,42 @@ pub fn get_funded_wallet(descriptor: &str, change_descriptor: &str) -> (Wallet,
 }
 
 fn new_funded_wallet(descriptor: &str, change_descriptor: Option<&str>) -> (Wallet, Txid) {
+    let (mut wallet, txid, update) = new_wallet_and_funding_update(descriptor, change_descriptor);
+    wallet.apply_update(update).unwrap();
+    (wallet, txid)
+}
+
+/// Return a fake wallet that appears to be funded for testing.
+///
+/// The funded wallet contains a tx with a 76_000 sats input and two outputs, one spending 25_000
+/// to a foreign address and one returning 50_000 back to the wallet. The remaining 1000
+/// sats are the transaction fee.
+pub fn get_funded_wallet_single(descriptor: &str) -> (Wallet, Txid) {
+    new_funded_wallet(descriptor, None)
+}
+
+/// Get funded segwit wallet
+pub fn get_funded_wallet_wpkh() -> (Wallet, Txid) {
+    let (desc, change_desc) = get_test_wpkh_and_change_desc();
+    get_funded_wallet(desc, change_desc)
+}
+
+/// Get unfunded wallet and wallet update that funds it
+///
+/// The funding update contains a tx with a 76_000 sats input and two outputs, one spending
+/// 25_000 to a foreign address and one returning 50_000 back to the wallet as
+/// change. The remaining 1000 sats are the transaction fee.
+pub fn new_wallet_and_funding_update(
+    descriptor: &str,
+    change_descriptor: Option<&str>,
+) -> (Wallet, Txid, Update) {
     let params = if let Some(change_desc) = change_descriptor {
         Wallet::create(descriptor.to_string(), change_desc.to_string())
     } else {
         Wallet::create_single(descriptor.to_string())
     };
 
-    let mut wallet = params
+    let wallet = params
         .network(Network::Regtest)
         .create_wallet_no_persist()
         .expect("descriptors must be valid");
@@ -39,6 +68,8 @@ fn new_funded_wallet(descriptor: &str, change_descriptor: Option<&str>) -> (Wall
         .require_network(Network::Regtest)
         .unwrap();
 
+    let mut update = Update::default();
+
     let tx0 = Transaction {
         output: vec![TxOut {
             value: Amount::from_sat(76_000),
@@ -67,71 +98,37 @@ fn new_funded_wallet(descriptor: &str, change_descriptor: Option<&str>) -> (Wall
         ],
         ..new_tx(0)
     };
+    let txid1 = tx1.compute_txid();
 
-    insert_checkpoint(
-        &mut wallet,
-        BlockId {
-            height: 42,
-            hash: BlockHash::all_zeros(),
-        },
-    );
-    insert_checkpoint(
-        &mut wallet,
-        BlockId {
-            height: 1_000,
-            hash: BlockHash::all_zeros(),
-        },
-    );
-    insert_checkpoint(
-        &mut wallet,
-        BlockId {
-            height: 2_000,
-            hash: BlockHash::all_zeros(),
-        },
-    );
-
-    insert_tx(&mut wallet, tx0.clone());
-    insert_anchor(
-        &mut wallet,
-        tx0.compute_txid(),
-        ConfirmationBlockTime {
-            block_id: BlockId {
-                height: 1_000,
-                hash: BlockHash::all_zeros(),
-            },
-            confirmation_time: 100,
-        },
-    );
-
-    insert_tx(&mut wallet, tx1.clone());
-    insert_anchor(
-        &mut wallet,
-        tx1.compute_txid(),
-        ConfirmationBlockTime {
-            block_id: BlockId {
-                height: 2_000,
-                hash: BlockHash::all_zeros(),
-            },
-            confirmation_time: 200,
-        },
-    );
-
-    (wallet, tx1.compute_txid())
-}
-
-/// Return a fake wallet that appears to be funded for testing.
-///
-/// The funded wallet contains a tx with a 76_000 sats input and two outputs, one spending 25_000
-/// to a foreign address and one returning 50_000 back to the wallet. The remaining 1000
-/// sats are the transaction fee.
-pub fn get_funded_wallet_single(descriptor: &str) -> (Wallet, Txid) {
-    new_funded_wallet(descriptor, None)
-}
+    let b0 = BlockId {
+        height: 0,
+        hash: BlockHash::from_slice(wallet.network().chain_hash().as_bytes()).unwrap(),
+    };
+    let b1 = BlockId {
+        height: 42,
+        hash: BlockHash::all_zeros(),
+    };
+    let b2 = BlockId {
+        height: 1000,
+        hash: BlockHash::all_zeros(),
+    };
+    let a2 = ConfirmationBlockTime {
+        block_id: b2,
+        confirmation_time: 100,
+    };
+    let b3 = BlockId {
+        height: 2000,
+        hash: BlockHash::all_zeros(),
+    };
+    let a3 = ConfirmationBlockTime {
+        block_id: b3,
+        confirmation_time: 200,
+    };
+    update.chain = CheckPoint::from_block_ids([b0, b1, b2, b3]).ok();
+    update.tx_update.anchors = [(a2, tx0.compute_txid()), (a3, tx1.compute_txid())].into();
+    update.tx_update.txs = [Arc::new(tx0), Arc::new(tx1)].into();
 
-/// Get funded segwit wallet
-pub fn get_funded_wallet_wpkh() -> (Wallet, Txid) {
-    let (desc, change_desc) = get_test_wpkh_and_change_desc();
-    get_funded_wallet(desc, change_desc)
+    (wallet, txid1, update)
 }
 
 /// `pkh` single key descriptor

wallet/src/wallet/changeset.rs

@@ -11,7 +11,7 @@ type IndexedTxGraphChangeSet =
 ///
 /// ## Definition
 ///
-/// The change set is responsible for transmiting data between the persistent storage layer and the
+/// The change set is responsible for transmitting data between the persistent storage layer and the
 /// core library components. Specifically, it serves two primary functions:
 ///
 /// 1) Recording incremental changes to the in-memory representation that need to be persisted to
@@ -46,7 +46,7 @@ type IndexedTxGraphChangeSet =
 /// at any point thereafter.
 ///
 /// Other fields of the change set are not required to be non-empty, that is they may be empty even
-/// in the aggregate. However in practice they should contain the data needed to recover a wallet
+/// in the aggregate. However, in practice they should contain the data needed to recover a wallet
 /// state between sessions. These include:
 /// * [`tx_graph`](Self::tx_graph)
 /// * [`indexer`](Self::indexer)