mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-24 23:08:36 +01:00
Code Issues Projects Releases Packages Wiki Activity

Fix two bugs which access the wrong peer's htlc_minimum_msat value

Browse source 
* Channel::get_counterparty_htlc_minimum_msat() returned
   holder_htlc_minimum_msat, which was obviously incorrect.
 * ChannelManager::get_channel_update set htlc_minimum_msat to
   Channel::get_holder_htlc_minimum_msat(), but the spec explicitly
   states we "MUST set htlc_minimum_msat to the minimum HTLC value
   (in millisatoshi) that the channel peer will accept." This makes
   sense because the reason we're rejecting the HTLC is because our
   counterparty's HTLC minimum value is too small for us to send to
   them, our own HTLC minimum value plays no role. Further, our
   router already expects this - looking at the same directional
   channel info as it does fees.

Finally, we add a test in the existing onion router test cases
which fails if either of the above is incorrect (the second issue
discovered in the process of writing the test).
This commit is contained in:
Matt Corallo 2020-09-14 17:39:42 -04:00
parent a8be9af7d3
commit 0f6b0004c4
3 changed files with 20 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
lightning/src/ln/channel.rs
View file

@ -3125,6 +3125,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}
/// Allowed in any state (including after shutdown)
#[cfg(test)]
pub fn get_holder_htlc_minimum_msat(&self) -> u64 {
self.holder_htlc_minimum_msat
}
@ -3143,7 +3144,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
/// Allowed in any state (including after shutdown)
pub fn get_counterparty_htlc_minimum_msat(&self) -> u64 {
self.holder_htlc_minimum_msat
self.counterparty_htlc_minimum_msat
}
pub fn get_value_satoshis(&self) -> u64 {

2
lightning/src/ln/channelmanager.rs
View file

@ -1228,7 +1228,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
timestamp: chan.get_update_time_counter(),
flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1),
cltv_expiry_delta: CLTV_EXPIRY_DELTA,
htlc_minimum_msat: chan.get_holder_htlc_minimum_msat(),
htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(),
htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()),
fee_base_msat: chan.get_holder_fee_base_msat(&self.fee_estimator),
fee_proportional_millionths: chan.get_fee_proportional_millionths(),

18
lightning/src/ln/onion_route_tests.rs
View file

@ -21,6 +21,7 @@ use ln::msgs::{ChannelMessageHandler, HTLCFailChannelUpdate, OptionalField};
use util::test_utils;
use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
use util::ser::{Writeable, Writer};
use util::config::UserConfig;
use bitcoin::blockdata::block::BlockHeader;
use bitcoin::hash_types::BlockHash;
@ -257,9 +258,19 @@ fn test_onion_failure() {
const NODE: u16 = 0x2000;
const UPDATE: u16 = 0x1000;
// When we check for amount_below_minimum below, we want to test that we're using the *right*
// amount, thus we need different htlc_minimum_msat values. We set node[2]'s htlc_minimum_msat
// to 2000, which is above the default value of 1000 set in create_node_chanmgrs.
// This exposed a previous bug because we were using the wrong value all the way down in
// Channel::get_counterparty_htlc_minimum_msat().
let mut node_2_cfg: UserConfig = Default::default();
node_2_cfg.own_channel_config.our_htlc_minimum_msat = 2000;
node_2_cfg.channel_options.announced_channel = true;
node_2_cfg.peer_channel_config_limits.force_announced_channel_preference = false;
let chanmon_cfgs = create_chanmon_cfgs(3);
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, Some(node_2_cfg)]);
let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
for node in nodes.iter() {
*node.keys_manager.override_session_priv.lock().unwrap() = Some([3; 32]);
@ -411,6 +422,11 @@ fn test_onion_failure() {
bogus_route.paths[0][route_len-1].fee_msat = amt_to_forward;
run_onion_failure_test("amount_below_minimum", 0, &nodes, &bogus_route, &payment_hash, |_| {}, ||{}, true, Some(UPDATE|11), Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage{msg: ChannelUpdate::dummy()}));
// Test a positive test-case with one extra msat, meeting the minimum.
bogus_route.paths[0][route_len-1].fee_msat = amt_to_forward + 1;
let (preimage, _) = send_along_route(&nodes[0], bogus_route, &[&nodes[1], &nodes[2]], amt_to_forward+1);
claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], preimage, amt_to_forward+1);
//TODO: with new config API, we will be able to generate both valid and
//invalid channel_update cases.
run_onion_failure_test("fee_insufficient", 0, &nodes, &route, &payment_hash, |msg| {