Skip to content

Commit 931868a

Browse files
Add ChannelScore struct
Adds a ChannelScore field under DirectionalInfoStruct to measure the performance of channels. Two methods were added to the NetworkGraph impl to allow for scoring PaymentSent and PaymentFailed events. Includes a test to ensure that NetworkGraph object is being properly updated as expected when the scoring functions are called on it.
1 parent fd71f8b commit 931868a

File tree

5 files changed

+231
-6
lines changed

5 files changed

+231
-6
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,7 +1845,12 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
18451845
// TODO: If we decided to blame ourselves (or one of our channels) in
18461846
// process_onion_failure we should close that channel as it implies our
18471847
// next-hop is needlessly blaming us!
1848+
let mut faultive_node = None;
18481849
if let Some(update) = channel_update {
1850+
faultive_node = match update.clone() {
1851+
msgs::HTLCFailChannelUpdate::NodeFailure {node_id, is_permanent: _} => Some(node_id),
1852+
_ => None,
1853+
};
18491854
self.channel_state.lock().unwrap().pending_msg_events.push(
18501855
events::MessageSendEvent::PaymentFailureNetworkUpdate {
18511856
update,
@@ -1856,6 +1861,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
18561861
events::Event::PaymentFailed {
18571862
payment_hash: payment_hash.clone(),
18581863
rejected_by_dest: !payment_retryable,
1864+
faultive_node: faultive_node,
18591865
#[cfg(test)]
18601866
error_code: onion_error_code,
18611867
#[cfg(test)]
@@ -1880,6 +1886,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
18801886
events::Event::PaymentFailed {
18811887
payment_hash: payment_hash.clone(),
18821888
rejected_by_dest: path.len() == 1,
1889+
faultive_node: None,
18831890
#[cfg(test)]
18841891
error_code: Some(*failure_code),
18851892
#[cfg(test)]

lightning/src/ln/functional_test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ macro_rules! expect_payment_failed {
770770
let events = $node.node.get_and_clear_pending_events();
771771
assert_eq!(events.len(), 1);
772772
match events[0] {
773-
Event::PaymentFailed { ref payment_hash, rejected_by_dest, ref error_code, ref error_data } => {
773+
Event::PaymentFailed { ref payment_hash, rejected_by_dest, faultive_node: _, ref error_code, ref error_data } => {
774774
assert_eq!(*payment_hash, $expected_payment_hash);
775775
assert_eq!(rejected_by_dest, $rejected_by_dest);
776776
assert!(error_code.is_some());

lightning/src/ln/functional_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5578,7 +5578,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
55785578

55795579
let events = nodes[0].node.get_and_clear_pending_events();
55805580
assert_eq!(events.len(), 1);
5581-
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code, error_data: _ } = &events[0] {
5581+
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, faultive_node: _, ref error_code, error_data: _ } = &events[0] {
55825582
assert_eq!(*rejected_by_dest, !expected_retryable);
55835583
assert_eq!(*error_code, expected_error_code);
55845584
} else {

lightning/src/routing/network_graph.rs

Lines changed: 217 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use chain::chaininterface::{ChainError, ChainWatchInterface};
1313
use ln::features::{ChannelFeatures, NodeFeatures};
1414
use ln::msgs::{DecodeError,ErrorAction,LightningError,RoutingMessageHandler,NetAddress};
1515
use ln::msgs;
16+
use routing::router::RouteHop;
1617
use util::ser::{Writeable, Readable, Writer};
1718
use util::logger::Logger;
1819

@@ -202,6 +203,36 @@ impl<C: Deref + Sync + Send, L: Deref + Sync + Send> RoutingMessageHandler for N
202203
}
203204
}
204205

206+
#[derive(PartialEq, Debug)]
207+
/// Tracks the score of a payment by holding onto certain metadata about
208+
/// the channel. Right now it stores a count of PaymentSent and PaymentFailed
209+
/// occurances for a given channel, but there's a lot more that could be
210+
/// added down the road (eg uptime, fee rates, variation, floppiness...)
211+
pub struct ChannelScore {
212+
/// Count of occurances of PaymentSent events
213+
pub payment_sent_score: u64,
214+
/// Count of occurances of PaymentFailed events
215+
pub payment_failed_score: u64,
216+
}
217+
218+
impl Readable for ChannelScore {
219+
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<ChannelScore, DecodeError> {
220+
let payment_sent_score: u64 = Readable::read(reader)?;
221+
let payment_failed_score: u64 = Readable::read(reader)?;
222+
Ok(ChannelScore {
223+
payment_sent_score,
224+
payment_failed_score,
225+
})
226+
}
227+
}
228+
229+
impl Writeable for ChannelScore {
230+
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
231+
self.payment_sent_score.write(writer)?;
232+
self.payment_failed_score.write(writer)?;
233+
Ok(())
234+
}
235+
}
205236
#[derive(PartialEq, Debug)]
206237
/// Details about one direction of a channel. Received
207238
/// within a channel update.
@@ -222,6 +253,8 @@ pub struct DirectionalChannelInfo {
222253
/// Everything else is useful only for sending out for initial routing sync.
223254
/// Not stored if contains excess data to prevent DoS.
224255
pub last_update_message: Option<msgs::ChannelUpdate>,
256+
/// A structure that tracks the reliablity of a channel.
257+
pub channel_score: ChannelScore,
225258
}
226259

227260
impl std::fmt::Display for DirectionalChannelInfo {
@@ -237,7 +270,8 @@ impl_writeable!(DirectionalChannelInfo, 0, {
237270
cltv_expiry_delta,
238271
htlc_minimum_msat,
239272
fees,
240-
last_update_message
273+
last_update_message,
274+
channel_score
241275
});
242276

243277
#[derive(PartialEq)]
@@ -506,6 +540,53 @@ impl NetworkGraph {
506540
None
507541
}
508542

543+
/// Increments payment_sent_score for appropriate DirectionalChannelInfos on the network graph.
544+
/// The appropriate directional channel is indentified by the RouteHop, which includes a
545+
/// PublicKey field. This is matched against the two PublicKey fields in ChannelInfo and is
546+
/// used to increment the score.
547+
pub fn score_payment_sent_for_route(&mut self, route: Vec<Vec<RouteHop>>) -> Result<bool, LightningError> {
548+
for path in route {
549+
for route_hop in path {
550+
let short_channel_id = route_hop.short_channel_id;
551+
let channel = self.channels.get_mut(&short_channel_id).unwrap();
552+
let directional_channel_info = if route_hop.pubkey == channel.node_one {
553+
channel.one_to_two.as_mut().unwrap()
554+
} else {
555+
channel.two_to_one.as_mut().unwrap()
556+
};
557+
let channel_score = &mut directional_channel_info.channel_score;
558+
channel_score.payment_sent_score += 1;
559+
}
560+
}
561+
Ok(true)
562+
}
563+
564+
/// Increments payment_failed_score for approriate DirectionalChannelInfos on the network
565+
/// graph. Like the case for scoring PaymentSent events, this method uses the node's PublicKey
566+
/// to identify the appropriate DirectionalChannelInfo to score. From there, there is a check
567+
/// against a list of at fault nodes that determines whether the node's channel score should be
568+
/// penalized for failing to execute or rewarded for executing properly.
569+
pub fn score_payment_failed_for_route(&mut self, route: Vec<Vec<RouteHop>>, faultive_nodes: Vec<PublicKey>) -> Result<bool, LightningError> {
570+
for path in route {
571+
for route_hop in path {
572+
let short_channel_id = route_hop.short_channel_id;
573+
let channel = self.channels.get_mut(&short_channel_id).unwrap();
574+
let directional_channel_info = if route_hop.pubkey == channel.node_one {
575+
channel.one_to_two.as_mut().unwrap()
576+
} else {
577+
channel.two_to_one.as_mut().unwrap()
578+
};
579+
let channel_score = &mut directional_channel_info.channel_score;
580+
if faultive_nodes.contains(&(route_hop.pubkey)) {
581+
channel_score.payment_failed_score += 1;
582+
} else {
583+
channel_score.payment_sent_score += 1;
584+
}
585+
}
586+
}
587+
Ok(true)
588+
}
589+
509590
/// For an already known node (from channel announcements), update its stored properties from a given node announcement
510591
/// Announcement signatures are checked here only if Secp256k1 object is provided.
511592
fn update_node_from_announcement(&mut self, msg: &msgs::NodeAnnouncement, secp_ctx: Option<&Secp256k1<secp256k1::VerifyOnly>>) -> Result<bool, LightningError> {
@@ -677,7 +758,11 @@ impl NetworkGraph {
677758
base_msat: msg.contents.fee_base_msat,
678759
proportional_millionths: msg.contents.fee_proportional_millionths,
679760
},
680-
last_update_message
761+
last_update_message,
762+
channel_score: ChannelScore {
763+
payment_sent_score: 0,
764+
payment_failed_score: 0,
765+
}
681766
};
682767
$target = Some(updated_channel_dir_info);
683768
}
@@ -765,12 +850,13 @@ impl NetworkGraph {
765850
mod tests {
766851
use chain::chaininterface;
767852
use ln::features::{ChannelFeatures, NodeFeatures};
768-
use routing::network_graph::{NetGraphMsgHandler, NetworkGraph};
853+
use routing::network_graph::{NetGraphMsgHandler, NetworkGraph, DirectionalChannelInfo, ChannelScore, RoutingFees};
769854
use ln::msgs::{RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement,
770855
UnsignedChannelAnnouncement, ChannelAnnouncement, UnsignedChannelUpdate, ChannelUpdate, HTLCFailChannelUpdate};
771856
use util::test_utils;
772857
use util::logger::Logger;
773858
use util::ser::{Readable, Writeable};
859+
use routing::router::RouteHop;
774860

775861
use bitcoin::hashes::sha256d::Hash as Sha256dHash;
776862
use bitcoin::hashes::Hash;
@@ -1658,4 +1744,132 @@ mod tests {
16581744
network.write(&mut w).unwrap();
16591745
assert!(<NetworkGraph>::read(&mut ::std::io::Cursor::new(&w.0)).unwrap() == *network);
16601746
}
1747+
1748+
#[test]
1749+
fn network_graph_score_payment() {
1750+
// Set up a network that consists of just a channel between node_1 and node_2
1751+
let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
1752+
let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
1753+
let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
1754+
let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_1_privkey);
1755+
let node_id_2 = PublicKey::from_secret_key(&secp_ctx, node_2_privkey);
1756+
let node_1_btckey = &SecretKey::from_slice(&[40; 32]).unwrap();
1757+
let node_2_btckey = &SecretKey::from_slice(&[39; 32]).unwrap();
1758+
1759+
let short_channel_id = 0;
1760+
let chain_hash = genesis_block(Network::Testnet).header.bitcoin_hash();
1761+
1762+
{
1763+
// There is no nodes in the table at the beginning.
1764+
let network = net_graph_msg_handler.network_graph.read().unwrap();
1765+
assert_eq!(network.get_nodes().len(), 0);
1766+
}
1767+
1768+
{
1769+
// Announce a channel we will update
1770+
let unsigned_announcement = UnsignedChannelAnnouncement {
1771+
features: ChannelFeatures::empty(),
1772+
chain_hash,
1773+
short_channel_id,
1774+
node_id_1,
1775+
node_id_2,
1776+
bitcoin_key_1: PublicKey::from_secret_key(&secp_ctx, node_1_btckey),
1777+
bitcoin_key_2: PublicKey::from_secret_key(&secp_ctx, node_2_btckey),
1778+
excess_data: Vec::new(),
1779+
};
1780+
1781+
let msghash = hash_to_message!(&Sha256dHash::hash(&unsigned_announcement.encode()[..])[..]);
1782+
let valid_channel_announcement = ChannelAnnouncement {
1783+
node_signature_1: secp_ctx.sign(&msghash, node_1_privkey),
1784+
node_signature_2: secp_ctx.sign(&msghash, node_2_privkey),
1785+
bitcoin_signature_1: secp_ctx.sign(&msghash, node_1_btckey),
1786+
bitcoin_signature_2: secp_ctx.sign(&msghash, node_2_btckey),
1787+
contents: unsigned_announcement.clone(),
1788+
};
1789+
match net_graph_msg_handler.handle_channel_announcement(&valid_channel_announcement) {
1790+
Ok(_) => (),
1791+
Err(_) => panic!()
1792+
};
1793+
}
1794+
1795+
{
1796+
// At the start, the node_1 <-> node_2 channel's DirectionalChannelInfos should be 0.
1797+
let mut network = net_graph_msg_handler.network_graph.write().unwrap();
1798+
let chan_id = network.get_nodes().get(&node_id_1).unwrap().channels[0];
1799+
let channel_info = network.channels.get_mut(&chan_id).unwrap();
1800+
// Assign a DirectionalChannelInfo and ChannelScore object to one_to_two of
1801+
// channel_info
1802+
let chan_score = ChannelScore {
1803+
payment_sent_score: 0,
1804+
payment_failed_score: 0,
1805+
};
1806+
let dummy_routing_fee = RoutingFees {
1807+
base_msat: 0,
1808+
proportional_millionths: 0,
1809+
};
1810+
let dir_chan_info = DirectionalChannelInfo {
1811+
last_update: 0,
1812+
enabled: true,
1813+
cltv_expiry_delta: 0,
1814+
htlc_minimum_msat: 0,
1815+
fees: dummy_routing_fee,
1816+
last_update_message: None,
1817+
channel_score: chan_score,
1818+
};
1819+
channel_info.one_to_two = Some(dir_chan_info);
1820+
let dir_one_to_two = channel_info.one_to_two.as_mut().unwrap();
1821+
assert_eq!(dir_one_to_two.channel_score.payment_sent_score, 0);
1822+
assert_eq!(dir_one_to_two.channel_score.payment_failed_score, 0);
1823+
}
1824+
1825+
{
1826+
// Assume a payment is made, and node 1 is learning of a PaymentSent event. It now calls
1827+
// its score_payment_sent_for_route on itself.
1828+
let route_hop = RouteHop {
1829+
pubkey: node_id_1,
1830+
node_features: NodeFeatures::empty(),
1831+
short_channel_id: 0,
1832+
channel_features: ChannelFeatures::empty(),
1833+
fee_msat: 0,
1834+
cltv_expiry_delta: 0,
1835+
};
1836+
let sent_paths: Vec<RouteHop> = vec![route_hop];
1837+
let sent_route: Vec<Vec<RouteHop>> = vec![sent_paths];
1838+
let mut network = net_graph_msg_handler.network_graph.write().unwrap();
1839+
let sent_res = network.score_payment_sent_for_route(sent_route);
1840+
assert!(sent_res.unwrap() == true);
1841+
// Check that score_payment_sent_for_route incremented the appropriate ChannelScore
1842+
let chan_id = network.get_nodes().get(&node_id_1).unwrap().channels[0];
1843+
let channel_info = network.channels.get_mut(&chan_id).unwrap();
1844+
let dir_one_to_two = &channel_info.one_to_two.as_ref();
1845+
assert_eq!(dir_one_to_two.unwrap().channel_score.payment_sent_score, 1);
1846+
assert_eq!(dir_one_to_two.unwrap().channel_score.payment_failed_score, 0);
1847+
}
1848+
1849+
{
1850+
let route_hop = RouteHop {
1851+
pubkey: node_id_1,
1852+
node_features: NodeFeatures::empty(),
1853+
short_channel_id: 0,
1854+
channel_features: ChannelFeatures::empty(),
1855+
fee_msat: 0,
1856+
cltv_expiry_delta: 0,
1857+
};
1858+
// Assume a payment fails due to node_1 (identified by its PublicKey), verify that its
1859+
// directional channel's score (node_1 -> node_2) has its PaymentFailed score
1860+
// incremented.
1861+
let faultive_nodes: Vec<PublicKey> = vec![node_id_1];
1862+
let attempted_path: Vec<RouteHop> = vec![route_hop];
1863+
let failed_route: Vec<Vec<RouteHop>> = vec![attempted_path];
1864+
let mut network = net_graph_msg_handler.network_graph.write().unwrap();
1865+
let failed_res = network.score_payment_failed_for_route(failed_route, faultive_nodes);
1866+
assert!(failed_res.unwrap() == true);
1867+
let chan_id = network.get_nodes().get(&node_id_1).unwrap().channels[0];
1868+
let channel_info = network.channels.get_mut(&chan_id).unwrap();
1869+
assert!(channel_info.node_two == node_id_2);
1870+
let dir_one_to_two = &channel_info.one_to_two.as_ref();
1871+
assert_eq!(dir_one_to_two.unwrap().channel_score.payment_sent_score, 1);
1872+
assert_eq!(dir_one_to_two.unwrap().channel_score.payment_failed_score, 1);
1873+
}
1874+
}
16611875
}

lightning/src/util/events.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ pub enum Event {
9898
/// the payment has failed, not just the route in question. If this is not set, you may
9999
/// retry the payment via a different route.
100100
rejected_by_dest: bool,
101+
/// Indicates the node responsible for the failure in the event of NodeFailure event.
102+
faultive_node: Option<PublicKey>,
101103
#[cfg(test)]
102104
error_code: Option<u16>,
103105
#[cfg(test)]
@@ -145,7 +147,7 @@ impl Writeable for Event {
145147
3u8.write(writer)?;
146148
payment_preimage.write(writer)?;
147149
},
148-
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest,
150+
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref faultive_node,
149151
#[cfg(test)]
150152
ref error_code,
151153
#[cfg(test)]
@@ -154,6 +156,7 @@ impl Writeable for Event {
154156
4u8.write(writer)?;
155157
payment_hash.write(writer)?;
156158
rejected_by_dest.write(writer)?;
159+
faultive_node.write(writer)?;
157160
#[cfg(test)]
158161
error_code.write(writer)?;
159162
#[cfg(test)]
@@ -194,6 +197,7 @@ impl MaybeReadable for Event {
194197
4u8 => Ok(Some(Event::PaymentFailed {
195198
payment_hash: Readable::read(reader)?,
196199
rejected_by_dest: Readable::read(reader)?,
200+
faultive_node: Readable::read(reader)?,
197201
#[cfg(test)]
198202
error_code: Readable::read(reader)?,
199203
#[cfg(test)]

0 commit comments

Comments
 (0)