Skip to content

Commit 826bb68

Browse files
committed
docs: improve wallet event docs and tests
per suggestions from ValuedMammal: 1. re-export WalletEvent type 2. add comments to wallet_events function 3. rename ambiguous variable names in wallet_events from cp to pos 4. remove signing from wallet_event tests 5. change wallet_events function assert_eq to debug_asset_eq 6. add option 4 to ADR 0003 re: creating events only from Update
1 parent 9e766dd commit 826bb68

File tree

4 files changed

+32
-17
lines changed

4 files changed

+32
-17
lines changed

docs/adr/0003_events.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,21 @@ return the list of `WalletEvent` enums.
8989
* Could be confusing to users which function to use, the original or new one.
9090
* If in a future breaking release we decide to always return events we'll need to deprecate `Wallet::apply_update_events`.
9191

92+
#### Option 4: Create events directly from Wallet::Update
93+
94+
The `wallet::Update` structure passed into the `Wallet::apply_update` function contains any new transaction or
95+
blockchain data found in a `FullScanResponse` or `SyncResponse`. Events could be generated from only this data.
96+
97+
**Pros:**
98+
99+
* No further wallet lookups is required to create events, it would be more efficient.
100+
* Could be implemented as a function directly on the `wallet::Update` structure, a non-breaking API change.
101+
102+
**Cons:**
103+
104+
* A `wallet::Update` only contains the blocks, tx, and anchors found during a sync or full scan. It does not show how
105+
this data changes the canonical status of already known blocks and tx.
106+
92107
## Decision Outcome
93108

94109
Chosen option: "Option 3", because it can be delivered to users in the next minor release. This option also lets us

wallet/src/wallet/event.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ pub enum WalletEvent {
8282
},
8383
}
8484

85+
/// Generate events by comparing the chain tip and wallet transactions before and after applying
86+
/// `wallet::Update` to `Wallet`. Any changes are added to the list of returned `WalletEvent`s.
8587
pub(crate) fn wallet_events(
8688
wallet: &mut Wallet,
8789
chain_tip1: BlockId,
@@ -91,17 +93,19 @@ pub(crate) fn wallet_events(
9193
) -> Vec<WalletEvent> {
9294
let mut events: Vec<WalletEvent> = Vec::new();
9395

96+
// find chain tip change
9497
if chain_tip1 != chain_tip2 {
9598
events.push(WalletEvent::ChainTipChanged {
9699
old_tip: chain_tip1,
97100
new_tip: chain_tip2,
98101
});
99102
}
100103

101-
wallet_txs2.iter().for_each(|(txid2, (tx2, cp2))| {
102-
if let Some((tx1, cp1)) = wallet_txs1.get(txid2) {
103-
assert_eq!(tx1.compute_txid(), *txid2);
104-
match (cp1, cp2) {
104+
// find transaction canonical status changes
105+
wallet_txs2.iter().for_each(|(txid2, (tx2, pos2))| {
106+
if let Some((tx1, pos1)) = wallet_txs1.get(txid2) {
107+
debug_assert_eq!(tx1.compute_txid(), *txid2);
108+
match (pos1, pos2) {
105109
(Unconfirmed { .. }, Confirmed { anchor, .. }) => {
106110
events.push(WalletEvent::TxConfirmed {
107111
txid: *txid2,
@@ -139,7 +143,7 @@ pub(crate) fn wallet_events(
139143
}
140144
}
141145
} else {
142-
match cp2 {
146+
match pos2 {
143147
Confirmed { anchor, .. } => {
144148
events.push(WalletEvent::TxConfirmed {
145149
txid: *txid2,

wallet/src/wallet/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,12 @@ use crate::wallet::{
7575
tx_builder::{FeePolicy, TxBuilder, TxParams},
7676
utils::{check_nsequence_rbf, After, Older, SecpCtx},
7777
};
78+
use event::wallet_events;
7879

7980
// re-exports
80-
use crate::event::{wallet_events, WalletEvent};
8181
pub use bdk_chain::Balance;
8282
pub use changeset::ChangeSet;
83+
pub use event::WalletEvent;
8384
pub use params::*;
8485
pub use persisted::*;
8586
pub use utils::IsDust;

wallet/tests/wallet_event.rs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use bdk_chain::{BlockId, CheckPoint, ConfirmationBlockTime};
22
use bdk_wallet::event::WalletEvent;
33
use bdk_wallet::test_utils::{get_test_wpkh_and_change_desc, new_wallet_and_funding_update};
4-
use bdk_wallet::{SignOptions, Update};
4+
use bdk_wallet::Update;
55
use bitcoin::hashes::Hash;
66
use bitcoin::{Address, Amount, BlockHash, FeeRate};
77
use core::str::FromStr;
@@ -76,8 +76,7 @@ fn test_tx_replaced_event() {
7676
.assume_checked(),
7777
Amount::from_sat(10_000),
7878
);
79-
let mut psbt = builder.finish().unwrap();
80-
wallet.sign(&mut psbt, SignOptions::default()).unwrap();
79+
let psbt = builder.finish().unwrap();
8180
let orig_tx = Arc::new(psbt.extract_tx().unwrap());
8281
let orig_txid = orig_tx.compute_txid();
8382

@@ -95,8 +94,7 @@ fn test_tx_replaced_event() {
9594
// create rbf tx
9695
let mut builder = wallet.build_fee_bump(orig_txid).unwrap();
9796
builder.fee_rate(FeeRate::from_sat_per_vb(10).unwrap());
98-
let mut psbt = builder.finish().unwrap();
99-
wallet.sign(&mut psbt, SignOptions::default()).unwrap();
97+
let psbt = builder.finish().unwrap();
10098
let rbf_tx = Arc::new(psbt.extract_tx().unwrap());
10199
let rbf_txid = rbf_tx.compute_txid();
102100

@@ -131,8 +129,7 @@ fn test_tx_confirmed_event() {
131129
.assume_checked(),
132130
Amount::from_sat(10_000),
133131
);
134-
let mut psbt = builder.finish().unwrap();
135-
wallet.sign(&mut psbt, SignOptions::default()).unwrap();
132+
let psbt = builder.finish().unwrap();
136133
let new_tx = Arc::new(psbt.extract_tx().unwrap());
137134
let new_txid = new_tx.compute_txid();
138135

@@ -189,8 +186,7 @@ fn test_tx_confirmed_new_block_event() {
189186
.assume_checked(),
190187
Amount::from_sat(10_000),
191188
);
192-
let mut psbt = builder.finish().unwrap();
193-
wallet.sign(&mut psbt, SignOptions::default()).unwrap();
189+
let psbt = builder.finish().unwrap();
194190
let new_tx = Arc::new(psbt.extract_tx().unwrap());
195191
let new_txid = new_tx.compute_txid();
196192

@@ -274,8 +270,7 @@ fn test_tx_dropped_event() {
274270
.assume_checked(),
275271
Amount::from_sat(10_000),
276272
);
277-
let mut psbt = builder.finish().unwrap();
278-
wallet.sign(&mut psbt, SignOptions::default()).unwrap();
273+
let psbt = builder.finish().unwrap();
279274
let new_tx = Arc::new(psbt.extract_tx().unwrap());
280275
let new_txid = new_tx.compute_txid();
281276

0 commit comments

Comments
 (0)