Skip to content

Commit 86efbb8

Browse files
feat(electrum): Handle spks with expected txids
Co-authored-by: Wei Chen <[email protected]>
1 parent b5b442a commit 86efbb8

File tree

2 files changed

+161
-13
lines changed

2 files changed

+161
-13
lines changed

crates/electrum/src/bdk_electrum_client.rs

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
use bdk_core::{
22
bitcoin::{block::Header, BlockHash, OutPoint, ScriptBuf, Transaction, Txid},
3-
collections::{BTreeMap, HashMap},
4-
spk_client::{FullScanRequest, FullScanResponse, SyncRequest, SyncResponse},
3+
collections::{BTreeMap, HashMap, HashSet},
4+
spk_client::{
5+
FullScanRequest, FullScanResponse, SpkWithExpectedTxids, SyncRequest, SyncResponse,
6+
},
57
BlockId, CheckPoint, ConfirmationBlockTime, TxUpdate,
68
};
79
use electrum_client::{ElectrumApi, Error, HeaderNotification};
8-
use std::{
9-
collections::HashSet,
10-
sync::{Arc, Mutex},
11-
};
10+
use std::sync::{Arc, Mutex};
1211

1312
/// We include a chain suffix of a certain length for the purpose of robustness.
1413
const CHAIN_SUFFIX_LENGTH: u32 = 8;
@@ -138,7 +137,9 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
138137
let mut tx_update = TxUpdate::<ConfirmationBlockTime>::default();
139138
let mut last_active_indices = BTreeMap::<K, u32>::default();
140139
for keychain in request.keychains() {
141-
let spks = request.iter_spks(keychain.clone());
140+
let spks = request
141+
.iter_spks(keychain.clone())
142+
.map(|(spk_i, spk)| (spk_i, SpkWithExpectedTxids::from(spk)));
142143
if let Some(last_active_index) =
143144
self.populate_with_spks(start_time, &mut tx_update, spks, stop_gap, batch_size)?
144145
{
@@ -209,7 +210,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
209210
start_time,
210211
&mut tx_update,
211212
request
212-
.iter_spks()
213+
.iter_spks_with_expected_txids()
213214
.enumerate()
214215
.map(|(i, spk)| (i as u32, spk)),
215216
usize::MAX,
@@ -247,7 +248,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
247248
&self,
248249
start_time: u64,
249250
tx_update: &mut TxUpdate<ConfirmationBlockTime>,
250-
mut spks: impl Iterator<Item = (u32, ScriptBuf)>,
251+
mut spks_with_expected_txids: impl Iterator<Item = (u32, SpkWithExpectedTxids)>,
251252
stop_gap: usize,
252253
batch_size: usize,
253254
) -> Result<Option<u32>, Error> {
@@ -256,17 +257,30 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
256257

257258
loop {
258259
let spks = (0..batch_size)
259-
.map_while(|_| spks.next())
260+
.map_while(|_| spks_with_expected_txids.next())
260261
.collect::<Vec<_>>();
261262
if spks.is_empty() {
262263
return Ok(last_active_index);
263264
}
264265

266+
let (spks, expected_spk_txids): (Vec<(u32, ScriptBuf)>, HashMap<ScriptBuf, _>) = spks
267+
.iter()
268+
.map(|(spk_i, spk_with_expected_txids)| {
269+
(
270+
(*spk_i, spk_with_expected_txids.spk.clone()),
271+
(
272+
spk_with_expected_txids.spk.clone(),
273+
spk_with_expected_txids.expected_txids.clone(),
274+
),
275+
)
276+
})
277+
.unzip();
278+
265279
let spk_histories = self
266280
.inner
267281
.batch_script_get_history(spks.iter().map(|(_, s)| s.as_script()))?;
268282

269-
for ((spk_index, _spk), spk_history) in spks.into_iter().zip(spk_histories) {
283+
for ((spk_index, spk), spk_history) in spks.into_iter().zip(spk_histories) {
270284
if spk_history.is_empty() {
271285
unused_spk_count = unused_spk_count.saturating_add(1);
272286
if unused_spk_count >= stop_gap {
@@ -277,6 +291,19 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
277291
unused_spk_count = 0;
278292
}
279293

294+
let spk_history_set = spk_history
295+
.iter()
296+
.map(|res| res.tx_hash)
297+
.collect::<HashSet<_>>();
298+
299+
if let Some(expected_txids) = expected_spk_txids.get(&spk) {
300+
tx_update.evicted_ats.extend(
301+
expected_txids
302+
.difference(&spk_history_set)
303+
.map(|&txid| (txid, start_time)),
304+
);
305+
}
306+
280307
for tx_res in spk_history {
281308
tx_update.txs.push(self.fetch_tx(tx_res.tx_hash)?);
282309
match tx_res.height.try_into() {

crates/electrum/tests/test_electrum.rs

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
use bdk_chain::{
22
bitcoin::{hashes::Hash, Address, Amount, ScriptBuf, WScriptHash},
3+
keychain_txout::KeychainTxOutIndex,
34
local_chain::LocalChain,
5+
miniscript::Descriptor,
46
spk_client::{FullScanRequest, SyncRequest, SyncResponse},
57
spk_txout::SpkTxOutIndex,
68
Balance, ConfirmationBlockTime, IndexedTxGraph, Indexer, Merge, TxGraph,
79
};
8-
use bdk_core::bitcoin::Network;
10+
use bdk_core::bitcoin::{key::Secp256k1, Network};
911
use bdk_electrum::BdkElectrumClient;
1012
use bdk_testenv::{
1113
anyhow,
@@ -14,7 +16,7 @@ use bdk_testenv::{
1416
};
1517
use core::time::Duration;
1618
use electrum_client::ElectrumApi;
17-
use std::collections::{BTreeSet, HashSet};
19+
use std::collections::{BTreeSet, HashMap, HashSet};
1820
use std::str::FromStr;
1921

2022
// Batch size for `sync_with_electrum`.
@@ -60,6 +62,125 @@ where
6062
Ok(update)
6163
}
6264

65+
// Ensure that a wallet can detect a malicious replacement of an incoming transaction.
66+
//
67+
// This checks that both the Electrum chain source and the receiving structures properly track the
68+
// replaced transaction as missing.
69+
#[test]
70+
pub fn detect_receive_tx_cancel() -> anyhow::Result<()> {
71+
const SEND_TX_FEE: Amount = Amount::from_sat(1000);
72+
const UNDO_SEND_TX_FEE: Amount = Amount::from_sat(2000);
73+
74+
use bdk_chain::keychain_txout::SyncRequestBuilderExt;
75+
let env = TestEnv::new()?;
76+
let rpc_client = env.rpc_client();
77+
let electrum_client = electrum_client::Client::new(env.electrsd.electrum_url.as_str())?;
78+
let client = BdkElectrumClient::new(electrum_client);
79+
80+
let (receiver_desc, _) = Descriptor::parse_descriptor(&Secp256k1::signing_only(), "tr([73c5da0a/86'/0'/0']xprv9xgqHN7yz9MwCkxsBPN5qetuNdQSUttZNKw1dcYTV4mkaAFiBVGQziHs3NRSWMkCzvgjEe3n9xV8oYywvM8at9yRqyaZVz6TYYhX98VjsUk/0/*)")
81+
.expect("must be valid");
82+
let mut graph = IndexedTxGraph::<ConfirmationBlockTime, _>::new(KeychainTxOutIndex::new(0));
83+
let _ = graph.index.insert_descriptor((), receiver_desc.clone())?;
84+
let (chain, _) = LocalChain::from_genesis_hash(env.bitcoind.client.get_block_hash(0)?);
85+
86+
// Derive the receiving address from the descriptor.
87+
let ((_, receiver_spk), _) = graph.index.reveal_next_spk(()).unwrap();
88+
let receiver_addr = Address::from_script(&receiver_spk, bdk_chain::bitcoin::Network::Regtest)?;
89+
90+
env.mine_blocks(101, None)?;
91+
92+
// Select a UTXO to use as an input for constructing our test transactions.
93+
let selected_utxo = rpc_client
94+
.list_unspent(None, None, None, Some(false), None)?
95+
.into_iter()
96+
// Find a block reward tx.
97+
.find(|utxo| utxo.amount == Amount::from_int_btc(50))
98+
.expect("Must find a block reward UTXO");
99+
100+
// Derive the sender's address from the selected UTXO.
101+
let sender_spk = selected_utxo.script_pub_key.clone();
102+
let sender_addr = Address::from_script(&sender_spk, bdk_chain::bitcoin::Network::Regtest)
103+
.expect("Failed to derive address from UTXO");
104+
105+
// Setup the common inputs used by both `send_tx` and `undo_send_tx`.
106+
let inputs = [CreateRawTransactionInput {
107+
txid: selected_utxo.txid,
108+
vout: selected_utxo.vout,
109+
sequence: None,
110+
}];
111+
112+
// Create and sign the `send_tx` that sends funds to the receiver address.
113+
let send_tx_outputs = HashMap::from([(
114+
receiver_addr.to_string(),
115+
selected_utxo.amount - SEND_TX_FEE,
116+
)]);
117+
let send_tx = rpc_client.create_raw_transaction(&inputs, &send_tx_outputs, None, Some(true))?;
118+
let send_tx = rpc_client
119+
.sign_raw_transaction_with_wallet(send_tx.raw_hex(), None, None)?
120+
.transaction()?;
121+
122+
// Create and sign the `undo_send_tx` transaction. This redirects funds back to the sender
123+
// address.
124+
let undo_send_outputs = HashMap::from([(
125+
sender_addr.to_string(),
126+
selected_utxo.amount - UNDO_SEND_TX_FEE,
127+
)]);
128+
let undo_send_tx =
129+
rpc_client.create_raw_transaction(&inputs, &undo_send_outputs, None, Some(true))?;
130+
let undo_send_tx = rpc_client
131+
.sign_raw_transaction_with_wallet(undo_send_tx.raw_hex(), None, None)?
132+
.transaction()?;
133+
134+
// Sync after broadcasting the `send_tx`. Ensure that we detect and receive the `send_tx`.
135+
let send_txid = env.rpc_client().send_raw_transaction(send_tx.raw_hex())?;
136+
env.wait_until_electrum_sees_txid(send_txid, Duration::from_secs(6))?;
137+
let sync_request = SyncRequest::builder()
138+
.chain_tip(chain.tip())
139+
.revealed_spks_from_indexer(&graph.index, ..)
140+
.expected_spk_txids(graph.list_expected_spk_txids(&chain, chain.tip().block_id(), ..));
141+
let sync_response = client.sync(sync_request, BATCH_SIZE, true)?;
142+
assert!(
143+
sync_response
144+
.tx_update
145+
.txs
146+
.iter()
147+
.any(|tx| tx.compute_txid() == send_txid),
148+
"sync response must include the send_tx"
149+
);
150+
let changeset = graph.apply_update(sync_response.tx_update.clone());
151+
assert!(
152+
changeset.tx_graph.txs.contains(&send_tx),
153+
"tx graph must deem send_tx relevant and include it"
154+
);
155+
156+
// Sync after broadcasting the `undo_send_tx`. Verify that `send_tx` is now missing from the
157+
// mempool.
158+
let undo_send_txid = env
159+
.rpc_client()
160+
.send_raw_transaction(undo_send_tx.raw_hex())?;
161+
env.wait_until_electrum_sees_txid(undo_send_txid, Duration::from_secs(6))?;
162+
let sync_request = SyncRequest::builder()
163+
.chain_tip(chain.tip())
164+
.revealed_spks_from_indexer(&graph.index, ..)
165+
.expected_spk_txids(graph.list_expected_spk_txids(&chain, chain.tip().block_id(), ..));
166+
let sync_response = client.sync(sync_request, BATCH_SIZE, true)?;
167+
assert!(
168+
sync_response
169+
.tx_update
170+
.evicted_ats
171+
.iter()
172+
.any(|(txid, _)| *txid == send_txid),
173+
"sync response must track send_tx as missing from mempool"
174+
);
175+
let changeset = graph.apply_update(sync_response.tx_update.clone());
176+
assert!(
177+
changeset.tx_graph.last_evicted.contains_key(&send_txid),
178+
"tx graph must track send_tx as missing"
179+
);
180+
181+
Ok(())
182+
}
183+
63184
/// If an spk history contains a tx that spends another unconfirmed tx (chained mempool history),
64185
/// the Electrum API will return the tx with a negative height. This should succeed and not panic.
65186
#[test]

0 commit comments

Comments
 (0)