-
Notifications
You must be signed in to change notification settings - Fork 456
Include interactive funding candidates on broadcast #4570
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,9 +15,11 @@ | |
|
|
||
| use core::{cmp, ops::Deref}; | ||
|
|
||
| use crate::ln::funding::FundingContribution; | ||
| use crate::ln::types::ChannelId; | ||
| use crate::prelude::*; | ||
|
|
||
| use bitcoin::hash_types::Txid; | ||
| use bitcoin::secp256k1::PublicKey; | ||
| use bitcoin::transaction::Transaction; | ||
|
|
||
|
|
@@ -104,19 +106,74 @@ pub enum TransactionType { | |
| /// A single sweep transaction may aggregate outputs from multiple channels. | ||
| channels: Vec<(PublicKey, ChannelId)>, | ||
| }, | ||
| /// A splice transaction modifying an existing channel's funding. | ||
| /// An interactively-negotiated funding transaction. | ||
| /// | ||
| /// A transaction of this type will be broadcast as a result of a [`ChannelManager::splice_channel`] operation. | ||
| /// A transaction of this type will be broadcast as a result of a | ||
| /// [`ChannelManager::splice_channel`] operation, or (once supported) V2 (dual-funded) channel | ||
| /// establishment. The same variant is used for batches of either or both. | ||
| /// | ||
| /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel | ||
| Splice { | ||
| /// The `node_id` of the channel counterparty. | ||
| counterparty_node_id: PublicKey, | ||
| /// The ID of the channel being spliced. | ||
| channel_id: ChannelId, | ||
| InteractiveFunding { | ||
| /// Every negotiated candidate for this funding in order: the original negotiation | ||
| /// followed by any RBF replacements. The last entry is the candidate being broadcast. | ||
| candidates: Vec<FundingCandidate>, | ||
| }, | ||
| } | ||
|
|
||
| /// A single negotiated candidate within a [`TransactionType::InteractiveFunding`] broadcast. | ||
| /// | ||
| /// The candidate is identified by its [`Txid`] and lists the channels participating in it. A | ||
| /// single candidate funds more than one channel only when batching splices and/or V2 channel | ||
| /// openings (not yet implemented). | ||
| #[derive(Clone, Debug, Hash, PartialEq, Eq)] | ||
| pub struct FundingCandidate { | ||
| /// The txid of this candidate. | ||
| pub txid: Txid, | ||
| /// The channels participating in this candidate. | ||
| pub channels: Vec<ChannelFunding>, | ||
| } | ||
|
|
||
| /// Information about a single channel's participation in a [`FundingCandidate`]. | ||
| #[derive(Clone, Debug, Hash, PartialEq, Eq)] | ||
| pub struct ChannelFunding { | ||
| /// The `node_id` of the channel counterparty. | ||
| pub counterparty_node_id: PublicKey, | ||
| /// The ID of the channel. | ||
| pub channel_id: ChannelId, | ||
| /// Whether this channel is being newly established or is an existing channel being spliced. | ||
| pub purpose: FundingPurpose, | ||
| /// The local node's contribution to this channel in this candidate, or `None` if we did | ||
| /// not contribute (e.g., a pure acceptor with zero value added, or a leading RBF round | ||
| /// before we began contributing). | ||
| pub contribution: Option<FundingContribution>, | ||
| } | ||
|
|
||
| /// The role of a channel within a [`FundingCandidate`]. | ||
| #[derive(Clone, Debug, Hash, PartialEq, Eq)] | ||
| pub enum FundingPurpose { | ||
| /// The channel is being newly established (V2 dual-funded open). | ||
| Establishment, | ||
| /// An existing channel is being spliced. | ||
| Splice, | ||
| } | ||
|
|
||
| impl_writeable_tlv_based!(FundingCandidate, { | ||
| (1, txid, required), | ||
| (3, channels, required_vec), | ||
| }); | ||
|
|
||
| impl_writeable_tlv_based!(ChannelFunding, { | ||
| (1, counterparty_node_id, required), | ||
| (3, channel_id, required), | ||
| (5, purpose, required), | ||
| (7, contribution, option), | ||
| }); | ||
|
|
||
| impl_writeable_tlv_based_enum!(FundingPurpose, | ||
| (0, Establishment) => {}, | ||
| (2, Splice) => {}, | ||
| ); | ||
|
Comment on lines
+160
to
+175
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: these three serialization impls ( If these are forward-looking (e.g., for future persistence or cross-process messaging), a brief comment explaining the intent would help. Otherwise they could be removed to avoid confusion.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need these for downstream
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth adding a comment? |
||
|
|
||
| // TODO: Define typed abstraction over feerates to handle their conversions. | ||
| pub(crate) fn compute_feerate_sat_per_1000_weight(fee_sat: u64, weight: u64) -> u32 { | ||
| (fee_sat * 1000 / weight).try_into().unwrap_or(u32::max_value()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,8 @@ use bitcoin::{secp256k1, sighash, FeeRate, Sequence, TxIn}; | |
|
|
||
| use crate::blinded_path::message::BlindedMessagePath; | ||
| use crate::chain::chaininterface::{ | ||
| ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator, TransactionType, | ||
| ChannelFunding, ConfirmationTarget, FeeEstimator, FundingCandidate, FundingPurpose, | ||
| LowerBoundedFeeEstimator, TransactionType, | ||
| }; | ||
| use crate::chain::channelmonitor::{ | ||
| ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, CommitmentHTLCData, | ||
|
|
@@ -9382,10 +9383,34 @@ where | |
| ); | ||
| } | ||
|
|
||
| let tx_type = TransactionType::Splice { | ||
| counterparty_node_id: self.context.counterparty_node_id, | ||
| channel_id: self.context.channel_id, | ||
| }; | ||
| let contrib_offset = pending_splice | ||
| .negotiated_candidates | ||
| .len() | ||
| .saturating_sub(pending_splice.contributions.len()); | ||
| let candidates = pending_splice | ||
| .negotiated_candidates | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(i, funding)| { | ||
| let txid = funding | ||
| .get_funding_txid() | ||
| .expect("negotiated candidates should have a funding txid"); | ||
| let contribution = i | ||
| .checked_sub(contrib_offset) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the offset always 0 here? We're currently exchanging |
||
| .and_then(|j| pending_splice.contributions.get(j)) | ||
| .cloned(); | ||
| FundingCandidate { | ||
| txid, | ||
| channels: vec![ChannelFunding { | ||
| counterparty_node_id: self.context.counterparty_node_id, | ||
| channel_id: self.context.channel_id, | ||
| purpose: FundingPurpose::Splice, | ||
| contribution, | ||
| }], | ||
| } | ||
| }) | ||
| .collect(); | ||
| let tx_type = TransactionType::InteractiveFunding { candidates }; | ||
| funding_tx_signed.funding_tx = Some((funding_tx, tx_type)); | ||
| funding_tx_signed.splice_negotiated = Some(splice_negotiated); | ||
| funding_tx_signed.splice_locked = splice_locked; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, unfortunate that
counterparty_node_id/channel_idare moved now. I see that keeping them would be prohibitive in the batching case, but it seems this will make classification / attribution way less intuitive downstream. But of course we already have essentially the same concern with theSweepvariant.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... they are still here just deeper in the structs. Similar how the the
Fundingvariant has them in aVecfor v1 batches. For interactive funding, we may not know which set will be applicable until confirmed since an RBF may add / remove channels when batching.