Skip to content

Commit c991643

Browse files
authored
Merge pull request #433 from TheBlueMatt/2019-12-features-in-routes
Plumb Features through into Routes
2 parents f263b37 + 617a680 commit c991643

12 files changed

+330
-106
lines changed

fuzz/src/chanmon_consistency.rs

+12-6
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ use lightning::ln::channelmonitor;
2929
use lightning::ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, HTLCUpdate};
3030
use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage, ChannelManagerReadArgs};
3131
use lightning::ln::router::{Route, RouteHop};
32-
use lightning::ln::features::InitFeatures;
33-
use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, ErrorAction, UpdateAddHTLC};
32+
use lightning::ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
33+
use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, ErrorAction, UpdateAddHTLC, Init};
3434
use lightning::util::enforcing_trait_impls::EnforcingChannelKeys;
3535
use lightning::util::events;
3636
use lightning::util::logger::Logger;
@@ -414,7 +414,9 @@ pub fn do_test(data: &[u8]) {
414414
if let Err(_) = $source.send_payment(Route {
415415
hops: vec![RouteHop {
416416
pubkey: $dest.0.get_our_node_id(),
417+
node_features: NodeFeatures::empty(),
417418
short_channel_id: $dest.1,
419+
channel_features: ChannelFeatures::empty(),
418420
fee_msat: 5000000,
419421
cltv_expiry_delta: 200,
420422
}],
@@ -429,12 +431,16 @@ pub fn do_test(data: &[u8]) {
429431
if let Err(_) = $source.send_payment(Route {
430432
hops: vec![RouteHop {
431433
pubkey: $middle.0.get_our_node_id(),
434+
node_features: NodeFeatures::empty(),
432435
short_channel_id: $middle.1,
436+
channel_features: ChannelFeatures::empty(),
433437
fee_msat: 50000,
434438
cltv_expiry_delta: 100,
435439
},RouteHop {
436440
pubkey: $dest.0.get_our_node_id(),
441+
node_features: NodeFeatures::empty(),
437442
short_channel_id: $dest.1,
443+
channel_features: ChannelFeatures::empty(),
438444
fee_msat: 5000000,
439445
cltv_expiry_delta: 200,
440446
}],
@@ -650,15 +656,15 @@ pub fn do_test(data: &[u8]) {
650656
},
651657
0x11 => {
652658
if chan_a_disconnected {
653-
nodes[0].peer_connected(&nodes[1].get_our_node_id());
654-
nodes[1].peer_connected(&nodes[0].get_our_node_id());
659+
nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: InitFeatures::empty() });
660+
nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: InitFeatures::empty() });
655661
chan_a_disconnected = false;
656662
}
657663
},
658664
0x12 => {
659665
if chan_b_disconnected {
660-
nodes[1].peer_connected(&nodes[2].get_our_node_id());
661-
nodes[2].peer_connected(&nodes[1].get_our_node_id());
666+
nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: InitFeatures::empty() });
667+
nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: InitFeatures::empty() });
662668
chan_b_disconnected = false;
663669
}
664670
},

fuzz/src/router.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ use bitcoin::blockdata::transaction::Transaction;
55

66
use lightning::chain::chaininterface::{ChainError,ChainWatchInterface};
77
use lightning::ln::channelmanager::ChannelDetails;
8+
use lightning::ln::features::InitFeatures;
89
use lightning::ln::msgs;
9-
use lightning::ln::msgs::{RoutingMessageHandler};
10+
use lightning::ln::msgs::RoutingMessageHandler;
1011
use lightning::ln::router::{Router, RouteHint};
1112
use lightning::util::logger::Logger;
1213
use lightning::util::ser::Readable;
@@ -198,6 +199,7 @@ pub fn do_test(data: &[u8]) {
198199
channel_id: [0; 32],
199200
short_channel_id: Some(slice_to_be64(get_slice!(8))),
200201
remote_network_id: get_pubkey!(),
202+
counterparty_features: InitFeatures::empty(),
201203
channel_value_satoshis: slice_to_be64(get_slice!(8)),
202204
user_id: 0,
203205
inbound_capacity_msat: 0,

lightning/src/ln/chanmon_update_fail_tests.rs

+12-12
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
215215
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
216216
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
217217

218-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
218+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
219219
let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
220220
assert_eq!(reestablish_1.len(), 1);
221-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
221+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
222222
let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
223223
assert_eq!(reestablish_2.len(), 1);
224224

@@ -237,10 +237,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
237237
assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
238238
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
239239

240-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
240+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
241241
let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
242242
assert_eq!(reestablish_1.len(), 1);
243-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
243+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
244244
let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
245245
assert_eq!(reestablish_2.len(), 1);
246246

@@ -938,8 +938,8 @@ fn test_monitor_update_fail_reestablish() {
938938
commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, false);
939939

940940
*nodes[1].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
941-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
942-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
941+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
942+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
943943

944944
let as_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
945945
let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
@@ -954,8 +954,8 @@ fn test_monitor_update_fail_reestablish() {
954954
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
955955
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
956956

957-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
958-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
957+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
958+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
959959

960960
assert!(as_reestablish == get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id()));
961961
assert!(bs_reestablish == get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id()));
@@ -1118,8 +1118,8 @@ fn claim_while_disconnected_monitor_update_fail() {
11181118
assert!(nodes[1].node.claim_funds(payment_preimage_1, 1_000_000));
11191119
check_added_monitors!(nodes[1], 1);
11201120

1121-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
1122-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
1121+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
1122+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
11231123

11241124
let as_reconnect = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
11251125
let bs_reconnect = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
@@ -1246,8 +1246,8 @@ fn monitor_failed_no_reestablish_response() {
12461246
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
12471247
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
12481248

1249-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
1250-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
1249+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
1250+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
12511251

12521252
let as_reconnect = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
12531253
let bs_reconnect = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());

lightning/src/ln/channelmanager.rs

+95-36
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,12 @@ pub(super) struct ChannelHolder<ChanSigner: ChannelKeys> {
275275
pub(super) pending_msg_events: Vec<events::MessageSendEvent>,
276276
}
277277

278+
/// State we hold per-peer. In the future we should put channels in here, but for now we only hold
279+
/// the latest Init features we heard from the peer.
280+
struct PeerState {
281+
latest_features: InitFeatures,
282+
}
283+
278284
#[cfg(not(any(target_pointer_width = "32", target_pointer_width = "64")))]
279285
const ERR: () = "You need at least 32 bit pointers (well, usize, but we'll assume they're the same) for ChannelManager::latest_block_height";
280286

@@ -328,6 +334,14 @@ pub struct ChannelManager<ChanSigner: ChannelKeys> {
328334
channel_state: Mutex<ChannelHolder<ChanSigner>>,
329335
our_network_key: SecretKey,
330336

337+
/// The bulk of our storage will eventually be here (channels and message queues and the like).
338+
/// If we are connected to a peer we always at least have an entry here, even if no channels
339+
/// are currently open with that peer.
340+
/// Because adding or removing an entry is rare, we usually take an outer read lock and then
341+
/// operate on the inner value freely. Sadly, this prevents parallel operation when opening a
342+
/// new channel.
343+
per_peer_state: RwLock<HashMap<PublicKey, Mutex<PeerState>>>,
344+
331345
pending_events: Mutex<Vec<events::Event>>,
332346
/// Used when we have to take a BIG lock to make sure everything is self-consistent.
333347
/// Essentially just when we're serializing ourselves out.
@@ -390,6 +404,10 @@ pub struct ChannelDetails {
390404
pub short_channel_id: Option<u64>,
391405
/// The node_id of our counterparty
392406
pub remote_network_id: PublicKey,
407+
/// The Features the channel counterparty provided upon last connection.
408+
/// Useful for routing as it is the most up-to-date copy of the counterparty's features and
409+
/// many routing-relevant features are present in the init context.
410+
pub counterparty_features: InitFeatures,
393411
/// The value, in satoshis, of this channel as appears in the funding output
394412
pub channel_value_satoshis: u64,
395413
/// The user_id passed in to create_channel, or 0 if the channel was inbound.
@@ -610,6 +628,8 @@ impl<ChanSigner: ChannelKeys> ChannelManager<ChanSigner> {
610628
}),
611629
our_network_key: keys_manager.get_node_secret(),
612630

631+
per_peer_state: RwLock::new(HashMap::new()),
632+
613633
pending_events: Mutex::new(Vec::new()),
614634
total_consistency_lock: RwLock::new(()),
615635

@@ -660,56 +680,53 @@ impl<ChanSigner: ChannelKeys> ChannelManager<ChanSigner> {
660680
Ok(())
661681
}
662682

663-
/// Gets the list of open channels, in random order. See ChannelDetail field documentation for
664-
/// more information.
665-
pub fn list_channels(&self) -> Vec<ChannelDetails> {
666-
let channel_state = self.channel_state.lock().unwrap();
667-
let mut res = Vec::with_capacity(channel_state.by_id.len());
668-
for (channel_id, channel) in channel_state.by_id.iter() {
669-
let (inbound_capacity_msat, outbound_capacity_msat) = channel.get_inbound_outbound_available_balance_msat();
670-
res.push(ChannelDetails {
671-
channel_id: (*channel_id).clone(),
672-
short_channel_id: channel.get_short_channel_id(),
673-
remote_network_id: channel.get_their_node_id(),
674-
channel_value_satoshis: channel.get_value_satoshis(),
675-
inbound_capacity_msat,
676-
outbound_capacity_msat,
677-
user_id: channel.get_user_id(),
678-
is_live: channel.is_live(),
679-
});
680-
}
681-
res
682-
}
683-
684-
/// Gets the list of usable channels, in random order. Useful as an argument to
685-
/// Router::get_route to ensure non-announced channels are used.
686-
///
687-
/// These are guaranteed to have their is_live value set to true, see the documentation for
688-
/// ChannelDetails::is_live for more info on exactly what the criteria are.
689-
pub fn list_usable_channels(&self) -> Vec<ChannelDetails> {
690-
let channel_state = self.channel_state.lock().unwrap();
691-
let mut res = Vec::with_capacity(channel_state.by_id.len());
692-
for (channel_id, channel) in channel_state.by_id.iter() {
693-
// Note we use is_live here instead of usable which leads to somewhat confused
694-
// internal/external nomenclature, but that's ok cause that's probably what the user
695-
// really wanted anyway.
696-
if channel.is_live() {
683+
fn list_channels_with_filter<F: FnMut(&(&[u8; 32], &Channel<ChanSigner>)) -> bool>(&self, f: F) -> Vec<ChannelDetails> {
684+
let mut res = Vec::new();
685+
{
686+
let channel_state = self.channel_state.lock().unwrap();
687+
res.reserve(channel_state.by_id.len());
688+
for (channel_id, channel) in channel_state.by_id.iter().filter(f) {
697689
let (inbound_capacity_msat, outbound_capacity_msat) = channel.get_inbound_outbound_available_balance_msat();
698690
res.push(ChannelDetails {
699691
channel_id: (*channel_id).clone(),
700692
short_channel_id: channel.get_short_channel_id(),
701693
remote_network_id: channel.get_their_node_id(),
694+
counterparty_features: InitFeatures::empty(),
702695
channel_value_satoshis: channel.get_value_satoshis(),
703696
inbound_capacity_msat,
704697
outbound_capacity_msat,
705698
user_id: channel.get_user_id(),
706-
is_live: true,
699+
is_live: channel.is_live(),
707700
});
708701
}
709702
}
703+
let per_peer_state = self.per_peer_state.read().unwrap();
704+
for chan in res.iter_mut() {
705+
if let Some(peer_state) = per_peer_state.get(&chan.remote_network_id) {
706+
chan.counterparty_features = peer_state.lock().unwrap().latest_features.clone();
707+
}
708+
}
710709
res
711710
}
712711

712+
/// Gets the list of open channels, in random order. See ChannelDetail field documentation for
713+
/// more information.
714+
pub fn list_channels(&self) -> Vec<ChannelDetails> {
715+
self.list_channels_with_filter(|_| true)
716+
}
717+
718+
/// Gets the list of usable channels, in random order. Useful as an argument to
719+
/// Router::get_route to ensure non-announced channels are used.
720+
///
721+
/// These are guaranteed to have their is_live value set to true, see the documentation for
722+
/// ChannelDetails::is_live for more info on exactly what the criteria are.
723+
pub fn list_usable_channels(&self) -> Vec<ChannelDetails> {
724+
// Note we use is_live here instead of usable which leads to somewhat confused
725+
// internal/external nomenclature, but that's ok cause that's probably what the user
726+
// really wanted anyway.
727+
self.list_channels_with_filter(|&(_, ref channel)| channel.is_live())
728+
}
729+
713730
/// Begins the process of closing a channel. After this call (plus some timeout), no new HTLCs
714731
/// will be accepted on the given channel, and after additional timeout/the closing of all
715732
/// pending HTLCs, the channel will be closed on chain.
@@ -2780,6 +2797,7 @@ impl<ChanSigner: ChannelKeys> ChannelMessageHandler for ChannelManager<ChanSigne
27802797
let _ = self.total_consistency_lock.read().unwrap();
27812798
let mut failed_channels = Vec::new();
27822799
let mut failed_payments = Vec::new();
2800+
let mut no_channels_remain = true;
27832801
{
27842802
let mut channel_state_lock = self.channel_state.lock().unwrap();
27852803
let channel_state = &mut *channel_state_lock;
@@ -2818,6 +2836,8 @@ impl<ChanSigner: ChannelKeys> ChannelMessageHandler for ChannelManager<ChanSigne
28182836
short_to_id.remove(&short_id);
28192837
}
28202838
return false;
2839+
} else {
2840+
no_channels_remain = false;
28212841
}
28222842
}
28232843
true
@@ -2843,6 +2863,10 @@ impl<ChanSigner: ChannelKeys> ChannelMessageHandler for ChannelManager<ChanSigne
28432863
}
28442864
});
28452865
}
2866+
if no_channels_remain {
2867+
self.per_peer_state.write().unwrap().remove(their_node_id);
2868+
}
2869+
28462870
for failure in failed_channels.drain(..) {
28472871
self.finish_force_close_channel(failure);
28482872
}
@@ -2853,10 +2877,25 @@ impl<ChanSigner: ChannelKeys> ChannelMessageHandler for ChannelManager<ChanSigne
28532877
}
28542878
}
28552879

2856-
fn peer_connected(&self, their_node_id: &PublicKey) {
2880+
fn peer_connected(&self, their_node_id: &PublicKey, init_msg: &msgs::Init) {
28572881
log_debug!(self, "Generating channel_reestablish events for {}", log_pubkey!(their_node_id));
28582882

28592883
let _ = self.total_consistency_lock.read().unwrap();
2884+
2885+
{
2886+
let mut peer_state_lock = self.per_peer_state.write().unwrap();
2887+
match peer_state_lock.entry(their_node_id.clone()) {
2888+
hash_map::Entry::Vacant(e) => {
2889+
e.insert(Mutex::new(PeerState {
2890+
latest_features: init_msg.features.clone(),
2891+
}));
2892+
},
2893+
hash_map::Entry::Occupied(e) => {
2894+
e.get().lock().unwrap().latest_features = init_msg.features.clone();
2895+
},
2896+
}
2897+
}
2898+
28602899
let mut channel_state_lock = self.channel_state.lock().unwrap();
28612900
let channel_state = &mut *channel_state_lock;
28622901
let pending_msg_events = &mut channel_state.pending_msg_events;
@@ -3123,6 +3162,14 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for ChannelManager<ChanSigne
31233162
}
31243163
}
31253164

3165+
let per_peer_state = self.per_peer_state.write().unwrap();
3166+
(per_peer_state.len() as u64).write(writer)?;
3167+
for (peer_pubkey, peer_state_mutex) in per_peer_state.iter() {
3168+
peer_pubkey.write(writer)?;
3169+
let peer_state = peer_state_mutex.lock().unwrap();
3170+
peer_state.latest_features.write(writer)?;
3171+
}
3172+
31263173
Ok(())
31273174
}
31283175
}
@@ -3256,6 +3303,16 @@ impl<'a, R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArg
32563303
claimable_htlcs.insert(payment_hash, previous_hops);
32573304
}
32583305

3306+
let peer_count: u64 = Readable::read(reader)?;
3307+
let mut per_peer_state = HashMap::with_capacity(cmp::min(peer_count as usize, 128));
3308+
for _ in 0..peer_count {
3309+
let peer_pubkey = Readable::read(reader)?;
3310+
let peer_state = PeerState {
3311+
latest_features: Readable::read(reader)?,
3312+
};
3313+
per_peer_state.insert(peer_pubkey, Mutex::new(peer_state));
3314+
}
3315+
32593316
let channel_manager = ChannelManager {
32603317
genesis_hash,
32613318
fee_estimator: args.fee_estimator,
@@ -3275,6 +3332,8 @@ impl<'a, R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArg
32753332
}),
32763333
our_network_key: args.keys_manager.get_node_secret(),
32773334

3335+
per_peer_state: RwLock::new(per_peer_state),
3336+
32783337
pending_events: Mutex::new(Vec::new()),
32793338
total_consistency_lock: RwLock::new(()),
32803339
keys_manager: args.keys_manager,

0 commit comments

Comments
 (0)