Skip to content

Commit 9d09f69

Browse files
TheBlueMattTheBlueMatt
committed
Relay/store channel/node announces w/ unknown req'd feature bits
This change was made in the flat features BOLT PR, as if a channel requires some unknown feature bits we should still rumor it, we just shouldn't route through it.
1 parent 2a8c6ee commit 9d09f69

File tree

2 files changed

+130
-34
lines changed

2 files changed

+130
-34
lines changed

‎lightning/src/ln/msgs.rs

+18-10
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,23 @@ impl<T: FeatureContext> Features<T> {
192192
pub(crate) fn byte_count(&self) -> usize {
193193
self.flags.len()
194194
}
195+
196+
#[cfg(test)]
197+
pub(crate) fn set_require_unknown_bits(&mut self) {
198+
let newlen = cmp::max(2, self.flags.len());
199+
self.flags.resize(newlen, 0u8);
200+
self.flags[1] |= 0x40;
201+
}
202+
203+
#[cfg(test)]
204+
pub(crate) fn clear_require_unknown_bits(&mut self) {
205+
let newlen = cmp::max(2, self.flags.len());
206+
self.flags.resize(newlen, 0u8);
207+
self.flags[1] &= !0x40;
208+
if self.flags.len() == 2 && self.flags[1] == 0 {
209+
self.flags.resize(1, 0u8);
210+
}
211+
}
195212
}
196213

197214
impl<T: FeatureContextInitNode> Features<T> {
@@ -1272,13 +1289,7 @@ impl Writeable for UnsignedChannelAnnouncement {
12721289
impl<R: Read> Readable<R> for UnsignedChannelAnnouncement {
12731290
fn read(r: &mut R) -> Result<Self, DecodeError> {
12741291
Ok(Self {
1275-
features: {
1276-
let f: ChannelFeatures = Readable::read(r)?;
1277-
if f.requires_unknown_bits() {
1278-
return Err(DecodeError::UnknownRequiredFeature);
1279-
}
1280-
f
1281-
},
1292+
features: Readable::read(r)?,
12821293
chain_hash: Readable::read(r)?,
12831294
short_channel_id: Readable::read(r)?,
12841295
node_id_1: Readable::read(r)?,
@@ -1406,9 +1417,6 @@ impl Writeable for UnsignedNodeAnnouncement {
14061417
impl<R: Read> Readable<R> for UnsignedNodeAnnouncement {
14071418
fn read(r: &mut R) -> Result<Self, DecodeError> {
14081419
let features: NodeFeatures = Readable::read(r)?;
1409-
if features.requires_unknown_bits() {
1410-
return Err(DecodeError::UnknownRequiredFeature);
1411-
}
14121420
let timestamp: u32 = Readable::read(r)?;
14131421
let node_id: PublicKey = Readable::read(r)?;
14141422
let mut rgb = [0; 3];

‎lightning/src/ln/router.rs

+112-24
Original file line numberDiff line numberDiff line change
@@ -414,10 +414,6 @@ impl RoutingMessageHandler for Router {
414414
let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]);
415415
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id);
416416

417-
if msg.contents.features.requires_unknown_bits() {
418-
panic!("Unknown-required-features NodeAnnouncements should never deserialize!");
419-
}
420-
421417
let mut network = self.network_map.write().unwrap();
422418
match network.nodes.get_mut(&msg.contents.node_id) {
423419
None => Err(LightningError{err: "No existing channels for node_announcement", action: ErrorAction::IgnoreError}),
@@ -432,7 +428,7 @@ impl RoutingMessageHandler for Router {
432428
node.alias = msg.contents.alias;
433429
node.addresses = msg.contents.addresses.clone();
434430

435-
let should_relay = msg.contents.excess_data.is_empty() && msg.contents.excess_address_data.is_empty() && !msg.contents.features.supports_unknown_bits();
431+
let should_relay = msg.contents.excess_data.is_empty() && msg.contents.excess_address_data.is_empty();
436432
node.announcement_message = if should_relay { Some(msg.clone()) } else { None };
437433
Ok(should_relay)
438434
}
@@ -450,10 +446,6 @@ impl RoutingMessageHandler for Router {
450446
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &msg.contents.bitcoin_key_1);
451447
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_2, &msg.contents.bitcoin_key_2);
452448

453-
if msg.contents.features.requires_unknown_bits() {
454-
panic!("Unknown-required-features ChannelAnnouncements should never deserialize!");
455-
}
456-
457449
let checked_utxo = match self.chain_monitor.get_chain_utxo(msg.contents.chain_hash, msg.contents.short_channel_id) {
458450
Ok((script_pubkey, _value)) => {
459451
let expected_script = Builder::new().push_opcode(opcodes::all::OP_PUSHNUM_2)
@@ -483,7 +475,7 @@ impl RoutingMessageHandler for Router {
483475
let mut network_lock = self.network_map.write().unwrap();
484476
let network = network_lock.borrow_parts();
485477

486-
let should_relay = msg.contents.excess_data.is_empty() && !msg.contents.features.supports_unknown_bits();
478+
let should_relay = msg.contents.excess_data.is_empty();
487479

488480
let chan_info = ChannelInfo {
489481
features: msg.contents.features.clone(),
@@ -939,19 +931,23 @@ impl Router {
939931
}
940932
}
941933

942-
for chan_id in $node.channels.iter() {
943-
let chan = network.channels.get(chan_id).unwrap();
944-
if chan.one_to_two.src_node_id == *$node_id {
945-
// ie $node is one, ie next hop in A* is two, via the two_to_one channel
946-
if first_hops.is_none() || chan.two_to_one.src_node_id != network.our_node_id {
947-
if chan.two_to_one.enabled {
948-
add_entry!(chan_id, chan.one_to_two.src_node_id, chan.two_to_one, $fee_to_target_msat);
949-
}
950-
}
951-
} else {
952-
if first_hops.is_none() || chan.one_to_two.src_node_id != network.our_node_id {
953-
if chan.one_to_two.enabled {
954-
add_entry!(chan_id, chan.two_to_one.src_node_id, chan.one_to_two, $fee_to_target_msat);
934+
if !$node.features.requires_unknown_bits() {
935+
for chan_id in $node.channels.iter() {
936+
let chan = network.channels.get(chan_id).unwrap();
937+
if !chan.features.requires_unknown_bits() {
938+
if chan.one_to_two.src_node_id == *$node_id {
939+
// ie $node is one, ie next hop in A* is two, via the two_to_one channel
940+
if first_hops.is_none() || chan.two_to_one.src_node_id != network.our_node_id {
941+
if chan.two_to_one.enabled {
942+
add_entry!(chan_id, chan.one_to_two.src_node_id, chan.two_to_one, $fee_to_target_msat);
943+
}
944+
}
945+
} else {
946+
if first_hops.is_none() || chan.one_to_two.src_node_id != network.our_node_id {
947+
if chan.one_to_two.enabled {
948+
add_entry!(chan_id, chan.two_to_one.src_node_id, chan.one_to_two, $fee_to_target_msat);
949+
}
950+
}
955951
}
956952
}
957953
}
@@ -1015,7 +1011,7 @@ mod tests {
10151011
use chain::chaininterface;
10161012
use ln::channelmanager;
10171013
use ln::router::{Router,NodeInfo,NetworkMap,ChannelInfo,DirectionalChannelInfo,RouteHint};
1018-
use ln::msgs::{ChannelFeatures, NodeFeatures};
1014+
use ln::msgs::{ChannelFeatures, NodeFeatures, LightningError, ErrorAction};
10191015
use util::test_utils;
10201016
use util::test_utils::TestVecWriter;
10211017
use util::logger::Logger;
@@ -1441,6 +1437,98 @@ mod tests {
14411437
assert_eq!(route.hops[1].cltv_expiry_delta, 42);
14421438
}
14431439

1440+
{ // Disable 4 and 12 by requiring unknown feature bits
1441+
let mut network = router.network_map.write().unwrap();
1442+
network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.set_require_unknown_bits();
1443+
network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.set_require_unknown_bits();
1444+
}
1445+
1446+
{ // If all the channels require some features we don't understand, route should fail
1447+
if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = router.get_route(&node3, None, &Vec::new(), 100, 42) {
1448+
assert_eq!(err, "Failed to find a path to the given destination");
1449+
} else { panic!(); }
1450+
}
1451+
1452+
{ // If we specify a channel to node8, that overrides our local channel view and that gets used
1453+
let our_chans = vec![channelmanager::ChannelDetails {
1454+
channel_id: [0; 32],
1455+
short_channel_id: Some(42),
1456+
remote_network_id: node8.clone(),
1457+
channel_value_satoshis: 0,
1458+
user_id: 0,
1459+
outbound_capacity_msat: 0,
1460+
inbound_capacity_msat: 0,
1461+
is_live: true,
1462+
}];
1463+
let route = router.get_route(&node3, Some(&our_chans), &Vec::new(), 100, 42).unwrap();
1464+
assert_eq!(route.hops.len(), 2);
1465+
1466+
assert_eq!(route.hops[0].pubkey, node8);
1467+
assert_eq!(route.hops[0].short_channel_id, 42);
1468+
assert_eq!(route.hops[0].fee_msat, 200);
1469+
assert_eq!(route.hops[0].cltv_expiry_delta, (13 << 8) | 1);
1470+
1471+
assert_eq!(route.hops[1].pubkey, node3);
1472+
assert_eq!(route.hops[1].short_channel_id, 13);
1473+
assert_eq!(route.hops[1].fee_msat, 100);
1474+
assert_eq!(route.hops[1].cltv_expiry_delta, 42);
1475+
}
1476+
1477+
{ // Re-enable 4 and 12 by wiping the unknown feature bits
1478+
let mut network = router.network_map.write().unwrap();
1479+
network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.clear_require_unknown_bits();
1480+
network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.clear_require_unknown_bits();
1481+
}
1482+
1483+
{ // Disable nodes 1, 2, and 8 by requiring unknown feature bits
1484+
let mut network = router.network_map.write().unwrap();
1485+
network.nodes.get_mut(&node1).unwrap().features.set_require_unknown_bits();
1486+
network.nodes.get_mut(&node2).unwrap().features.set_require_unknown_bits();
1487+
network.nodes.get_mut(&node8).unwrap().features.set_require_unknown_bits();
1488+
}
1489+
1490+
{ // If all nodes require some features we don't understand, route should fail
1491+
if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = router.get_route(&node3, None, &Vec::new(), 100, 42) {
1492+
assert_eq!(err, "Failed to find a path to the given destination");
1493+
} else { panic!(); }
1494+
}
1495+
1496+
{ // If we specify a channel to node8, that overrides our local channel view and that gets used
1497+
let our_chans = vec![channelmanager::ChannelDetails {
1498+
channel_id: [0; 32],
1499+
short_channel_id: Some(42),
1500+
remote_network_id: node8.clone(),
1501+
channel_value_satoshis: 0,
1502+
user_id: 0,
1503+
outbound_capacity_msat: 0,
1504+
inbound_capacity_msat: 0,
1505+
is_live: true,
1506+
}];
1507+
let route = router.get_route(&node3, Some(&our_chans), &Vec::new(), 100, 42).unwrap();
1508+
assert_eq!(route.hops.len(), 2);
1509+
1510+
assert_eq!(route.hops[0].pubkey, node8);
1511+
assert_eq!(route.hops[0].short_channel_id, 42);
1512+
assert_eq!(route.hops[0].fee_msat, 200);
1513+
assert_eq!(route.hops[0].cltv_expiry_delta, (13 << 8) | 1);
1514+
1515+
assert_eq!(route.hops[1].pubkey, node3);
1516+
assert_eq!(route.hops[1].short_channel_id, 13);
1517+
assert_eq!(route.hops[1].fee_msat, 100);
1518+
assert_eq!(route.hops[1].cltv_expiry_delta, 42);
1519+
}
1520+
1521+
{ // Re-enable nodes 1, 2, and 8
1522+
let mut network = router.network_map.write().unwrap();
1523+
network.nodes.get_mut(&node1).unwrap().features.clear_require_unknown_bits();
1524+
network.nodes.get_mut(&node2).unwrap().features.clear_require_unknown_bits();
1525+
network.nodes.get_mut(&node8).unwrap().features.clear_require_unknown_bits();
1526+
}
1527+
1528+
// Note that we don't test disabling node 3 and failing to route to it, as we (somewhat
1529+
// naively) assume that the user checked the feature bits on the invoice, which override
1530+
// the node_announcement.
1531+
14441532
{ // Route to 1 via 2 and 3 because our channel to 1 is disabled
14451533
let route = router.get_route(&node1, None, &Vec::new(), 100, 42).unwrap();
14461534
assert_eq!(route.hops.len(), 3);

0 commit comments

Comments
 (0)