Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Convert to attributable errors #2256

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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: 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