Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 480
Allow forwarding less than the amount in the onion #2319
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
Merged
tnull
merged 12 commits into
lightningdevkit:main
from
valentinewallace:2023-05-forward-less-than-onionJun 21, 2023
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a2a7fef
Move PendingHTLCStatus construction inside channel lock
valentinewallace 672d666
Move next hop packet pubkey calculation to outside channel lock
valentinewallace 85a5b31
Add config knob for accepting underpaying HTLCs
valentinewallace 52f2901
Set extra skimmed fee on intercepted forward
valentinewallace 9485304
Persist counterparty skimmed fee in ClaimableHTLC
valentinewallace 27b99ac
Add PaymentClaimable::counterparty_skimmed_fee_msat
valentinewallace 9a3914d
Allow receiving less than the onion claims to pay
valentinewallace 47567e4
Track the sender's skimmed fee in UpdateAddHTLC
valentinewallace 5a79d6c
Persist update_add sender skimmed fee in Channel
valentinewallace 4cee622
Set UpdateAddHTLC::skimmed_fee_msat on forward
valentinewallace 0d94d9f
Check UpdateAddHTLC::skimmed_fee_msat on receive
valentinewallace 2127eb8
Document on claim events that amount_msat may be > invoice amount
valentinewallace File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -224,6 +224,7 @@ struct OutboundHTLCOutput { | ||
| payment_hash: PaymentHash, | ||
| state: OutboundHTLCState, | ||
| source: HTLCSource, | ||
| skimmed_fee_msat: Option<u64>, | ||
| } | ||
| /// See AwaitingRemoteRevoke ChannelState for more info | ||
| @@ -235,6 +236,8 @@ enum HTLCUpdateAwaitingACK { | ||
| payment_hash: PaymentHash, | ||
| source: HTLCSource, | ||
| onion_routing_packet: msgs::OnionPacket, | ||
| // The extra fee we're skimming off the top of this HTLC. | ||
| skimmed_fee_msat: Option<u64>, | ||
| }, | ||
| ClaimHTLC { | ||
| payment_preimage: PaymentPreimage, | ||
| @@ -3052,8 +3055,13 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | ||
| // handling this case better and maybe fulfilling some of the HTLCs while attempting | ||
| // to rebalance channels. | ||
| match &htlc_update { | ||
| &HTLCUpdateAwaitingACK::AddHTLC {amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet, ..} => { | ||
| match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(), onion_routing_packet.clone(), false, logger) { | ||
| &HTLCUpdateAwaitingACK::AddHTLC { | ||
| amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet, | ||
| skimmed_fee_msat, .. | ||
| } => { | ||
| match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(), | ||
| onion_routing_packet.clone(), false, skimmed_fee_msat, logger) | ||
| { | ||
| Ok(update_add_msg_option) => update_add_htlcs.push(update_add_msg_option.unwrap()), | ||
| Err(e) => { | ||
| match e { | ||
| @@ -3695,6 +3703,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | ||
| payment_hash: htlc.payment_hash, | ||
| cltv_expiry: htlc.cltv_expiry, | ||
| onion_routing_packet: (**onion_packet).clone(), | ||
| skimmed_fee_msat: htlc.skimmed_fee_msat, | ||
| }); | ||
| } | ||
| } | ||
| @@ -5042,11 +5051,13 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | ||
| /// commitment update. | ||
| /// | ||
| /// `Err`s will only be [`ChannelError::Ignore`]. | ||
| pub fn queue_add_htlc<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, | ||
| onion_routing_packet: msgs::OnionPacket, logger: &L) | ||
| -> Result<(), ChannelError> where L::Target: Logger { | ||
| pub fn queue_add_htlc<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>, logger: &L | ||
| ) -> Result<(), ChannelError> where L::Target: Logger { | ||
| self | ||
| .send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, true, logger) | ||
| .send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, true, | ||
| skimmed_fee_msat, logger) | ||
| .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?")) | ||
| .map_err(|err| { | ||
| if let ChannelError::Ignore(_) = err { /* fine */ } | ||
| @@ -5071,9 +5082,11 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | ||
| /// on this [`Channel`] if `force_holding_cell` is false. | ||
| /// | ||
| /// `Err`s will only be [`ChannelError::Ignore`]. | ||
| fn send_htlc<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, | ||
| onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool, logger: &L) | ||
| -> Result<Option<msgs::UpdateAddHTLC>, ChannelError> where L::Target: Logger { | ||
| fn send_htlc<L: Deref>( | ||
| &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>, logger: &L | ||
| ) -> Result<Option<msgs::UpdateAddHTLC>, ChannelError> where L::Target: Logger { | ||
| if (self.context.channel_state & (ChannelState::ChannelReady as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelReady as u32) { | ||
| return Err(ChannelError::Ignore("Cannot send HTLC until channel is fully established and we haven't started shutting down".to_owned())); | ||
| } | ||
| @@ -5125,6 +5138,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | ||
| cltv_expiry, | ||
| source, | ||
| onion_routing_packet, | ||
| skimmed_fee_msat, | ||
| }); | ||
| return Ok(None); | ||
| } | ||
| @@ -5136,6 +5150,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | ||
| cltv_expiry, | ||
| state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())), | ||
| source, | ||
| skimmed_fee_msat, | ||
| }); | ||
| let res = msgs::UpdateAddHTLC { | ||
| @@ -5145,6 +5160,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | ||
| payment_hash, | ||
| cltv_expiry, | ||
| onion_routing_packet, | ||
| skimmed_fee_msat, | ||
| }; | ||
| self.context.next_holder_htlc_id += 1; | ||
| @@ -5283,8 +5299,12 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | ||
| /// | ||
| /// Shorthand for calling [`Self::send_htlc`] followed by a commitment update, see docs on | ||
| /// [`Self::send_htlc`] and [`Self::build_commitment_no_state_update`] for more info. | ||
| pub fn send_htlc_and_commit<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket, logger: &L) -> Result<Option<&ChannelMonitorUpdate>, ChannelError> where L::Target: Logger { | ||
| let send_res = self.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, false, logger); | ||
| pub fn send_htlc_and_commit<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>, logger: &L | ||
| ) -> Result<Option<&ChannelMonitorUpdate>, ChannelError> where L::Target: Logger { | ||
| let send_res = self.send_htlc(amount_msat, payment_hash, cltv_expiry, source, | ||
| onion_routing_packet, false, skimmed_fee_msat, logger); | ||
| if let Err(e) = &send_res { if let ChannelError::Ignore(_) = e {} else { debug_assert!(false, "Sending cannot trigger channel failure"); } } | ||
| match send_res? { | ||
| Some(_) => { | ||
| @@ -6609,9 +6629,10 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> { | ||
| } | ||
| let mut preimages: Vec<&Option<PaymentPreimage>> = vec![]; | ||
| let mut pending_outbound_skimmed_fees: Vec<Option<u64>> = Vec::new(); | ||
| (self.context.pending_outbound_htlcs.len() as u64).write(writer)?; | ||
| for htlc in self.context.pending_outbound_htlcs.iter() { | ||
| for (idx, htlc) in self.context.pending_outbound_htlcs.iter().enumerate() { | ||
| htlc.htlc_id.write(writer)?; | ||
| htlc.amount_msat.write(writer)?; | ||
| htlc.cltv_expiry.write(writer)?; | ||
| @@ -6647,18 +6668,37 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> { | ||
| reason.write(writer)?; | ||
| } | ||
| } | ||
| if let Some(skimmed_fee) = htlc.skimmed_fee_msat { | ||
| if pending_outbound_skimmed_fees.is_empty() { | ||
| for _ in 0..idx { pending_outbound_skimmed_fees.push(None); } | ||
| } | ||
| pending_outbound_skimmed_fees.push(Some(skimmed_fee)); | ||
| } else if !pending_outbound_skimmed_fees.is_empty() { | ||
| pending_outbound_skimmed_fees.push(None); | ||
| } | ||
| } | ||
| let mut holding_cell_skimmed_fees: Vec<Option<u64>> = Vec::new(); | ||
| (self.context.holding_cell_htlc_updates.len() as u64).write(writer)?; | ||
| for update in self.context.holding_cell_htlc_updates.iter() { | ||
| for (idx, update) in self.context.holding_cell_htlc_updates.iter().enumerate() { | ||
| match update { | ||
| &HTLCUpdateAwaitingACK::AddHTLC { ref amount_msat, ref cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet } => { | ||
| &HTLCUpdateAwaitingACK::AddHTLC { | ||
| ref amount_msat, ref cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet, | ||
| skimmed_fee_msat, | ||
| } => { | ||
| 0u8.write(writer)?; | ||
| amount_msat.write(writer)?; | ||
| cltv_expiry.write(writer)?; | ||
| payment_hash.write(writer)?; | ||
| source.write(writer)?; | ||
| onion_routing_packet.write(writer)?; | ||
| if let Some(skimmed_fee) = skimmed_fee_msat { | ||
| if holding_cell_skimmed_fees.is_empty() { | ||
| for _ in 0..idx { holding_cell_skimmed_fees.push(None); } | ||
| } | ||
| holding_cell_skimmed_fees.push(Some(skimmed_fee)); | ||
| } else if !holding_cell_skimmed_fees.is_empty() { holding_cell_skimmed_fees.push(None); } | ||
| }, | ||
| &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, ref htlc_id } => { | ||
| 1u8.write(writer)?; | ||
| @@ -6825,6 +6865,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> { | ||
| (29, self.context.temporary_channel_id, option), | ||
| (31, channel_pending_event_emitted, option), | ||
| (33, self.context.pending_monitor_updates, vec_type), | ||
| (35, pending_outbound_skimmed_fees, optional_vec), | ||
| (37, holding_cell_skimmed_fees, optional_vec), | ||
| }); | ||
| Ok(()) | ||
| @@ -6935,6 +6977,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch | ||
| }, | ||
| _ => return Err(DecodeError::InvalidValue), | ||
| }, | ||
| skimmed_fee_msat: None, | ||
| }); | ||
| } | ||
| @@ -6948,6 +6991,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch | ||
| payment_hash: Readable::read(reader)?, | ||
| source: Readable::read(reader)?, | ||
| onion_routing_packet: Readable::read(reader)?, | ||
| skimmed_fee_msat: None, | ||
| }, | ||
| 1 => HTLCUpdateAwaitingACK::ClaimHTLC { | ||
| payment_preimage: Readable::read(reader)?, | ||
| @@ -7103,6 +7147,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch | ||
| let mut pending_monitor_updates = Some(Vec::new()); | ||
| let mut pending_outbound_skimmed_fees_opt: Option<Vec<Option<u64>>> = None; | ||
| let mut holding_cell_skimmed_fees_opt: Option<Vec<Option<u64>>> = None; | ||
| read_tlv_fields!(reader, { | ||
| (0, announcement_sigs, option), | ||
| (1, minimum_depth, option), | ||
| @@ -7126,6 +7173,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch | ||
| (29, temporary_channel_id, option), | ||
| (31, channel_pending_event_emitted, option), | ||
| (33, pending_monitor_updates, vec_type), | ||
| (35, pending_outbound_skimmed_fees_opt, optional_vec), | ||
TheBlueMatt marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| (37, holding_cell_skimmed_fees_opt, optional_vec), | ||
| }); | ||
| let (channel_keys_id, holder_signer) = if let Some(channel_keys_id) = channel_keys_id { | ||
| @@ -7180,6 +7229,25 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch | ||
| let holder_max_accepted_htlcs = holder_max_accepted_htlcs.unwrap_or(DEFAULT_MAX_HTLCS); | ||
| if let Some(skimmed_fees) = pending_outbound_skimmed_fees_opt { | ||
| let mut iter = skimmed_fees.into_iter(); | ||
| for htlc in pending_outbound_htlcs.iter_mut() { | ||
| htlc.skimmed_fee_msat = iter.next().ok_or(DecodeError::InvalidValue)?; | ||
| } | ||
| // We expect all skimmed fees to be consumed above | ||
| if iter.next().is_some() { return Err(DecodeError::InvalidValue) } | ||
| } | ||
| if let Some(skimmed_fees) = holding_cell_skimmed_fees_opt { | ||
| let mut iter = skimmed_fees.into_iter(); | ||
| for htlc in holding_cell_htlc_updates.iter_mut() { | ||
| if let HTLCUpdateAwaitingACK::AddHTLC { ref mut skimmed_fee_msat, .. } = htlc { | ||
| *skimmed_fee_msat = iter.next().ok_or(DecodeError::InvalidValue)?; | ||
| } | ||
| } | ||
| // We expect all skimmed fees to be consumed above | ||
| if iter.next().is_some() { return Err(DecodeError::InvalidValue) } | ||
| } | ||
| Ok(Channel { | ||
| context: ChannelContext { | ||
| user_id, | ||
| @@ -7522,7 +7590,8 @@ mod tests { | ||
| session_priv: SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(), | ||
| first_hop_htlc_msat: 548, | ||
| payment_id: PaymentId([42; 32]), | ||
| } | ||
| }, | ||
| skimmed_fee_msat: None, | ||
| }); | ||
| // Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass | ||
| @@ -8079,6 +8148,7 @@ mod tests { | ||
| payment_hash: PaymentHash([0; 32]), | ||
| state: OutboundHTLCState::Committed, | ||
| source: HTLCSource::dummy(), | ||
| skimmed_fee_msat: None, | ||
| }; | ||
| out.payment_hash.0 = Sha256::hash(&hex::decode("0202020202020202020202020202020202020202020202020202020202020202").unwrap()).into_inner(); | ||
| out | ||
| @@ -8091,6 +8161,7 @@ mod tests { | ||
| payment_hash: PaymentHash([0; 32]), | ||
| state: OutboundHTLCState::Committed, | ||
| source: HTLCSource::dummy(), | ||
| skimmed_fee_msat: None, | ||
| }; | ||
| out.payment_hash.0 = Sha256::hash(&hex::decode("0303030303030303030303030303030303030303030303030303030303030303").unwrap()).into_inner(); | ||
| out | ||
| @@ -8492,6 +8563,7 @@ mod tests { | ||
| payment_hash: PaymentHash([0; 32]), | ||
| state: OutboundHTLCState::Committed, | ||
| source: HTLCSource::dummy(), | ||
| skimmed_fee_msat: None, | ||
| }; | ||
| out.payment_hash.0 = Sha256::hash(&hex::decode("0505050505050505050505050505050505050505050505050505050505050505").unwrap()).into_inner(); | ||
| out | ||
| @@ -8504,6 +8576,7 @@ mod tests { | ||
| payment_hash: PaymentHash([0; 32]), | ||
| state: OutboundHTLCState::Committed, | ||
| source: HTLCSource::dummy(), | ||
| skimmed_fee_msat: None, | ||
| }; | ||
| out.payment_hash.0 = Sha256::hash(&hex::decode("0505050505050505050505050505050505050505050505050505050505050505").unwrap()).into_inner(); | ||
| out | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.