Swap PublicKey for NodeId in UnsignedChannelAnnouncement

Adds the macro `get_pubkey_from_node_id`
to parse `PublicKey`s back from `NodeId`s for signature
verification, as well as `make_funding_redeemscript_from_slices`
to avoid parsing back and forth between types.
This commit is contained in:
Alec Chen 2023-02-06 14:41:18 -06:00
parent 760ab65dbd
commit 1fd95496d1
9 changed files with 94 additions and 57 deletions

View file

@ -149,6 +149,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
}
}
macro_rules! get_pubkey_from_node_id {
($node_id: expr ) => {
match PublicKey::from_slice($node_id.as_slice()) {
Ok(pk) => pk,
Err(_) => return,
}
}
}
macro_rules! get_pubkey {
() => {
match PublicKey::from_slice(get_slice!(33)) {
@ -180,14 +189,14 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
},
1 => {
let msg = decode_msg_with_len16!(msgs::UnsignedChannelAnnouncement, 32+8+33*4);
node_pks.insert(msg.node_id_1);
node_pks.insert(msg.node_id_2);
node_pks.insert(get_pubkey_from_node_id!(msg.node_id_1));
node_pks.insert(get_pubkey_from_node_id!(msg.node_id_2));
let _ = net_graph.update_channel_from_unsigned_announcement::<&FuzzChainSource>(&msg, &None);
},
2 => {
let msg = decode_msg_with_len16!(msgs::UnsignedChannelAnnouncement, 32+8+33*4);
node_pks.insert(msg.node_id_1);
node_pks.insert(msg.node_id_2);
node_pks.insert(get_pubkey_from_node_id!(msg.node_id_1));
node_pks.insert(get_pubkey_from_node_id!(msg.node_id_2));
let _ = net_graph.update_channel_from_unsigned_announcement(&msg, &Some(&FuzzChainSource { input: Arc::clone(&input) }));
},
3 => {

View file

@ -23,6 +23,7 @@ use bitcoin::hash_types::{Txid, PubkeyHash};
use crate::ln::{PaymentHash, PaymentPreimage};
use crate::ln::msgs::DecodeError;
use crate::routing::gossip::NodeId;
use crate::util::ser::{Readable, Writeable, Writer};
use crate::util::transaction_utils;
@ -660,13 +661,17 @@ pub fn make_funding_redeemscript(broadcaster: &PublicKey, countersignatory: &Pub
let broadcaster_funding_key = broadcaster.serialize();
let countersignatory_funding_key = countersignatory.serialize();
make_funding_redeemscript_from_slices(&broadcaster_funding_key, &countersignatory_funding_key)
}
pub(crate) fn make_funding_redeemscript_from_slices(broadcaster_funding_key: &[u8], countersignatory_funding_key: &[u8]) -> Script {
let builder = Builder::new().push_opcode(opcodes::all::OP_PUSHNUM_2);
if broadcaster_funding_key[..] < countersignatory_funding_key[..] {
builder.push_slice(&broadcaster_funding_key)
.push_slice(&countersignatory_funding_key)
builder.push_slice(broadcaster_funding_key)
.push_slice(countersignatory_funding_key)
} else {
builder.push_slice(&countersignatory_funding_key)
.push_slice(&broadcaster_funding_key)
builder.push_slice(countersignatory_funding_key)
.push_slice(broadcaster_funding_key)
}.push_opcode(opcodes::all::OP_PUSHNUM_2).push_opcode(opcodes::all::OP_CHECKMULTISIG).into_script()
}

View file

@ -36,6 +36,7 @@ use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBounde
use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
use crate::chain::transaction::{OutPoint, TransactionData};
use crate::chain::keysinterface::{WriteableEcdsaChannelSigner, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
use crate::routing::gossip::NodeId;
use crate::util::events::ClosureReason;
use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
use crate::util::logger::Logger;
@ -5416,18 +5417,19 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned()));
}
let node_id = node_signer.get_node_id(Recipient::Node)
.map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?;
let were_node_one = node_id.serialize()[..] < self.counterparty_node_id.serialize()[..];
let node_id = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node)
.map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?);
let counterparty_node_id = NodeId::from_pubkey(&self.get_counterparty_node_id());
let were_node_one = node_id.as_slice() < counterparty_node_id.as_slice();
let msg = msgs::UnsignedChannelAnnouncement {
features: channelmanager::provided_channel_features(&user_config),
chain_hash,
short_channel_id: self.get_short_channel_id().unwrap(),
node_id_1: if were_node_one { node_id } else { self.get_counterparty_node_id() },
node_id_2: if were_node_one { self.get_counterparty_node_id() } else { node_id },
bitcoin_key_1: if were_node_one { self.get_holder_pubkeys().funding_pubkey } else { self.counterparty_funding_pubkey().clone() },
bitcoin_key_2: if were_node_one { self.counterparty_funding_pubkey().clone() } else { self.get_holder_pubkeys().funding_pubkey },
node_id_1: if were_node_one { node_id } else { counterparty_node_id },
node_id_2: if were_node_one { counterparty_node_id } else { node_id },
bitcoin_key_1: NodeId::from_pubkey(if were_node_one { &self.get_holder_pubkeys().funding_pubkey } else { self.counterparty_funding_pubkey() }),
bitcoin_key_2: NodeId::from_pubkey(if were_node_one { self.counterparty_funding_pubkey() } else { &self.get_holder_pubkeys().funding_pubkey }),
excess_data: Vec::new(),
};
@ -5497,8 +5499,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
&self, node_signer: &NS, announcement: msgs::UnsignedChannelAnnouncement
) -> Result<msgs::ChannelAnnouncement, ChannelError> where NS::Target: NodeSigner {
if let Some((their_node_sig, their_bitcoin_sig)) = self.announcement_sigs {
let our_node_key = node_signer.get_node_id(Recipient::Node)
.map_err(|_| ChannelError::Ignore("Signer failed to retrieve own public key".to_owned()))?;
let our_node_key = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node)
.map_err(|_| ChannelError::Ignore("Signer failed to retrieve own public key".to_owned()))?);
let were_node_one = announcement.node_id_1 == our_node_key;
let our_node_sig = node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelAnnouncement(&announcement))

View file

@ -46,6 +46,8 @@ use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer
use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
use crate::routing::gossip::NodeId;
/// 21 million * 10^8 * 1000
pub(crate) const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;
@ -698,13 +700,13 @@ pub struct UnsignedChannelAnnouncement {
/// The short channel ID
pub short_channel_id: u64,
/// One of the two `node_id`s which are endpoints of this channel
pub node_id_1: PublicKey,
pub node_id_1: NodeId,
/// The other of the two `node_id`s which are endpoints of this channel
pub node_id_2: PublicKey,
pub node_id_2: NodeId,
/// The funding key for the first node
pub bitcoin_key_1: PublicKey,
pub bitcoin_key_1: NodeId,
/// The funding key for the second node
pub bitcoin_key_2: PublicKey,
pub bitcoin_key_2: NodeId,
pub(crate) excess_data: Vec<u8>,
}
/// A [`channel_announcement`] message to be sent to or received from a peer.
@ -2053,6 +2055,7 @@ mod tests {
use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
use crate::ln::msgs;
use crate::ln::msgs::{FinalOnionHopData, OptionalField, OnionErrorPacket, OnionHopDataFormat};
use crate::routing::gossip::NodeId;
use crate::util::ser::{Writeable, Readable, Hostname};
use bitcoin::hashes::hex::FromHex;
@ -2160,10 +2163,10 @@ mod tests {
features,
chain_hash: BlockHash::from_hex("6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000").unwrap(),
short_channel_id: 2316138423780173,
node_id_1: pubkey_1,
node_id_2: pubkey_2,
bitcoin_key_1: pubkey_3,
bitcoin_key_2: pubkey_4,
node_id_1: NodeId::from_pubkey(&pubkey_1),
node_id_2: NodeId::from_pubkey(&pubkey_2),
bitcoin_key_1: NodeId::from_pubkey(&pubkey_3),
bitcoin_key_2: NodeId::from_pubkey(&pubkey_4),
excess_data: if excess_data { vec![10, 0, 0, 20, 0, 0, 30, 0, 0, 40] } else { Vec::new() },
};
let channel_announcement = msgs::ChannelAnnouncement {

View file

@ -27,7 +27,7 @@ use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor,NextNoiseStep};
use crate::ln::wire;
use crate::ln::wire::Encode;
use crate::onion_message::{CustomOnionMessageContents, CustomOnionMessageHandler, SimpleArcOnionMessenger, SimpleRefOnionMessenger};
use crate::routing::gossip::{NetworkGraph, P2PGossipSync};
use crate::routing::gossip::{NetworkGraph, P2PGossipSync, NodeId};
use crate::util::atomic_counter::AtomicCounter;
use crate::util::events::{MessageSendEvent, MessageSendEventsProvider, OnionMessageProvider};
use crate::util::logger::Logger;
@ -1467,9 +1467,11 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
log_gossip!(self.logger, "Skipping broadcast message to {:?} as its outbound buffer is full", peer.their_node_id);
continue;
}
if peer.their_node_id.as_ref() == Some(&msg.contents.node_id_1) ||
peer.their_node_id.as_ref() == Some(&msg.contents.node_id_2) {
continue;
if let Some(their_node_id) = peer.their_node_id {
let their_node_id = NodeId::from_pubkey(&their_node_id);
if their_node_id == msg.contents.node_id_1 || their_node_id == msg.contents.node_id_2 {
continue;
}
}
if except_node.is_some() && peer.their_node_id.as_ref() == except_node {
continue;

View file

@ -21,7 +21,7 @@ use bitcoin::hash_types::BlockHash;
use crate::chain;
use crate::chain::Access;
use crate::ln::chan_utils::make_funding_redeemscript;
use crate::ln::chan_utils::make_funding_redeemscript_from_slices;
use crate::ln::features::{ChannelFeatures, NodeFeatures, InitFeatures};
use crate::ln::msgs::{DecodeError, ErrorAction, Init, LightningError, RoutingMessageHandler, NetAddress, MAX_VALUE_MSAT};
use crate::ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, GossipTimestampFilter};
@ -326,6 +326,22 @@ macro_rules! secp_verify_sig {
};
}
macro_rules! get_pubkey_from_node_id {
( $node_id: expr, $msg_type: expr ) => {
PublicKey::from_slice($node_id.as_slice())
.map_err(|_| LightningError {
err: format!("Invalid public key on {} message", $msg_type),
action: ErrorAction::SendWarningMessage {
msg: msgs::WarningMessage {
channel_id: [0; 32],
data: format!("Invalid public key on {} message", $msg_type),
},
log_level: Level::Trace
}
})?
}
}
impl<G: Deref<Target=NetworkGraph<L>>, C: Deref, L: Deref> RoutingMessageHandler for P2PGossipSync<G, C, L>
where C::Target: chain::Access, L::Target: Logger
{
@ -1330,10 +1346,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
C::Target: chain::Access,
{
let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]);
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.node_signature_1, &msg.contents.node_id_1, "channel_announcement");
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.node_signature_2, &msg.contents.node_id_2, "channel_announcement");
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &msg.contents.bitcoin_key_1, "channel_announcement");
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_2, &msg.contents.bitcoin_key_2, "channel_announcement");
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.node_signature_1, &get_pubkey_from_node_id!(msg.contents.node_id_1, "channel_announcement"), "channel_announcement");
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.node_signature_2, &get_pubkey_from_node_id!(msg.contents.node_id_2, "channel_announcement"), "channel_announcement");
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &get_pubkey_from_node_id!(msg.contents.bitcoin_key_1, "channel_announcement"), "channel_announcement");
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_2, &get_pubkey_from_node_id!(msg.contents.bitcoin_key_2, "channel_announcement"), "channel_announcement");
self.update_channel_from_unsigned_announcement_intern(&msg.contents, Some(msg), chain_access)
}
@ -1438,9 +1454,6 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
return Err(LightningError{err: "Channel announcement node had a channel with itself".to_owned(), action: ErrorAction::IgnoreError});
}
let node_one = NodeId::from_pubkey(&msg.node_id_1);
let node_two = NodeId::from_pubkey(&msg.node_id_2);
{
let channels = self.channels.read().unwrap();
@ -1457,7 +1470,7 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
// We use the Node IDs rather than the bitcoin_keys to check for "equivalence"
// as we didn't (necessarily) store the bitcoin keys, and we only really care
// if the peers on the channel changed anyway.
if node_one == chan.node_one && node_two == chan.node_two {
if msg.node_id_1 == chan.node_one && msg.node_id_2 == chan.node_two {
return Err(LightningError {
err: "Already have chain-validated channel".to_owned(),
action: ErrorAction::IgnoreDuplicateGossip
@ -1478,8 +1491,8 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
let removed_channels = self.removed_channels.lock().unwrap();
let removed_nodes = self.removed_nodes.lock().unwrap();
if removed_channels.contains_key(&msg.short_channel_id) ||
removed_nodes.contains_key(&node_one) ||
removed_nodes.contains_key(&node_two) {
removed_nodes.contains_key(&msg.node_id_1) ||
removed_nodes.contains_key(&msg.node_id_2) {
return Err(LightningError{
err: format!("Channel with SCID {} or one of its nodes was removed from our network graph recently", &msg.short_channel_id),
action: ErrorAction::IgnoreAndLog(Level::Gossip)});
@ -1495,7 +1508,7 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
match chain_access.get_utxo(&msg.chain_hash, msg.short_channel_id) {
Ok(TxOut { value, script_pubkey }) => {
let expected_script =
make_funding_redeemscript(&msg.bitcoin_key_1, &msg.bitcoin_key_2).to_v0_p2wsh();
make_funding_redeemscript_from_slices(msg.bitcoin_key_1.as_slice(), msg.bitcoin_key_2.as_slice()).to_v0_p2wsh();
if script_pubkey != expected_script {
return Err(LightningError{err: format!("Channel announcement key ({}) didn't match on-chain script ({})", expected_script.to_hex(), script_pubkey.to_hex()), action: ErrorAction::IgnoreError});
}
@ -1522,9 +1535,9 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
let chan_info = ChannelInfo {
features: msg.features.clone(),
node_one,
node_one: msg.node_id_1,
one_to_two: None,
node_two,
node_two: msg.node_id_2,
two_to_one: None,
capacity_sats: utxo_value,
announcement_message: if msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY
@ -1987,10 +2000,10 @@ mod tests {
features: channelmanager::provided_channel_features(&UserConfig::default()),
chain_hash: genesis_block(Network::Testnet).header.block_hash(),
short_channel_id: 0,
node_id_1,
node_id_2,
bitcoin_key_1: PublicKey::from_secret_key(&secp_ctx, node_1_btckey),
bitcoin_key_2: PublicKey::from_secret_key(&secp_ctx, node_2_btckey),
node_id_1: NodeId::from_pubkey(&node_id_1),
node_id_2: NodeId::from_pubkey(&node_id_2),
bitcoin_key_1: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_1_btckey)),
bitcoin_key_2: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_2_btckey)),
excess_data: Vec::new(),
};
f(&mut unsigned_announcement);

View file

@ -1779,10 +1779,10 @@ mod tests {
features: channelmanager::provided_channel_features(&UserConfig::default()),
chain_hash: genesis_hash,
short_channel_id,
node_id_1: PublicKey::from_secret_key(&secp_ctx, &node_1_key),
node_id_2: PublicKey::from_secret_key(&secp_ctx, &node_2_key),
bitcoin_key_1: PublicKey::from_secret_key(&secp_ctx, &node_1_secret),
bitcoin_key_2: PublicKey::from_secret_key(&secp_ctx, &node_2_secret),
node_id_1: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_1_key)),
node_id_2: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_2_key)),
bitcoin_key_1: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_1_secret)),
bitcoin_key_2: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_2_secret)),
excess_data: Vec::new(),
};
let msghash = hash_to_message!(&Sha256dHash::hash(&unsigned_announcement.encode()[..])[..]);

View file

@ -27,13 +27,15 @@ use bitcoin::secp256k1::{Secp256k1, All};
use crate::prelude::*;
use crate::sync::{self, Arc};
use crate::routing::gossip::NodeId;
// Using the same keys for LN and BTC ids
pub(super) 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
) {
let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_1_privkey);
let node_id_2 = PublicKey::from_secret_key(&secp_ctx, node_2_privkey);
let node_id_1 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_1_privkey));
let node_id_2 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_2_privkey));
let unsigned_announcement = UnsignedChannelAnnouncement {
features,

View file

@ -23,6 +23,7 @@ use crate::ln::{msgs, wire};
use crate::ln::msgs::LightningError;
use crate::ln::script::ShutdownScript;
use crate::routing::gossip::NetworkGraph;
use crate::routing::gossip::NodeId;
use crate::routing::router::{find_route, InFlightHtlcs, Route, RouteHop, RouteParameters, Router, ScorerAccountingForInFlightHtlcs};
use crate::routing::scoring::FixedPenaltyScorer;
use crate::util::config::UserConfig;
@ -435,10 +436,10 @@ fn get_dummy_channel_announcement(short_chan_id: u64) -> msgs::ChannelAnnounceme
features: ChannelFeatures::empty(),
chain_hash: genesis_block(network).header.block_hash(),
short_channel_id: short_chan_id,
node_id_1: PublicKey::from_secret_key(&secp_ctx, &node_1_privkey),
node_id_2: PublicKey::from_secret_key(&secp_ctx, &node_2_privkey),
bitcoin_key_1: PublicKey::from_secret_key(&secp_ctx, &node_1_btckey),
bitcoin_key_2: PublicKey::from_secret_key(&secp_ctx, &node_2_btckey),
node_id_1: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_1_privkey)),
node_id_2: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_2_privkey)),
bitcoin_key_1: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_1_btckey)),
bitcoin_key_2: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_2_btckey)),
excess_data: Vec::new(),
};