Skip to content

Commit 6fc65fa

Browse files
tankyleotankyleo
committed
Validate submitpackage behavior for v29+
Require Electrum and Esplora zero-fee commitments validation to observe the Bitcoin Core v29+ failure shape for the dummy TRUC package, instead of accepting any structured submitpackage response. Set the locktime field of the transaction to zero so that the test works at any chain-height, which is particularly helpful when starting ldk-node against test networks. Map HTTP 400 errors returned to `ChainSourceNotSupported` as this error code is returned by blockstream-electrs and mempool-electrs when running against Bitcoin Core v26. We previously would map this error to a general `ConnectionFailed` error, which is not correct for Bitcoin Core v26. Co-Authored-By: HAL 9000
1 parent 052303a commit 6fc65fa

3 files changed

Lines changed: 87 additions & 33 deletions

File tree

‎src/chain/electrum.rs‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ use bdk_wallet::{KeychainKind as BdkKeyChainKind, Update as BdkUpdate};
1919
use bitcoin::transaction::Version;
2020
use bitcoin::{FeeRate, Network, Script, ScriptBuf, Transaction, Txid};
2121
use electrum_client::{
22-
Batch, Client as ElectrumClient, ConfigBuilder as ElectrumConfigBuilder, ElectrumApi,
22+
Batch, BroadcastPackageRes, Client as ElectrumClient, ConfigBuilder as ElectrumConfigBuilder,
23+
ElectrumApi,
2324
};
2425
use lightning::chain::{Confirm, Filter, WatchedOutput};
2526
use lightning::util::ser::Writeable;
@@ -321,11 +322,6 @@ impl ElectrumChainSource {
321322
return Err(Error::ConnectionFailed);
322323
};
323324

324-
// TODO: Use `protocol_version` API once shipped in
325-
// https://github.com/bitcoindevkit/rust-electrum-client/pull/213.
326-
//
327-
// This could still accept an Electrum server running against Bitcoin Core v26
328-
// through v28, which does not relay ephemeral dust.
329325
let spawn_fut = electrum_client.runtime.spawn_blocking({
330326
let electrum_client = Arc::clone(&electrum_client.electrum_client);
331327
move || electrum_client.transaction_broadcast_package(&super::dummy_package())
@@ -336,11 +332,19 @@ impl ElectrumChainSource {
336332
);
337333

338334
match timeout_fut.await {
339-
Ok(Ok(Ok(_))) => Ok(()),
340-
Ok(Ok(Err(
341-
e @ (electrum_client::Error::Protocol(_)
342-
| electrum_client::Error::AllAttemptsErrored(_)),
343-
))) => {
335+
Ok(Ok(Ok(result))) => {
336+
if dummy_submit_package_result_matches_v29_or_later(&result) {
337+
Ok(())
338+
} else {
339+
log_error!(
340+
self.logger,
341+
"Electrum server does not support submitpackage: {:?}",
342+
result
343+
);
344+
Err(Error::ChainSourceNotSupported)
345+
}
346+
},
347+
Ok(Ok(Err(e))) if electrum_submitpackage_error_implies_unsupported(&e) => {
344348
log_error!(self.logger, "Electrum server does not support submitpackage: {:?}", e);
345349
Err(Error::ChainSourceNotSupported)
346350
},
@@ -377,6 +381,22 @@ impl ElectrumChainSource {
377381
}
378382
}
379383

384+
fn electrum_submitpackage_error_implies_unsupported(e: &electrum_client::Error) -> bool {
385+
matches!(e, electrum_client::Error::Protocol(_) | electrum_client::Error::AllAttemptsErrored(_))
386+
}
387+
388+
fn dummy_submit_package_result_matches_v29_or_later(result: &BroadcastPackageRes) -> bool {
389+
if result.success {
390+
return false;
391+
}
392+
393+
super::dummy_package_txids().iter().all(|expected_txid| {
394+
result.errors.iter().any(|error| {
395+
&error.txid == expected_txid && error.error == super::DUMMY_PACKAGE_EXPECTED_ERROR
396+
})
397+
})
398+
}
399+
380400
impl Filter for ElectrumChainSource {
381401
fn register_tx(&self, txid: &Txid, script_pubkey: &Script) {
382402
self.electrum_runtime_status.write().expect("lock").register_tx(txid, script_pubkey)

‎src/chain/esplora.rs‎

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
1313
use bdk_esplora::EsploraAsyncExt;
1414
use bitcoin::transaction::Version;
1515
use bitcoin::{FeeRate, Network, Script, Txid};
16-
use esplora_client::AsyncClient as EsploraAsyncClient;
16+
use esplora_client::{AsyncClient as EsploraAsyncClient, SubmitPackageResult};
1717
use lightning::chain::{Confirm, Filter, WatchedOutput};
1818
use lightning::util::ser::Writeable;
1919
use lightning_transaction_sync::EsploraSyncClient;
@@ -86,16 +86,13 @@ impl EsploraChainSource {
8686
}
8787

8888
pub(super) async fn validate_zero_fee_commitments_support(&self) -> Result<(), Error> {
89-
// This could still accept an Esplora server running against Bitcoin Core v26
90-
// through v28, which does not relay ephemeral dust.
91-
self.esplora_client.submit_package(&super::dummy_package(), None, None).await.map_err(
92-
|e| {
93-
if let esplora_client::Error::HttpResponse { status: 404, message } = e {
94-
log_error!(
95-
self.logger,
96-
"Esplora server does not support submitpackage: {}",
97-
message
98-
);
89+
let result = self
90+
.esplora_client
91+
.submit_package(&super::dummy_package(), None, None)
92+
.await
93+
.map_err(|e| {
94+
if esplora_submitpackage_error_implies_unsupported(&e) {
95+
log_error!(self.logger, "Esplora server does not support submitpackage: {}", e);
9996
Error::ChainSourceNotSupported
10097
} else {
10198
log_error!(
@@ -105,8 +102,11 @@ impl EsploraChainSource {
105102
);
106103
Error::ConnectionFailed
107104
}
108-
},
109-
)?;
105+
})?;
106+
if !dummy_submit_package_result_matches_v29_or_later(&result) {
107+
log_error!(self.logger, "Esplora server does not support submitpackage: {:?}", result);
108+
return Err(Error::ChainSourceNotSupported);
109+
}
110110
Ok(())
111111
}
112112

@@ -521,6 +521,23 @@ impl EsploraChainSource {
521521
}
522522
}
523523

524+
fn esplora_submitpackage_error_implies_unsupported(e: &esplora_client::Error) -> bool {
525+
matches!(e, esplora_client::Error::HttpResponse { status: 400 | 404, .. })
526+
}
527+
528+
fn dummy_submit_package_result_matches_v29_or_later(result: &SubmitPackageResult) -> bool {
529+
if result.package_msg != "transaction failed" {
530+
return false;
531+
}
532+
533+
super::dummy_package_txids().iter().all(|expected_txid| {
534+
result.tx_results.values().any(|tx_result| {
535+
&tx_result.txid == expected_txid
536+
&& tx_result.error.as_deref() == Some(super::DUMMY_PACKAGE_EXPECTED_ERROR)
537+
})
538+
})
539+
}
540+
524541
impl Filter for EsploraChainSource {
525542
fn register_tx(&self, txid: &Txid, script_pubkey: &Script) {
526543
self.tx_sync.register_tx(txid, script_pubkey);

‎src/chain/mod.rs‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,20 @@ use crate::{Error, PersistedNodeMetrics};
3131

3232
/// We use this parent-child TRUC package to make sure the configured chain source supports
3333
/// broadcasting packages via the `submitpackage` Bitcoin Core RPC.
34-
const PARENT_TXID: &str = "9a015f93fac6cb203c2b994e18b85176eb0354a22a468255516f3c6002d3f696";
34+
const DUMMY_PACKAGE_EXPECTED_ERROR: &str = "bad-txns-inputs-missingorspent";
35+
const PARENT_TXID: &str = "11105ba7c94f2fdc1b870a9fbdb136ca006bcf598c372548a1a3051001f3238f";
3536
const PARENT_HEX: &str =
36-
"0300000000010160d0cdb72f2ddf719f40ca32f44614c67577fc75996140544003915683c34a310000000000fd\
37+
"0300000000010160d0cdb72f2ddf719f40ca32f44614c67577fc75996140544003915683c34a310000000000ff\
3738
ffffff0201000000000000000451024e73876100000000000022512042731375894dad3b25092cd0f713dc5bee4\
3839
a71e30a95e1db3d880906d7eba1fa01409327942924218e4eb1635a7cce6706fcb37b8bbb61a2f0b86357356681\
39-
4e09419a3501e02252043bb237d479304632282fe9159db9e9a6ae6ec5bedea9f0f115a97b0e00";
40-
const CHILD_TXID: &str = "d011b3ff78cdfb8b93822639ea87771847936b04bb83afc8763a7c02a386ae26";
40+
4e09419a3501e02252043bb237d479304632282fe9159db9e9a6ae6ec5bedea9f0f11500000000";
41+
const CHILD_TXID: &str = "6a051464dbf0534a060a61355a6a971559de004795328f8dfd19f3197f9bb4b0";
4142
const CHILD_HEX: &str =
42-
"0300000000010296f6d302603c6f515582462aa25403eb7651b8184e992b3c20cbc6fa935f019a0000000000ff\
43-
ffffff96f6d302603c6f515582462aa25403eb7651b8184e992b3c20cbc6fa935f019a0100000000fdffffff015\
43+
"030000000001028f23f3011005a3a14825378c59cf6b00ca36b1bd9f0a871bdc2f4fc9a75b10110000000000ff\
44+
ffffff8f23f3011005a3a14825378c59cf6b00ca36b1bd9f0a871bdc2f4fc9a75b10110100000000ffffffff015\
4445
660000000000000225120ac18cd599a1be003595854e2eeec18dbe1c92d04b0ba05812d04445e3fcf16bc000140\
4546
1462a35808d77a164f0a23a84c4721d1545befd09ad19945bb8aa0ea5576953a9699038725f944b1bc429942ef4\
46-
7e6504a554babf022cb15db53be2d8c1dbfe5a97b0e00";
47+
7e6504a554babf022cb15db53be2d8c1dbfe500000000";
4748

4849
fn dummy_package() -> [bitcoin::Transaction; 2] {
4950
use bitcoin::consensus::Decodable;
@@ -55,11 +56,27 @@ fn dummy_package() -> [bitcoin::Transaction; 2] {
5556
Transaction::consensus_decode(&mut &parent_tx_bytes[..]).expect("read from a constant");
5657
let child =
5758
Transaction::consensus_decode(&mut &child_tx_bytes[..]).expect("read from a constant");
58-
assert_eq!(parent.compute_txid().to_string(), PARENT_TXID);
59-
assert_eq!(child.compute_txid().to_string(), CHILD_TXID);
59+
let [parent_txid, child_txid] = dummy_package_txids();
60+
assert_eq!(parent.compute_txid(), parent_txid);
61+
assert_eq!(child.compute_txid(), child_txid);
62+
assert_eq!(parent.lock_time, bitcoin::absolute::LockTime::ZERO);
63+
assert_eq!(child.lock_time, bitcoin::absolute::LockTime::ZERO);
64+
assert!(parent
65+
.input
66+
.iter()
67+
.chain(child.input.iter())
68+
.all(|input| input.sequence == bitcoin::Sequence::MAX));
69+
assert!(child.input.iter().all(|input| input.previous_output.txid == parent.compute_txid()));
6070
[parent, child]
6171
}
6272

73+
fn dummy_package_txids() -> [Txid; 2] {
74+
[
75+
PARENT_TXID.parse().expect("read from a constant"),
76+
CHILD_TXID.parse().expect("read from a constant"),
77+
]
78+
}
79+
6380
pub(crate) enum WalletSyncStatus {
6481
Completed,
6582
InProgress { subscribers: tokio::sync::broadcast::Sender<Result<(), Error>> },

0 commit comments

Comments
 (0)