-
Notifications
You must be signed in to change notification settings - Fork 417
Update channel_reestablish
for splicing
#3886
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?
Conversation
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
🔔 1st Reminder Hey @wpaulino! This PR has been waiting for your review. |
06c0dfc
to
f000b76
Compare
@wpaulino Ready for review now. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3886 +/- ##
==========================================
+ Coverage 88.83% 89.51% +0.68%
==========================================
Files 166 166
Lines 119259 128842 +9583
Branches 119259 128842 +9583
==========================================
+ Hits 105943 115339 +9396
- Misses 10988 11101 +113
- Partials 2328 2402 +74 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🔔 2nd Reminder Hey @wpaulino! This PR has been waiting for your review. |
🔔 3rd Reminder Hey @wpaulino! This PR has been waiting for your review. |
🔔 4th Reminder Hey @wpaulino! This PR has been waiting for your review. |
🔔 5th Reminder Hey @wpaulino! This PR has been waiting for your review. |
🔔 6th Reminder Hey @wpaulino! This PR has been waiting for your review. |
f000b76
to
e2ea3bf
Compare
Rebased. |
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.
We may need to double check our incoming channel_ready
handling to make sure we can handle receiving one long after channel_ready
was already exchanged due to the new logic surrounding your_last_funding_locked_txid
.
lightning/src/ln/channel.rs
Outdated
@@ -10128,10 +10135,52 @@ where | |||
} | |||
} | |||
|
|||
#[cfg(splicing)] | |||
fn maybe_get_your_last_funding_locked_txid(&self, features: &InitFeatures) -> Option<Txid> { | |||
if !features.supports_splicing() { |
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.
Since the TLVs are optional, I don't think we really need to check this and can always set them
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.. if the implementation supports splicing but didn't set the feature bit, won't they understand the TLV?
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.
Yes. This method is only for an outgoing channel_reestablish
so they'll just ignore the TLV if they don't support splicing.
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.
Ah, I was overloading "supporting" here. What I meant was that the other implementation supported splicing in the sense they have code written. But for whatever reason they chose not to set the feature bit in their init message. In that sense, they might not ignore the TLV?
I guess in our implementation here we don't really do any checks around this but should always do the right thing (i.e., never send splice_locked
) since in the above scenario as we'd never have any FundingScope
where is_splice
is true.
Though, it doesn't seem like we check their feature bits when handling any splice-related messages. Do we care if they tell us they don't support splicing but send us those messages anyway? At very least it seems we should check if they support splicing before attempting to initiate a splice.
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.
Do we care if they tell us they don't support splicing but send us those messages anyway?
I don't think so, them sending the message implies they should support it?
At very least it seems we should check if they support splicing before attempting to initiate a splice.
Yeah we should always check they support a feature before we attempt to use it. The channel_reestablish
TLVs are a bit different as they're not a feature on their own, and are odd so they can be ignored if not supported.
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.
They also have a perfectly valid meaning outside of a splicing context. I agree, I see no reason to not just always send them (even without the cfg).
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.
Alright, dropped the part needing init features.
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
e2ea3bf
to
69be701
Compare
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.
We may need to double check our incoming
channel_ready
handling to make sure we can handle receiving one long afterchannel_ready
was already exchanged due to the new logic surroundingyour_last_funding_locked_txid
.
ACK. Also need to address https://github.com/lightningdevkit/rust-lightning/pull/3736/files#r2133028859.
lightning/src/ln/channel.rs
Outdated
@@ -10128,10 +10135,52 @@ where | |||
} | |||
} | |||
|
|||
#[cfg(splicing)] | |||
fn maybe_get_your_last_funding_locked_txid(&self, features: &InitFeatures) -> Option<Txid> { | |||
if !features.supports_splicing() { |
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.. if the implementation supports splicing but didn't set the feature bit, won't they understand the TLV?
69be701
to
f7b9785
Compare
While splicing is not yet fully supported, checking if the feature has been negotiated is needed for changes to the channel_reestablish logic.
The splicing spec extends the channel_reestablish message with two more TLVs indicating which funding txid the sender has sent/received either explicitly via splice_locked or implicitly via channel_ready. This allows peers to detect if a splice_locked was lost during disconnection and must be retransmitted. This commit updates channel_reestablish with the TLVs. Subsequent commits will implement the spec requirements.
The previous commit extended the channel_reestablish message with your_last_funding_locked_txid and my_current_funding_locked_txid for use as described there. This commit sets those fields to the funding txid most recently sent/received accordingly.
f7b9785
to
b0291f4
Compare
Latest push adds some commits to address this. |
Looks like we have some logic below. I think the catch-all case still holds? We should only receive a rust-lightning/lightning/src/ln/channel.rs Lines 6491 to 6519 in 42085b9
|
// Clear the interactive transaction constructor | ||
self.interactive_tx_constructor.take(); | ||
self.interactive_tx_signing_session = Some(signing_session); | ||
self.context.channel_state.set_interactive_signing(); |
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.
Why not set the flag above?
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.
There are other error conditions, which could prevent us from assigning interactive_tx_signing_session
to Some
.
}) | ||
let signature = self.get_initial_counterparty_commitment_signature(funding, logger); | ||
if let Some(signature) = signature { | ||
log_info!(logger, "Generated commitment_signed for peer for channel {}", &self.channel_id()); |
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.
Debug/trace instead?
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.
This is maintaining the existing logging level. Should we reduce it?
@@ -5524,38 +5525,35 @@ where | |||
#[rustfmt::skip] | |||
fn get_initial_commitment_signed<L: Deref>( | |||
&mut self, funding: &FundingScope, logger: &L | |||
) -> Result<msgs::CommitmentSigned, ChannelError> | |||
) -> Option<msgs::CommitmentSigned> |
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.
Let's also rename the method to be v2 specific
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.
Added a commit renaming this and another similarly named method to make them consistent.
lightning/src/ln/channel.rs
Outdated
.ok_or_else(|| ChannelError::Close( | ||
( | ||
"Failed to get signatures for new commitment_signed".to_owned(), | ||
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }, |
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.
Should be a ProcessingError
instead
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.
Should we do the same in funding_tx_constructed
? It was already using HolderForceClosed
.
if !self.context.channel_state.is_their_tx_signatures_sent() | ||
&& !session.has_received_commitment_signed() | ||
{ | ||
return next_local_commitment_number + 1; |
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.
A comment here would be nice
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.
Added the spec requirements. Let me know if there is something more specific to our implementation that we should say. I can't quite recall the exact reason behind this.
lightning/src/ln/channel.rs
Outdated
// `splice_locked` it has sent: | ||
// - MUST retransmit `splice_locked`. | ||
let sent_splice_txid = self | ||
.maybe_get_my_current_funding_locked(their_features) |
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.
Isn't it enough to just check pending_splice.sent_funding_txid
?
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.
IIUC, we may have already promoted the splice.
.or_else(|| { | ||
msg.your_last_funding_locked_txid | ||
.and_then(|last_funding_txid| { | ||
funding_txid.filter(|funding_txid| last_funding_txid != *funding_txid) |
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.
So this is handling the case where we sent channel_ready
on txid A, they processed it, but now we have a channel_ready
for txid B that we need to send? Basically the following case in splicing but for channel_ready
:
let splice_locked = msg
// A receiving node:
// - if `your_last_funding_locked` is set and it does not match the most recent
// `splice_locked` it has sent:
// - MUST retransmit `splice_locked`.
.your_last_funding_locked_txid
.and_then(|last_funding_txid| {
sent_splice_txid.filter(|sent_splice_txid| last_funding_txid != *sent_splice_txid)
})
I don't think this is possible in LDK today, we seem to close the channel once the funding becomes unconfirmed after having sent channel_ready
(see do_best_block_updated
). I guess it can happen once we support dual funding RBF? One RBF confirms to one node, but another confirms to the other, and now we need to recover via this mechanism.
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.
Yeah, I think you're right. Though your_last_funding_locked_txid
is the one they said we sent them in channel_ready
. So for dual funding RBF it would need to be a re-org? i.e., we initially sent channel_ready
for your_last_funding_locked_txid
but while disconnected there was a re-org and another RBF candidate confirmed.
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.
Didn't get all the way through but a few comments.
lightning/src/ln/channel.rs
Outdated
@@ -10128,10 +10135,52 @@ where | |||
} | |||
} | |||
|
|||
#[cfg(splicing)] | |||
fn maybe_get_your_last_funding_locked_txid(&self, features: &InitFeatures) -> Option<Txid> { | |||
if !features.supports_splicing() { |
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.
They also have a perfectly valid meaning outside of a splicing context. I agree, I see no reason to not just always send them (even without the cfg).
// A node: | ||
// - if `option_splice` was negotiated and `your_last_funding_locked` is not | ||
// set in the `channel_reestablish` it received: | ||
// - MUST retransmit `channel_ready`. |
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.
Honestly I'm not sure what the harm is in duplicatively sending channel_ready
/splice_locked
? We could cut down on a lot of the code here if we just always sent it, and it seems like that should be fine, does the spec allow for it/require clients ignore extra retransmissions?
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.
Immediately after that part of the spec, it states:
- otherwise:
- MUST NOT retransmit `channel_ready`, but MAY send `channel_ready` with
a different `short_channel_id` `alias` field.
- upon reconnection:
- MUST ignore any redundant `channel_ready` it receives.
So it seems we may be able to just always re-transmit channel_ready
. I don't see anything explicit about splice_locked
. FWIW, I broached the subject about cleaning up the language in the spec recently: https://github.com/lightning/bolts/pull/1160/files#r2167803850
@@ -8401,94 +8401,215 @@ where | |||
let is_awaiting_remote_revoke = self.context.channel_state.is_awaiting_remote_revoke(); | |||
let next_counterparty_commitment_number = INITIAL_COMMITMENT_NUMBER - self.context.cur_counterparty_commitment_transaction_number + if is_awaiting_remote_revoke { 1 } else { 0 }; | |||
|
|||
let channel_ready = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.holder_commitment_point.transaction_number() == 1 { | |||
let splicing_negotiated = their_features.supports_splicing(); |
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.
We haven't "negtotiated" splicing unless we also set the bit, which is controlled by our config, so we need to check that.
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.
Will we want a config option? Or should we always send init
with the feature bit set?
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.
Oh, I guess maybe not? Might be worth just calling provided_init_features(config)
and checking if its set cause we may decide something else later.
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.
Right, though seems like we should also check their_features
given this would otherwise affect channel_ready
behavior when they don't support splicing but we do.
|
||
if let Some(session) = &self.interactive_tx_signing_session { | ||
if !self.context.channel_state.is_their_tx_signatures_sent() | ||
&& !session.has_received_commitment_signed() |
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.
Gah, so now we have a commitment number for which we only have a commitment tx on one funding context but not the others? Bleh.
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.
Hmmm... @wpaulino and I were walking through this the other day, but I can't recall exactly why self.holder_commitment_point.transaction_number()
would have advanced here.
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.
It feels wrong? Like, we should get a new commitment_signed
for the new funding at the same index as the current commitment_signed
we have for the existing funding, but its also possible the spec wants a new index, that just sucks.
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.
So the original requirements are:
The sending node:
- MUST set `next_commitment_number` to the commitment number of the
next `commitment_signed` it expects to receive.
Which in our code we had represented as:
INITIAL_COMMITMENT_NUMBER - self.holder_commitment_point.transaction_number()
IIUC, we are subtracting from INITIAL_COMMITMENT_NUMBER
because transaction_number
is initialized by INITIAL_COMMITMENT_NUMBER
(counting down) but next_commitment_number
is suppose to count up from 0, according to the spec:
`next_commitment_number`: A commitment number is a 48-bit
incrementing counter for each commitment transaction; counters
are independent for each peer in the channel and start at 0.
They're only explicitly relayed to the other node in the case of
re-establishment, otherwise they are implicit.
Then the splicing spec added the last two lines of the following:
- if it has sent `commitment_signed` for an interactive transaction construction but
it has not received `tx_signatures`:
- MUST set `next_funding_txid` to the txid of that interactive transaction.
- if it has not received `commitment_signed` for that interactive transaction:
- MUST set `next_commitment_number` to the commitment number of the `commitment_signed` it sent.
So I think the intention of adding 1
here was to get the previous commitment number (i.e., the one for thecommitment_signed
we had sent)... are we assuming was it advanced then? This is where I'm a little foggy. But if that's suppose to be the intention, shouldn't we subtract given next_commitment_number
is suppose to be counting up?
When splicing is negotiated, channel_ready must be retransmitted when your_last_funding_locked is not set. Further, the current logic for retransmitting channel_ready is only applicable when splicing is not negotiated.
The ChannelState::NegotiatingFunding assertion check in ChannelContext::get_initial_commitment_signed will fail when implementing splicing's channel_reestablish logic. In order to support it and channel establishment, enter ChannelState::FundingNegotiated prior to calling the method and update the assertion accordingly.
When ChannelContext::get_initial_commitment_signed is called for V2 channel establishment, any errors should result in closing the channel. However, in the future, when this is used for splicing it should abort instead of closing the channel. Move the error construction to the call sites in anticipation of this.
The splicing spec updates the logic pertaining to next_funding_txid when handling a channel_reestablish message. Specifically: A receiving node: - if `next_funding_txid` is set: - if `next_funding_txid` matches the latest interactive funding transaction or the current channel funding transaction: - if `next_commitment_number` is equal to the commitment number of the `commitment_signed` message it sent for this funding transaction: - MUST retransmit its `commitment_signed` for that funding transaction. - if it has already received `commitment_signed` and it should sign first, as specified in the [`tx_signatures` requirements](#the-tx_signatures-message): - MUST send its `tx_signatures` for that funding transaction. - if it has already received `tx_signatures` for that funding transaction: - MUST send its `tx_signatures` for that funding transaction. - if it also sets `next_funding_txid` in its own `channel_reestablish`, but the values don't match: - MUST send an `error` and fail the channel. - otherwise: - MUST send `tx_abort` to let the sending node know that they can forget this funding transaction. This commit updates FundedChannel::channel_reestablish accordingly. Co-authored-by: Wilmer Paulino <wilmer@wilmerpaulino.com> Co-authored-by: Jeffrey Czyz <jkczyz@gmail.com>
The splicing spec updates the logic pertaining to next_commitment_number when sending a channel_reestablish message. Specifically: The sending node: - if it has sent `commitment_signed` for an interactive transaction construction but it has not received `tx_signatures`: - MUST set `next_funding_txid` to the txid of that interactive transaction. - if it has not received `commitment_signed` for that interactive transaction: - MUST set `next_commitment_number` to the commitment number of the `commitment_signed` it sent.
The channel_reestablish protocol supports retransmitting splice_locked messages as needed. Add support for doing such when handling channel_reestablish messages.
The splicing spec updates channel_establishment logic to retransmit channel_ready or splice_locked for announced channels. Specifically: - if `my_current_funding_locked` is included: - if `announce_channel` is set for this channel: - if it has not received `announcement_signatures` for that transaction: - MUST retransmit `channel_ready` or `splice_locked` after exchanging `channel_reestablish`.
When a splice transaction is promoted (i.e., when splice_locked has been exchanged), announcement_signatures must be sent. However, if we try to send a channel_announcement before they are received, then the signatures will be incorrect. To avoid this, clear the counterparty's announcement_signatures upon promoting a FundingScope.
The channel_reestablish protocol supports retransmitting channel_ready messages as needed. Add support for doing such when handling channel_reestablish messages.
When handling a counterparties channel_reestablish, the spec dictates that a splice_locked may be implied by my_current_funding_locked. Compare that against any pending splices and handle an implicit splice_locked message when applicable.
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.
Need to dig into the spec for a couple comments still. Wanted to respond to everything else in the meanwhile.
@@ -8401,94 +8401,215 @@ where | |||
let is_awaiting_remote_revoke = self.context.channel_state.is_awaiting_remote_revoke(); | |||
let next_counterparty_commitment_number = INITIAL_COMMITMENT_NUMBER - self.context.cur_counterparty_commitment_transaction_number + if is_awaiting_remote_revoke { 1 } else { 0 }; | |||
|
|||
let channel_ready = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.holder_commitment_point.transaction_number() == 1 { | |||
let splicing_negotiated = their_features.supports_splicing(); |
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.
Will we want a config option? Or should we always send init
with the feature bit set?
// Clear the interactive transaction constructor | ||
self.interactive_tx_constructor.take(); | ||
self.interactive_tx_signing_session = Some(signing_session); | ||
self.context.channel_state.set_interactive_signing(); |
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.
There are other error conditions, which could prevent us from assigning interactive_tx_signing_session
to Some
.
}) | ||
let signature = self.get_initial_counterparty_commitment_signature(funding, logger); | ||
if let Some(signature) = signature { | ||
log_info!(logger, "Generated commitment_signed for peer for channel {}", &self.channel_id()); |
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.
This is maintaining the existing logging level. Should we reduce it?
lightning/src/ln/channel.rs
Outdated
.ok_or_else(|| ChannelError::Close( | ||
( | ||
"Failed to get signatures for new commitment_signed".to_owned(), | ||
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }, |
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.
Should we do the same in funding_tx_constructed
? It was already using HolderForceClosed
.
|
||
if let Some(session) = &self.interactive_tx_signing_session { | ||
if !self.context.channel_state.is_their_tx_signatures_sent() | ||
&& !session.has_received_commitment_signed() |
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.
Hmmm... @wpaulino and I were walking through this the other day, but I can't recall exactly why self.holder_commitment_point.transaction_number()
would have advanced here.
if !self.context.channel_state.is_their_tx_signatures_sent() | ||
&& !session.has_received_commitment_signed() | ||
{ | ||
return next_local_commitment_number + 1; |
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.
Added the spec requirements. Let me know if there is something more specific to our implementation that we should say. I can't quite recall the exact reason behind this.
lightning/src/ln/channel.rs
Outdated
// `splice_locked` it has sent: | ||
// - MUST retransmit `splice_locked`. | ||
let sent_splice_txid = self | ||
.maybe_get_my_current_funding_locked(their_features) |
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.
IIUC, we may have already promoted the splice.
lightning/src/ln/channel.rs
Outdated
@@ -10128,10 +10135,52 @@ where | |||
} | |||
} | |||
|
|||
#[cfg(splicing)] | |||
fn maybe_get_your_last_funding_locked_txid(&self, features: &InitFeatures) -> Option<Txid> { | |||
if !features.supports_splicing() { |
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.
Alright, dropped the part needing init features.
@@ -5524,38 +5525,35 @@ where | |||
#[rustfmt::skip] | |||
fn get_initial_commitment_signed<L: Deref>( | |||
&mut self, funding: &FundingScope, logger: &L | |||
) -> Result<msgs::CommitmentSigned, ChannelError> | |||
) -> Option<msgs::CommitmentSigned> |
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.
Added a commit renaming this and another similarly named method to make them consistent.
b0291f4
to
8004c6f
Compare
The splicing spec extends the
channel_reestablish
message with two more TLVs indicating which funding txid the sender has sent/received either explicitly viasplice_locked
or implicitly viachannel_ready
. This allows peers to detect if asplice_locked
was lost during disconnection and must be retransmitted.To this end, the spec updates the
channel_reestablish
logic to support splicing.