Skip to content

Integrate Splicing with Quiescence #4007

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
138 changes: 106 additions & 32 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2443,13 +2443,42 @@ impl PendingSplice {
}
}

pub(crate) struct SpliceInstructions {
Copy link
Contributor

Choose a reason for hiding this comment

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

Meh, this is all changing very soon with #3979, can we just pull this commit out and rebase it on top of that?

our_funding_contribution_satoshis: i64,
our_funding_inputs: Vec<(TxIn, Transaction, Weight)>,
change_script: Option<ScriptBuf>,
funding_feerate_per_kw: u32,
locktime: u32,
}

impl_writeable_tlv_based!(SpliceInstructions, {
(1, our_funding_contribution_satoshis, required),
(3, our_funding_inputs, required_vec),
(5, change_script, option),
(7, funding_feerate_per_kw, required),
(9, locktime, 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`].
Expand Down Expand Up @@ -5926,7 +5955,7 @@ fn estimate_v2_funding_transaction_fee(
fn check_v2_funding_inputs_sufficient(
contribution_amount: i64, funding_inputs: &[(TxIn, Transaction, Weight)], is_initiator: bool,
is_splice: bool, funding_feerate_sat_per_1000_weight: u32,
) -> Result<u64, ChannelError> {
) -> Result<u64, String> {
let mut total_input_witness_weight = Weight::from_wu(funding_inputs.iter().map(|(_, _, w)| w.to_wu()).sum());
let mut funding_inputs_len = funding_inputs.len();
if is_initiator && is_splice {
Expand All @@ -5941,10 +5970,10 @@ fn check_v2_funding_inputs_sufficient(
if let Some(output) = input.1.output.get(input.0.previous_output.vout as usize) {
total_input_sats = total_input_sats.saturating_add(output.value.to_sat());
} else {
return Err(ChannelError::Warn(format!(
return Err(format!(
"Transaction with txid {} does not have an output with vout of {} corresponding to TxIn at funding_inputs[{}]",
input.1.compute_txid(), input.0.previous_output.vout, idx
)));
));
}
}

Expand All @@ -5961,10 +5990,10 @@ 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!(
Err(format!(
"Total input amount {} is lower than needed for contribution {}, considering fees of {}. Need more inputs.",
total_input_sats, contribution_amount, estimated_fee,
)))
))
} else {
Ok(estimated_fee)
}
Expand Down Expand Up @@ -10614,11 +10643,14 @@ 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, our_funding_contribution_satoshis: i64,
our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, change_script: Option<ScriptBuf>,
funding_feerate_per_kw: u32, locktime: u32,
) -> Result<msgs::SpliceInit, APIError> {
funding_feerate_per_kw: u32, locktime: u32, logger: &L,
) -> Result<Option<msgs::Stfu>, APIError>
where
L::Target: Logger,
{
// Check if a splice has been initiated already.
// Note: only a single outstanding splice is supported (per spec)
if self.pending_splice.is_some() {
Expand Down Expand Up @@ -10671,11 +10703,44 @@ where
err,
),
})?;
// Convert inputs
let mut funding_inputs = Vec::new();
for (tx_in, tx, _w) in our_funding_inputs.into_iter() {
let tx16 = TransactionU16LenLimited::new(tx)
.map_err(|_e| APIError::APIMisuseError { err: format!("Too large transaction") })?;

// TODO(splicing): Check that transactions aren't too big for the splice_init message here.

let action = QuiescentAction::Splice(SpliceInstructions {
our_funding_contribution_satoshis,
our_funding_inputs,
change_script,
funding_feerate_per_kw,
locktime,
});
self.propose_quiescence(logger, action)
.map_err(|e| APIError::APIMisuseError { err: e.to_owned() })
}

#[cfg(splicing)]
fn send_splice_init(
&mut self, instructions: SpliceInstructions,
) -> Result<msgs::SpliceInit, String> {
let SpliceInstructions {
our_funding_contribution_satoshis,
our_funding_inputs,
change_script,
funding_feerate_per_kw,
locktime,
} = instructions;

// Check that the channel value hasn't changed out from under us.
let _fee = check_v2_funding_inputs_sufficient(
our_funding_contribution_satoshis,
&our_funding_inputs,
true,
true,
funding_feerate_per_kw,
)?;

let mut funding_inputs = Vec::with_capacity(our_funding_inputs.len());
for (tx_in, tx, _weight) in our_funding_inputs {
let tx16 = TransactionU16LenLimited::new(tx).map_err(|_e| "tx too big".to_owned())?;
funding_inputs.push((tx_in, tx16));
}

Expand Down Expand Up @@ -11567,23 +11632,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.context.post_quiescence_action.is_some() {
return Err(ChannelError::Ignore("Channel is already quiescing".to_owned()));
return Err("Channel is already quiescing");
}

self.context.post_quiescence_action = Some(action);
Expand All @@ -11604,7 +11667,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>
where
L::Target: Logger,
{
Expand All @@ -11618,9 +11681,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() {
Expand All @@ -11646,7 +11707,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()));
}
Expand Down Expand Up @@ -11677,7 +11738,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
Expand Down Expand Up @@ -11718,6 +11782,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::Ignore(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.
Expand Down Expand Up @@ -11750,7 +11821,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:
Expand Down Expand Up @@ -15920,8 +15994,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 1730. Need more inputs.",
res.err().unwrap(),
"Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1730. Need more inputs.",
);
}

Expand Down Expand Up @@ -15956,8 +16030,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 2495. Need more inputs.",
res.err().unwrap(),
"Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2495. Need more inputs.",
);
}

Expand Down
49 changes: 30 additions & 19 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ use crate::ln::channel::QuiescentAction;
use crate::ln::channel::{
self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, FundedChannel,
InboundV1Channel, OutboundV1Channel, PendingV2Channel, ReconnectionMsg, ShutdownResult,
UpdateFulfillCommitFetch, WithChannelContext,
StfuResponse, UpdateFulfillCommitFetch, WithChannelContext,
};
use crate::ln::channel_state::ChannelDetails;
use crate::ln::inbound_payment;
Expand Down Expand Up @@ -4503,17 +4503,21 @@ where
hash_map::Entry::Occupied(mut chan_phase_entry) => {
let locktime = locktime.unwrap_or_else(|| self.current_best_block().height);
if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() {
let msg = chan.splice_channel(
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
let msg_opt = chan.splice_channel(
our_funding_contribution_satoshis,
our_funding_inputs,
change_script,
funding_feerate_per_kw,
locktime,
&&logger,
)?;
peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceInit {
node_id: *counterparty_node_id,
msg,
});
if let Some(msg) = msg_opt {
peer_state.pending_msg_events.push(MessageSendEvent::SendStfu {
node_id: *counterparty_node_id,
msg,
});
}
Ok(())
} else {
Err(APIError::ChannelUnavailable {
Expand Down Expand Up @@ -10841,22 +10845,31 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
));
}

let mut sent_stfu = false;
match peer_state.channel_by_id.entry(msg.channel_id) {
hash_map::Entry::Occupied(mut chan_entry) => {
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
let logger = WithContext::from(
&self.logger, Some(*counterparty_node_id), Some(msg.channel_id), None
);

if let Some(stfu) = try_channel_entry!(
self, peer_state, chan.stfu(&msg, &&logger), chan_entry
) {
sent_stfu = true;
peer_state.pending_msg_events.push(MessageSendEvent::SendStfu {
node_id: *counterparty_node_id,
msg: stfu,
});
let res = chan.stfu(&msg, &&logger);
let resp = try_channel_entry!(self, peer_state, res, chan_entry);
match resp {
None => Ok(false),
Some(StfuResponse::Stfu(msg)) => {
peer_state.pending_msg_events.push(MessageSendEvent::SendStfu {
node_id: *counterparty_node_id,
msg,
});
Ok(true)
},
Some(StfuResponse::SpliceInit(msg)) => {
peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceInit {
node_id: *counterparty_node_id,
msg,
});
Ok(true)
},
}
} else {
let msg = "Peer sent `stfu` for an unfunded channel";
Expand All @@ -10871,8 +10884,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
msg.channel_id
))
}

Ok(sent_stfu)
}

#[rustfmt::skip]
Expand Down Expand Up @@ -13884,8 +13895,8 @@ where
let persist = match &res {
Err(e) if e.closes_channel() => NotifyOption::DoPersist,
Err(_) => NotifyOption::SkipPersistHandleEvents,
Ok(sent_stfu) => {
if *sent_stfu {
Ok(responded) => {
if *responded {
NotifyOption::SkipPersistHandleEvents
} else {
NotifyOption::SkipPersistNoEvents
Expand Down
16 changes: 11 additions & 5 deletions lightning/src/ln/splicing_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ fn test_v1_splice_in() {
let acceptor_node_index = 1;
let initiator_node = &nodes[initiator_node_index];
let acceptor_node = &nodes[acceptor_node_index];
let initiator_node_id = initiator_node.node.get_our_node_id();
let acceptor_node_id = acceptor_node.node.get_our_node_id();

let channel_value_sat = 100_000;
let channel_reserve_amnt_sat = 1_000;
Expand Down Expand Up @@ -79,12 +81,16 @@ fn test_v1_splice_in() {
None, // locktime
)
.unwrap();

let init_stfu = get_event_msg!(initiator_node, MessageSendEvent::SendStfu, acceptor_node_id);
acceptor_node.node.handle_stfu(initiator_node_id, &init_stfu);

let ack_stfu = get_event_msg!(acceptor_node, MessageSendEvent::SendStfu, initiator_node_id);
initiator_node.node.handle_stfu(acceptor_node_id, &ack_stfu);

// Extract the splice_init message
let splice_init_msg = get_event_msg!(
initiator_node,
MessageSendEvent::SendSpliceInit,
acceptor_node.node.get_our_node_id()
);
let splice_init_msg =
get_event_msg!(initiator_node, MessageSendEvent::SendSpliceInit, acceptor_node_id);
assert_eq!(splice_init_msg.funding_contribution_satoshis, splice_in_sats as i64);
assert_eq!(splice_init_msg.funding_feerate_per_kw, funding_feerate_per_kw);
assert_eq!(splice_init_msg.funding_pubkey.to_string(), expected_initiator_funding_key);
Expand Down