Skip to content

Commit

Permalink
convert to attributable errors
Browse files Browse the repository at this point in the history
  • Loading branch information
joostjager committed May 3, 2023
1 parent c182567 commit e4ff468
Show file tree
Hide file tree
Showing 6 changed files with 386 additions and 52 deletions.
5 changes: 4 additions & 1 deletion lightning-invoice/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,12 +519,15 @@ impl InvoiceBuilder<tb::False, tb::False, tb::False, tb::False, tb::False, tb::F
/// Construct new, empty `InvoiceBuilder`. All necessary fields have to be filled first before
/// `InvoiceBuilder::build(self)` becomes available.
pub fn new(currency: Currency) -> Self {
let mut features = InvoiceFeatures::empty();
features.set_attributable_errors_optional();

InvoiceBuilder {
currency,
amount: None,
si_prefix: None,
timestamp: None,
tagged_fields: Vec::new(),
tagged_fields: vec![TaggedField::Features(features)],
error: None,

phantom_d: core::marker::PhantomData,
Expand Down
69 changes: 58 additions & 11 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, Pa
use crate::routing::scoring::ProbabilisticScorer;
use crate::ln::msgs;
use crate::ln::onion_utils;
use crate::ln::onion_utils::HTLCFailReason;
use crate::ln::onion_utils::{HTLCFailReason,FailureStructure};
use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
#[cfg(test)]
use crate::ln::outbound_payment;
Expand Down Expand Up @@ -128,6 +128,7 @@ pub(super) struct PendingHTLCInfo {
/// may overshoot this in either case)
pub(super) outgoing_amt_msat: u64,
pub(super) outgoing_cltv_value: u32,
pub(super) structure: Option<FailureStructure>,
}

#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
Expand Down Expand Up @@ -167,7 +168,7 @@ pub(super) enum HTLCForwardInfo {
}

/// Tracks the inbound corresponding to an outbound HTLC
#[derive(Clone, Hash, PartialEq, Eq)]
#[derive(Clone, Hash, PartialEq, Eq, Debug)]
pub(crate) struct HTLCPreviousHopData {
// Note that this may be an outbound SCID alias for the associated channel.
short_channel_id: u64,
Expand All @@ -178,6 +179,8 @@ pub(crate) struct HTLCPreviousHopData {
// This field is consumed by `claim_funds_from_hop()` when updating a force-closed backwards
// channel with a preimage provided by the forward channel.
outpoint: OutPoint,

structure: Option<FailureStructure>,
}

enum OnionPayload {
Expand Down Expand Up @@ -278,7 +281,7 @@ impl_writeable_tlv_based_enum!(SentHTLCId,

/// Tracks the inbound corresponding to an outbound HTLC
#[allow(clippy::derive_hash_xor_eq)] // Our Hash is faithful to the data, we just don't have SecretKey::hash
#[derive(Clone, PartialEq, Eq)]
#[derive(Clone, PartialEq, Eq, Debug)]
pub(crate) enum HTLCSource {
PreviousHopData(HTLCPreviousHopData),
OutboundRoute {
Expand Down Expand Up @@ -2327,13 +2330,15 @@ where
}
},
};

Ok(PendingHTLCInfo {
routing,
payment_hash,
incoming_shared_secret: shared_secret,
incoming_amt_msat: Some(amt_msat),
outgoing_amt_msat: hop_data.amt_to_forward,
outgoing_cltv_value: hop_data.outgoing_cltv_value,
structure: hop_data.structure,
})
}

Expand Down Expand Up @@ -2373,11 +2378,15 @@ where
($msg: expr, $err_code: expr, $data: expr) => {
{
log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg);
let no_structure = FailureStructure { // TODO: Return legacy failure.
max_hops: 3,
payload_len: 8,
};
return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
reason: HTLCFailReason::reason($err_code, $data.to_vec())
.get_encrypted_failure_packet(&shared_secret, &None),
.get_encrypted_failure_packet(&shared_secret, &None, &no_structure),
}));
}
}
Expand Down Expand Up @@ -2433,6 +2442,7 @@ where
incoming_amt_msat: Some(msg.amount_msat),
outgoing_amt_msat: next_hop_data.amt_to_forward,
outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
structure: next_hop_data.structure,
})
}
};
Expand Down Expand Up @@ -3219,6 +3229,7 @@ where
htlc_id: payment.prev_htlc_id,
incoming_packet_shared_secret: payment.forward_info.incoming_shared_secret,
phantom_shared_secret: None,
structure: payment.forward_info.structure,
});

let failure_reason = HTLCFailReason::from_failure_code(0x4000 | 10);
Expand Down Expand Up @@ -3253,7 +3264,8 @@ where
prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
forward_info: PendingHTLCInfo {
routing, incoming_shared_secret, payment_hash, outgoing_amt_msat,
outgoing_cltv_value, incoming_amt_msat: _
outgoing_cltv_value, incoming_amt_msat: _,
structure,
}
}) => {
macro_rules! failure_handler {
Expand All @@ -3266,6 +3278,7 @@ where
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret,
phantom_shared_secret: $phantom_ss,
structure,
});

let reason = if $next_hop_unknown {
Expand Down Expand Up @@ -3367,6 +3380,7 @@ where
forward_info: PendingHTLCInfo {
incoming_shared_secret, payment_hash, outgoing_amt_msat, outgoing_cltv_value,
routing: PendingHTLCRouting::Forward { onion_packet, .. }, incoming_amt_msat: _,
structure,
},
}) => {
log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id);
Expand All @@ -3377,6 +3391,7 @@ where
incoming_packet_shared_secret: incoming_shared_secret,
// Phantom payments are only PendingHTLCRouting::Receive.
phantom_shared_secret: None,
structure,
});
if let Err(e) = chan.get_mut().queue_add_htlc(outgoing_amt_msat,
payment_hash, outgoing_cltv_value, htlc_source.clone(),
Expand Down Expand Up @@ -3424,7 +3439,8 @@ where
HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
forward_info: PendingHTLCInfo {
routing, incoming_shared_secret, payment_hash, incoming_amt_msat, outgoing_amt_msat, ..
routing, incoming_shared_secret, payment_hash, incoming_amt_msat, outgoing_amt_msat,
structure, ..
}
}) => {
let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret, mut onion_fields) = match routing {
Expand All @@ -3451,6 +3467,7 @@ where
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret,
phantom_shared_secret,
structure: structure,
},
// We differentiate the received value from the sender intended value
// if possible so that we don't prematurely mark MPP payments complete
Expand Down Expand Up @@ -3479,6 +3496,7 @@ where
htlc_id: $htlc.prev_hop.htlc_id,
incoming_packet_shared_secret: $htlc.prev_hop.incoming_packet_shared_secret,
phantom_shared_secret,
structure: $htlc.prev_hop.structure,
}), payment_hash,
HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
HTLCDestination::FailedPayment { payment_hash: $payment_hash },
Expand Down Expand Up @@ -3676,6 +3694,7 @@ where
HTLCForwardInfo::FailHTLC { .. } => {
panic!("Got pending fail of our own HTLC");
}
_ => panic!("Unsupported forward info"),
}
}
}
Expand Down Expand Up @@ -4099,9 +4118,11 @@ where
&self.pending_events, &self.logger)
{ self.push_pending_forwards_ev(); }
},
HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint, structure: Some(structure) }) => {
log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with {:?}", log_bytes!(payment_hash.0), onion_error);
let err_packet = onion_error.get_encrypted_failure_packet(incoming_packet_shared_secret, phantom_shared_secret);
let err_packet = onion_error.get_encrypted_failure_packet(incoming_packet_shared_secret, phantom_shared_secret, structure);

log_trace!(self.logger, "Failure packet length: {}", err_packet.data.len());

let mut push_forward_ev = false;
let mut forward_htlcs = self.forward_htlcs.lock().unwrap();
Expand All @@ -4124,6 +4145,7 @@ where
failed_next_destination: destination,
});
},
_ => panic!("Unhandled htlc source type {:?}", source),
}
}

Expand Down Expand Up @@ -5034,13 +5056,13 @@ where
// but if we've sent a shutdown and they haven't acknowledged it yet, we just
// want to reject the new HTLC and fail it backwards instead of forwarding.
match pending_forward_info {
PendingHTLCStatus::Forward(PendingHTLCInfo { ref incoming_shared_secret, .. }) => {
PendingHTLCStatus::Forward(PendingHTLCInfo { ref incoming_shared_secret, structure: Some(structure), .. }) => { // TODO: Implement legacy.
let reason = if (error_code & 0x1000) != 0 {
let (real_code, error_data) = self.get_htlc_inbound_temp_fail_err_and_data(error_code, chan);
HTLCFailReason::reason(real_code, error_data)
} else {
HTLCFailReason::from_failure_code(error_code)
}.get_encrypted_failure_packet(incoming_shared_secret, &None);
}.get_encrypted_failure_packet(incoming_shared_secret, &None, &structure);
let msg = msgs::UpdateFailHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
Expand Down Expand Up @@ -5190,6 +5212,7 @@ where
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: forward_info.incoming_shared_secret,
phantom_shared_secret: None,
structure: forward_info.structure,
});

failed_intercept_forwards.push((htlc_source, forward_info.payment_hash,
Expand Down Expand Up @@ -6273,6 +6296,7 @@ where
incoming_packet_shared_secret: htlc.forward_info.incoming_shared_secret,
phantom_shared_secret: None,
outpoint: htlc.prev_funding_outpoint,
structure: htlc.forward_info.structure.clone(),
});

let requested_forward_scid /* intercept scid */ = match htlc.forward_info.routing {
Expand Down Expand Up @@ -6695,6 +6719,7 @@ pub fn provided_init_features(_config: &UserConfig) -> InitFeatures {
features.set_channel_type_optional();
features.set_scid_privacy_optional();
features.set_zero_conf_optional();
features.set_attributable_errors_optional();
#[cfg(anchors)]
{ // Attributes are not allowed on if expressions on our current MSRV of 1.41.
if _config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx {
Expand Down Expand Up @@ -6855,13 +6880,34 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting,
},
;);

impl Writeable for FailureStructure {
fn write<W:Writer>(&self,w: &mut W) -> Result<(),io::Error>{
self.max_hops.write(w)?;
self.payload_len.write(w)?;

Ok(())
}
}

impl Readable for FailureStructure {
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
let max_hops: u8 = Readable::read(r)?;
let payload_len: u8 = Readable::read(r)?;
Ok(FailureStructure {
max_hops,
payload_len,
})
}
}

impl_writeable_tlv_based!(PendingHTLCInfo, {
(0, routing, required),
(2, incoming_shared_secret, required),
(4, payment_hash, required),
(6, outgoing_amt_msat, required),
(8, outgoing_cltv_value, required),
(9, incoming_amt_msat, option),
(11, structure, option),
});


Expand Down Expand Up @@ -6942,7 +6988,8 @@ impl_writeable_tlv_based!(HTLCPreviousHopData, {
(1, phantom_shared_secret, option),
(2, outpoint, required),
(4, htlc_id, required),
(6, incoming_packet_shared_secret, required)
(6, incoming_packet_shared_secret, required),
(7, structure, option)
});

impl Writeable for ClaimableHTLC {
Expand Down
10 changes: 7 additions & 3 deletions lightning/src/ln/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ mod sealed {
// Byte 2
BasicMPP | Wumbo | AnchorsZeroFeeHtlcTx,
// Byte 3
ShutdownAnySegwit,
ShutdownAnySegwit | AttributableErrors,
// Byte 4
OnionMessages,
// Byte 5
Expand All @@ -152,7 +152,7 @@ mod sealed {
// Byte 2
BasicMPP | Wumbo | AnchorsZeroFeeHtlcTx,
// Byte 3
ShutdownAnySegwit,
ShutdownAnySegwit | AttributableErrors,
// Byte 4
OnionMessages,
// Byte 5
Expand All @@ -169,7 +169,7 @@ mod sealed {
// Byte 2
BasicMPP,
// Byte 3
,
AttributableErrors,
// Byte 4
,
// Byte 5
Expand Down Expand Up @@ -384,6 +384,9 @@ mod sealed {
define_feature!(27, ShutdownAnySegwit, [InitContext, NodeContext],
"Feature flags for `opt_shutdown_anysegwit`.", set_shutdown_any_segwit_optional,
set_shutdown_any_segwit_required, supports_shutdown_anysegwit, requires_shutdown_anysegwit);
define_feature!(29, AttributableErrors, [InitContext, NodeContext, InvoiceContext],
"Feature flags for `option_attributable_errors`.", set_attributable_errors_optional,
set_attributable_errors_required, supports_attributable_errors, requires_attributable_errors);
define_feature!(39, OnionMessages, [InitContext, NodeContext],
"Feature flags for `option_onion_messages`.", set_onion_messages_optional,
set_onion_messages_required, supports_onion_messages, requires_onion_messages);
Expand Down Expand Up @@ -863,6 +866,7 @@ mod tests {
init_features.set_basic_mpp_optional();
init_features.set_wumbo_optional();
init_features.set_shutdown_any_segwit_optional();
init_features.set_attributable_errors_optional();
init_features.set_onion_messages_optional();
init_features.set_channel_type_optional();
init_features.set_scid_privacy_optional();
Expand Down
Loading

0 comments on commit e4ff468

Please sign in to comment.