Skip to content

Commit 22225fa

Browse files
TheBlueMattTheBlueMatt
committed
Flatten OnionHopData struct with the Realm0 struct.
Previously OnionHopData contained a OnionRealm0HopData field however instead of bumping the realm number, it has been replaced with a length, used to indicte the length of a TLV-formatted object. Because a TLV-formatted hop data can contain the same information as a realm-0 hop data, we flatten the field and simply keep track of what format it was in.
1 parent fac80d6 commit 22225fa

File tree

4 files changed

+63
-83
lines changed

4 files changed

+63
-83
lines changed

‎lightning/src/ln/channelmanager.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -959,11 +959,11 @@ impl<ChanSigner: ChannelKeys> ChannelManager<ChanSigner> {
959959
return_err!("The final CLTV expiry is too soon to handle", 17, &[0;0]);
960960
}
961961
// final_incorrect_htlc_amount
962-
if next_hop_data.data.amt_to_forward > msg.amount_msat {
962+
if next_hop_data.amt_to_forward > msg.amount_msat {
963963
return_err!("Upstream node sent less than we were supposed to receive in payment", 19, &byte_utils::be64_to_array(msg.amount_msat));
964964
}
965965
// final_incorrect_cltv_expiry
966-
if next_hop_data.data.outgoing_cltv_value != msg.cltv_expiry {
966+
if next_hop_data.outgoing_cltv_value != msg.cltv_expiry {
967967
return_err!("Upstream node set CLTV to the wrong value", 18, &byte_utils::be32_to_array(msg.cltv_expiry));
968968
}
969969

@@ -977,8 +977,8 @@ impl<ChanSigner: ChannelKeys> ChannelManager<ChanSigner> {
977977
payment_hash: msg.payment_hash.clone(),
978978
short_channel_id: 0,
979979
incoming_shared_secret: shared_secret,
980-
amt_to_forward: next_hop_data.data.amt_to_forward,
981-
outgoing_cltv_value: next_hop_data.data.outgoing_cltv_value,
980+
amt_to_forward: next_hop_data.amt_to_forward,
981+
outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
982982
})
983983
} else {
984984
let mut new_packet_data = [0; 20*65];
@@ -1008,10 +1008,10 @@ impl<ChanSigner: ChannelKeys> ChannelManager<ChanSigner> {
10081008
PendingHTLCStatus::Forward(PendingForwardHTLCInfo {
10091009
onion_packet: Some(outgoing_packet),
10101010
payment_hash: msg.payment_hash.clone(),
1011-
short_channel_id: next_hop_data.data.short_channel_id,
1011+
short_channel_id: next_hop_data.short_channel_id,
10121012
incoming_shared_secret: shared_secret,
1013-
amt_to_forward: next_hop_data.data.amt_to_forward,
1014-
outgoing_cltv_value: next_hop_data.data.outgoing_cltv_value,
1013+
amt_to_forward: next_hop_data.amt_to_forward,
1014+
outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
10151015
})
10161016
};
10171017

‎lightning/src/ln/functional_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4854,7 +4854,7 @@ fn test_onion_failure() {
48544854
let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
48554855
let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route, &session_priv).unwrap();
48564856
let (mut onion_payloads, _htlc_msat, _htlc_cltv) = onion_utils::build_onion_payloads(&route, cur_height).unwrap();
4857-
onion_payloads[0].realm = 3;
4857+
onion_payloads[0].format = msgs::OnionHopDataFormat::BogusRealm(3);
48584858
msg.onion_routing_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
48594859
}, ||{}, true, Some(PERM|1), Some(msgs::HTLCFailChannelUpdate::ChannelClosed{short_channel_id: channels[1].0.contents.short_channel_id, is_permanent: true}));//XXX incremented channels idx here
48604860

@@ -4864,7 +4864,7 @@ fn test_onion_failure() {
48644864
let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
48654865
let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route, &session_priv).unwrap();
48664866
let (mut onion_payloads, _htlc_msat, _htlc_cltv) = onion_utils::build_onion_payloads(&route, cur_height).unwrap();
4867-
onion_payloads[1].realm = 3;
4867+
onion_payloads[1].format = msgs::OnionHopDataFormat::BogusRealm(3);
48684868
msg.onion_routing_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
48694869
}, ||{}, false, Some(PERM|1), Some(msgs::HTLCFailChannelUpdate::ChannelClosed{short_channel_id: channels[1].0.contents.short_channel_id, is_permanent: true}));
48704870

‎lightning/src/ln/msgs.rs

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -811,21 +811,25 @@ pub trait RoutingMessageHandler : Send + Sync {
811811
fn get_next_node_announcements(&self, starting_point: Option<&PublicKey>, batch_amount: u8) -> Vec<NodeAnnouncement>;
812812
}
813813

814-
pub(crate) struct OnionRealm0HopData {
815-
pub(crate) short_channel_id: u64,
816-
pub(crate) amt_to_forward: u64,
817-
pub(crate) outgoing_cltv_value: u32,
818-
// 12 bytes of 0-padding
819-
}
820-
821814
mod fuzzy_internal_msgs {
822815
// These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize
823816
// them from untrusted input):
824817

825-
use super::OnionRealm0HopData;
818+
pub(crate) enum OnionHopDataFormat {
819+
Legacy, // aka Realm-0
820+
// Some tests expect to be able to generate bogus non-deserializable OnionHopDatas. In the
821+
// future we can use bogus TLV attributes, but for now we have to expose a "bogus realm"
822+
// option.
823+
#[cfg(test)]
824+
BogusRealm(u8),
825+
}
826+
826827
pub struct OnionHopData {
827-
pub(crate) realm: u8,
828-
pub(crate) data: OnionRealm0HopData,
828+
pub(crate) format: OnionHopDataFormat,
829+
pub(crate) short_channel_id: u64,
830+
pub(crate) amt_to_forward: u64,
831+
pub(crate) outgoing_cltv_value: u32,
832+
// 12 bytes of 0-padding
829833
pub(crate) hmac: [u8; 32],
830834
}
831835

@@ -1199,36 +1203,18 @@ impl_writeable!(UpdateAddHTLC, 32+8+8+32+4+1366, {
11991203
onion_routing_packet
12001204
});
12011205

1202-
impl Writeable for OnionRealm0HopData {
1206+
impl Writeable for OnionHopData {
12031207
fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
1204-
w.size_hint(32);
1208+
w.size_hint(65);
1209+
match self.format {
1210+
OnionHopDataFormat::Legacy => 0u8.write(w)?,
1211+
#[cfg(test)]
1212+
OnionHopDataFormat::BogusRealm(v) => v.write(w)?,
1213+
}
12051214
self.short_channel_id.write(w)?;
12061215
self.amt_to_forward.write(w)?;
12071216
self.outgoing_cltv_value.write(w)?;
12081217
w.write_all(&[0;12])?;
1209-
Ok(())
1210-
}
1211-
}
1212-
1213-
impl<R: Read> Readable<R> for OnionRealm0HopData {
1214-
fn read(r: &mut R) -> Result<Self, DecodeError> {
1215-
Ok(OnionRealm0HopData {
1216-
short_channel_id: Readable::read(r)?,
1217-
amt_to_forward: Readable::read(r)?,
1218-
outgoing_cltv_value: {
1219-
let v: u32 = Readable::read(r)?;
1220-
r.read_exact(&mut [0; 12])?;
1221-
v
1222-
}
1223-
})
1224-
}
1225-
}
1226-
1227-
impl Writeable for OnionHopData {
1228-
fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
1229-
w.size_hint(65);
1230-
self.realm.write(w)?;
1231-
self.data.write(w)?;
12321218
self.hmac.write(w)?;
12331219
Ok(())
12341220
}
@@ -1237,14 +1223,20 @@ impl Writeable for OnionHopData {
12371223
impl<R: Read> Readable<R> for OnionHopData {
12381224
fn read(r: &mut R) -> Result<Self, DecodeError> {
12391225
Ok(OnionHopData {
1240-
realm: {
1226+
format: {
12411227
let r: u8 = Readable::read(r)?;
12421228
if r != 0 {
12431229
return Err(DecodeError::UnknownVersion);
12441230
}
1245-
r
1231+
OnionHopDataFormat::Legacy
1232+
},
1233+
short_channel_id: Readable::read(r)?,
1234+
amt_to_forward: Readable::read(r)?,
1235+
outgoing_cltv_value: {
1236+
let v: u32 = Readable::read(r)?;
1237+
r.read_exact(&mut [0; 12])?;
1238+
v
12461239
},
1247-
data: Readable::read(r)?,
12481240
hmac: Readable::read(r)?,
12491241
})
12501242
}

‎lightning/src/ln/onion_utils.rs

Lines changed: 24 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,10 @@ pub(super) fn build_onion_payloads(route: &Route, starting_htlc_offset: u32) ->
121121
let value_msat = if cur_value_msat == 0 { hop.fee_msat } else { cur_value_msat };
122122
let cltv = if cur_cltv == starting_htlc_offset { hop.cltv_expiry_delta + starting_htlc_offset } else { cur_cltv };
123123
res.insert(0, msgs::OnionHopData {
124-
realm: 0,
125-
data: msgs::OnionRealm0HopData {
126-
short_channel_id: last_short_channel_id,
127-
amt_to_forward: value_msat,
128-
outgoing_cltv_value: cltv,
129-
},
124+
format: msgs::OnionHopDataFormat::Legacy,
125+
short_channel_id: last_short_channel_id,
126+
amt_to_forward: value_msat,
127+
outgoing_cltv_value: cltv,
130128
hmac: [0; 32],
131129
});
132130
cur_value_msat += hop.fee_msat;
@@ -515,48 +513,38 @@ mod tests {
515513
// Test vectors below are flat-out wrong: they claim to set outgoing_cltv_value to non-0 :/
516514
let payloads = vec!(
517515
msgs::OnionHopData {
518-
realm: 0,
519-
data: msgs::OnionRealm0HopData {
520-
short_channel_id: 0,
521-
amt_to_forward: 0,
522-
outgoing_cltv_value: 0,
523-
},
516+
format: msgs::OnionHopDataFormat::Legacy,
517+
short_channel_id: 0,
518+
amt_to_forward: 0,
519+
outgoing_cltv_value: 0,
524520
hmac: [0; 32],
525521
},
526522
msgs::OnionHopData {
527-
realm: 0,
528-
data: msgs::OnionRealm0HopData {
529-
short_channel_id: 0x0101010101010101,
530-
amt_to_forward: 0x0100000001,
531-
outgoing_cltv_value: 0,
532-
},
523+
format: msgs::OnionHopDataFormat::Legacy,
524+
short_channel_id: 0x0101010101010101,
525+
amt_to_forward: 0x0100000001,
526+
outgoing_cltv_value: 0,
533527
hmac: [0; 32],
534528
},
535529
msgs::OnionHopData {
536-
realm: 0,
537-
data: msgs::OnionRealm0HopData {
538-
short_channel_id: 0x0202020202020202,
539-
amt_to_forward: 0x0200000002,
540-
outgoing_cltv_value: 0,
541-
},
530+
format: msgs::OnionHopDataFormat::Legacy,
531+
short_channel_id: 0x0202020202020202,
532+
amt_to_forward: 0x0200000002,
533+
outgoing_cltv_value: 0,
542534
hmac: [0; 32],
543535
},
544536
msgs::OnionHopData {
545-
realm: 0,
546-
data: msgs::OnionRealm0HopData {
547-
short_channel_id: 0x0303030303030303,
548-
amt_to_forward: 0x0300000003,
549-
outgoing_cltv_value: 0,
550-
},
537+
format: msgs::OnionHopDataFormat::Legacy,
538+
short_channel_id: 0x0303030303030303,
539+
amt_to_forward: 0x0300000003,
540+
outgoing_cltv_value: 0,
551541
hmac: [0; 32],
552542
},
553543
msgs::OnionHopData {
554-
realm: 0,
555-
data: msgs::OnionRealm0HopData {
556-
short_channel_id: 0x0404040404040404,
557-
amt_to_forward: 0x0400000004,
558-
outgoing_cltv_value: 0,
559-
},
544+
format: msgs::OnionHopDataFormat::Legacy,
545+
short_channel_id: 0x0404040404040404,
546+
amt_to_forward: 0x0400000004,
547+
outgoing_cltv_value: 0,
560548
hmac: [0; 32],
561549
},
562550
);

0 commit comments

Comments
 (0)