Skip to content
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

Jump to bottom

[WIP] New ChannelId enum #2451

Closed
wants to merge 2 commits into from
Closed

[WIP] New ChannelId enum #2451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2601287
New ChannelId type, wrapper for [u8; 32]
optout21 Jul 26, 2023
70fb868
ChannelId is an enum
optout21 Jul 26, 2023
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
3 changes: 2 additions & 1 deletion fuzz/src/full_stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use lightning::chain::transaction::OutPoint;
use lightning::sign::{InMemorySigner, Recipient, KeyMaterial, EntropySource, NodeSigner, SignerProvider};
use lightning::events::Event;
use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
use lightning::ln::channel::ChannelId;
use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentId, RecipientOnionFields, Retry};
use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor,IgnoringMessageHandler};
use lightning::ln::msgs::{self, DecodeError};
Expand Down Expand Up @@ -466,7 +467,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
let mut should_forward = false;
let mut payments_received: Vec<PaymentHash> = Vec::new();
let mut payments_sent = 0;
let mut pending_funding_generation: Vec<([u8; 32], PublicKey, u64, Script)> = Vec::new();
let mut pending_funding_generation: Vec<(ChannelId, PublicKey, u64, Script)> = Vec::new();
let mut pending_funding_signatures = HashMap::new();

loop {
Expand Down
3 changes: 2 additions & 1 deletion fuzz/src/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::BlockHash;

use lightning::chain::transaction::OutPoint;
use lightning::ln::channel::ChannelId;
use lightning::ln::channelmanager::{self, ChannelDetails, ChannelCounterparty};
use lightning::ln::msgs;
use lightning::routing::gossip::{NetworkGraph, RoutingFees};
Expand Down Expand Up @@ -240,7 +241,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
let rnid = node_pks.iter().skip(u16::from_be_bytes(get_slice!(2).try_into().unwrap()) as usize % node_pks.len()).next().unwrap();
let capacity = u64::from_be_bytes(get_slice!(8).try_into().unwrap());
first_hops_vec.push(ChannelDetails {
channel_id: [0; 32],
channel_id: ChannelId::new_zero(),
counterparty: ChannelCounterparty {
node_id: *rnid,
features: channelmanager::provided_init_features(&UserConfig::default()),
Expand Down
18 changes: 9 additions & 9 deletions lightning-invoice/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ where
log_trace!(logger, "Considering {} channels for invoice route hints", channels.len());
for channel in channels.into_iter().filter(|chan| chan.is_channel_ready) {
if channel.get_inbound_payment_scid().is_none() || channel.counterparty.forwarding_info.is_none() {
log_trace!(logger, "Ignoring channel {} for invoice route hints", log_bytes!(channel.channel_id));
log_trace!(logger, "Ignoring channel {} for invoice route hints", log_bytes!(channel.channel_id.bytes()[..]));
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're already touching it it would be nice to implement Display/Debug for ChannelId and log in a more readable form, e.g., as a hex string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just done this (in #2485), and I've seen this comment only now :)

continue;
}

Expand All @@ -641,7 +641,7 @@ where
// If any public channel exists, return no hints and let the sender
// look at the public channels instead.
log_trace!(logger, "Not including channels in invoice route hints on account of public channel {}",
log_bytes!(channel.channel_id));
log_bytes!(channel.channel_id.bytes()[..]));
return vec![].into_iter().take(MAX_CHANNEL_HINTS).map(route_hint_from_channel);
}
}
Expand Down Expand Up @@ -681,18 +681,18 @@ where
log_trace!(logger,
"Preferring counterparty {} channel {} (SCID {:?}, {} msats) over {} (SCID {:?}, {} msats) for invoice route hints",
log_pubkey!(channel.counterparty.node_id),
log_bytes!(channel.channel_id), channel.short_channel_id,
log_bytes!(channel.channel_id.bytes()[..]), channel.short_channel_id,
channel.inbound_capacity_msat,
log_bytes!(entry.get().channel_id), entry.get().short_channel_id,
log_bytes!(entry.get().channel_id.bytes()[..]), entry.get().short_channel_id,
current_max_capacity);
entry.insert(channel);
} else {
log_trace!(logger,
"Preferring counterparty {} channel {} (SCID {:?}, {} msats) over {} (SCID {:?}, {} msats) for invoice route hints",
log_pubkey!(channel.counterparty.node_id),
log_bytes!(entry.get().channel_id), entry.get().short_channel_id,
log_bytes!(entry.get().channel_id.bytes()[..]), entry.get().short_channel_id,
current_max_capacity,
log_bytes!(channel.channel_id), channel.short_channel_id,
log_bytes!(channel.channel_id.bytes()[..]), channel.short_channel_id,
channel.inbound_capacity_msat);
}
}
Expand Down Expand Up @@ -731,14 +731,14 @@ where

if include_channel {
log_trace!(logger, "Including channel {} in invoice route hints",
log_bytes!(channel.channel_id));
log_bytes!(channel.channel_id.bytes()[..]));
} else if !has_enough_capacity {
log_trace!(logger, "Ignoring channel {} without enough capacity for invoice route hints",
log_bytes!(channel.channel_id));
log_bytes!(channel.channel_id.bytes()[..]));
} else {
debug_assert!(!channel.is_usable || (has_pub_unconf_chan && !channel.is_public));
log_trace!(logger, "Ignoring channel {} with disconnected peer",
log_bytes!(channel.channel_id));
log_bytes!(channel.channel_id.bytes()[..]));
}

include_channel
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2538,7 +2538,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
} else if !self.holder_tx_signed {
log_error!(logger, "WARNING: You have a potentially-unsafe holder commitment transaction available to broadcast");
log_error!(logger, " in channel monitor for channel {}!", log_bytes!(self.funding_info.0.to_channel_id()));
log_error!(logger, " in channel monitor for channel {}!", log_bytes!(self.funding_info.0.to_channel_id().bytes()[..]));
log_error!(logger, " Read the docs for ChannelMonitor::get_latest_holder_commitment_txn and take manual action!");
} else {
// If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager
Expand Down Expand Up @@ -3196,7 +3196,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
if prevout.txid == self.funding_info.0.txid && prevout.vout == self.funding_info.0.index as u32 {
let mut balance_spendable_csv = None;
log_info!(logger, "Channel {} closed by funding output spend in txid {}.",
log_bytes!(self.funding_info.0.to_channel_id()), txid);
log_bytes!(self.funding_info.0.to_channel_id().bytes()[..]), txid);
self.funding_spend_seen = true;
let mut commitment_tx_to_counterparty_output = None;
if (tx.input[0].sequence.0 >> 8*3) as u8 == 0x80 && (tx.lock_time.0 >> 8*3) as u8 == 0x20 {
Expand Down
9 changes: 5 additions & 4 deletions lightning/src/chain/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

//! Types describing on-chain transactions.

use crate::ln::channel::ChannelId;
use bitcoin::hash_types::Txid;
use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
use bitcoin::blockdata::transaction::Transaction;
Expand Down Expand Up @@ -57,12 +58,12 @@ pub struct OutPoint {

impl OutPoint {
/// Convert an `OutPoint` to a lightning channel id.
pub fn to_channel_id(&self) -> [u8; 32] {
pub fn to_channel_id(&self) -> ChannelId {
let mut res = [0; 32];
res[..].copy_from_slice(&self.txid[..]);
res[30] ^= ((self.index >> 8) & 0xff) as u8;
res[31] ^= ((self.index >> 0) & 0xff) as u8;
res
ChannelId::new_funding_tx_based(res)
}

/// Converts this OutPoint into the OutPoint field as used by rust-bitcoin
Expand Down Expand Up @@ -94,10 +95,10 @@ mod tests {
assert_eq!(&OutPoint {
txid: tx.txid(),
index: 0
}.to_channel_id(), &hex::decode("3e88dd7165faf7be58b3c5bb2c9c452aebef682807ea57080f62e6f6e113c25e").unwrap()[..]);
}.to_channel_id().bytes()[..], &hex::decode("3e88dd7165faf7be58b3c5bb2c9c452aebef682807ea57080f62e6f6e113c25e").unwrap()[..]);
assert_eq!(&OutPoint {
txid: tx.txid(),
index: 1
}.to_channel_id(), &hex::decode("3e88dd7165faf7be58b3c5bb2c9c452aebef682807ea57080f62e6f6e113c25f").unwrap()[..]);
}.to_channel_id().bytes()[..], &hex::decode("3e88dd7165faf7be58b3c5bb2c9c452aebef682807ea57080f62e6f6e113c25f").unwrap()[..]);
}
}
36 changes: 18 additions & 18 deletions lightning/src/events/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub use bump_transaction::BumpTransactionEvent;

use crate::sign::SpendableOutputDescriptor;
use crate::ln::channelmanager::{InterceptId, PaymentId, RecipientOnionFields};
use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS;
use crate::ln::channel::{ChannelId, FUNDING_CONF_DEADLINE_BLOCKS};
use crate::ln::features::ChannelTypeFeatures;
use crate::ln::msgs;
use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
Expand Down Expand Up @@ -215,7 +215,7 @@ pub enum HTLCDestination {
/// counterparty node information.
node_id: Option<PublicKey>,
/// The outgoing `channel_id` between us and the next node.
channel_id: [u8; 32],
channel_id: ChannelId,
},
/// Scenario where we are unsure of the next node to forward the HTLC to.
UnknownNextHop {
Expand Down Expand Up @@ -333,7 +333,7 @@ pub enum Event {
/// [`ChannelManager::funding_transaction_generated`].
///
/// [`ChannelManager::funding_transaction_generated`]: crate::ln::channelmanager::ChannelManager::funding_transaction_generated
temporary_channel_id: [u8; 32],
temporary_channel_id: ChannelId,
/// The counterparty's node_id, which you'll need to pass back into
/// [`ChannelManager::funding_transaction_generated`].
///
Expand Down Expand Up @@ -411,7 +411,7 @@ pub enum Event {
/// payment is to pay an invoice or to send a spontaneous payment.
purpose: PaymentPurpose,
/// The `channel_id` indicating over which channel we received the payment.
via_channel_id: Option<[u8; 32]>,
via_channel_id: Option<ChannelId>,
/// The `user_channel_id` indicating over which channel we received the payment.
via_user_channel_id: Option<u128>,
/// The block height at which this payment will be failed back and will no longer be
Expand Down Expand Up @@ -663,10 +663,10 @@ pub enum Event {
PaymentForwarded {
/// The incoming channel between the previous node and us. This is only `None` for events
/// generated or serialized by versions prior to 0.0.107.
prev_channel_id: Option<[u8; 32]>,
prev_channel_id: Option<ChannelId>,
/// The outgoing channel between the next node and us. This is only `None` for events
/// generated or serialized by versions prior to 0.0.107.
next_channel_id: Option<[u8; 32]>,
next_channel_id: Option<ChannelId>,
/// The fee, in milli-satoshis, which was earned as a result of the payment.
///
/// Note that if we force-closed the channel over which we forwarded an HTLC while the HTLC
Expand Down Expand Up @@ -697,7 +697,7 @@ pub enum Event {
/// [`Event::ChannelReady`] event.
ChannelPending {
/// The `channel_id` of the channel that is pending confirmation.
channel_id: [u8; 32],
channel_id: ChannelId,
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound
/// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if
/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
Expand All @@ -710,7 +710,7 @@ pub enum Event {
/// The `temporary_channel_id` this channel used to be known by during channel establishment.
///
/// Will be `None` for channels created prior to LDK version 0.0.115.
former_temporary_channel_id: Option<[u8; 32]>,
former_temporary_channel_id: Option<ChannelId>,
/// The `node_id` of the channel counterparty.
counterparty_node_id: PublicKey,
/// The outpoint of the channel's funding transaction.
Expand All @@ -722,7 +722,7 @@ pub enum Event {
/// establishment.
ChannelReady {
/// The `channel_id` of the channel that is ready.
channel_id: [u8; 32],
channel_id: ChannelId,
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound
/// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if
/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
Expand All @@ -742,7 +742,7 @@ pub enum Event {
ChannelClosed {
/// The `channel_id` of the channel which has been closed. Note that on-chain transactions
/// resolving the channel are likely still awaiting confirmation.
channel_id: [u8; 32],
channel_id: ChannelId,
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound
/// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if
/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
Expand All @@ -761,7 +761,7 @@ pub enum Event {
/// inputs for another purpose.
DiscardFunding {
/// The channel_id of the channel which has been closed.
channel_id: [u8; 32],
channel_id: ChannelId,
/// The full transaction received from the user
transaction: Transaction
},
Expand All @@ -785,7 +785,7 @@ pub enum Event {
///
/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
/// [`ChannelManager::force_close_without_broadcasting_txn`]: crate::ln::channelmanager::ChannelManager::force_close_without_broadcasting_txn
temporary_channel_id: [u8; 32],
temporary_channel_id: ChannelId,
/// The node_id of the counterparty requesting to open the channel.
///
/// When responding to the request, the `counterparty_node_id` should be passed
Expand Down Expand Up @@ -831,7 +831,7 @@ pub enum Event {
/// requirements (i.e. insufficient fees paid, or a CLTV that is too soon).
HTLCHandlingFailed {
/// The channel over which the HTLC was received.
prev_channel_id: [u8; 32],
prev_channel_id: ChannelId,
/// Destination of the HTLC that failed to be processed.
failed_next_destination: HTLCDestination,
},
Expand Down Expand Up @@ -1248,7 +1248,7 @@ impl MaybeReadable for Event {
},
9u8 => {
let f = || {
let mut channel_id = [0; 32];
let mut channel_id = ChannelId::new_zero();
let mut reason = UpgradableRequired(None);
let mut user_channel_id_low_opt: Option<u64> = None;
let mut user_channel_id_high_opt: Option<u64> = None;
Expand All @@ -1271,7 +1271,7 @@ impl MaybeReadable for Event {
},
11u8 => {
let f = || {
let mut channel_id = [0; 32];
let mut channel_id = ChannelId::new_zero();
let mut transaction = Transaction{ version: 2, lock_time: PackedLockTime::ZERO, input: Vec::new(), output: Vec::new() };
read_tlv_fields!(reader, {
(0, channel_id, required),
Expand Down Expand Up @@ -1376,7 +1376,7 @@ impl MaybeReadable for Event {
},
25u8 => {
let f = || {
let mut prev_channel_id = [0; 32];
let mut prev_channel_id = ChannelId::new_zero();
let mut failed_next_destination_opt = UpgradableRequired(None);
read_tlv_fields!(reader, {
(0, prev_channel_id, required),
Expand All @@ -1392,7 +1392,7 @@ impl MaybeReadable for Event {
27u8 => Ok(None),
29u8 => {
let f = || {
let mut channel_id = [0; 32];
let mut channel_id = ChannelId::new_zero();
let mut user_channel_id: u128 = 0;
let mut counterparty_node_id = RequiredWrapper(None);
let mut channel_type = RequiredWrapper(None);
Expand All @@ -1414,7 +1414,7 @@ impl MaybeReadable for Event {
},
31u8 => {
let f = || {
let mut channel_id = [0; 32];
let mut channel_id = ChannelId::new_zero();
let mut user_channel_id: u128 = 0;
let mut former_temporary_channel_id = None;
let mut counterparty_node_id = RequiredWrapper(None);
Expand Down
Loading
Loading