Skip to content
Open
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
27 changes: 27 additions & 0 deletions src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,33 @@ where
counterparty_node_id,
funding_txo,
);

let chans =
self.channel_manager.list_channels_with_counterparty(&counterparty_node_id);
let chan_output = chans
.iter()
.find(|c| c.user_channel_id == user_channel_id)
.and_then(|c| c.get_funding_output());
Comment on lines +1462 to +1467
Copy link
Contributor

Choose a reason for hiding this comment

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

Doing this in ChannelReady wouldn't work for splicing preexisting channels. Wonder if we should instead do this when initiating a splice? There we already look up the channel. It would also let us use the real shared_input when selecting UTXOs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing this in ChannelReady wouldn't work for splicing preexisting channels.

I dont think this is right, in the docs it says we get a ChannelReady for splices, also my test seems to confirm

match chan_output {
None => {
log_error!(
self.logger,
"Failed to find channel info for pending channel {channel_id} with counterparty {counterparty_node_id}"
);
debug_assert!(false,
"Failed to find channel info for pending channel {channel_id} with counterparty {counterparty_node_id}"
);
},
Some(output) => {
if let Err(e) = self.wallet.insert_txo(funding_txo, output) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm... I guess we didn't really need the redeem script in ChannelPending and SplicePending since we only need the previous funding output in the wallet for the next splice. Might have been better in ChannelReady. We'd be able to avoid the look-up, although it wouldn't remove the Option check.

Copy link
Contributor Author

@benthecarman benthecarman Dec 9, 2025

Choose a reason for hiding this comment

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

Yeah sadly I messed up here, i think we needed to add the redeem script along with the new channel size so we had the full utxo information

log_error!(
self.logger,
"Failed to insert funding TXO into wallet: {e}"
);
return Err(ReplayEvent());
}
},
}
} else {
log_info!(
Copy link
Collaborator

Choose a reason for hiding this comment

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

In what case would we end up in this else branch, and would we need to do something related to the redeemscript, too?

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.

According to the docs, the funding_txo "Will be None if the channel's funding transaction reached an acceptable depth prior to version 0.2". Since this was also added in 0.2, the redeem script would also be None so there shouldn't be anything to add. Either way, we need the funding txo to be able to insert so we can't add it.

self.logger,
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
10 changes: 10 additions & 0 deletions src/types.rs
Original file line number Diff line number Diff line change
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<bitcoin::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
11 changes: 8 additions & 3 deletions tests/integration_tests_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,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?

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

generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await;
Expand All @@ -1006,11 +1006,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 Down
Loading