Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ enum NodeError {
"InvalidNodeAlias",
"InvalidDateTime",
"InvalidFeeRate",
"InvalidScriptPubKey",
"DuplicatePayment",
"UnsupportedCurrency",
"InsufficientFunds",
Expand Down Expand Up @@ -575,6 +576,7 @@ dictionary ChannelDetails {
ChannelId channel_id;
PublicKey counterparty_node_id;
OutPoint? funding_txo;
ScriptBuf? funding_redeem_script;
u64? short_channel_id;
u64? outbound_scid_alias;
u64? inbound_scid_alias;
Expand Down Expand Up @@ -901,3 +903,6 @@ typedef string LSPS1OrderId;

[Custom]
typedef string LSPSDateTime;

[Custom]
typedef string ScriptBuf;
3 changes: 3 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ pub enum Error {
InvalidDateTime,
/// The given fee rate is invalid.
InvalidFeeRate,
/// The given script public key is invalid.
InvalidScriptPubKey,
/// A payment with the given hash has already been initiated.
DuplicatePayment,
/// The provided offer was denonminated in an unsupported currency.
Expand Down Expand Up @@ -186,6 +188,7 @@ impl fmt::Display for Error {
Self::InvalidNodeAlias => write!(f, "The given node alias is invalid."),
Self::InvalidDateTime => write!(f, "The given date time is invalid."),
Self::InvalidFeeRate => write!(f, "The given fee rate is invalid."),
Self::InvalidScriptPubKey => write!(f, "The given script pubkey is invalid."),
Self::DuplicatePayment => {
write!(f, "A payment with the given hash has already been initiated.")
},
Expand Down
18 changes: 17 additions & 1 deletion src/ffi/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub use bip39::Mnemonic;
use bitcoin::hashes::sha256::Hash as Sha256;
use bitcoin::hashes::Hash;
use bitcoin::secp256k1::PublicKey;
pub use bitcoin::{Address, BlockHash, FeeRate, Network, OutPoint, Txid};
pub use bitcoin::{Address, BlockHash, FeeRate, Network, OutPoint, ScriptBuf, Txid};
pub use lightning::chain::channelmonitor::BalanceSource;
pub use lightning::events::{ClosureReason, PaymentFailureReason};
use lightning::ln::channelmanager::PaymentId;
Expand Down Expand Up @@ -106,6 +106,22 @@ impl UniffiCustomTypeConverter for Address {
}
}

impl UniffiCustomTypeConverter for ScriptBuf {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: While this should be fine for now, we generally try to get away from the UniffiCustomTypeConverter approach, and instead convert more and more objects to proper interfaces.

type Builtin = String;

fn into_custom(val: Self::Builtin) -> uniffi::Result<Self> {
if let Ok(key) = ScriptBuf::from_hex(&val) {
return Ok(key);
}

Err(Error::InvalidScriptPubKey.into())
}

fn from_custom(obj: Self) -> Self::Builtin {
obj.to_string()
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum OfferAmount {
Bitcoin { amount_msats: u64 },
Expand Down
38 changes: 26 additions & 12 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ use io::utils::write_node_metrics;
use lightning::chain::BestBlock;
use lightning::events::bump_transaction::{Input, Wallet as LdkWallet};
use lightning::impl_writeable_tlv_based;
use lightning::ln::chan_utils::{make_funding_redeemscript, FUNDING_TRANSACTION_WITNESS_WEIGHT};
use lightning::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT;
use lightning::ln::channel_state::{ChannelDetails as LdkChannelDetails, ChannelShutdownState};
use lightning::ln::channelmanager::PaymentId;
use lightning::ln::funding::SpliceContribution;
Expand Down Expand Up @@ -1267,29 +1267,27 @@ impl Node {
const EMPTY_SCRIPT_SIG_WEIGHT: u64 =
1 /* empty script_sig */ * bitcoin::constants::WITNESS_SCALE_FACTOR as u64;

// Used for creating a redeem script for the previous funding txo and the new funding
// txo. Only needed when selecting which UTXOs to include in the funding tx that would
// be sufficient to pay for fees. Hence, the value does not matter.
let dummy_pubkey = PublicKey::from_slice(&[2; 33]).unwrap();

let funding_txo = channel_details.funding_txo.ok_or_else(|| {
log_error!(self.logger, "Failed to splice channel: channel not yet ready",);
Error::ChannelSplicingFailed
})?;

let funding_output = channel_details.get_funding_output().ok_or_else(|| {
log_error!(self.logger, "Failed to splice channel: channel not yet ready");
Error::ChannelSplicingFailed
})?;

let shared_input = Input {
outpoint: funding_txo.into_bitcoin_outpoint(),
previous_utxo: bitcoin::TxOut {
value: Amount::from_sat(channel_details.channel_value_satoshis),
script_pubkey: make_funding_redeemscript(&dummy_pubkey, &dummy_pubkey)
.to_p2wsh(),
},
previous_utxo: funding_output.clone(),
satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + FUNDING_TRANSACTION_WITNESS_WEIGHT,
};

let shared_output = bitcoin::TxOut {
value: shared_input.previous_utxo.value + Amount::from_sat(splice_amount_sats),
script_pubkey: make_funding_redeemscript(&dummy_pubkey, &dummy_pubkey).to_p2wsh(),
// will not actually be the exact same script pubkey after splice
// but it is the same size and good enough for coin selection purposes
script_pubkey: funding_output.script_pubkey.clone(),
};

let fee_rate = self.fee_estimator.estimate_fee_rate(ConfirmationTarget::ChannelFunding);
Expand All @@ -1305,6 +1303,10 @@ impl Node {
Error::ChannelSplicingFailed
})?;

// insert channel's funding utxo into the wallet so we can later calculate fees
// correctly when viewing this splice-in.
self.wallet.insert_txo(funding_txo.into_bitcoin_outpoint(), funding_output)?;

let change_address = self.wallet.get_new_internal_address()?;

let contribution = SpliceContribution::SpliceIn {
Expand Down Expand Up @@ -1400,6 +1402,18 @@ impl Node {
},
};

let funding_txo = channel_details.funding_txo.ok_or_else(|| {
log_error!(self.logger, "Failed to splice channel: channel not yet ready",);
Error::ChannelSplicingFailed
})?;

let funding_output = channel_details.get_funding_output().ok_or_else(|| {
log_error!(self.logger, "Failed to splice channel: channel not yet ready");
Error::ChannelSplicingFailed
})?;

self.wallet.insert_txo(funding_txo.into_bitcoin_outpoint(), funding_output)?;

self.channel_manager
.splice_channel(
&channel_details.channel_id,
Expand Down
12 changes: 11 additions & 1 deletion src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::fmt;
use std::sync::{Arc, Mutex};

use bitcoin::secp256k1::PublicKey;
use bitcoin::OutPoint;
use bitcoin::{OutPoint, ScriptBuf};
use lightning::chain::chainmonitor;
use lightning::impl_writeable_tlv_based;
use lightning::ln::channel_state::ChannelDetails as LdkChannelDetails;
Expand Down Expand Up @@ -222,6 +222,15 @@ pub struct ChannelDetails {
/// state until the splice transaction reaches sufficient confirmations to be locked (and we
/// exchange `splice_locked` messages with our peer).
pub funding_txo: Option<OutPoint>,
/// The witness script that is used to lock the channel's funding output to commitment transactions.
///
/// This field will be `None` if we have not negotiated the funding transaction with our
/// counterparty already.
///
/// When a channel is spliced, this continues to refer to the original pre-splice channel
/// state until the splice transaction reaches sufficient confirmations to be locked (and we
/// exchange `splice_locked` messages with our peer).
pub funding_redeem_script: Option<ScriptBuf>,
/// The position of the funding transaction in the chain. None if the funding transaction has
/// not yet been confirmed and the channel fully opened.
///
Expand Down Expand Up @@ -378,6 +387,7 @@ impl From<LdkChannelDetails> for ChannelDetails {
channel_id: value.channel_id,
counterparty_node_id: value.counterparty.node_id,
funding_txo: value.funding_txo.map(|o| o.into_bitcoin_outpoint()),
funding_redeem_script: value.funding_redeem_script,
short_channel_id: value.short_channel_id,
outbound_scid_alias: value.outbound_scid_alias,
inbound_scid_alias: value.inbound_scid_alias,
Expand Down
15 changes: 14 additions & 1 deletion src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use bitcoin::secp256k1::ecdh::SharedSecret;
use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature};
use bitcoin::secp256k1::{All, PublicKey, Scalar, Secp256k1, SecretKey};
use bitcoin::{
Address, Amount, FeeRate, ScriptBuf, Transaction, TxOut, Txid, WPubkeyHash, Weight,
Address, Amount, FeeRate, OutPoint, ScriptBuf, Transaction, TxOut, Txid, WPubkeyHash, Weight,
WitnessProgram, WitnessVersion,
};
use lightning::chain::chaininterface::BroadcasterInterface;
Expand Down Expand Up @@ -153,6 +153,19 @@ impl Wallet {
Ok(())
}

pub(crate) fn insert_txo(&self, outpoint: OutPoint, txout: TxOut) -> Result<(), Error> {
let mut locked_wallet = self.inner.lock().unwrap();
locked_wallet.insert_txout(outpoint, txout);

let mut locked_persister = self.persister.lock().unwrap();
locked_wallet.persist(&mut locked_persister).map_err(|e| {
log_error!(self.logger, "Failed to persist wallet: {}", e);
Error::PersistenceFailed
})?;

Ok(())
}

fn update_payment_store<'a>(
&self, locked_wallet: &'a mut PersistedWallet<KVStoreWalletPersister>,
) -> Result<(), Error> {
Expand Down
37 changes: 28 additions & 9 deletions tests/integration_tests_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,10 +927,13 @@ async fn concurrent_connections_succeed() {
}
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn splice_channel() {
async fn run_splice_channel_test(bitcoind_chain_source: bool) {
let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
let chain_source = TestChainSource::Esplora(&electrsd);
let chain_source = if bitcoind_chain_source {
TestChainSource::BitcoindRpcSync(&bitcoind)
} else {
TestChainSource::Esplora(&electrsd)
};
let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false);

let address_a = node_a.onchain_payment().new_address().unwrap();
Expand Down Expand Up @@ -995,7 +998,7 @@ async fn splice_channel() {
// Splice-in funds for Node B so that it has outbound liquidity to make a payment
node_b.splice_in(&user_channel_id_b, node_a.node_id(), 4_000_000).unwrap();

expect_splice_pending_event!(node_a, node_b.node_id());
let txo = expect_splice_pending_event!(node_a, node_b.node_id());
Copy link
Collaborator

Choose a reason for hiding this comment

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

These test changes pass on main. Can you add coverage for the cases where the previous approach would lead to inaccurate fee estimations?

Copy link
Contributor Author

@benthecarman benthecarman Dec 4, 2025

Choose a reason for hiding this comment

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

The bug/issue only exists with bitcoind syncing so to do so, we'd have to change the test to sync with bitcoind rpc. Is that wanted?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The bug/issue only exists with bitcoind syncing so to do so, we'd have to change the test to sync with bitcoind rpc. Is that wanted?

Sure, feel free to do so. Alternatively, you could also add a _bitcoind variant test case (ofc reusing the logic) so we have two test cases for bitcoind and esplora - that's if we think there would be more chain source specific edge cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

expect_splice_pending_event!(node_b, node_a.node_id());

generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await;
Expand All @@ -1006,11 +1009,16 @@ async fn splice_channel() {
expect_channel_ready_event!(node_a, node_b.node_id());
expect_channel_ready_event!(node_b, node_a.node_id());

let splice_in_fee_sat = 252;
let expected_splice_in_fee_sat = 252;

let payments = node_b.list_payments();
let payment =
payments.into_iter().find(|p| p.id == PaymentId(txo.txid.to_byte_array())).unwrap();
assert_eq!(payment.fee_paid_msat, Some(expected_splice_in_fee_sat * 1_000));

assert_eq!(
node_b.list_balances().total_onchain_balance_sats,
premine_amount_sat - 4_000_000 - splice_in_fee_sat
premine_amount_sat - 4_000_000 - expected_splice_in_fee_sat
);
assert_eq!(node_b.list_balances().total_lightning_balance_sats, 4_000_000);

Expand All @@ -1033,7 +1041,7 @@ async fn splice_channel() {
let address = node_a.onchain_payment().new_address().unwrap();
node_a.splice_out(&user_channel_id_a, node_b.node_id(), &address, amount_msat / 1000).unwrap();

expect_splice_pending_event!(node_a, node_b.node_id());
let txo = expect_splice_pending_event!(node_a, node_b.node_id());
expect_splice_pending_event!(node_b, node_a.node_id());

generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await;
Expand All @@ -1044,18 +1052,29 @@ async fn splice_channel() {
expect_channel_ready_event!(node_a, node_b.node_id());
expect_channel_ready_event!(node_b, node_a.node_id());

let splice_out_fee_sat = 183;
let expected_splice_out_fee_sat = 183;

let payments = node_a.list_payments();
let payment =
payments.into_iter().find(|p| p.id == PaymentId(txo.txid.to_byte_array())).unwrap();
assert_eq!(payment.fee_paid_msat, Some(expected_splice_out_fee_sat * 1_000));

assert_eq!(
node_a.list_balances().total_onchain_balance_sats,
premine_amount_sat - 4_000_000 - opening_transaction_fee_sat + amount_msat / 1000
);
assert_eq!(
node_a.list_balances().total_lightning_balance_sats,
4_000_000 - closing_transaction_fee_sat - anchor_output_sat - splice_out_fee_sat
4_000_000 - closing_transaction_fee_sat - anchor_output_sat - expected_splice_out_fee_sat
);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn splice_channel() {
run_splice_channel_test(false).await;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Not that it matters much, but elsewhere in LDK Node / LDK tests the usual pattern would be to name the 'inner' test methods do_splice_channel. Feel free to make that rename if you touch the code again.

run_splice_channel_test(true).await;
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn simple_bolt12_send_receive() {
let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
Expand Down