Skip to content

Commit

Permalink
Encapsulate HTLCFailReason to not expose struct variants
Browse files Browse the repository at this point in the history
Now that `HTLCFailReason` is opaque and in `onion_utils`, we should
encapsulate it so that `ChannelManager` can no longer directly
access its inner fields.
  • Loading branch information
TheBlueMatt committed Dec 1, 2022
1 parent dabae73 commit 5ced282
Showing 1 changed file with 28 additions and 14 deletions.
42 changes: 28 additions & 14 deletions lightning/src/ln/onion_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,10 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
}

#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
pub(super) enum HTLCFailReason {
pub(super) struct HTLCFailReason(HTLCFailReasonRepr);

#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
enum HTLCFailReasonRepr {
LightningError {
err: msgs::OnionErrorPacket,
},
Expand All @@ -605,18 +608,29 @@ pub(super) enum HTLCFailReason {

impl core::fmt::Debug for HTLCFailReason {
fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
match self {
HTLCFailReason::Reason { ref failure_code, .. } => {
match self.0 {
HTLCFailReasonRepr::Reason { ref failure_code, .. } => {
write!(f, "HTLC error code {}", failure_code)
},
HTLCFailReason::LightningError { .. } => {
HTLCFailReasonRepr::LightningError { .. } => {
write!(f, "pre-built LightningError")
}
}
}
}

impl_writeable_tlv_based_enum!(HTLCFailReason,
impl Writeable for HTLCFailReason {
fn write<W: crate::util::ser::Writer>(&self, writer: &mut W) -> Result<(), crate::io::Error> {
self.0.write(writer)
}
}
impl Readable for HTLCFailReason {
fn read<R: crate::io::Read>(reader: &mut R) -> Result<Self, crate::ln::msgs::DecodeError> {
Ok(Self(Readable::read(reader)?))
}
}

impl_writeable_tlv_based_enum!(HTLCFailReasonRepr,
(0, LightningError) => {
(0, err, required),
},
Expand All @@ -628,20 +642,20 @@ impl_writeable_tlv_based_enum!(HTLCFailReason,

impl HTLCFailReason {
pub(super) fn reason(failure_code: u16, data: Vec<u8>) -> Self {
Self::Reason { failure_code, data }
Self(HTLCFailReasonRepr::Reason { failure_code, data })
}

pub(super) fn from_failure_code(failure_code: u16) -> Self {
Self::Reason { failure_code, data: Vec::new() }
Self(HTLCFailReasonRepr::Reason { failure_code, data: Vec::new() })
}

pub(super) fn from_msg(msg: &msgs::UpdateFailHTLC) -> Self {
Self::LightningError { err: msg.reason.clone() }
Self(HTLCFailReasonRepr::LightningError { err: msg.reason.clone() })
}

pub(super) fn get_encrypted_failure_packet(&self, incoming_packet_shared_secret: &[u8; 32], phantom_shared_secret: &Option<[u8; 32]>) -> msgs::OnionErrorPacket {
match self {
HTLCFailReason::Reason { ref failure_code, ref data } => {
match self.0 {
HTLCFailReasonRepr::Reason { ref failure_code, ref data } => {
if let Some(phantom_ss) = phantom_shared_secret {
let phantom_packet = build_failure_packet(phantom_ss, *failure_code, &data[..]).encode();
let encrypted_phantom_packet = encrypt_failure_packet(phantom_ss, &phantom_packet);
Expand All @@ -651,18 +665,18 @@ impl HTLCFailReason {
encrypt_failure_packet(incoming_packet_shared_secret, &packet)
}
},
HTLCFailReason::LightningError { err } => {
HTLCFailReasonRepr::LightningError { ref err } => {
encrypt_failure_packet(incoming_packet_shared_secret, &err.data)
}
}
}

pub(super) fn decode_onion_failure<T: secp256k1::Signing, L: Deref>(&self, secp_ctx: &Secp256k1<T>, logger: &L, htlc_source: &HTLCSource) -> (Option<crate::routing::gossip::NetworkUpdate>, Option<u64>, bool, Option<u16>, Option<Vec<u8>>) where L::Target: Logger {
match self {
HTLCFailReason::LightningError { ref err } => {
match self.0 {
HTLCFailReasonRepr::LightningError { ref err } => {
process_onion_failure(secp_ctx, logger, &htlc_source, err.data.clone())
},
HTLCFailReason::Reason { ref failure_code, ref data, .. } => {
HTLCFailReasonRepr::Reason { ref failure_code, ref data, .. } => {
// we get a fail_malformed_htlc from the first hop
// TODO: We'd like to generate a NetworkUpdate for temporary
// failures here, but that would be insufficient as find_route
Expand Down

0 comments on commit 5ced282

Please sign in to comment.