Skip to content

Commit

Permalink
Validate channel_update signatures without holding a graph lock
Browse files Browse the repository at this point in the history
We often process many gossip messages in parallel across different
peer connections, making the `NetworkGraph` mutexes fairly
contention-sensitive (not to mention the potential that we want to
send a payment and need to find a path to do so).

Because we need to look up a node's public key to validate a
signature on `channel_update` messages, we always need to take a
`NetworkGraph::channels` lock before we can validate the message.

For simplicity, and to avoid taking a lock twice, we'd always
validated the `channel_update` signature while holding the same
lock, but here we address the contention issues by doing a
`channel_update` validation in three stages.

First we take a read lock on `NetworkGraph::channels` and check if
the `channel_update` is new, then release the lock and validate the
message signature, and finally take a write lock, (re-check if the
`channel_update` is new) and update the graph.
  • Loading branch information
TheBlueMatt committed Sep 23, 2024
1 parent 0b7838b commit 131849f
Showing 1 changed file with 57 additions and 54 deletions.
111 changes: 57 additions & 54 deletions lightning/src/routing/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2294,62 +2294,65 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
}
};

let mut channels = self.channels.write().unwrap();
match channels.get_mut(&msg.short_channel_id) {
None => {
core::mem::drop(channels);
self.pending_checks.check_hold_pending_channel_update(msg, full_msg)?;
return Err(LightningError {
err: "Couldn't find channel for update".to_owned(),
action: ErrorAction::IgnoreAndLog(Level::Gossip),
});
},
Some(channel) => {
check_msg_sanity(channel)?;

macro_rules! get_new_channel_info {
() => { {
let last_update_message = if msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY
{ full_msg.cloned() } else { None };

let updated_channel_update_info = ChannelUpdateInfo {
enabled: chan_enabled,
last_update: msg.timestamp,
cltv_expiry_delta: msg.cltv_expiry_delta,
htlc_minimum_msat: msg.htlc_minimum_msat,
htlc_maximum_msat: msg.htlc_maximum_msat,
fees: RoutingFees {
base_msat: msg.fee_base_msat,
proportional_millionths: msg.fee_proportional_millionths,
},
last_update_message
};
Some(updated_channel_update_info)
} }
}

let msg_hash = hash_to_message!(&message_sha256d_hash(&msg)[..]);
if msg.channel_flags & 1 == 1 {
if let Some(sig) = sig {
secp_verify_sig!(self.secp_ctx, &msg_hash, &sig, &PublicKey::from_slice(channel.node_two.as_slice()).map_err(|_| LightningError{
let node_pubkey;
{
let channels = self.channels.read().unwrap();
match channels.get(&msg.short_channel_id) {
None => {
core::mem::drop(channels);
self.pending_checks.check_hold_pending_channel_update(msg, full_msg)?;
return Err(LightningError {
err: "Couldn't find channel for update".to_owned(),
action: ErrorAction::IgnoreAndLog(Level::Gossip),
});
},
Some(channel) => {
check_msg_sanity(channel)?;
let node_id = if msg.channel_flags & 1 == 1 {
channel.node_two.as_slice()
} else {
channel.node_one.as_slice()
};
node_pubkey = PublicKey::from_slice(node_id)
.map_err(|_| LightningError{
err: "Couldn't parse source node pubkey".to_owned(),
action: ErrorAction::IgnoreAndLog(Level::Debug)
})?, "channel_update");
}
if !only_verify {
channel.two_to_one = get_new_channel_info!();
}
} else {
if let Some(sig) = sig {
secp_verify_sig!(self.secp_ctx, &msg_hash, &sig, &PublicKey::from_slice(channel.node_one.as_slice()).map_err(|_| LightningError{
err: "Couldn't parse destination node pubkey".to_owned(),
action: ErrorAction::IgnoreAndLog(Level::Debug)
})?, "channel_update");
}
if !only_verify {
channel.one_to_two = get_new_channel_info!();
}
}
})?;
},
}
}

let msg_hash = hash_to_message!(&message_sha256d_hash(&msg)[..]);
if let Some(sig) = sig {
secp_verify_sig!(self.secp_ctx, &msg_hash, &sig, &node_pubkey, "channel_update");
}

if only_verify { return Ok(()); }

let mut channels = self.channels.write().unwrap();
if let Some(channel) = channels.get_mut(&msg.short_channel_id) {
check_msg_sanity(channel)?;

let last_update_message = if msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY
{ full_msg.cloned() } else { None };

let new_channel_info = Some(ChannelUpdateInfo {
enabled: chan_enabled,
last_update: msg.timestamp,
cltv_expiry_delta: msg.cltv_expiry_delta,
htlc_minimum_msat: msg.htlc_minimum_msat,
htlc_maximum_msat: msg.htlc_maximum_msat,
fees: RoutingFees {
base_msat: msg.fee_base_msat,
proportional_millionths: msg.fee_proportional_millionths,
},
last_update_message
});

if msg.channel_flags & 1 == 1 {
channel.two_to_one = new_channel_info;
} else {
channel.one_to_two = new_channel_info;
}
}

Expand Down

0 comments on commit 131849f

Please sign in to comment.