Skip to content

Commit e541265

Browse files
jkczyzclaude
andcommitted
Report the confirmed splice candidate's fee, not the last broadcast's
A splice funding payment can be fee-bumped via RBF, producing several candidate transactions with increasing fees. The payment recorded the last-broadcast candidate's amount and fee and kept them on confirmation, but the candidate that actually confirms need not be the last one broadcast — so an earlier, lower-fee candidate confirming left the payment over-reporting its fee. Record each candidate's amount and fee, keyed by txid, so that on confirmation the payment reflects the candidate that actually confirmed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9f98db5 commit e541265

4 files changed

Lines changed: 205 additions & 24 deletions

File tree

src/payment/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ pub use bolt11::Bolt11Payment;
2020
pub(crate) use bolt11::PaymentMetadata;
2121
pub use bolt12::Bolt12Payment;
2222
pub use onchain::OnchainPayment;
23+
pub(crate) use pending_payment_store::FundingTxCandidate;
2324
pub(crate) use pending_payment_store::PendingPaymentDetails;
2425
pub use spontaneous::SpontaneousPayment;
2526
pub use store::{

src/payment/pending_payment_store.rs

Lines changed: 107 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,31 +13,67 @@ use crate::data_store::{StorableObject, StorableObjectUpdate};
1313
use crate::payment::store::PaymentDetailsUpdate;
1414
use crate::payment::{PaymentDetails, PaymentKind};
1515

16+
/// One candidate transaction in an interactive-funding (splice) RBF history, holding this node's
17+
/// share of the funding amount and fee for that candidate. Both are `None` for a candidate this
18+
/// node did not contribute to — e.g. a counterparty-initiated round before our `splice_in` joined
19+
/// it via RBF. Recorded per pending payment so that, on confirmation, the payment reports the
20+
/// figures of the candidate that actually confirmed, which need not be the last one broadcast.
21+
#[derive(Clone, Debug, PartialEq, Eq)]
22+
pub(crate) struct FundingTxCandidate {
23+
/// The candidate's broadcast transaction id.
24+
pub txid: Txid,
25+
/// This node's share of the funding amount for this candidate, in millisatoshis, or `None` if
26+
/// this node did not contribute to it.
27+
pub amount_msat: Option<u64>,
28+
/// This node's share of the on-chain fee for this candidate, in millisatoshis, or `None` if
29+
/// this node did not contribute to it.
30+
pub fee_paid_msat: Option<u64>,
31+
}
32+
33+
impl_writeable_tlv_based!(FundingTxCandidate, {
34+
(0, txid, required),
35+
(2, amount_msat, option),
36+
(4, fee_paid_msat, option),
37+
});
38+
1639
/// Represents a pending payment
1740
#[derive(Clone, Debug, PartialEq, Eq)]
1841
pub struct PendingPaymentDetails {
1942
/// The full payment details
2043
pub details: PaymentDetails,
2144
/// Transaction IDs that have replaced or conflict with this payment.
2245
pub conflicting_txids: Vec<Txid>,
46+
/// For interactive funding (splices), this node's per-candidate funding figures across the
47+
/// RBF history, keyed by each candidate's txid. Empty for non-funding payments and for
48+
/// records written before per-candidate tracking existed.
49+
pub(crate) candidates: Vec<FundingTxCandidate>,
2350
}
2451

2552
impl PendingPaymentDetails {
26-
pub(crate) fn new(details: PaymentDetails, conflicting_txids: Vec<Txid>) -> Self {
27-
Self { details, conflicting_txids }
53+
pub(crate) fn new(
54+
details: PaymentDetails, conflicting_txids: Vec<Txid>, candidates: Vec<FundingTxCandidate>,
55+
) -> Self {
56+
Self { details, conflicting_txids, candidates }
57+
}
58+
59+
/// Returns this node's recorded funding figures for the candidate with the given txid, if any.
60+
pub(crate) fn candidate(&self, txid: Txid) -> Option<&FundingTxCandidate> {
61+
self.candidates.iter().find(|candidate| candidate.txid == txid)
2862
}
2963
}
3064

3165
impl_writeable_tlv_based!(PendingPaymentDetails, {
3266
(0, details, required),
3367
(2, conflicting_txids, optional_vec),
68+
(4, candidates, optional_vec),
3469
});
3570

3671
#[derive(Clone, Debug, PartialEq, Eq)]
3772
pub(crate) struct PendingPaymentDetailsUpdate {
3873
pub id: PaymentId,
3974
pub payment_update: Option<PaymentDetailsUpdate>,
4075
pub conflicting_txids: Option<Vec<Txid>>,
76+
pub candidates: Vec<FundingTxCandidate>,
4177
}
4278

4379
impl StorableObject for PendingPaymentDetails {
@@ -69,6 +105,13 @@ impl StorableObject for PendingPaymentDetails {
69105
updated |= self.conflicting_txids.len() != conflicts_len;
70106
}
71107

108+
// Each classify passes the complete candidate history, so a non-empty update replaces the
109+
// stored list. An empty update (e.g. a non-funding payment) leaves it untouched.
110+
if !update.candidates.is_empty() && self.candidates != update.candidates {
111+
self.candidates = update.candidates;
112+
updated = true;
113+
}
114+
72115
updated
73116
}
74117

@@ -90,16 +133,73 @@ impl From<&PendingPaymentDetails> for PendingPaymentDetailsUpdate {
90133
} else {
91134
Some(value.conflicting_txids.clone())
92135
};
93-
Self { id: value.id(), payment_update: Some(value.details.to_update()), conflicting_txids }
136+
Self {
137+
id: value.id(),
138+
payment_update: Some(value.details.to_update()),
139+
conflicting_txids,
140+
candidates: value.candidates.clone(),
141+
}
94142
}
95143
}
96144

97145
#[cfg(test)]
98146
mod tests {
147+
use super::*;
148+
use crate::payment::store::ConfirmationStatus;
149+
use crate::payment::{PaymentDirection, PaymentKind, PaymentStatus};
99150
use bitcoin::hashes::Hash;
100151

101-
use super::*;
102-
use crate::payment::{ConfirmationStatus, PaymentDirection, PaymentKind, PaymentStatus};
152+
#[test]
153+
fn pending_payment_candidate_lookup() {
154+
let payment_id = PaymentId([1u8; 32]);
155+
let first_txid = Txid::from_byte_array([2u8; 32]);
156+
let rbf_txid = Txid::from_byte_array([3u8; 32]);
157+
158+
// A leading counterparty-initiated round we didn't contribute to (no figures), then our own
159+
// original and RBF candidates.
160+
let counterparty_txid = Txid::from_byte_array([4u8; 32]);
161+
let candidates = vec![
162+
FundingTxCandidate { txid: counterparty_txid, amount_msat: None, fee_paid_msat: None },
163+
FundingTxCandidate {
164+
txid: first_txid,
165+
amount_msat: Some(1_000_000),
166+
fee_paid_msat: Some(1_000),
167+
},
168+
FundingTxCandidate {
169+
txid: rbf_txid,
170+
amount_msat: Some(1_000_000),
171+
fee_paid_msat: Some(5_000),
172+
},
173+
];
174+
175+
// The stored details only need to be a valid funding payment; `candidate` resolves figures
176+
// purely from the recorded candidate list.
177+
let details = PaymentDetails::new(
178+
payment_id,
179+
PaymentKind::Onchain {
180+
txid: rbf_txid,
181+
status: ConfirmationStatus::Unconfirmed,
182+
tx_type: None,
183+
},
184+
Some(1_000_000),
185+
Some(5_000),
186+
PaymentDirection::Outbound,
187+
PaymentStatus::Pending,
188+
);
189+
let pending =
190+
PendingPaymentDetails::new(details, vec![first_txid, counterparty_txid], candidates);
191+
192+
// Each candidate resolves to its own figures, so a non-last candidate that confirms reports
193+
// its own (lower) fee rather than the last-broadcast candidate's.
194+
assert_eq!(pending.candidate(first_txid).and_then(|c| c.fee_paid_msat), Some(1_000));
195+
assert_eq!(pending.candidate(rbf_txid).and_then(|c| c.fee_paid_msat), Some(5_000));
196+
// A candidate we didn't contribute to carries no figures, so the payment reports `None`
197+
// rather than another candidate's stale figures.
198+
let counterparty = pending.candidate(counterparty_txid).expect("candidate is recorded");
199+
assert_eq!(counterparty.amount_msat, None);
200+
assert_eq!(counterparty.fee_paid_msat, None);
201+
assert_eq!(pending.candidate(Txid::from_byte_array([9u8; 32])), None);
202+
}
103203

104204
fn test_txid(byte: u8) -> Txid {
105205
Txid::from_byte_array([byte; 32])
@@ -125,10 +225,12 @@ mod tests {
125225
let mut pending_payment = PendingPaymentDetails::new(
126226
pending_onchain_payment(payment_id, replacement_txid),
127227
vec![original_txid],
228+
Vec::new(),
128229
);
129230
let update = PendingPaymentDetails::new(
130231
pending_onchain_payment(payment_id, original_txid),
131232
Vec::new(),
233+
Vec::new(),
132234
)
133235
.to_update();
134236

src/wallet/mod.rs

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ use crate::fee_estimator::{ConfirmationTarget, FeeEstimator, OnchainFeeEstimator
5858
use crate::logger::{log_debug, log_error, log_info, log_trace, LdkLogger, Logger};
5959
use crate::payment::store::ConfirmationStatus;
6060
use crate::payment::{
61-
PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus, PendingPaymentDetails,
62-
TransactionType,
61+
FundingTxCandidate, PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus,
62+
PendingPaymentDetails, TransactionType,
6363
};
6464
use crate::runtime::Runtime;
6565
use crate::types::{Broadcaster, PaymentStore, PendingPaymentStore};
@@ -1235,7 +1235,7 @@ impl Wallet {
12351235
direction,
12361236
PaymentStatus::Pending,
12371237
);
1238-
self.persist_funding_payment(details).await?;
1238+
self.persist_funding_payment(details, Vec::new()).await?;
12391239
log_debug!(
12401240
self.logger,
12411241
"Recorded channel-funding broadcast {} for channel {}",
@@ -1298,6 +1298,23 @@ impl Wallet {
12981298
// Anchor the `PaymentId` to the first negotiated candidate so the record stays stable
12991299
// across RBF replacements.
13001300
let payment_id = PaymentId(first.txid.to_byte_array());
1301+
1302+
// Record every candidate's figures (`None` for any round we didn't contribute to, e.g. a
1303+
// counterparty-initiated splice our `splice_in` later joined via RBF) so the confirmed
1304+
// candidate's amount/fee can be applied on confirmation, even if it isn't the last one
1305+
// broadcast or one we contributed to.
1306+
let candidate_records: Vec<FundingTxCandidate> = candidates
1307+
.iter()
1308+
.map(|candidate| {
1309+
let aggregate = aggregate_local_stakes(candidate);
1310+
FundingTxCandidate {
1311+
txid: candidate.txid,
1312+
amount_msat: aggregate.amount_msat,
1313+
fee_paid_msat: aggregate.fee_paid_msat,
1314+
}
1315+
})
1316+
.collect();
1317+
13011318
let details = PaymentDetails::new(
13021319
payment_id,
13031320
PaymentKind::Onchain {
@@ -1310,7 +1327,7 @@ impl Wallet {
13101327
direction,
13111328
PaymentStatus::Pending,
13121329
);
1313-
self.persist_funding_payment(details).await?;
1330+
self.persist_funding_payment(details, candidate_records).await?;
13141331
log_debug!(
13151332
self.logger,
13161333
"Recorded interactive-funding broadcast {} ({} candidates, {} channels)",
@@ -1323,9 +1340,11 @@ impl Wallet {
13231340

13241341
/// Writes a freshly-classified funding payment to the authoritative payment store and adds a
13251342
/// pending-store index entry, so wallet sync graduates it through `ANTI_REORG_DELAY`.
1326-
async fn persist_funding_payment(&self, details: PaymentDetails) -> Result<(), Error> {
1343+
async fn persist_funding_payment(
1344+
&self, details: PaymentDetails, candidates: Vec<FundingTxCandidate>,
1345+
) -> Result<(), Error> {
13271346
self.payment_store.insert_or_update(details.clone()).await?;
1328-
let pending = PendingPaymentDetails::new(details, Vec::new());
1347+
let pending = PendingPaymentDetails::new(details, Vec::new(), candidates);
13291348
self.pending_payment_store.insert_or_update(pending).await?;
13301349
Ok(())
13311350
}
@@ -1395,7 +1414,7 @@ impl Wallet {
13951414
fn create_pending_payment_from_tx(
13961415
&self, payment: PaymentDetails, conflicting_txids: Vec<Txid>,
13971416
) -> PendingPaymentDetails {
1398-
PendingPaymentDetails::new(payment, conflicting_txids)
1417+
PendingPaymentDetails::new(payment, conflicting_txids, Vec::new())
13991418
}
14001419

14011420
fn find_payment_by_txid(&self, target_txid: Txid) -> Option<PaymentId> {
@@ -1441,6 +1460,17 @@ impl Wallet {
14411460
} => tx_type.clone(),
14421461
_ => return Ok(false),
14431462
};
1463+
// Report the figures of the candidate that actually confirmed, which need not be the last
1464+
// one broadcast (an earlier, lower-fee candidate may win) and may carry no figures at all
1465+
// (`None`) for a round we didn't contribute to. (`direction` is invariant across a splice's
1466+
// candidates and cannot be changed through the store anyway.)
1467+
if let Some(pending) = self.pending_payment_store.get(&payment_id) {
1468+
if let Some(candidate) = pending.candidate(event_txid) {
1469+
payment.amount_msat = candidate.amount_msat;
1470+
payment.fee_paid_msat = candidate.fee_paid_msat;
1471+
}
1472+
}
1473+
14441474
payment.kind =
14451475
PaymentKind::Onchain { txid: event_txid, status: confirmation_status, tx_type };
14461476
self.runtime.block_on(self.payment_store.insert_or_update(payment.clone()))?;

0 commit comments

Comments
 (0)