Skip to content

approach-1: Set outgoing signal in PendingHTLCInfo #23

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.

Sign up for GitHub

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

approach-1: Set outgoing signal in PendingHTLCInfo #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
235d5ec
ln: add experimental accountable signal to update_add_htlc
carlaKC Oct 23, 2025
8fb0681
ln: add incoming_accountable to PendingHTLCInfo
carlaKC Oct 23, 2025
ef58f94
[approach1]: Set outgoing_accountable in PendingHTLCInfo
carlaKC Oct 23, 2025
1f00685
ln: add accountable signal to HTLCUpdateAwaitingACK::AddHTLC
carlaKC Oct 23, 2025
c62eb0f
ln: add accountable signal to OutboundHTLCOutput
carlaKC Oct 23, 2025
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
1 change: 1 addition & 0 deletions lightning/src/ln/blinded_payment_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1523,6 +1523,7 @@ fn update_add_msg(
skimmed_fee_msat: None,
blinding_point,
hold_htlc: None,
accountable: None,
}
}

Expand Down
61 changes: 57 additions & 4 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ struct OutboundHTLCOutput {
skimmed_fee_msat: Option<u64>,
send_timestamp: Option<Duration>,
hold_htlc: Option<()>,
accountable: Option<u8>,
}

/// See AwaitingRemoteRevoke ChannelState for more info
Expand All @@ -445,6 +446,7 @@ enum HTLCUpdateAwaitingACK {
skimmed_fee_msat: Option<u64>,
blinding_point: Option<PublicKey>,
hold_htlc: Option<()>,
accountable: Option<u8>,
},
ClaimHTLC {
payment_preimage: PaymentPreimage,
Expand Down Expand Up @@ -8314,7 +8316,7 @@ where
skimmed_fee_msat,
blinding_point,
hold_htlc,
..
accountable,
} => {
match self.send_htlc(
amount_msat,
Expand All @@ -8326,6 +8328,7 @@ where
skimmed_fee_msat,
blinding_point,
hold_htlc.is_some(),
accountable,
fee_estimator,
logger,
) {
Expand Down Expand Up @@ -9639,6 +9642,7 @@ where
skimmed_fee_msat: htlc.skimmed_fee_msat,
blinding_point: htlc.blinding_point,
hold_htlc: htlc.hold_htlc,
accountable: htlc.accountable,
});
}
}
Expand Down Expand Up @@ -12428,7 +12432,8 @@ where
pub fn queue_add_htlc<F: Deref, L: Deref>(
&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32,
source: HTLCSource, onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>,
blinding_point: Option<PublicKey>, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
blinding_point: Option<PublicKey>, accountable: Option<u8>,
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
) -> Result<(), (LocalHTLCFailureReason, String)>
where
F::Target: FeeEstimator,
Expand All @@ -12445,6 +12450,7 @@ where
blinding_point,
// This method is only called for forwarded HTLCs, which are never held at the next hop
false,
accountable,
fee_estimator,
logger,
)
Expand Down Expand Up @@ -12476,7 +12482,7 @@ where
&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32,
source: HTLCSource, onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool,
skimmed_fee_msat: Option<u64>, blinding_point: Option<PublicKey>, hold_htlc: bool,
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
accountable: Option<u8>, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
) -> Result<bool, (LocalHTLCFailureReason, String)>
where
F::Target: FeeEstimator,
Expand Down Expand Up @@ -12558,6 +12564,7 @@ where
skimmed_fee_msat,
blinding_point,
hold_htlc: hold_htlc.then(|| ()),
accountable,
});
return Ok(false);
}
Expand All @@ -12580,6 +12587,7 @@ where
skimmed_fee_msat,
send_timestamp,
hold_htlc: hold_htlc.then(|| ()),
accountable,
});
self.context.next_holder_htlc_id += 1;

Expand Down Expand Up @@ -12806,7 +12814,8 @@ where
pub fn send_htlc_and_commit<F: Deref, L: Deref>(
&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32,
source: HTLCSource, onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>,
hold_htlc: bool, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
hold_htlc: bool, accountable: Option<u8>, fee_estimator: &LowerBoundedFeeEstimator<F>,
logger: &L,
) -> Result<Option<ChannelMonitorUpdate>, ChannelError>
where
F::Target: FeeEstimator,
Expand All @@ -12822,6 +12831,7 @@ where
skimmed_fee_msat,
None,
hold_htlc,
accountable,
fee_estimator,
logger,
);
Expand Down Expand Up @@ -14426,6 +14436,7 @@ where
let mut pending_outbound_skimmed_fees: Vec<Option<u64>> = Vec::new();
let mut pending_outbound_blinding_points: Vec<Option<PublicKey>> = Vec::new();
let mut pending_outbound_held_htlc_flags: Vec<Option<()>> = Vec::new();
let mut pending_outbound_accountable: Vec<Option<u8>> = Vec::new();

(self.context.pending_outbound_htlcs.len() as u64).write(writer)?;
for htlc in self.context.pending_outbound_htlcs.iter() {
Expand Down Expand Up @@ -14469,6 +14480,7 @@ where
pending_outbound_skimmed_fees.push(htlc.skimmed_fee_msat);
pending_outbound_blinding_points.push(htlc.blinding_point);
pending_outbound_held_htlc_flags.push(htlc.hold_htlc);
pending_outbound_accountable.push(htlc.accountable);
}

let holding_cell_htlc_update_count = self.context.holding_cell_htlc_updates.len();
Expand All @@ -14480,6 +14492,8 @@ where
Vec::with_capacity(holding_cell_htlc_update_count);
let mut holding_cell_held_htlc_flags: Vec<Option<()>> =
Vec::with_capacity(holding_cell_htlc_update_count);
let mut holding_cell_accountable_flags: Vec<Option<u8>> =
Vec::with_capacity(holding_cell_htlc_update_count);
// Vec of (htlc_id, failure_code, sha256_of_onion)
let mut malformed_htlcs: Vec<(u64, u16, [u8; 32])> = Vec::new();
(holding_cell_htlc_update_count as u64).write(writer)?;
Expand All @@ -14494,6 +14508,7 @@ where
blinding_point,
skimmed_fee_msat,
hold_htlc,
accountable,
} => {
0u8.write(writer)?;
amount_msat.write(writer)?;
Expand All @@ -14505,6 +14520,7 @@ where
holding_cell_skimmed_fees.push(skimmed_fee_msat);
holding_cell_blinding_points.push(blinding_point);
holding_cell_held_htlc_flags.push(hold_htlc);
holding_cell_accountable_flags.push(accountable);
},
&HTLCUpdateAwaitingACK::ClaimHTLC {
ref payment_preimage,
Expand Down Expand Up @@ -14759,6 +14775,8 @@ where
(65, self.quiescent_action, option), // Added in 0.2
(67, pending_outbound_held_htlc_flags, optional_vec), // Added in 0.2
(69, holding_cell_held_htlc_flags, optional_vec), // Added in 0.2
(71, holding_cell_accountable_flags, optional_vec), // Added in 0.3
(73, pending_outbound_accountable, optional_vec), // Added in 0.3
});

Ok(())
Expand Down Expand Up @@ -14908,6 +14926,7 @@ where
blinding_point: None,
send_timestamp: None,
hold_htlc: None,
accountable: None,
});
}

Expand All @@ -14927,6 +14946,7 @@ where
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
accountable: None,
},
1 => HTLCUpdateAwaitingACK::ClaimHTLC {
payment_preimage: Readable::read(reader)?,
Expand Down Expand Up @@ -15126,6 +15146,8 @@ where

let mut pending_outbound_held_htlc_flags_opt: Option<Vec<Option<()>>> = None;
let mut holding_cell_held_htlc_flags_opt: Option<Vec<Option<()>>> = None;
let mut pending_outbound_accountable_opt: Option<Vec<Option<u8>>> = None;
let mut holding_cell_accountable_opt: Option<Vec<Option<u8>>> = None;

read_tlv_fields!(reader, {
(0, announcement_sigs, option),
Expand Down Expand Up @@ -15173,6 +15195,8 @@ where
(65, quiescent_action, upgradable_option), // Added in 0.2
(67, pending_outbound_held_htlc_flags_opt, optional_vec), // Added in 0.2
(69, holding_cell_held_htlc_flags_opt, optional_vec), // Added in 0.2
(71, holding_cell_accountable_opt, optional_vec), // Added in 0.3
(73, pending_outbound_accountable_opt, optional_vec), // Added in 0.3
});

let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
Expand Down Expand Up @@ -15297,6 +15321,28 @@ where
}
}

if let Some(accountable_htlcs) = holding_cell_accountable_opt {
let mut iter = accountable_htlcs.into_iter();
for htlc in holding_cell_htlc_updates.iter_mut() {
if let HTLCUpdateAwaitingACK::AddHTLC { ref mut accountable, .. } = htlc {
*accountable = iter.next().ok_or(DecodeError::InvalidValue)?;
}
}
// We expect all accountable HTLC signals to be consumed above
if iter.next().is_some() {
return Err(DecodeError::InvalidValue);
}
}
if let Some(held_htlcs) = pending_outbound_accountable_opt {
let mut iter = held_htlcs.into_iter();
for htlc in pending_outbound_htlcs.iter_mut() {
htlc.accountable = iter.next().ok_or(DecodeError::InvalidValue)?;
}
// We expect all accountable HTLC signals to be consumed above
if iter.next().is_some() {
return Err(DecodeError::InvalidValue);
}
}
if let Some(attribution_data_list) = removed_htlc_attribution_data {
let mut removed_htlcs = pending_inbound_htlcs.iter_mut().filter_map(|status| {
if let InboundHTLCState::LocalRemoved(reason) = &mut status.state {
Expand Down Expand Up @@ -15878,6 +15924,7 @@ mod tests {
blinding_point: None,
send_timestamp: None,
hold_htlc: None,
accountable: None,
});

// Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass
Expand Down Expand Up @@ -16333,6 +16380,7 @@ mod tests {
blinding_point: None,
send_timestamp: None,
hold_htlc: None,
accountable: None,
};
let mut pending_outbound_htlcs = vec![dummy_outbound_output.clone(); 10];
for (idx, htlc) in pending_outbound_htlcs.iter_mut().enumerate() {
Expand All @@ -16359,6 +16407,7 @@ mod tests {
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
accountable: None,
};
let dummy_holding_cell_claim_htlc = |attribution_data| HTLCUpdateAwaitingACK::ClaimHTLC {
payment_preimage: PaymentPreimage([42; 32]),
Expand Down Expand Up @@ -16684,6 +16733,7 @@ mod tests {
blinding_point: None,
send_timestamp: None,
hold_htlc: None,
accountable: None,
};
out.payment_hash.0 = Sha256::hash(&<Vec<u8>>::from_hex("0202020202020202020202020202020202020202020202020202020202020202").unwrap()).to_byte_array();
out
Expand All @@ -16700,6 +16750,7 @@ mod tests {
blinding_point: None,
send_timestamp: None,
hold_htlc: None,
accountable: None,
};
out.payment_hash.0 = Sha256::hash(&<Vec<u8>>::from_hex("0303030303030303030303030303030303030303030303030303030303030303").unwrap()).to_byte_array();
out
Expand Down Expand Up @@ -17114,6 +17165,7 @@ mod tests {
blinding_point: None,
send_timestamp: None,
hold_htlc: None,
accountable: None,
};
out.payment_hash.0 = Sha256::hash(&<Vec<u8>>::from_hex("0505050505050505050505050505050505050505050505050505050505050505").unwrap()).to_byte_array();
out
Expand All @@ -17130,6 +17182,7 @@ mod tests {
blinding_point: None,
send_timestamp: None,
hold_htlc: None,
accountable: None,
};
out.payment_hash.0 = Sha256::hash(&<Vec<u8>>::from_hex("0505050505050505050505050505050505050505050505050505050505050505").unwrap()).to_byte_array();
out
Expand Down
30 changes: 25 additions & 5 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,12 @@ pub struct PendingHTLCInfo {
/// This is used to allow LSPs to take fees as a part of payments, without the sender having to
/// shoulder them.
pub skimmed_fee_msat: Option<u64>,
/// An experimental field indicating whether our node's reputation would be held accountable
/// for the timely resolution of the received HTLC.
pub incoming_accountable: Option<u8>,
/// An experimental field indicating whether the outgoing node's reputation would be held
/// accountable for the timely resolution of the offered HTLC.
pub outgoing_accountable: Option<u8>,
Comment on lines +432 to +434
Copy link

@elnosh elnosh Oct 28, 2025

Choose a reason for hiding this comment

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

since this field won't apply for receives, would it be better to put it accordingly for each variant in PendingHTLCRouting?

}

#[derive(Clone)] // See FundedChannel::revoke_and_ack for why, tl;dr: Rust bug
Expand Down Expand Up @@ -5048,7 +5054,7 @@ where
let current_height: u32 = self.best_block.read().unwrap().height;
create_recv_pending_htlc_info(decoded_hop, shared_secret, msg.payment_hash,
msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat,
current_height)
msg.accountable, current_height)
},
onion_utils::Hop::Forward { .. } | onion_utils::Hop::BlindedForward { .. } => {
create_fwd_pending_htlc_info(msg, decoded_hop, shared_secret, next_packet_pubkey_opt)
Expand Down Expand Up @@ -5268,6 +5274,7 @@ where
onion_packet,
None,
hold_htlc_at_next_hop,
Some(0),
&self.fee_estimator,
&&logger,
);
Expand Down Expand Up @@ -7150,6 +7157,7 @@ where
payment_hash,
outgoing_amt_msat,
outgoing_cltv_value,
incoming_accountable,
..
},
} = payment;
Expand Down Expand Up @@ -7248,6 +7256,7 @@ where
Some(phantom_shared_secret),
false,
None,
incoming_accountable,
current_height,
);
match create_res {
Expand Down Expand Up @@ -7357,6 +7366,7 @@ where
outgoing_cltv_value,
routing,
skimmed_fee_msat,
outgoing_accountable,
..
},
..
Expand Down Expand Up @@ -7452,6 +7462,7 @@ where
onion_packet.clone(),
*skimmed_fee_msat,
next_blinding_point,
*outgoing_accountable,
&self.fee_estimator,
&&logger,
) {
Expand Down Expand Up @@ -11221,7 +11232,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
Some(prev_channel_id),
Some(payment_hash),
);
let pending_add = PendingAddHTLCInfo {
let mut pending_add = PendingAddHTLCInfo {
prev_outbound_scid_alias,
prev_counterparty_node_id,
prev_funding_outpoint,
Expand Down Expand Up @@ -11315,6 +11326,13 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
},
}
} else {
// Set the value for our outgoing accountable signal to copy the received
// incoming value (or just set zero if not present). This point is where we
// could introduce an interceptor that provides us with custom accountable
// values if desired.
pending_add.forward_info.outgoing_accountable =
pending_add.forward_info.incoming_accountable.or(Some(0));

Comment on lines +11329 to +11335
Copy link

@elnosh elnosh Oct 28, 2025

Choose a reason for hiding this comment

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

I'm not sure if this should be here or in process_forward_htlcs? IIUC, the outgoing channel on which the HTLC will end up being forwarded could potentially change there.

Copy link
Owner Author

@carlaKC carlaKC Oct 30, 2025

Choose a reason for hiding this comment

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

Taken a shot at this here - I think that I prefer this approach for now, because it gives us good flexibility (see PR description). Also addresses the PendingHTLCRouting comment below nicely (in that we don't have to put it anywhere, which I like).

match self.forward_htlcs.lock().unwrap().entry(scid) {
hash_map::Entry::Occupied(mut entry) => {
entry.get_mut().push(HTLCForwardInfo::AddHTLC(pending_add));
Expand Down Expand Up @@ -15764,6 +15782,8 @@ impl_writeable_tlv_based!(PendingHTLCInfo, {
(8, outgoing_cltv_value, required),
(9, incoming_amt_msat, option),
(10, skimmed_fee_msat, option),
(11, incoming_accountable, option),
(13, outgoing_accountable, option),
});

impl Writeable for HTLCFailureMsg {
Expand Down Expand Up @@ -19198,7 +19218,7 @@ mod tests {
if let Err(crate::ln::channelmanager::InboundHTLCErr { reason, .. }) =
create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
sender_intended_amt_msat - extra_fee_msat - 1, 42, None, true, Some(extra_fee_msat),
current_height)
None, current_height)
{
assert_eq!(reason, LocalHTLCFailureReason::FinalIncorrectHTLCAmount);
} else { panic!(); }
Expand All @@ -19221,7 +19241,7 @@ mod tests {
let current_height: u32 = node[0].node.best_block.read().unwrap().height;
assert!(create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
sender_intended_amt_msat - extra_fee_msat, 42, None, true, Some(extra_fee_msat),
current_height).is_ok());
None, current_height).is_ok());
}

#[test]
Expand All @@ -19246,7 +19266,7 @@ mod tests {
custom_tlvs: Vec::new(),
},
shared_secret: SharedSecret::from_bytes([0; 32]),
}, [0; 32], PaymentHash([0; 32]), 100, TEST_FINAL_CLTV + 1, None, true, None, current_height);
}, [0; 32], PaymentHash([0; 32]), 100, TEST_FINAL_CLTV + 1, None, true, None, None, current_height);

// Should not return an error as this condition:
// https://github.com/lightning/bolts/blob/4dcc377209509b13cf89a4b91fde7d478f5b46d8/04-onion-routing.md?plain=1#L334
Expand Down
Loading
Loading