-
Notifications
You must be signed in to change notification settings - Fork 436
Integrate Splicing with Quiescence #4019
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
Changes from all commits
2a6501a
92f1961
a206b4d
73a6d97
9200308
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 |
|---|---|---|
|
|
@@ -2448,13 +2448,46 @@ impl PendingSplice { | |
| } | ||
| } | ||
|
|
||
| pub(crate) struct SpliceInstructions { | ||
| adjusted_funding_contribution: SignedAmount, | ||
| our_funding_inputs: Vec<FundingTxInput>, | ||
| our_funding_outputs: Vec<TxOut>, | ||
| change_script: Option<ScriptBuf>, | ||
| funding_feerate_per_kw: u32, | ||
| locktime: u32, | ||
| original_funding_txo: OutPoint, | ||
| } | ||
|
|
||
| impl_writeable_tlv_based!(SpliceInstructions, { | ||
| (1, adjusted_funding_contribution, required), | ||
| (3, our_funding_inputs, required_vec), | ||
| (5, our_funding_outputs, required_vec), | ||
| (7, change_script, option), | ||
| (9, funding_feerate_per_kw, required), | ||
| (11, locktime, required), | ||
| (13, original_funding_txo, required), | ||
| }); | ||
|
|
||
| pub(crate) enum QuiescentAction { | ||
| // TODO: Make this test-only once we have another variant (as some code requires *a* variant). | ||
| Splice(SpliceInstructions), | ||
| #[cfg(any(test, fuzzing))] | ||
| DoNothing, | ||
| } | ||
|
|
||
| pub(crate) enum StfuResponse { | ||
| Stfu(msgs::Stfu), | ||
| #[cfg_attr(not(splicing), allow(unused))] | ||
| SpliceInit(msgs::SpliceInit), | ||
| } | ||
|
|
||
| #[cfg(any(test, fuzzing))] | ||
| impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, | ||
| (99, DoNothing) => {}, | ||
| (0, DoNothing) => {}, | ||
| {1, Splice} => (), | ||
| ); | ||
| #[cfg(not(any(test, fuzzing)))] | ||
| impl_writeable_tlv_based_enum_upgradable!(QuiescentAction,, | ||
| {1, Splice} => (), | ||
| ); | ||
|
|
||
| /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`]. | ||
|
|
@@ -5982,7 +6015,7 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos | |
| fn check_splice_contribution_sufficient( | ||
| channel_balance: Amount, contribution: &SpliceContribution, is_initiator: bool, | ||
| funding_feerate: FeeRate, | ||
| ) -> Result<Amount, ChannelError> { | ||
| ) -> Result<Amount, String> { | ||
| let contribution_amount = contribution.value(); | ||
| if contribution_amount < SignedAmount::ZERO { | ||
| let estimated_fee = Amount::from_sat(estimate_v2_funding_transaction_fee( | ||
|
|
@@ -5996,10 +6029,10 @@ fn check_splice_contribution_sufficient( | |
| if channel_balance >= contribution_amount.unsigned_abs() + estimated_fee { | ||
| Ok(estimated_fee) | ||
| } else { | ||
| Err(ChannelError::Warn(format!( | ||
| "Available channel balance {} is lower than needed for splicing out {}, considering fees of {}", | ||
| channel_balance, contribution_amount.unsigned_abs(), estimated_fee, | ||
| ))) | ||
| Err(format!( | ||
| "Available channel balance {channel_balance} is lower than needed for splicing out {}, considering fees of {estimated_fee}", | ||
| contribution_amount.unsigned_abs(), | ||
| )) | ||
| } | ||
| } else { | ||
| check_v2_funding_inputs_sufficient( | ||
|
|
@@ -6066,7 +6099,7 @@ fn estimate_v2_funding_transaction_fee( | |
| fn check_v2_funding_inputs_sufficient( | ||
| contribution_amount: i64, funding_inputs: &[FundingTxInput], is_initiator: bool, | ||
| is_splice: bool, funding_feerate_sat_per_1000_weight: u32, | ||
| ) -> Result<u64, ChannelError> { | ||
| ) -> Result<u64, String> { | ||
| let estimated_fee = estimate_v2_funding_transaction_fee( | ||
| funding_inputs, &[], is_initiator, is_splice, funding_feerate_sat_per_1000_weight, | ||
| ); | ||
|
|
@@ -6089,10 +6122,9 @@ fn check_v2_funding_inputs_sufficient( | |
|
|
||
| let minimal_input_amount_needed = contribution_amount.saturating_add(estimated_fee as i64); | ||
| if (total_input_sats as i64) < minimal_input_amount_needed { | ||
| Err(ChannelError::Warn(format!( | ||
| "Total input amount {} is lower than needed for contribution {}, considering fees of {}. Need more inputs.", | ||
| total_input_sats, contribution_amount, estimated_fee, | ||
| ))) | ||
| Err(format!( | ||
| "Total input amount {total_input_sats} is lower than needed for contribution {contribution_amount}, considering fees of {estimated_fee}. Need more inputs.", | ||
| )) | ||
| } else { | ||
| Ok(estimated_fee) | ||
| } | ||
|
|
@@ -10749,9 +10781,13 @@ where | |
| /// - `change_script`: an option change output script. If `None` and needed, one will be | ||
| /// generated by `SignerProvider::get_destination_script`. | ||
| #[cfg(splicing)] | ||
| pub fn splice_channel( | ||
| pub fn splice_channel<L: Deref>( | ||
| &mut self, contribution: SpliceContribution, funding_feerate_per_kw: u32, locktime: u32, | ||
| ) -> Result<msgs::SpliceInit, APIError> { | ||
| logger: &L, | ||
| ) -> Result<Option<msgs::Stfu>, APIError> | ||
| where | ||
| L::Target: Logger, | ||
| { | ||
| if self.holder_commitment_point.current_point().is_none() { | ||
| return Err(APIError::APIMisuseError { | ||
| err: format!( | ||
|
|
@@ -10763,7 +10799,7 @@ where | |
|
|
||
| // Check if a splice has been initiated already. | ||
| // Note: only a single outstanding splice is supported (per spec) | ||
| if self.pending_splice.is_some() { | ||
| if self.pending_splice.is_some() || self.quiescent_action.is_some() { | ||
| return Err(APIError::APIMisuseError { | ||
| err: format!( | ||
| "Channel {} cannot be spliced, as it has already a splice pending", | ||
|
|
@@ -10781,8 +10817,6 @@ where | |
| }); | ||
| } | ||
|
|
||
| // TODO(splicing): check for quiescence | ||
|
|
||
| let our_funding_contribution = contribution.value(); | ||
| if our_funding_contribution == SignedAmount::ZERO { | ||
| return Err(APIError::APIMisuseError { | ||
|
|
@@ -10877,8 +10911,50 @@ where | |
| } | ||
| } | ||
|
|
||
| let prev_funding_input = self.funding.to_splice_funding_input(); | ||
| let original_funding_txo = self.funding.get_funding_txo().ok_or_else(|| { | ||
wpaulino marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| debug_assert!(false); | ||
| APIError::APIMisuseError { err: "Channel isn't yet fully funded".to_owned() } | ||
| })?; | ||
|
|
||
| let (our_funding_inputs, our_funding_outputs, change_script) = contribution.into_tx_parts(); | ||
|
|
||
| let action = QuiescentAction::Splice(SpliceInstructions { | ||
| adjusted_funding_contribution, | ||
| our_funding_inputs, | ||
| our_funding_outputs, | ||
| change_script, | ||
| funding_feerate_per_kw, | ||
| locktime, | ||
| original_funding_txo, | ||
| }); | ||
| self.propose_quiescence(logger, action) | ||
| .map_err(|e| APIError::APIMisuseError { err: e.to_owned() }) | ||
TheBlueMatt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| #[cfg(splicing)] | ||
| fn send_splice_init( | ||
| &mut self, instructions: SpliceInstructions, | ||
| ) -> Result<msgs::SpliceInit, String> { | ||
| let SpliceInstructions { | ||
| adjusted_funding_contribution, | ||
| our_funding_inputs, | ||
| our_funding_outputs, | ||
| change_script, | ||
| funding_feerate_per_kw, | ||
| locktime, | ||
| original_funding_txo, | ||
| } = instructions; | ||
|
|
||
| // Check if a splice has been initiated already. | ||
| // Note: only a single outstanding splice is supported (per spec) | ||
| if self.pending_splice.is_some() { | ||
| return Err(format!( | ||
| "Channel {} cannot be spliced, as it has already a splice pending", | ||
| self.context.channel_id(), | ||
| )); | ||
| } | ||
|
|
||
| let prev_funding_input = self.funding.to_splice_funding_input(); | ||
| let funding_negotiation_context = FundingNegotiationContext { | ||
| is_initiator: true, | ||
| our_funding_contribution: adjusted_funding_contribution, | ||
|
|
@@ -11034,6 +11110,10 @@ where | |
| ES::Target: EntropySource, | ||
| L::Target: Logger, | ||
| { | ||
| if !self.context.channel_state.is_quiescent() { | ||
wpaulino marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return Err(ChannelError::WarnAndDisconnect("Quiescence needed to splice".to_owned())); | ||
| } | ||
|
|
||
| let our_funding_contribution = SignedAmount::from_sat(our_funding_contribution_satoshis); | ||
| let splice_funding = self.validate_splice_init(msg, our_funding_contribution)?; | ||
|
|
||
|
|
@@ -11073,6 +11153,11 @@ where | |
| })?; | ||
| debug_assert!(interactive_tx_constructor.take_initiator_first_message().is_none()); | ||
|
|
||
| // TODO(splicing): if quiescent_action is set, integrate what the user wants to do into the | ||
| // counterparty-initiated splice. For always-on nodes this probably isn't a useful | ||
| // optimization, but for often-offline nodes it may be, as we may connect and immediately | ||
| // go into splicing from both sides. | ||
|
|
||
| let funding_pubkey = splice_funding.get_holder_pubkeys().funding_pubkey; | ||
|
|
||
| self.pending_splice = Some(PendingSplice { | ||
|
|
@@ -11821,23 +11906,21 @@ where | |
| ); | ||
| } | ||
|
|
||
| #[cfg(any(test, fuzzing))] | ||
| #[cfg(any(splicing, test, fuzzing))] | ||
| #[rustfmt::skip] | ||
| pub fn propose_quiescence<L: Deref>( | ||
| &mut self, logger: &L, action: QuiescentAction, | ||
| ) -> Result<Option<msgs::Stfu>, ChannelError> | ||
| ) -> Result<Option<msgs::Stfu>, &'static str> | ||
| where | ||
| L::Target: Logger, | ||
| { | ||
| log_debug!(logger, "Attempting to initiate quiescence"); | ||
|
|
||
| if !self.context.is_usable() { | ||
| return Err(ChannelError::Ignore( | ||
| "Channel is not in a usable state to propose quiescence".to_owned() | ||
| )); | ||
| return Err("Channel is not in a usable state to propose quiescence"); | ||
| } | ||
| if self.quiescent_action.is_some() { | ||
| return Err(ChannelError::Ignore("Channel is already quiescing".to_owned())); | ||
| return Err("Channel already has a pending quiescent action and cannot start another"); | ||
| } | ||
|
|
||
| self.quiescent_action = Some(action); | ||
|
|
@@ -11858,7 +11941,7 @@ where | |
|
|
||
| // Assumes we are either awaiting quiescence or our counterparty has requested quiescence. | ||
| #[rustfmt::skip] | ||
| pub fn send_stfu<L: Deref>(&mut self, logger: &L) -> Result<msgs::Stfu, ChannelError> | ||
| pub fn send_stfu<L: Deref>(&mut self, logger: &L) -> Result<msgs::Stfu, &'static str> | ||
|
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. May as well return
Collaborator
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. I think we'd end up with strictly more |
||
| where | ||
| L::Target: Logger, | ||
| { | ||
|
|
@@ -11872,9 +11955,7 @@ where | |
| if self.context.is_waiting_on_peer_pending_channel_update() | ||
| || self.context.is_monitor_or_signer_pending_channel_update() | ||
| { | ||
| return Err(ChannelError::Ignore( | ||
| "We cannot send `stfu` while state machine is pending".to_owned() | ||
| )); | ||
| return Err("We cannot send `stfu` while state machine is pending") | ||
| } | ||
|
|
||
| let initiator = if self.context.channel_state.is_remote_stfu_sent() { | ||
|
|
@@ -11900,7 +11981,7 @@ where | |
| #[rustfmt::skip] | ||
| pub fn stfu<L: Deref>( | ||
| &mut self, msg: &msgs::Stfu, logger: &L | ||
| ) -> Result<Option<msgs::Stfu>, ChannelError> where L::Target: Logger { | ||
| ) -> Result<Option<StfuResponse>, ChannelError> where L::Target: Logger { | ||
| if self.context.channel_state.is_quiescent() { | ||
| return Err(ChannelError::Warn("Channel is already quiescent".to_owned())); | ||
| } | ||
|
|
@@ -11931,7 +12012,10 @@ where | |
| self.context.channel_state.set_remote_stfu_sent(); | ||
|
|
||
| log_debug!(logger, "Received counterparty stfu proposing quiescence"); | ||
| return self.send_stfu(logger).map(|stfu| Some(stfu)); | ||
| return self | ||
| .send_stfu(logger) | ||
| .map(|stfu| Some(StfuResponse::Stfu(stfu))) | ||
| .map_err(|e| ChannelError::Ignore(e.to_owned())); | ||
| } | ||
|
|
||
| // We already sent `stfu` and are now processing theirs. It may be in response to ours, or | ||
|
|
@@ -11972,6 +12056,13 @@ where | |
| "Internal Error: Didn't have anything to do after reaching quiescence".to_owned() | ||
| )); | ||
| }, | ||
| Some(QuiescentAction::Splice(_instructions)) => { | ||
| #[cfg(splicing)] | ||
| return self.send_splice_init(_instructions) | ||
| .map(|splice_init| Some(StfuResponse::SpliceInit(splice_init))) | ||
| .map_err(|e| ChannelError::WarnAndDisconnect(e.to_owned())); | ||
| }, | ||
| #[cfg(any(test, fuzzing))] | ||
| Some(QuiescentAction::DoNothing) => { | ||
| // In quiescence test we want to just hang out here, letting the test manually | ||
| // leave quiescence. | ||
|
|
@@ -12004,7 +12095,10 @@ where | |
| || (self.context.channel_state.is_remote_stfu_sent() | ||
| && !self.context.channel_state.is_local_stfu_sent()) | ||
| { | ||
| return self.send_stfu(logger).map(|stfu| Some(stfu)); | ||
| return self | ||
| .send_stfu(logger) | ||
| .map(|stfu| Some(stfu)) | ||
| .map_err(|e| ChannelError::Ignore(e.to_owned())); | ||
| } | ||
|
|
||
| // We're either: | ||
|
|
@@ -16205,8 +16299,8 @@ mod tests { | |
| 2000, | ||
| ); | ||
| assert_eq!( | ||
| format!("{:?}", res.err().unwrap()), | ||
| "Warn: Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1746. Need more inputs.", | ||
| res.err().unwrap(), | ||
| "Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1746. Need more inputs.", | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -16241,8 +16335,8 @@ mod tests { | |
| 2200, | ||
| ); | ||
| assert_eq!( | ||
| format!("{:?}", res.err().unwrap()), | ||
| "Warn: Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2522. Need more inputs.", | ||
| res.err().unwrap(), | ||
| "Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2522. Need more inputs.", | ||
| ); | ||
| } | ||
|
|
||
|
|
||
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 can be dropped now
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 do in a followup, thanks.