Drop the async-setting of the `P2PGossipSync` `utxo_verifier`
What changed, and why it matters
This commit is a routine internal cleanup in the Lightning Dev Kit's gossip message handling code. It removes a method that let users set a UTXO (unspent transaction output) lookup provider after the object was created, because the codebase no longer needs that late-setup workaround. The change only affects how the code is structured and how tests are set up; it does not fix a security bug or introduce a known vulnerability.
No security action required. Developers using LDK should note the removed `add_utxo_lookup` method and pass the UTXO lookup provider directly to `P2PGossipSync::new` instead.
Security signals we found
No security-relevant signal in the diff: no bounds checks, no input validation changes, no cryptographic fixes, no privilege changes, no memory-safety fixes.
The removed `add_utxo_lookup` API could theoretically have allowed runtime replacement of the UTXO verifier, but the diff does not indicate that this was exploitable or treated as a vulnerability by the project.
The change is purely API/test refactoring.
Evidence from the diff
The patch removes P2PGossipSync::add_utxo_lookup, an async/late setter for the utxo_lookup field. The field is changed from RwLock<Option<U>> to a plain Option<U>, initialized only in P2PGossipSync::new. Tests are refactored to supply the UTXO verifier at construction time (build_graph_with_gossip_validation) or to skip UTXO validation explicitly (add_channel_skipping_utxo_update). The commit message frames this as eliminating a ‘hacky’ circular-reference workaround now that the architecture no longer relies on circular references.
Changed components
lightning/src/routing/gossip.rslightning/src/routing/router.rslightning/src/routing/test_utils.rsInspect captured patch +59 / −24
diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs
index e8fcb7b..040a28c 100644
--- a/lightning/src/routing/gossip.rs
+++ b/lightning/src/routing/gossip.rs
@@ -328,7 +328,10 @@ where
L::Target: Logger,
{
network_graph: G,
- utxo_lookup: RwLock<Option<U>>,
+ #[cfg(any(feature = "_test_utils", test))]
+ pub(super) utxo_lookup: Option<U>,
+ #[cfg(not(any(feature = "_test_utils", test)))]
+ utxo_lookup: Option<U>,
full_syncs_requested: AtomicUsize,
pending_events: Mutex<Vec<MessageSendEvent>>,
logger: L,
@@ -341,25 +344,19 @@ where
{
/// Creates a new tracker of the actual state of the network of channels and nodes,
/// assuming an existing [`NetworkGraph`].
+ ///
/// UTXO lookup is used to make sure announced channels exist on-chain, channel data is
/// correct, and the announcement is signed with channel owners' keys.
pub fn new(network_graph: G, utxo_lookup: Option<U>, logger: L) -> Self {
P2PGossipSync {
network_graph,
full_syncs_requested: AtomicUsize::new(0),
- utxo_lookup: RwLock::new(utxo_lookup),
+ utxo_lookup,
pending_events: Mutex::new(vec![]),
logger,
}
}
- /// Adds a provider used to check new announcements. Does not affect
- /// existing announcements unless they are updated.
- /// Add, update or remove the provider would replace the current one.
- pub fn add_utxo_lookup(&self, utxo_lookup: Option<U>) {
- *self.utxo_lookup.write().unwrap() = utxo_lookup;
- }
-
/// Gets a reference to the underlying [`NetworkGraph`] which was provided in
/// [`P2PGossipSync::new`].
///
@@ -564,8 +561,7 @@ where
fn handle_channel_announcement(
&self, _their_node_id: Option<PublicKey>, msg: &msgs::ChannelAnnouncement,
) -> Result<bool, LightningError> {
- self.network_graph
- .update_channel_from_announcement(msg, &*self.utxo_lookup.read().unwrap())?;
+ self.network_graph.update_channel_from_announcement(msg, &self.utxo_lookup)?;
Ok(msg.contents.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY)
}
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index c06e517..40580a0 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -3943,10 +3943,7 @@ mod tests {
ChannelUsage, FixedPenaltyScorer, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
ProbabilisticScoringFeeParameters, ScoreLookUp,
};
- use crate::routing::test_utils::{
- add_channel, add_or_update_node, build_graph, build_line_graph, get_nodes,
- id_to_feature_flags, update_channel,
- };
+ use crate::routing::test_utils::*;
use crate::routing::utxo::UtxoResult;
use crate::types::features::{BlindedHopFeatures, ChannelFeatures, InitFeatures, NodeFeatures};
use crate::util::config::UserConfig;
@@ -5368,7 +5365,7 @@ mod tests {
fn available_amount_while_routing_test() {
// Tests whether we choose the correct available channel amount while routing.
- let (secp_ctx, network_graph, gossip_sync, chain_monitor, logger) = build_graph();
+ let (secp_ctx, network_graph, gossip_sync, chain_monitor, logger) = build_graph_with_gossip_validation();
let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
let scorer = ln_test_utils::TestScorer::new();
let random_seed_bytes = [42; 32];
@@ -5588,11 +5585,10 @@ mod tests {
.push_opcode(opcodes::all::OP_PUSHNUM_2)
.push_opcode(opcodes::all::OP_CHECKMULTISIG).into_script().to_p2wsh();
+
*chain_monitor.utxo_ret.lock().unwrap() =
UtxoResult::Sync(Ok(TxOut { value: Amount::from_sat(15), script_pubkey: good_script.clone() }));
- gossip_sync.add_utxo_lookup(Some(chain_monitor));
-
- add_channel(&gossip_sync, &secp_ctx, &privkeys[0], &privkeys[2], ChannelFeatures::from_le_bytes(id_to_feature_flags(3)), 333);
+ add_channel_skipping_utxo_update(&gossip_sync, &secp_ctx, &privkeys[0], &privkeys[2], ChannelFeatures::from_le_bytes(id_to_feature_flags(3)), 333);
update_channel(&gossip_sync, &secp_ctx, &privkeys[0], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 333,
diff --git a/lightning/src/routing/test_utils.rs b/lightning/src/routing/test_utils.rs
index c5c35c9..a433fa3 100644
--- a/lightning/src/routing/test_utils.rs
+++ b/lightning/src/routing/test_utils.rs
@@ -10,7 +10,9 @@
// licenses.
use crate::routing::gossip::{NetworkGraph, NodeAlias, P2PGossipSync};
+use crate::routing::utxo::UtxoResult;
use crate::types::features::{ChannelFeatures, NodeFeatures};
+use crate::ln::chan_utils::make_funding_redeemscript;
use crate::ln::msgs::{ChannelAnnouncement, ChannelUpdate, MAX_VALUE_MSAT, NodeAnnouncement, RoutingMessageHandler, SocketAddress, UnsignedChannelAnnouncement, UnsignedChannelUpdate, UnsignedNodeAnnouncement};
use crate::util::test_utils;
use crate::util::ser::Writeable;
@@ -22,6 +24,7 @@ use bitcoin::hex::FromHex;
use bitcoin::network::Network;
use bitcoin::secp256k1::{PublicKey,SecretKey};
use bitcoin::secp256k1::{Secp256k1, All};
+use bitcoin::{Amount, TxOut};
#[allow(unused)]
use crate::prelude::*;
@@ -58,19 +61,34 @@ pub(crate) fn channel_announcement(
}
// Using the same keys for LN and BTC ids
-pub(crate) fn add_channel(
+pub(crate) fn add_channel_skipping_utxo_update(
gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
- secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey, node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64
+ secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey, node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64,
) {
let valid_announcement =
channel_announcement(node_1_privkey, node_2_privkey, features, short_channel_id, secp_ctx);
- let node_1_pubkey = PublicKey::from_secret_key(&secp_ctx, node_1_privkey);
+
+ let node_1_pubkey = PublicKey::from_secret_key(&secp_ctx, &node_1_privkey);
match gossip_sync.handle_channel_announcement(Some(node_1_pubkey), &valid_announcement) {
Ok(res) => assert!(res),
- _ => panic!()
+ Err(e) => panic!("{:?}", e),
};
}
+pub(crate) fn add_channel(
+ gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
+ secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey, node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64,
+) {
+ gossip_sync.utxo_lookup.as_ref().map(|checker| {
+ let node_1_pubkey = PublicKey::from_secret_key(&secp_ctx, &node_1_privkey);
+ let node_2_pubkey = PublicKey::from_secret_key(&secp_ctx, &node_2_privkey);
+ let script_pubkey = make_funding_redeemscript(&node_1_pubkey, &node_2_pubkey).to_p2wsh();
+ *checker.utxo_ret.lock().unwrap() =
+ UtxoResult::Sync(Ok(TxOut { value: Amount::from_sat(21_000_000_0000_0000), script_pubkey }));
+ });
+ add_channel_skipping_utxo_update(gossip_sync, secp_ctx, node_1_privkey, node_2_privkey, features, short_channel_id);
+}
+
pub(crate) fn add_or_update_node(
gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
secp_ctx: &Secp256k1<All>, node_privkey: &SecretKey, features: NodeFeatures, timestamp: u32
@@ -197,18 +215,43 @@ pub(super) fn build_line_graph() -> (
(secp_ctx, network_graph, gossip_sync, chain_monitor, logger)
}
+pub(super) fn build_graph_with_gossip_validation() -> (
+ Secp256k1<All>,
+ sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
+ P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
+ sync::Arc<test_utils::TestChainSource>,
+ sync::Arc<test_utils::TestLogger>,
+) {
+ do_build_graph(true)
+}
+
pub(super) fn build_graph() -> (
Secp256k1<All>,
sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
sync::Arc<test_utils::TestChainSource>,
sync::Arc<test_utils::TestLogger>,
+) {
+ do_build_graph(false)
+}
+
+fn do_build_graph(with_validation: bool) -> (
+ Secp256k1<All>,
+ sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
+ P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
+ sync::Arc<test_utils::TestChainSource>,
+ sync::Arc<test_utils::TestLogger>,
) {
let secp_ctx = Secp256k1::new();
let logger = Arc::new(test_utils::TestLogger::new());
let chain_monitor = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, Arc::clone(&logger)));
- let gossip_sync = P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger));
+ let checker = if with_validation {
+ Some(Arc::clone(&chain_monitor))
+ } else {
+ None
+ };
+ let gossip_sync = P2PGossipSync::new(Arc::clone(&network_graph), checker, Arc::clone(&logger));
// Build network from our_id to node6:
//
// -1(1)2- node0 -1(3)2-
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.