Skip to content

Splice fixes for chanmon_consistency fuzz target #4655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wpaulino wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Splice fixes for chanmon_consistency fuzz target #4655

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
89bdf95
Allow pending splice broadcasts at fuzz EOF
wpaulino Jun 1, 2026
9397749
Ignore stale splice signing fuzz events
wpaulino Jun 1, 2026
561f3ab
Raise iteration capacity in chanmon consistency when settling state
wpaulino Jun 1, 2026
baa5d0c
Replay seen blocks after node reload in chanmon_consistency
wpaulino Jun 1, 2026
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
80 changes: 71 additions & 9 deletions fuzz/src/chanmon_consistency.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use lightning::blinded_path::message::{BlindedMessagePath, MessageContext, Messa
use lightning::blinded_path::payment::{BlindedPaymentPath, ReceiveTlvs};
use lightning::chain;
use lightning::chain::chaininterface::{
BroadcasterInterface, ConfirmationTarget, FeeEstimator, TransactionType,
BroadcasterInterface, ConfirmationTarget, FeeEstimator, FundingPurpose, TransactionType,
};
use lightning::chain::channelmonitor::ChannelMonitor;
use lightning::chain::{
Expand Down Expand Up @@ -170,12 +170,12 @@ impl MessageRouter for FuzzRouter {
}

pub struct TestBroadcaster {
txn_broadcasted: RefCell<Vec<Transaction>>,
txn_broadcasted: RefCell<Vec<(Transaction, TransactionType)>>,
}
impl BroadcasterInterface for TestBroadcaster {
fn broadcast_transactions(&self, txs: &[(&Transaction, TransactionType)]) {
for (tx, _broadcast_type) in txs {
self.txn_broadcasted.borrow_mut().push((*tx).clone());
for (tx, broadcast_type) in txs {
self.txn_broadcasted.borrow_mut().push(((*tx).clone(), broadcast_type.clone()));
}
}
}
Expand Down Expand Up @@ -2008,10 +2008,71 @@ fn assert_test_invariants(nodes: &[HarnessNode<'_>; 3]) {
assert_eq!(nodes[1].list_channels().len(), 6);
assert_eq!(nodes[2].list_channels().len(), 3);

// All broadcasters should be empty. Broadcast transactions are handled explicitly.
assert!(nodes[0].broadcaster.txn_broadcasted.borrow().is_empty());
assert!(nodes[1].broadcaster.txn_broadcasted.borrow().is_empty());
assert!(nodes[2].broadcaster.txn_broadcasted.borrow().is_empty());
// Broadcast transactions are handled explicitly. If the input ends immediately after

@joostjager joostjager Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For context, #4657 is changing chanmon_consistency broadcast handling so broadcast transactions are routed through an explicit modeled mempool, with finish draining remaining broadcasts through relay/mining cleanup.

@TheBlueMatt TheBlueMatt Jun 8, 2026

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.

Should we fix this failure there, then?

@joostjager joostjager Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something needs to be re-aligned either way, depending on merge order. Not sure what parts of this PR can be omitted with the more realistic mempool.

Prefer to keep #4657 a pure refactor though, and not include splice bug fixes. It's big enough already.

// `tx_signatures`, however, the corresponding `SpliceNegotiated` event may still be pending,
// leaving the valid interactive funding transaction in the test broadcaster.
for (idx, node) in nodes.iter().enumerate() {
if node.broadcaster.txn_broadcasted.borrow().is_empty() {
continue;
}

let pending_events = node.get_and_clear_pending_events();
Comment on lines +2016 to +2021

@ldk-claude-review-bot ldk-claude-review-bot Jun 1, 2026

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.

The verification is one-directional: for each broadcast tx, you check a matching SpliceNegotiated event exists, but you don't verify the reverse — that every pending SpliceNegotiated event has a corresponding broadcast tx. A node with no broadcasts (skipped at line 2017) but with dangling SpliceNegotiated events passes silently.

Consider also checking nodes that have no broadcasts but do have pending SpliceNegotiated events, as that would indicate the splice tx was not broadcast despite an event being generated.

Suggested change
for (idx, node) in nodes.iter().enumerate() {
if node.broadcaster.txn_broadcasted.borrow().is_empty() {
continue;
}
let pending_events = node.get_and_clear_pending_events();
for (idx, node) in nodes.iter().enumerate() {
let pending_events = node.get_and_clear_pending_events();
if node.broadcaster.txn_broadcasted.borrow().is_empty() {
assert!(
!pending_events.iter().any(|e| matches!(e, events::Event::SpliceNegotiated { .. })),
"node {} has pending SpliceNegotiated event(s) but no broadcast tx",
idx,
);
continue;
}

@TheBlueMatt TheBlueMatt Jun 8, 2026

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.

Is it not a cleaner fix to just handle the events here? We could filter the events by FundingNegotiated if we want to retain previous behavior, though not sure it matters much.

let expected_splice_events = {
let txs = node.broadcaster.txn_broadcasted.borrow();
let mut expected_splice_events = Vec::new();
for (tx, tx_type) in txs.iter() {
let txid = tx.compute_txid();
let candidates = match tx_type {
TransactionType::InteractiveFunding { candidates } => candidates,
_ => panic!("node {} had unexpected broadcast transaction: {:?}", idx, tx_type),
};
for funding in &candidates.last().unwrap().channels {
assert!(
matches!(&funding.purpose, FundingPurpose::Splice),
"node {} had leftover non-splice interactive funding broadcast: {:?}",
idx,
funding
);
expected_splice_events.push((
txid.clone(),
funding.counterparty_node_id.clone(),
funding.channel_id.clone(),
));
}
}
expected_splice_events
};

let mut pending_splice_events = pending_events
.iter()
.filter_map(|event| match event {
events::Event::SpliceNegotiated {
new_funding_txo,
counterparty_node_id,
channel_id,
..
} => Some((
new_funding_txo.txid.clone(),
counterparty_node_id.clone(),
channel_id.clone(),
)),
_ => None,
})
.collect::<Vec<_>>();
for expected_splice_event in expected_splice_events {
let pending_idx =
pending_splice_events.iter().position(|event| event == &expected_splice_event);
assert!(
pending_idx.is_some(),
"node {} had leftover interactive funding broadcast without matching \
pending SpliceNegotiated event: {:?}; pending events: {:?}",
idx,
expected_splice_event,
pending_events
);
pending_splice_events.remove(pending_idx.unwrap());
}
}
}

fn connect_peers(source: &ChanMan<'_>, dest: &ChanMan<'_>) {
Expand Down Expand Up @@ -2821,7 +2882,8 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> {
events::Event::SpliceNegotiated { new_funding_txo, .. } => {
let mut txs = nodes[node_idx].broadcaster.txn_broadcasted.borrow_mut();
assert!(txs.len() >= 1);
let splice_tx = txs.remove(0);
let (splice_tx, tx_type) = txs.remove(0);
assert!(matches!(tx_type, TransactionType::InteractiveFunding { .. }));
assert_eq!(new_funding_txo.txid, splice_tx.compute_txid());
chain_state.add_pending_tx(splice_tx);
},
Expand Down