Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions src/wallet/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1551,7 +1551,54 @@ impl Wallet {
let txid = tx.compute_txid();
let (amount_msat, fee_paid_msat, direction) = self.onchain_payment_fields(tx);

// A funding transaction that moves no wallet funds carries nothing to record — e.g. LDK
// re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding path,
// including splices the interactive-funding classification deliberately declined (no
// local contribution, or a splice-out moving no wallet funds). Recording it here would
// mint a zero-amount payment that nothing ever confirms. Skip on the wallet-derived
// amount alone — the condition `classify_interactive_funding` declines on; anything
// declined there must be skipped here, or its re-broadcast resurrects the record. The fee
// is no participation signal: the wallet resolves a splice's shared input whenever the
// previous funding transaction touched it (e.g. it funded the original channel open).
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at
// all. `zero_conf_splice_out_funding_rebroadcast_canary` pins the current behavior by
// asserting the log line below; when it fails against a newer LDK, re-evaluate whether
// this skip still sees traffic.
if amount_msat == Some(0) {
log_trace!(
self.logger,
"Not recording channel-funding broadcast {} as a payment: no wallet-level activity",
txid,
);
return Ok(());
}

let payment_id = PaymentId(txid.to_byte_array());

// A promoted-but-unconfirmed 0conf splice comes back through this generic path re-typed
// and carrying wallet-view figures; `funding_reclassification_update` declines the
// downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can
// observe the traffic. The read cannot go stale: only the broadcast loop writes
// interactive-funding classifications, and it runs this classification too.
if let Some(current) = self.payment_store.get(&payment_id) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, there are now so many edge cases to handle in this code. I increasingly think we need to reconsider the chosen approach (classifying through the broadcaster interface rather than keeping state in LDK and providing an API for it). Reopened https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/3566#issuecomment-440916 and added it to the v0.4 milestone.

if matches!(
current.kind,
PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}
) {
log_trace!(
self.logger,
"Keeping interactive-funding classification over funding-typed rebroadcast {}",
txid,
);
}
}

let details = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
Expand DownExpand Up@@ -2585,6 +2632,28 @@ fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight {
fn funding_reclassification_update(
details: PaymentDetails, candidates: &[FundingTxCandidate], current: Option<&PaymentDetails>,
) -> PaymentDetailsUpdate {
// A funding-typed classification of a record already classified as interactive funding is a
// downgrade, not news: LDK re-broadcasts a promoted-but-unconfirmed splice through its
// generic funding path, where the figures are wallet-view rather than contribution-derived.
// Keep the record as classified; wallet-sync events own its confirmation state.
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at all.
// `zero_conf_splice_in_funding_rebroadcast_canary` pins the current behavior via the
// arrival log in `classify_funding`; when it fails against a newer LDK, re-evaluate
// whether this guard still sees traffic.
if let (
Some(PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}),
PaymentKind::Onchain { tx_type: Some(TransactionType::Funding { .. }), .. },
) = (current.map(|payment| &payment.kind), &details.kind)
{
return PaymentDetailsUpdate::new(details.id);
}

let mut update = PaymentDetailsUpdate::funding_reclassification(details);
if let Some(PaymentKind::Onchain {
txid: confirmed_txid,
Expand DownExpand Up@@ -3689,6 +3758,35 @@ mod tests {
assert_eq!(update.txid, Some(active_txid));
}

/// A funding-typed (re)classification of a record already classified as interactive funding
/// carries nothing the record doesn't have — LDK re-broadcasts a promoted-but-unconfirmed
/// splice through its generic funding path with wallet-view figures — so the update must
/// move nothing.
#[test]
fn funding_reclassification_update_skips_funding_over_interactive_funding() {
let txid = Txid::from_byte_array([1u8; 32]);
let payment_id = PaymentId(txid.to_byte_array());
let current = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));

let rebroadcast = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
txid,
status: ConfirmationStatus::Unconfirmed,
tx_type: Some(TransactionType::Funding { channels: vec![] }),
},
Some(10_000_000),
Some(0),
PaymentDirection::Inbound,
PaymentStatus::Pending,
);

let update = funding_reclassification_update(rebroadcast, &[], Some(&current));
let mut updated = current.clone();
assert!(!updated.update(update), "the rebroadcast must not move the record");
assert_eq!(updated, current);
}

/// Graduation must decide from the live record and write only the status: a pending-store
/// snapshot taken before a concurrent classification landed must not roll the record's
/// figures back when the payment graduates to `Succeeded`.
Expand DownExpand Up@@ -3831,6 +3929,148 @@ mod tests {
assert_eq!(wallet.find_payment_by_txid(txid2), Some(payment_id));
}

/// A funding-typed broadcast that doesn't touch the on-chain wallet must not be recorded.
/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path, so a splice the interactive-funding classification deliberately declined — no local
/// contribution, or none of the moved funds are the wallet's — would otherwise come back as
/// a spurious zero-amount record that nothing ever confirms.
#[tokio::test]
async fn funding_broadcast_without_wallet_activity_is_not_recorded() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

// No inputs or outputs involve the wallet: nothing to record.
wallet.classify_funding(&dummy_tx(), &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());
assert!(wallet.pending_payment_store.list_filter(|_| true).is_empty());

// A computable fee is not wallet participation. The wallet can resolve a splice's shared
// input whenever the previous funding transaction touched it (e.g. it funded the original
// channel open), so it derives the splice's fee even when no wallet funds move.
let prev_funding_outpoint = OutPoint { txid: Txid::from_byte_array([8u8; 32]), vout: 0 };
wallet.inner.lock().unwrap().insert_txout(
prev_funding_outpoint,
TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() },
);
let splice_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: vec![bitcoin::TxIn {
previous_output: prev_funding_outpoint,
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(99_000),
script_pubkey: ScriptBuf::new(),
}],
};
wallet.classify_funding(&splice_tx, &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());

// Control: a funding transaction the wallet participates in is still recorded.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let funded_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
wallet.classify_funding(&funded_tx, &channels, tx_type).await.unwrap();
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1);
assert_eq!(payments[0].id, PaymentId(funded_tx.compute_txid().to_byte_array()));
}

/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path: same txid, but typed as a plain funding transaction with wallet-view figures and no
/// contribution metadata. The rebroadcast must not overwrite the contribution-derived
/// figures or the interactive-funding classification — neither while the record is
/// unconfirmed nor once it confirmed under that same txid, where updates naming the
/// confirmed txid may otherwise move figures.
#[tokio::test]
async fn funding_rebroadcast_keeps_interactive_funding_classification() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

// The rebroadcast passes the wallet-activity guard: a splice-in funds the new channel
// output partly from the wallet, so the wallet sees movement.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
let txid = tx.compute_txid();
let payment_id = PaymentId(txid.to_byte_array());

let candidates = vec![FundingTxCandidate {
txid,
amount_msat: Some(1_000_000),
fee_paid_msat: Some(500),
}];
let details = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));
wallet.persist_funding_payment(details, candidates).await.unwrap();

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

let assert_unchanged = |confirmed: bool| {
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1, "the rebroadcast must not mint a second record");
let payment = &payments[0];
assert_eq!(payment.id, payment_id);
assert_eq!(payment.amount_msat, Some(1_000_000));
assert_eq!(payment.fee_paid_msat, Some(500));
match &payment.kind {
PaymentKind::Onchain {
status,
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
} => assert_eq!(matches!(status, ConfirmationStatus::Confirmed { .. }), confirmed),
kind => panic!("unexpected kind {:?}", kind),
}
};

wallet.classify_funding(&tx, &channels, tx_type.clone()).await.unwrap();
assert_unchanged(false);

// Confirm the record, then replay the rebroadcast: a monitor-update completion can race
// wallet sync around confirmation.
let event = WalletEvent::TxConfirmed {
txid,
tx: Arc::new(tx.clone()),
block_time: confirmed_block_time(5),
old_block_time: None,
};
wallet.update_payment_store(vec![event]).await.unwrap();
wallet.classify_funding(&tx, &channels, tx_type).await.unwrap();
assert_unchanged(true);
}

/// Barrier test, classification-first ordering: wallet sync's confirmation handling must
/// wait for classification's two-store write pair. Classification is parked between its
/// payment-store and pending-store writes (the torn window) and only then is the
Expand Down
44 changes: 44 additions & 0 deletions tests/common/logging.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -173,3 +173,47 @@ impl LogWriter for MultiNodeLogger {
print!("{}", log);
}
}

/// Collects every log message a node emits, for tests that assert a specific line was logged.
pub(crate) struct CollectingLogWriter {
logs: Mutex<Vec<String>>,
}

impl CollectingLogWriter {
pub(crate) fn new() -> Self {
Self { logs: Mutex::new(Vec::new()) }
}

pub(crate) fn contains(&self, text: &str) -> bool {
self.count(text) > 0
}

pub(crate) fn count(&self, text: &str) -> usize {
self.logs.lock().unwrap().iter().filter(|message| message.contains(text)).count()
}

/// Waits up to ten seconds for a logged message containing `text`, returning whether one
/// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays
/// the full timeout when the line never comes.
pub(crate) async fn wait_for(&self, text: &str) -> bool {
self.wait_for_count(text, 1).await
}

/// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning
/// whether they arrived.
pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool {
for _ in 0..100 {
if self.count(text) >= occurrences {
return true;
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
false
}
}

impl LogWriter for CollectingLogWriter {
fn log(&self, record: LogRecord) {
self.logs.lock().unwrap().push(record.args.to_string());
}
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Guard payment records against splice funding rebroadcasts by jkczyz · Pull Request #1049 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions src/wallet/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1551,7 +1551,54 @@ impl Wallet {
let txid = tx.compute_txid();
let (amount_msat, fee_paid_msat, direction) = self.onchain_payment_fields(tx);

// A funding transaction that moves no wallet funds carries nothing to record — e.g. LDK
// re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding path,
// including splices the interactive-funding classification deliberately declined (no
// local contribution, or a splice-out moving no wallet funds). Recording it here would
// mint a zero-amount payment that nothing ever confirms. Skip on the wallet-derived
// amount alone — the condition `classify_interactive_funding` declines on; anything
// declined there must be skipped here, or its re-broadcast resurrects the record. The fee
// is no participation signal: the wallet resolves a splice's shared input whenever the
// previous funding transaction touched it (e.g. it funded the original channel open).
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at
// all. `zero_conf_splice_out_funding_rebroadcast_canary` pins the current behavior by
// asserting the log line below; when it fails against a newer LDK, re-evaluate whether
// this skip still sees traffic.
if amount_msat == Some(0) {
log_trace!(
self.logger,
"Not recording channel-funding broadcast {} as a payment: no wallet-level activity",
txid,
);
return Ok(());
}

let payment_id = PaymentId(txid.to_byte_array());

// A promoted-but-unconfirmed 0conf splice comes back through this generic path re-typed
// and carrying wallet-view figures; `funding_reclassification_update` declines the
// downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can
// observe the traffic. The read cannot go stale: only the broadcast loop writes
// interactive-funding classifications, and it runs this classification too.
if let Some(current) = self.payment_store.get(&payment_id) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, there are now so many edge cases to handle in this code. I increasingly think we need to reconsider the chosen approach (classifying through the broadcaster interface rather than keeping state in LDK and providing an API for it). Reopened https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/3566#issuecomment-440916 and added it to the v0.4 milestone.

if matches!(
current.kind,
PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}
) {
log_trace!(
self.logger,
"Keeping interactive-funding classification over funding-typed rebroadcast {}",
txid,
);
}
}

let details = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
Expand DownExpand Up@@ -2585,6 +2632,28 @@ fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight {
fn funding_reclassification_update(
details: PaymentDetails, candidates: &[FundingTxCandidate], current: Option<&PaymentDetails>,
) -> PaymentDetailsUpdate {
// A funding-typed classification of a record already classified as interactive funding is a
// downgrade, not news: LDK re-broadcasts a promoted-but-unconfirmed splice through its
// generic funding path, where the figures are wallet-view rather than contribution-derived.
// Keep the record as classified; wallet-sync events own its confirmation state.
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at all.
// `zero_conf_splice_in_funding_rebroadcast_canary` pins the current behavior via the
// arrival log in `classify_funding`; when it fails against a newer LDK, re-evaluate
// whether this guard still sees traffic.
if let (
Some(PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}),
PaymentKind::Onchain { tx_type: Some(TransactionType::Funding { .. }), .. },
) = (current.map(|payment| &payment.kind), &details.kind)
{
return PaymentDetailsUpdate::new(details.id);
}

let mut update = PaymentDetailsUpdate::funding_reclassification(details);
if let Some(PaymentKind::Onchain {
txid: confirmed_txid,
Expand DownExpand Up@@ -3689,6 +3758,35 @@ mod tests {
assert_eq!(update.txid, Some(active_txid));
}

/// A funding-typed (re)classification of a record already classified as interactive funding
/// carries nothing the record doesn't have — LDK re-broadcasts a promoted-but-unconfirmed
/// splice through its generic funding path with wallet-view figures — so the update must
/// move nothing.
#[test]
fn funding_reclassification_update_skips_funding_over_interactive_funding() {
let txid = Txid::from_byte_array([1u8; 32]);
let payment_id = PaymentId(txid.to_byte_array());
let current = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));

let rebroadcast = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
txid,
status: ConfirmationStatus::Unconfirmed,
tx_type: Some(TransactionType::Funding { channels: vec![] }),
},
Some(10_000_000),
Some(0),
PaymentDirection::Inbound,
PaymentStatus::Pending,
);

let update = funding_reclassification_update(rebroadcast, &[], Some(&current));
let mut updated = current.clone();
assert!(!updated.update(update), "the rebroadcast must not move the record");
assert_eq!(updated, current);
}

/// Graduation must decide from the live record and write only the status: a pending-store
/// snapshot taken before a concurrent classification landed must not roll the record's
/// figures back when the payment graduates to `Succeeded`.
Expand DownExpand Up@@ -3831,6 +3929,148 @@ mod tests {
assert_eq!(wallet.find_payment_by_txid(txid2), Some(payment_id));
}

/// A funding-typed broadcast that doesn't touch the on-chain wallet must not be recorded.
/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path, so a splice the interactive-funding classification deliberately declined — no local
/// contribution, or none of the moved funds are the wallet's — would otherwise come back as
/// a spurious zero-amount record that nothing ever confirms.
#[tokio::test]
async fn funding_broadcast_without_wallet_activity_is_not_recorded() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

// No inputs or outputs involve the wallet: nothing to record.
wallet.classify_funding(&dummy_tx(), &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());
assert!(wallet.pending_payment_store.list_filter(|_| true).is_empty());

// A computable fee is not wallet participation. The wallet can resolve a splice's shared
// input whenever the previous funding transaction touched it (e.g. it funded the original
// channel open), so it derives the splice's fee even when no wallet funds move.
let prev_funding_outpoint = OutPoint { txid: Txid::from_byte_array([8u8; 32]), vout: 0 };
wallet.inner.lock().unwrap().insert_txout(
prev_funding_outpoint,
TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() },
);
let splice_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: vec![bitcoin::TxIn {
previous_output: prev_funding_outpoint,
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(99_000),
script_pubkey: ScriptBuf::new(),
}],
};
wallet.classify_funding(&splice_tx, &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());

// Control: a funding transaction the wallet participates in is still recorded.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let funded_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
wallet.classify_funding(&funded_tx, &channels, tx_type).await.unwrap();
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1);
assert_eq!(payments[0].id, PaymentId(funded_tx.compute_txid().to_byte_array()));
}

/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path: same txid, but typed as a plain funding transaction with wallet-view figures and no
/// contribution metadata. The rebroadcast must not overwrite the contribution-derived
/// figures or the interactive-funding classification — neither while the record is
/// unconfirmed nor once it confirmed under that same txid, where updates naming the
/// confirmed txid may otherwise move figures.
#[tokio::test]
async fn funding_rebroadcast_keeps_interactive_funding_classification() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

// The rebroadcast passes the wallet-activity guard: a splice-in funds the new channel
// output partly from the wallet, so the wallet sees movement.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
let txid = tx.compute_txid();
let payment_id = PaymentId(txid.to_byte_array());

let candidates = vec![FundingTxCandidate {
txid,
amount_msat: Some(1_000_000),
fee_paid_msat: Some(500),
}];
let details = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));
wallet.persist_funding_payment(details, candidates).await.unwrap();

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

let assert_unchanged = |confirmed: bool| {
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1, "the rebroadcast must not mint a second record");
let payment = &payments[0];
assert_eq!(payment.id, payment_id);
assert_eq!(payment.amount_msat, Some(1_000_000));
assert_eq!(payment.fee_paid_msat, Some(500));
match &payment.kind {
PaymentKind::Onchain {
status,
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
} => assert_eq!(matches!(status, ConfirmationStatus::Confirmed { .. }), confirmed),
kind => panic!("unexpected kind {:?}", kind),
}
};

wallet.classify_funding(&tx, &channels, tx_type.clone()).await.unwrap();
assert_unchanged(false);

// Confirm the record, then replay the rebroadcast: a monitor-update completion can race
// wallet sync around confirmation.
let event = WalletEvent::TxConfirmed {
txid,
tx: Arc::new(tx.clone()),
block_time: confirmed_block_time(5),
old_block_time: None,
};
wallet.update_payment_store(vec![event]).await.unwrap();
wallet.classify_funding(&tx, &channels, tx_type).await.unwrap();
assert_unchanged(true);
}

/// Barrier test, classification-first ordering: wallet sync's confirmation handling must
/// wait for classification's two-store write pair. Classification is parked between its
/// payment-store and pending-store writes (the torn window) and only then is the
Expand Down
44 changes: 44 additions & 0 deletions tests/common/logging.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -173,3 +173,47 @@ impl LogWriter for MultiNodeLogger {
print!("{}", log);
}
}

/// Collects every log message a node emits, for tests that assert a specific line was logged.
pub(crate) struct CollectingLogWriter {
logs: Mutex<Vec<String>>,
}

impl CollectingLogWriter {
pub(crate) fn new() -> Self {
Self { logs: Mutex::new(Vec::new()) }
}

pub(crate) fn contains(&self, text: &str) -> bool {
self.count(text) > 0
}

pub(crate) fn count(&self, text: &str) -> usize {
self.logs.lock().unwrap().iter().filter(|message| message.contains(text)).count()
}

/// Waits up to ten seconds for a logged message containing `text`, returning whether one
/// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays
/// the full timeout when the line never comes.
pub(crate) async fn wait_for(&self, text: &str) -> bool {
self.wait_for_count(text, 1).await
}

/// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning
/// whether they arrived.
pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool {
for _ in 0..100 {
if self.count(text) >= occurrences {
return true;
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
false
}
}

impl LogWriter for CollectingLogWriter {
fn log(&self, record: LogRecord) {
self.logs.lock().unwrap().push(record.args.to_string());
}
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Guard payment records against splice funding rebroadcasts by jkczyz · Pull Request #1049 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions src/wallet/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1551,7 +1551,54 @@ impl Wallet {
let txid = tx.compute_txid();
let (amount_msat, fee_paid_msat, direction) = self.onchain_payment_fields(tx);

// A funding transaction that moves no wallet funds carries nothing to record — e.g. LDK
// re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding path,
// including splices the interactive-funding classification deliberately declined (no
// local contribution, or a splice-out moving no wallet funds). Recording it here would
// mint a zero-amount payment that nothing ever confirms. Skip on the wallet-derived
// amount alone — the condition `classify_interactive_funding` declines on; anything
// declined there must be skipped here, or its re-broadcast resurrects the record. The fee
// is no participation signal: the wallet resolves a splice's shared input whenever the
// previous funding transaction touched it (e.g. it funded the original channel open).
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at
// all. `zero_conf_splice_out_funding_rebroadcast_canary` pins the current behavior by
// asserting the log line below; when it fails against a newer LDK, re-evaluate whether
// this skip still sees traffic.
if amount_msat == Some(0) {
log_trace!(
self.logger,
"Not recording channel-funding broadcast {} as a payment: no wallet-level activity",
txid,
);
return Ok(());
}

let payment_id = PaymentId(txid.to_byte_array());

// A promoted-but-unconfirmed 0conf splice comes back through this generic path re-typed
// and carrying wallet-view figures; `funding_reclassification_update` declines the
// downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can
// observe the traffic. The read cannot go stale: only the broadcast loop writes
// interactive-funding classifications, and it runs this classification too.
if let Some(current) = self.payment_store.get(&payment_id) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, there are now so many edge cases to handle in this code. I increasingly think we need to reconsider the chosen approach (classifying through the broadcaster interface rather than keeping state in LDK and providing an API for it). Reopened https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/3566#issuecomment-440916 and added it to the v0.4 milestone.

if matches!(
current.kind,
PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}
) {
log_trace!(
self.logger,
"Keeping interactive-funding classification over funding-typed rebroadcast {}",
txid,
);
}
}

let details = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
Expand DownExpand Up@@ -2585,6 +2632,28 @@ fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight {
fn funding_reclassification_update(
details: PaymentDetails, candidates: &[FundingTxCandidate], current: Option<&PaymentDetails>,
) -> PaymentDetailsUpdate {
// A funding-typed classification of a record already classified as interactive funding is a
// downgrade, not news: LDK re-broadcasts a promoted-but-unconfirmed splice through its
// generic funding path, where the figures are wallet-view rather than contribution-derived.
// Keep the record as classified; wallet-sync events own its confirmation state.
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at all.
// `zero_conf_splice_in_funding_rebroadcast_canary` pins the current behavior via the
// arrival log in `classify_funding`; when it fails against a newer LDK, re-evaluate
// whether this guard still sees traffic.
if let (
Some(PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}),
PaymentKind::Onchain { tx_type: Some(TransactionType::Funding { .. }), .. },
) = (current.map(|payment| &payment.kind), &details.kind)
{
return PaymentDetailsUpdate::new(details.id);
}

let mut update = PaymentDetailsUpdate::funding_reclassification(details);
if let Some(PaymentKind::Onchain {
txid: confirmed_txid,
Expand DownExpand Up@@ -3689,6 +3758,35 @@ mod tests {
assert_eq!(update.txid, Some(active_txid));
}

/// A funding-typed (re)classification of a record already classified as interactive funding
/// carries nothing the record doesn't have — LDK re-broadcasts a promoted-but-unconfirmed
/// splice through its generic funding path with wallet-view figures — so the update must
/// move nothing.
#[test]
fn funding_reclassification_update_skips_funding_over_interactive_funding() {
let txid = Txid::from_byte_array([1u8; 32]);
let payment_id = PaymentId(txid.to_byte_array());
let current = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));

let rebroadcast = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
txid,
status: ConfirmationStatus::Unconfirmed,
tx_type: Some(TransactionType::Funding { channels: vec![] }),
},
Some(10_000_000),
Some(0),
PaymentDirection::Inbound,
PaymentStatus::Pending,
);

let update = funding_reclassification_update(rebroadcast, &[], Some(&current));
let mut updated = current.clone();
assert!(!updated.update(update), "the rebroadcast must not move the record");
assert_eq!(updated, current);
}

/// Graduation must decide from the live record and write only the status: a pending-store
/// snapshot taken before a concurrent classification landed must not roll the record's
/// figures back when the payment graduates to `Succeeded`.
Expand DownExpand Up@@ -3831,6 +3929,148 @@ mod tests {
assert_eq!(wallet.find_payment_by_txid(txid2), Some(payment_id));
}

/// A funding-typed broadcast that doesn't touch the on-chain wallet must not be recorded.
/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path, so a splice the interactive-funding classification deliberately declined — no local
/// contribution, or none of the moved funds are the wallet's — would otherwise come back as
/// a spurious zero-amount record that nothing ever confirms.
#[tokio::test]
async fn funding_broadcast_without_wallet_activity_is_not_recorded() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

// No inputs or outputs involve the wallet: nothing to record.
wallet.classify_funding(&dummy_tx(), &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());
assert!(wallet.pending_payment_store.list_filter(|_| true).is_empty());

// A computable fee is not wallet participation. The wallet can resolve a splice's shared
// input whenever the previous funding transaction touched it (e.g. it funded the original
// channel open), so it derives the splice's fee even when no wallet funds move.
let prev_funding_outpoint = OutPoint { txid: Txid::from_byte_array([8u8; 32]), vout: 0 };
wallet.inner.lock().unwrap().insert_txout(
prev_funding_outpoint,
TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() },
);
let splice_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: vec![bitcoin::TxIn {
previous_output: prev_funding_outpoint,
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(99_000),
script_pubkey: ScriptBuf::new(),
}],
};
wallet.classify_funding(&splice_tx, &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());

// Control: a funding transaction the wallet participates in is still recorded.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let funded_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
wallet.classify_funding(&funded_tx, &channels, tx_type).await.unwrap();
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1);
assert_eq!(payments[0].id, PaymentId(funded_tx.compute_txid().to_byte_array()));
}

/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path: same txid, but typed as a plain funding transaction with wallet-view figures and no
/// contribution metadata. The rebroadcast must not overwrite the contribution-derived
/// figures or the interactive-funding classification — neither while the record is
/// unconfirmed nor once it confirmed under that same txid, where updates naming the
/// confirmed txid may otherwise move figures.
#[tokio::test]
async fn funding_rebroadcast_keeps_interactive_funding_classification() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

// The rebroadcast passes the wallet-activity guard: a splice-in funds the new channel
// output partly from the wallet, so the wallet sees movement.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
let txid = tx.compute_txid();
let payment_id = PaymentId(txid.to_byte_array());

let candidates = vec![FundingTxCandidate {
txid,
amount_msat: Some(1_000_000),
fee_paid_msat: Some(500),
}];
let details = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));
wallet.persist_funding_payment(details, candidates).await.unwrap();

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

let assert_unchanged = |confirmed: bool| {
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1, "the rebroadcast must not mint a second record");
let payment = &payments[0];
assert_eq!(payment.id, payment_id);
assert_eq!(payment.amount_msat, Some(1_000_000));
assert_eq!(payment.fee_paid_msat, Some(500));
match &payment.kind {
PaymentKind::Onchain {
status,
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
} => assert_eq!(matches!(status, ConfirmationStatus::Confirmed { .. }), confirmed),
kind => panic!("unexpected kind {:?}", kind),
}
};

wallet.classify_funding(&tx, &channels, tx_type.clone()).await.unwrap();
assert_unchanged(false);

// Confirm the record, then replay the rebroadcast: a monitor-update completion can race
// wallet sync around confirmation.
let event = WalletEvent::TxConfirmed {
txid,
tx: Arc::new(tx.clone()),
block_time: confirmed_block_time(5),
old_block_time: None,
};
wallet.update_payment_store(vec![event]).await.unwrap();
wallet.classify_funding(&tx, &channels, tx_type).await.unwrap();
assert_unchanged(true);
}

/// Barrier test, classification-first ordering: wallet sync's confirmation handling must
/// wait for classification's two-store write pair. Classification is parked between its
/// payment-store and pending-store writes (the torn window) and only then is the
Expand Down
44 changes: 44 additions & 0 deletions tests/common/logging.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -173,3 +173,47 @@ impl LogWriter for MultiNodeLogger {
print!("{}", log);
}
}

/// Collects every log message a node emits, for tests that assert a specific line was logged.
pub(crate) struct CollectingLogWriter {
logs: Mutex<Vec<String>>,
}

impl CollectingLogWriter {
pub(crate) fn new() -> Self {
Self { logs: Mutex::new(Vec::new()) }
}

pub(crate) fn contains(&self, text: &str) -> bool {
self.count(text) > 0
}

pub(crate) fn count(&self, text: &str) -> usize {
self.logs.lock().unwrap().iter().filter(|message| message.contains(text)).count()
}

/// Waits up to ten seconds for a logged message containing `text`, returning whether one
/// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays
/// the full timeout when the line never comes.
pub(crate) async fn wait_for(&self, text: &str) -> bool {
self.wait_for_count(text, 1).await
}

/// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning
/// whether they arrived.
pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool {
for _ in 0..100 {
if self.count(text) >= occurrences {
return true;
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
false
}
}

impl LogWriter for CollectingLogWriter {
fn log(&self, record: LogRecord) {
self.logs.lock().unwrap().push(record.args.to_string());
}
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Guard payment records against splice funding rebroadcasts by jkczyz · Pull Request #1049 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions src/wallet/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1551,7 +1551,54 @@ impl Wallet {
let txid = tx.compute_txid();
let (amount_msat, fee_paid_msat, direction) = self.onchain_payment_fields(tx);

// A funding transaction that moves no wallet funds carries nothing to record — e.g. LDK
// re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding path,
// including splices the interactive-funding classification deliberately declined (no
// local contribution, or a splice-out moving no wallet funds). Recording it here would
// mint a zero-amount payment that nothing ever confirms. Skip on the wallet-derived
// amount alone — the condition `classify_interactive_funding` declines on; anything
// declined there must be skipped here, or its re-broadcast resurrects the record. The fee
// is no participation signal: the wallet resolves a splice's shared input whenever the
// previous funding transaction touched it (e.g. it funded the original channel open).
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at
// all. `zero_conf_splice_out_funding_rebroadcast_canary` pins the current behavior by
// asserting the log line below; when it fails against a newer LDK, re-evaluate whether
// this skip still sees traffic.
if amount_msat == Some(0) {
log_trace!(
self.logger,
"Not recording channel-funding broadcast {} as a payment: no wallet-level activity",
txid,
);
return Ok(());
}

let payment_id = PaymentId(txid.to_byte_array());

// A promoted-but-unconfirmed 0conf splice comes back through this generic path re-typed
// and carrying wallet-view figures; `funding_reclassification_update` declines the
// downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can
// observe the traffic. The read cannot go stale: only the broadcast loop writes
// interactive-funding classifications, and it runs this classification too.
if let Some(current) = self.payment_store.get(&payment_id) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, there are now so many edge cases to handle in this code. I increasingly think we need to reconsider the chosen approach (classifying through the broadcaster interface rather than keeping state in LDK and providing an API for it). Reopened https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/3566#issuecomment-440916 and added it to the v0.4 milestone.

if matches!(
current.kind,
PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}
) {
log_trace!(
self.logger,
"Keeping interactive-funding classification over funding-typed rebroadcast {}",
txid,
);
}
}

let details = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
Expand DownExpand Up@@ -2585,6 +2632,28 @@ fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight {
fn funding_reclassification_update(
details: PaymentDetails, candidates: &[FundingTxCandidate], current: Option<&PaymentDetails>,
) -> PaymentDetailsUpdate {
// A funding-typed classification of a record already classified as interactive funding is a
// downgrade, not news: LDK re-broadcasts a promoted-but-unconfirmed splice through its
// generic funding path, where the figures are wallet-view rather than contribution-derived.
// Keep the record as classified; wallet-sync events own its confirmation state.
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at all.
// `zero_conf_splice_in_funding_rebroadcast_canary` pins the current behavior via the
// arrival log in `classify_funding`; when it fails against a newer LDK, re-evaluate
// whether this guard still sees traffic.
if let (
Some(PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}),
PaymentKind::Onchain { tx_type: Some(TransactionType::Funding { .. }), .. },
) = (current.map(|payment| &payment.kind), &details.kind)
{
return PaymentDetailsUpdate::new(details.id);
}

let mut update = PaymentDetailsUpdate::funding_reclassification(details);
if let Some(PaymentKind::Onchain {
txid: confirmed_txid,
Expand DownExpand Up@@ -3689,6 +3758,35 @@ mod tests {
assert_eq!(update.txid, Some(active_txid));
}

/// A funding-typed (re)classification of a record already classified as interactive funding
/// carries nothing the record doesn't have — LDK re-broadcasts a promoted-but-unconfirmed
/// splice through its generic funding path with wallet-view figures — so the update must
/// move nothing.
#[test]
fn funding_reclassification_update_skips_funding_over_interactive_funding() {
let txid = Txid::from_byte_array([1u8; 32]);
let payment_id = PaymentId(txid.to_byte_array());
let current = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));

let rebroadcast = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
txid,
status: ConfirmationStatus::Unconfirmed,
tx_type: Some(TransactionType::Funding { channels: vec![] }),
},
Some(10_000_000),
Some(0),
PaymentDirection::Inbound,
PaymentStatus::Pending,
);

let update = funding_reclassification_update(rebroadcast, &[], Some(&current));
let mut updated = current.clone();
assert!(!updated.update(update), "the rebroadcast must not move the record");
assert_eq!(updated, current);
}

/// Graduation must decide from the live record and write only the status: a pending-store
/// snapshot taken before a concurrent classification landed must not roll the record's
/// figures back when the payment graduates to `Succeeded`.
Expand DownExpand Up@@ -3831,6 +3929,148 @@ mod tests {
assert_eq!(wallet.find_payment_by_txid(txid2), Some(payment_id));
}

/// A funding-typed broadcast that doesn't touch the on-chain wallet must not be recorded.
/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path, so a splice the interactive-funding classification deliberately declined — no local
/// contribution, or none of the moved funds are the wallet's — would otherwise come back as
/// a spurious zero-amount record that nothing ever confirms.
#[tokio::test]
async fn funding_broadcast_without_wallet_activity_is_not_recorded() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

// No inputs or outputs involve the wallet: nothing to record.
wallet.classify_funding(&dummy_tx(), &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());
assert!(wallet.pending_payment_store.list_filter(|_| true).is_empty());

// A computable fee is not wallet participation. The wallet can resolve a splice's shared
// input whenever the previous funding transaction touched it (e.g. it funded the original
// channel open), so it derives the splice's fee even when no wallet funds move.
let prev_funding_outpoint = OutPoint { txid: Txid::from_byte_array([8u8; 32]), vout: 0 };
wallet.inner.lock().unwrap().insert_txout(
prev_funding_outpoint,
TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() },
);
let splice_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: vec![bitcoin::TxIn {
previous_output: prev_funding_outpoint,
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(99_000),
script_pubkey: ScriptBuf::new(),
}],
};
wallet.classify_funding(&splice_tx, &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());

// Control: a funding transaction the wallet participates in is still recorded.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let funded_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
wallet.classify_funding(&funded_tx, &channels, tx_type).await.unwrap();
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1);
assert_eq!(payments[0].id, PaymentId(funded_tx.compute_txid().to_byte_array()));
}

/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path: same txid, but typed as a plain funding transaction with wallet-view figures and no
/// contribution metadata. The rebroadcast must not overwrite the contribution-derived
/// figures or the interactive-funding classification — neither while the record is
/// unconfirmed nor once it confirmed under that same txid, where updates naming the
/// confirmed txid may otherwise move figures.
#[tokio::test]
async fn funding_rebroadcast_keeps_interactive_funding_classification() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

// The rebroadcast passes the wallet-activity guard: a splice-in funds the new channel
// output partly from the wallet, so the wallet sees movement.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
let txid = tx.compute_txid();
let payment_id = PaymentId(txid.to_byte_array());

let candidates = vec![FundingTxCandidate {
txid,
amount_msat: Some(1_000_000),
fee_paid_msat: Some(500),
}];
let details = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));
wallet.persist_funding_payment(details, candidates).await.unwrap();

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

let assert_unchanged = |confirmed: bool| {
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1, "the rebroadcast must not mint a second record");
let payment = &payments[0];
assert_eq!(payment.id, payment_id);
assert_eq!(payment.amount_msat, Some(1_000_000));
assert_eq!(payment.fee_paid_msat, Some(500));
match &payment.kind {
PaymentKind::Onchain {
status,
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
} => assert_eq!(matches!(status, ConfirmationStatus::Confirmed { .. }), confirmed),
kind => panic!("unexpected kind {:?}", kind),
}
};

wallet.classify_funding(&tx, &channels, tx_type.clone()).await.unwrap();
assert_unchanged(false);

// Confirm the record, then replay the rebroadcast: a monitor-update completion can race
// wallet sync around confirmation.
let event = WalletEvent::TxConfirmed {
txid,
tx: Arc::new(tx.clone()),
block_time: confirmed_block_time(5),
old_block_time: None,
};
wallet.update_payment_store(vec![event]).await.unwrap();
wallet.classify_funding(&tx, &channels, tx_type).await.unwrap();
assert_unchanged(true);
}

/// Barrier test, classification-first ordering: wallet sync's confirmation handling must
/// wait for classification's two-store write pair. Classification is parked between its
/// payment-store and pending-store writes (the torn window) and only then is the
Expand Down
44 changes: 44 additions & 0 deletions tests/common/logging.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -173,3 +173,47 @@ impl LogWriter for MultiNodeLogger {
print!("{}", log);
}
}

/// Collects every log message a node emits, for tests that assert a specific line was logged.
pub(crate) struct CollectingLogWriter {
logs: Mutex<Vec<String>>,
}

impl CollectingLogWriter {
pub(crate) fn new() -> Self {
Self { logs: Mutex::new(Vec::new()) }
}

pub(crate) fn contains(&self, text: &str) -> bool {
self.count(text) > 0
}

pub(crate) fn count(&self, text: &str) -> usize {
self.logs.lock().unwrap().iter().filter(|message| message.contains(text)).count()
}

/// Waits up to ten seconds for a logged message containing `text`, returning whether one
/// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays
/// the full timeout when the line never comes.
pub(crate) async fn wait_for(&self, text: &str) -> bool {
self.wait_for_count(text, 1).await
}

/// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning
/// whether they arrived.
pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool {
for _ in 0..100 {
if self.count(text) >= occurrences {
return true;
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
false
}
}

impl LogWriter for CollectingLogWriter {
fn log(&self, record: LogRecord) {
self.logs.lock().unwrap().push(record.args.to_string());
}
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Guard payment records against splice funding rebroadcasts by jkczyz · Pull Request #1049 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions src/wallet/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1551,7 +1551,54 @@ impl Wallet {
let txid = tx.compute_txid();
let (amount_msat, fee_paid_msat, direction) = self.onchain_payment_fields(tx);

// A funding transaction that moves no wallet funds carries nothing to record — e.g. LDK
// re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding path,
// including splices the interactive-funding classification deliberately declined (no
// local contribution, or a splice-out moving no wallet funds). Recording it here would
// mint a zero-amount payment that nothing ever confirms. Skip on the wallet-derived
// amount alone — the condition `classify_interactive_funding` declines on; anything
// declined there must be skipped here, or its re-broadcast resurrects the record. The fee
// is no participation signal: the wallet resolves a splice's shared input whenever the
// previous funding transaction touched it (e.g. it funded the original channel open).
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at
// all. `zero_conf_splice_out_funding_rebroadcast_canary` pins the current behavior by
// asserting the log line below; when it fails against a newer LDK, re-evaluate whether
// this skip still sees traffic.
if amount_msat == Some(0) {
log_trace!(
self.logger,
"Not recording channel-funding broadcast {} as a payment: no wallet-level activity",
txid,
);
return Ok(());
}

let payment_id = PaymentId(txid.to_byte_array());

// A promoted-but-unconfirmed 0conf splice comes back through this generic path re-typed
// and carrying wallet-view figures; `funding_reclassification_update` declines the
// downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can
// observe the traffic. The read cannot go stale: only the broadcast loop writes
// interactive-funding classifications, and it runs this classification too.
if let Some(current) = self.payment_store.get(&payment_id) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, there are now so many edge cases to handle in this code. I increasingly think we need to reconsider the chosen approach (classifying through the broadcaster interface rather than keeping state in LDK and providing an API for it). Reopened https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/3566#issuecomment-440916 and added it to the v0.4 milestone.

if matches!(
current.kind,
PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}
) {
log_trace!(
self.logger,
"Keeping interactive-funding classification over funding-typed rebroadcast {}",
txid,
);
}
}

let details = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
Expand DownExpand Up@@ -2585,6 +2632,28 @@ fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight {
fn funding_reclassification_update(
details: PaymentDetails, candidates: &[FundingTxCandidate], current: Option<&PaymentDetails>,
) -> PaymentDetailsUpdate {
// A funding-typed classification of a record already classified as interactive funding is a
// downgrade, not news: LDK re-broadcasts a promoted-but-unconfirmed splice through its
// generic funding path, where the figures are wallet-view rather than contribution-derived.
// Keep the record as classified; wallet-sync events own its confirmation state.
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at all.
// `zero_conf_splice_in_funding_rebroadcast_canary` pins the current behavior via the
// arrival log in `classify_funding`; when it fails against a newer LDK, re-evaluate
// whether this guard still sees traffic.
if let (
Some(PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}),
PaymentKind::Onchain { tx_type: Some(TransactionType::Funding { .. }), .. },
) = (current.map(|payment| &payment.kind), &details.kind)
{
return PaymentDetailsUpdate::new(details.id);
}

let mut update = PaymentDetailsUpdate::funding_reclassification(details);
if let Some(PaymentKind::Onchain {
txid: confirmed_txid,
Expand DownExpand Up@@ -3689,6 +3758,35 @@ mod tests {
assert_eq!(update.txid, Some(active_txid));
}

/// A funding-typed (re)classification of a record already classified as interactive funding
/// carries nothing the record doesn't have — LDK re-broadcasts a promoted-but-unconfirmed
/// splice through its generic funding path with wallet-view figures — so the update must
/// move nothing.
#[test]
fn funding_reclassification_update_skips_funding_over_interactive_funding() {
let txid = Txid::from_byte_array([1u8; 32]);
let payment_id = PaymentId(txid.to_byte_array());
let current = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));

let rebroadcast = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
txid,
status: ConfirmationStatus::Unconfirmed,
tx_type: Some(TransactionType::Funding { channels: vec![] }),
},
Some(10_000_000),
Some(0),
PaymentDirection::Inbound,
PaymentStatus::Pending,
);

let update = funding_reclassification_update(rebroadcast, &[], Some(&current));
let mut updated = current.clone();
assert!(!updated.update(update), "the rebroadcast must not move the record");
assert_eq!(updated, current);
}

/// Graduation must decide from the live record and write only the status: a pending-store
/// snapshot taken before a concurrent classification landed must not roll the record's
/// figures back when the payment graduates to `Succeeded`.
Expand DownExpand Up@@ -3831,6 +3929,148 @@ mod tests {
assert_eq!(wallet.find_payment_by_txid(txid2), Some(payment_id));
}

/// A funding-typed broadcast that doesn't touch the on-chain wallet must not be recorded.
/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path, so a splice the interactive-funding classification deliberately declined — no local
/// contribution, or none of the moved funds are the wallet's — would otherwise come back as
/// a spurious zero-amount record that nothing ever confirms.
#[tokio::test]
async fn funding_broadcast_without_wallet_activity_is_not_recorded() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

// No inputs or outputs involve the wallet: nothing to record.
wallet.classify_funding(&dummy_tx(), &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());
assert!(wallet.pending_payment_store.list_filter(|_| true).is_empty());

// A computable fee is not wallet participation. The wallet can resolve a splice's shared
// input whenever the previous funding transaction touched it (e.g. it funded the original
// channel open), so it derives the splice's fee even when no wallet funds move.
let prev_funding_outpoint = OutPoint { txid: Txid::from_byte_array([8u8; 32]), vout: 0 };
wallet.inner.lock().unwrap().insert_txout(
prev_funding_outpoint,
TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() },
);
let splice_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: vec![bitcoin::TxIn {
previous_output: prev_funding_outpoint,
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(99_000),
script_pubkey: ScriptBuf::new(),
}],
};
wallet.classify_funding(&splice_tx, &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());

// Control: a funding transaction the wallet participates in is still recorded.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let funded_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
wallet.classify_funding(&funded_tx, &channels, tx_type).await.unwrap();
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1);
assert_eq!(payments[0].id, PaymentId(funded_tx.compute_txid().to_byte_array()));
}

/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path: same txid, but typed as a plain funding transaction with wallet-view figures and no
/// contribution metadata. The rebroadcast must not overwrite the contribution-derived
/// figures or the interactive-funding classification — neither while the record is
/// unconfirmed nor once it confirmed under that same txid, where updates naming the
/// confirmed txid may otherwise move figures.
#[tokio::test]
async fn funding_rebroadcast_keeps_interactive_funding_classification() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

// The rebroadcast passes the wallet-activity guard: a splice-in funds the new channel
// output partly from the wallet, so the wallet sees movement.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
let txid = tx.compute_txid();
let payment_id = PaymentId(txid.to_byte_array());

let candidates = vec![FundingTxCandidate {
txid,
amount_msat: Some(1_000_000),
fee_paid_msat: Some(500),
}];
let details = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));
wallet.persist_funding_payment(details, candidates).await.unwrap();

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

let assert_unchanged = |confirmed: bool| {
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1, "the rebroadcast must not mint a second record");
let payment = &payments[0];
assert_eq!(payment.id, payment_id);
assert_eq!(payment.amount_msat, Some(1_000_000));
assert_eq!(payment.fee_paid_msat, Some(500));
match &payment.kind {
PaymentKind::Onchain {
status,
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
} => assert_eq!(matches!(status, ConfirmationStatus::Confirmed { .. }), confirmed),
kind => panic!("unexpected kind {:?}", kind),
}
};

wallet.classify_funding(&tx, &channels, tx_type.clone()).await.unwrap();
assert_unchanged(false);

// Confirm the record, then replay the rebroadcast: a monitor-update completion can race
// wallet sync around confirmation.
let event = WalletEvent::TxConfirmed {
txid,
tx: Arc::new(tx.clone()),
block_time: confirmed_block_time(5),
old_block_time: None,
};
wallet.update_payment_store(vec![event]).await.unwrap();
wallet.classify_funding(&tx, &channels, tx_type).await.unwrap();
assert_unchanged(true);
}

/// Barrier test, classification-first ordering: wallet sync's confirmation handling must
/// wait for classification's two-store write pair. Classification is parked between its
/// payment-store and pending-store writes (the torn window) and only then is the
Expand Down
44 changes: 44 additions & 0 deletions tests/common/logging.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -173,3 +173,47 @@ impl LogWriter for MultiNodeLogger {
print!("{}", log);
}
}

/// Collects every log message a node emits, for tests that assert a specific line was logged.
pub(crate) struct CollectingLogWriter {
logs: Mutex<Vec<String>>,
}

impl CollectingLogWriter {
pub(crate) fn new() -> Self {
Self { logs: Mutex::new(Vec::new()) }
}

pub(crate) fn contains(&self, text: &str) -> bool {
self.count(text) > 0
}

pub(crate) fn count(&self, text: &str) -> usize {
self.logs.lock().unwrap().iter().filter(|message| message.contains(text)).count()
}

/// Waits up to ten seconds for a logged message containing `text`, returning whether one
/// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays
/// the full timeout when the line never comes.
pub(crate) async fn wait_for(&self, text: &str) -> bool {
self.wait_for_count(text, 1).await
}

/// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning
/// whether they arrived.
pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool {
for _ in 0..100 {
if self.count(text) >= occurrences {
return true;
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
false
}
}

impl LogWriter for CollectingLogWriter {
fn log(&self, record: LogRecord) {
self.logs.lock().unwrap().push(record.args.to_string());
}
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Guard payment records against splice funding rebroadcasts by jkczyz · Pull Request #1049 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions src/wallet/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1551,7 +1551,54 @@ impl Wallet {
let txid = tx.compute_txid();
let (amount_msat, fee_paid_msat, direction) = self.onchain_payment_fields(tx);

// A funding transaction that moves no wallet funds carries nothing to record — e.g. LDK
// re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding path,
// including splices the interactive-funding classification deliberately declined (no
// local contribution, or a splice-out moving no wallet funds). Recording it here would
// mint a zero-amount payment that nothing ever confirms. Skip on the wallet-derived
// amount alone — the condition `classify_interactive_funding` declines on; anything
// declined there must be skipped here, or its re-broadcast resurrects the record. The fee
// is no participation signal: the wallet resolves a splice's shared input whenever the
// previous funding transaction touched it (e.g. it funded the original channel open).
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at
// all. `zero_conf_splice_out_funding_rebroadcast_canary` pins the current behavior by
// asserting the log line below; when it fails against a newer LDK, re-evaluate whether
// this skip still sees traffic.
if amount_msat == Some(0) {
log_trace!(
self.logger,
"Not recording channel-funding broadcast {} as a payment: no wallet-level activity",
txid,
);
return Ok(());
}

let payment_id = PaymentId(txid.to_byte_array());

// A promoted-but-unconfirmed 0conf splice comes back through this generic path re-typed
// and carrying wallet-view figures; `funding_reclassification_update` declines the
// downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can
// observe the traffic. The read cannot go stale: only the broadcast loop writes
// interactive-funding classifications, and it runs this classification too.
if let Some(current) = self.payment_store.get(&payment_id) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, there are now so many edge cases to handle in this code. I increasingly think we need to reconsider the chosen approach (classifying through the broadcaster interface rather than keeping state in LDK and providing an API for it). Reopened https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/3566#issuecomment-440916 and added it to the v0.4 milestone.

if matches!(
current.kind,
PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}
) {
log_trace!(
self.logger,
"Keeping interactive-funding classification over funding-typed rebroadcast {}",
txid,
);
}
}

let details = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
Expand DownExpand Up@@ -2585,6 +2632,28 @@ fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight {
fn funding_reclassification_update(
details: PaymentDetails, candidates: &[FundingTxCandidate], current: Option<&PaymentDetails>,
) -> PaymentDetailsUpdate {
// A funding-typed classification of a record already classified as interactive funding is a
// downgrade, not news: LDK re-broadcasts a promoted-but-unconfirmed splice through its
// generic funding path, where the figures are wallet-view rather than contribution-derived.
// Keep the record as classified; wallet-sync events own its confirmation state.
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at all.
// `zero_conf_splice_in_funding_rebroadcast_canary` pins the current behavior via the
// arrival log in `classify_funding`; when it fails against a newer LDK, re-evaluate
// whether this guard still sees traffic.
if let (
Some(PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}),
PaymentKind::Onchain { tx_type: Some(TransactionType::Funding { .. }), .. },
) = (current.map(|payment| &payment.kind), &details.kind)
{
return PaymentDetailsUpdate::new(details.id);
}

let mut update = PaymentDetailsUpdate::funding_reclassification(details);
if let Some(PaymentKind::Onchain {
txid: confirmed_txid,
Expand DownExpand Up@@ -3689,6 +3758,35 @@ mod tests {
assert_eq!(update.txid, Some(active_txid));
}

/// A funding-typed (re)classification of a record already classified as interactive funding
/// carries nothing the record doesn't have — LDK re-broadcasts a promoted-but-unconfirmed
/// splice through its generic funding path with wallet-view figures — so the update must
/// move nothing.
#[test]
fn funding_reclassification_update_skips_funding_over_interactive_funding() {
let txid = Txid::from_byte_array([1u8; 32]);
let payment_id = PaymentId(txid.to_byte_array());
let current = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));

let rebroadcast = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
txid,
status: ConfirmationStatus::Unconfirmed,
tx_type: Some(TransactionType::Funding { channels: vec![] }),
},
Some(10_000_000),
Some(0),
PaymentDirection::Inbound,
PaymentStatus::Pending,
);

let update = funding_reclassification_update(rebroadcast, &[], Some(&current));
let mut updated = current.clone();
assert!(!updated.update(update), "the rebroadcast must not move the record");
assert_eq!(updated, current);
}

/// Graduation must decide from the live record and write only the status: a pending-store
/// snapshot taken before a concurrent classification landed must not roll the record's
/// figures back when the payment graduates to `Succeeded`.
Expand DownExpand Up@@ -3831,6 +3929,148 @@ mod tests {
assert_eq!(wallet.find_payment_by_txid(txid2), Some(payment_id));
}

/// A funding-typed broadcast that doesn't touch the on-chain wallet must not be recorded.
/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path, so a splice the interactive-funding classification deliberately declined — no local
/// contribution, or none of the moved funds are the wallet's — would otherwise come back as
/// a spurious zero-amount record that nothing ever confirms.
#[tokio::test]
async fn funding_broadcast_without_wallet_activity_is_not_recorded() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

// No inputs or outputs involve the wallet: nothing to record.
wallet.classify_funding(&dummy_tx(), &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());
assert!(wallet.pending_payment_store.list_filter(|_| true).is_empty());

// A computable fee is not wallet participation. The wallet can resolve a splice's shared
// input whenever the previous funding transaction touched it (e.g. it funded the original
// channel open), so it derives the splice's fee even when no wallet funds move.
let prev_funding_outpoint = OutPoint { txid: Txid::from_byte_array([8u8; 32]), vout: 0 };
wallet.inner.lock().unwrap().insert_txout(
prev_funding_outpoint,
TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() },
);
let splice_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: vec![bitcoin::TxIn {
previous_output: prev_funding_outpoint,
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(99_000),
script_pubkey: ScriptBuf::new(),
}],
};
wallet.classify_funding(&splice_tx, &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());

// Control: a funding transaction the wallet participates in is still recorded.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let funded_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
wallet.classify_funding(&funded_tx, &channels, tx_type).await.unwrap();
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1);
assert_eq!(payments[0].id, PaymentId(funded_tx.compute_txid().to_byte_array()));
}

/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path: same txid, but typed as a plain funding transaction with wallet-view figures and no
/// contribution metadata. The rebroadcast must not overwrite the contribution-derived
/// figures or the interactive-funding classification — neither while the record is
/// unconfirmed nor once it confirmed under that same txid, where updates naming the
/// confirmed txid may otherwise move figures.
#[tokio::test]
async fn funding_rebroadcast_keeps_interactive_funding_classification() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

// The rebroadcast passes the wallet-activity guard: a splice-in funds the new channel
// output partly from the wallet, so the wallet sees movement.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
let txid = tx.compute_txid();
let payment_id = PaymentId(txid.to_byte_array());

let candidates = vec![FundingTxCandidate {
txid,
amount_msat: Some(1_000_000),
fee_paid_msat: Some(500),
}];
let details = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));
wallet.persist_funding_payment(details, candidates).await.unwrap();

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

let assert_unchanged = |confirmed: bool| {
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1, "the rebroadcast must not mint a second record");
let payment = &payments[0];
assert_eq!(payment.id, payment_id);
assert_eq!(payment.amount_msat, Some(1_000_000));
assert_eq!(payment.fee_paid_msat, Some(500));
match &payment.kind {
PaymentKind::Onchain {
status,
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
} => assert_eq!(matches!(status, ConfirmationStatus::Confirmed { .. }), confirmed),
kind => panic!("unexpected kind {:?}", kind),
}
};

wallet.classify_funding(&tx, &channels, tx_type.clone()).await.unwrap();
assert_unchanged(false);

// Confirm the record, then replay the rebroadcast: a monitor-update completion can race
// wallet sync around confirmation.
let event = WalletEvent::TxConfirmed {
txid,
tx: Arc::new(tx.clone()),
block_time: confirmed_block_time(5),
old_block_time: None,
};
wallet.update_payment_store(vec![event]).await.unwrap();
wallet.classify_funding(&tx, &channels, tx_type).await.unwrap();
assert_unchanged(true);
}

/// Barrier test, classification-first ordering: wallet sync's confirmation handling must
/// wait for classification's two-store write pair. Classification is parked between its
/// payment-store and pending-store writes (the torn window) and only then is the
Expand Down
44 changes: 44 additions & 0 deletions tests/common/logging.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -173,3 +173,47 @@ impl LogWriter for MultiNodeLogger {
print!("{}", log);
}
}

/// Collects every log message a node emits, for tests that assert a specific line was logged.
pub(crate) struct CollectingLogWriter {
logs: Mutex<Vec<String>>,
}

impl CollectingLogWriter {
pub(crate) fn new() -> Self {
Self { logs: Mutex::new(Vec::new()) }
}

pub(crate) fn contains(&self, text: &str) -> bool {
self.count(text) > 0
}

pub(crate) fn count(&self, text: &str) -> usize {
self.logs.lock().unwrap().iter().filter(|message| message.contains(text)).count()
}

/// Waits up to ten seconds for a logged message containing `text`, returning whether one
/// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays
/// the full timeout when the line never comes.
pub(crate) async fn wait_for(&self, text: &str) -> bool {
self.wait_for_count(text, 1).await
}

/// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning
/// whether they arrived.
pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool {
for _ in 0..100 {
if self.count(text) >= occurrences {
return true;
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
false
}
}

impl LogWriter for CollectingLogWriter {
fn log(&self, record: LogRecord) {
self.logs.lock().unwrap().push(record.args.to_string());
}
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Guard payment records against splice funding rebroadcasts by jkczyz · Pull Request #1049 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions src/wallet/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1551,7 +1551,54 @@ impl Wallet {
let txid = tx.compute_txid();
let (amount_msat, fee_paid_msat, direction) = self.onchain_payment_fields(tx);

// A funding transaction that moves no wallet funds carries nothing to record — e.g. LDK
// re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding path,
// including splices the interactive-funding classification deliberately declined (no
// local contribution, or a splice-out moving no wallet funds). Recording it here would
// mint a zero-amount payment that nothing ever confirms. Skip on the wallet-derived
// amount alone — the condition `classify_interactive_funding` declines on; anything
// declined there must be skipped here, or its re-broadcast resurrects the record. The fee
// is no participation signal: the wallet resolves a splice's shared input whenever the
// previous funding transaction touched it (e.g. it funded the original channel open).
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at
// all. `zero_conf_splice_out_funding_rebroadcast_canary` pins the current behavior by
// asserting the log line below; when it fails against a newer LDK, re-evaluate whether
// this skip still sees traffic.
if amount_msat == Some(0) {
log_trace!(
self.logger,
"Not recording channel-funding broadcast {} as a payment: no wallet-level activity",
txid,
);
return Ok(());
}

let payment_id = PaymentId(txid.to_byte_array());

// A promoted-but-unconfirmed 0conf splice comes back through this generic path re-typed
// and carrying wallet-view figures; `funding_reclassification_update` declines the
// downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can
// observe the traffic. The read cannot go stale: only the broadcast loop writes
// interactive-funding classifications, and it runs this classification too.
if let Some(current) = self.payment_store.get(&payment_id) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, there are now so many edge cases to handle in this code. I increasingly think we need to reconsider the chosen approach (classifying through the broadcaster interface rather than keeping state in LDK and providing an API for it). Reopened https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/3566#issuecomment-440916 and added it to the v0.4 milestone.

if matches!(
current.kind,
PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}
) {
log_trace!(
self.logger,
"Keeping interactive-funding classification over funding-typed rebroadcast {}",
txid,
);
}
}

let details = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
Expand DownExpand Up@@ -2585,6 +2632,28 @@ fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight {
fn funding_reclassification_update(
details: PaymentDetails, candidates: &[FundingTxCandidate], current: Option<&PaymentDetails>,
) -> PaymentDetailsUpdate {
// A funding-typed classification of a record already classified as interactive funding is a
// downgrade, not news: LDK re-broadcasts a promoted-but-unconfirmed splice through its
// generic funding path, where the figures are wallet-view rather than contribution-derived.
// Keep the record as classified; wallet-sync events own its confirmation state.
//
// TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The
// re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`:
// the re-offer ought to keep its `InteractiveFunding` classification, or not recur at all.
// `zero_conf_splice_in_funding_rebroadcast_canary` pins the current behavior via the
// arrival log in `classify_funding`; when it fails against a newer LDK, re-evaluate
// whether this guard still sees traffic.
if let (
Some(PaymentKind::Onchain {
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
}),
PaymentKind::Onchain { tx_type: Some(TransactionType::Funding { .. }), .. },
) = (current.map(|payment| &payment.kind), &details.kind)
{
return PaymentDetailsUpdate::new(details.id);
}

let mut update = PaymentDetailsUpdate::funding_reclassification(details);
if let Some(PaymentKind::Onchain {
txid: confirmed_txid,
Expand DownExpand Up@@ -3689,6 +3758,35 @@ mod tests {
assert_eq!(update.txid, Some(active_txid));
}

/// A funding-typed (re)classification of a record already classified as interactive funding
/// carries nothing the record doesn't have — LDK re-broadcasts a promoted-but-unconfirmed
/// splice through its generic funding path with wallet-view figures — so the update must
/// move nothing.
#[test]
fn funding_reclassification_update_skips_funding_over_interactive_funding() {
let txid = Txid::from_byte_array([1u8; 32]);
let payment_id = PaymentId(txid.to_byte_array());
let current = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));

let rebroadcast = PaymentDetails::new(
payment_id,
PaymentKind::Onchain {
txid,
status: ConfirmationStatus::Unconfirmed,
tx_type: Some(TransactionType::Funding { channels: vec![] }),
},
Some(10_000_000),
Some(0),
PaymentDirection::Inbound,
PaymentStatus::Pending,
);

let update = funding_reclassification_update(rebroadcast, &[], Some(&current));
let mut updated = current.clone();
assert!(!updated.update(update), "the rebroadcast must not move the record");
assert_eq!(updated, current);
}

/// Graduation must decide from the live record and write only the status: a pending-store
/// snapshot taken before a concurrent classification landed must not roll the record's
/// figures back when the payment graduates to `Succeeded`.
Expand DownExpand Up@@ -3831,6 +3929,148 @@ mod tests {
assert_eq!(wallet.find_payment_by_txid(txid2), Some(payment_id));
}

/// A funding-typed broadcast that doesn't touch the on-chain wallet must not be recorded.
/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path, so a splice the interactive-funding classification deliberately declined — no local
/// contribution, or none of the moved funds are the wallet's — would otherwise come back as
/// a spurious zero-amount record that nothing ever confirms.
#[tokio::test]
async fn funding_broadcast_without_wallet_activity_is_not_recorded() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

// No inputs or outputs involve the wallet: nothing to record.
wallet.classify_funding(&dummy_tx(), &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());
assert!(wallet.pending_payment_store.list_filter(|_| true).is_empty());

// A computable fee is not wallet participation. The wallet can resolve a splice's shared
// input whenever the previous funding transaction touched it (e.g. it funded the original
// channel open), so it derives the splice's fee even when no wallet funds move.
let prev_funding_outpoint = OutPoint { txid: Txid::from_byte_array([8u8; 32]), vout: 0 };
wallet.inner.lock().unwrap().insert_txout(
prev_funding_outpoint,
TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() },
);
let splice_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: vec![bitcoin::TxIn {
previous_output: prev_funding_outpoint,
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(99_000),
script_pubkey: ScriptBuf::new(),
}],
};
wallet.classify_funding(&splice_tx, &channels, tx_type.clone()).await.unwrap();
assert!(wallet.payment_store.list_filter(|_| true).is_empty());

// Control: a funding transaction the wallet participates in is still recorded.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let funded_tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
wallet.classify_funding(&funded_tx, &channels, tx_type).await.unwrap();
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1);
assert_eq!(payments[0].id, PaymentId(funded_tx.compute_txid().to_byte_array()));
}

/// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding
/// path: same txid, but typed as a plain funding transaction with wallet-view figures and no
/// contribution metadata. The rebroadcast must not overwrite the contribution-derived
/// figures or the interactive-funding classification — neither while the record is
/// unconfirmed nor once it confirmed under that same txid, where updates naming the
/// confirmed txid may otherwise move figures.
#[tokio::test]
async fn funding_rebroadcast_keeps_interactive_funding_classification() {
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
let wallet = new_test_wallet(store, false).await;

// The rebroadcast passes the wallet-activity guard: a splice-in funds the new channel
// output partly from the wallet, so the wallet sees movement.
let script_pubkey = wallet
.inner
.lock()
.unwrap()
.reveal_next_address(KeychainKind::External)
.address
.script_pubkey();
let tx = Transaction {
version: bitcoin::transaction::Version::TWO,
lock_time: LockTime::ZERO,
input: Vec::new(),
output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }],
};
let txid = tx.compute_txid();
let payment_id = PaymentId(txid.to_byte_array());

let candidates = vec![FundingTxCandidate {
txid,
amount_msat: Some(1_000_000),
fee_paid_msat: Some(500),
}];
let details = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500));
wallet.persist_funding_payment(details, candidates).await.unwrap();

let counterparty_node_id = PublicKey::from_str(
"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
)
.unwrap();
let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))];
let tx_type = TransactionType::Funding { channels: vec![] };

let assert_unchanged = |confirmed: bool| {
let payments = wallet.payment_store.list_filter(|_| true);
assert_eq!(payments.len(), 1, "the rebroadcast must not mint a second record");
let payment = &payments[0];
assert_eq!(payment.id, payment_id);
assert_eq!(payment.amount_msat, Some(1_000_000));
assert_eq!(payment.fee_paid_msat, Some(500));
match &payment.kind {
PaymentKind::Onchain {
status,
tx_type: Some(TransactionType::InteractiveFunding { .. }),
..
} => assert_eq!(matches!(status, ConfirmationStatus::Confirmed { .. }), confirmed),
kind => panic!("unexpected kind {:?}", kind),
}
};

wallet.classify_funding(&tx, &channels, tx_type.clone()).await.unwrap();
assert_unchanged(false);

// Confirm the record, then replay the rebroadcast: a monitor-update completion can race
// wallet sync around confirmation.
let event = WalletEvent::TxConfirmed {
txid,
tx: Arc::new(tx.clone()),
block_time: confirmed_block_time(5),
old_block_time: None,
};
wallet.update_payment_store(vec![event]).await.unwrap();
wallet.classify_funding(&tx, &channels, tx_type).await.unwrap();
assert_unchanged(true);
}

/// Barrier test, classification-first ordering: wallet sync's confirmation handling must
/// wait for classification's two-store write pair. Classification is parked between its
/// payment-store and pending-store writes (the torn window) and only then is the
Expand Down
44 changes: 44 additions & 0 deletions tests/common/logging.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -173,3 +173,47 @@ impl LogWriter for MultiNodeLogger {
print!("{}", log);
}
}

/// Collects every log message a node emits, for tests that assert a specific line was logged.
pub(crate) struct CollectingLogWriter {
logs: Mutex<Vec<String>>,
}

impl CollectingLogWriter {
pub(crate) fn new() -> Self {
Self { logs: Mutex::new(Vec::new()) }
}

pub(crate) fn contains(&self, text: &str) -> bool {
self.count(text) > 0
}

pub(crate) fn count(&self, text: &str) -> usize {
self.logs.lock().unwrap().iter().filter(|message| message.contains(text)).count()
}

/// Waits up to ten seconds for a logged message containing `text`, returning whether one
/// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays
/// the full timeout when the line never comes.
pub(crate) async fn wait_for(&self, text: &str) -> bool {
self.wait_for_count(text, 1).await
}

/// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning
/// whether they arrived.
pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool {
for _ in 0..100 {
if self.count(text) >= occurrences {
return true;
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
false
}
}

impl LogWriter for CollectingLogWriter {
fn log(&self, record: LogRecord) {
self.logs.lock().unwrap().push(record.args.to_string());
}
}
Loading
Loading