Skip to content

Commit

Permalink
Merge pull request #1519 from tnull/2022-06-require-htlc-max
Browse files Browse the repository at this point in the history 
Make `htlc_maximum_msat` a required field.
  • Loading branch information
@TheBlueMatt
TheBlueMatt committed Jul 25, 2022
2 parents f26e854 + 8b86ed7 commit 1988cb2
Show file tree
Hide file tree
Showing 13 changed files with 488 additions and 236 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
View file
Expand Up @@ -232,14 +232,14 @@ jobs:
EXPECTED_ROUTING_GRAPH_SNAPSHOT_SHASUM: 05a5361278f68ee2afd086cc04a1f927a63924be451f3221d380533acfacc303
- name: Fetch rapid graph sync reference input
run: |
curl --verbose -L -o lightning-rapid-gossip-sync/res/full_graph.lngossip https://bitcoin.ninja/ldk-compressed_graph-bc08df7542-2022-05-05.bin
curl --verbose -L -o lightning-rapid-gossip-sync/res/full_graph.lngossip https://bitcoin.ninja/ldk-compressed_graph-285cb27df79-2022-07-21.bin
echo "Sha sum: $(sha256sum lightning-rapid-gossip-sync/res/full_graph.lngossip | awk '{ print $1 }')"
if [ "$(sha256sum lightning-rapid-gossip-sync/res/full_graph.lngossip | awk '{ print $1 }')" != "${EXPECTED_RAPID_GOSSIP_SHASUM}" ]; then
echo "Bad hash"
exit 1
fi
env:
EXPECTED_RAPID_GOSSIP_SHASUM: 9637b91cea9d64320cf48fc0787c70fe69fc062f90d3512e207044110cadfd7b
EXPECTED_RAPID_GOSSIP_SHASUM: e0f5d11641c11896d7af3a2246d3d6c3f1720b7d2d17aab321ecce82e6b7deb8
- name: Test with Network Graph on Rust ${{ matrix.toolchain }}
run: |
cd lightning
Expand Down
2 changes: 1 addition & 1 deletion lightning-rapid-gossip-sync/src/lib.rs
View file
Expand Up @@ -240,7 +240,7 @@ mod tests {
let sync_result = rapid_sync
.sync_network_graph_with_file_path("./res/full_graph.lngossip");
if let Err(crate::error::GraphSyncError::DecodeError(DecodeError::Io(io_error))) = &sync_result {
let error_string = format!("Input file lightning-rapid-gossip-sync/res/full_graph.lngossip is missing! Download it from https://bitcoin.ninja/ldk-compressed_graph-bc08df7542-2022-05-05.bin\n\n{:?}", io_error);
let error_string = format!("Input file lightning-rapid-gossip-sync/res/full_graph.lngossip is missing! Download it from https://bitcoin.ninja/ldk-compressed_graph-285cb27df79-2022-07-21.bin\n\n{:?}", io_error);
#[cfg(not(require_route_graph_test))]
{
println!("{}", error_string);
Expand Down
29 changes: 6 additions & 23 deletions lightning-rapid-gossip-sync/src/processing.rs
View file
Expand Up @@ -8,7 +8,7 @@ use bitcoin::BlockHash;
use bitcoin::secp256k1::PublicKey;

use lightning::ln::msgs::{
DecodeError, ErrorAction, LightningError, OptionalField, UnsignedChannelUpdate,
DecodeError, ErrorAction, LightningError, UnsignedChannelUpdate,
};
use lightning::routing::gossip::NetworkGraph;
use lightning::util::logger::Logger;
Expand Down Expand Up @@ -119,12 +119,7 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
let default_htlc_minimum_msat: u64 = Readable::read(&mut read_cursor)?;
let default_fee_base_msat: u32 = Readable::read(&mut read_cursor)?;
let default_fee_proportional_millionths: u32 = Readable::read(&mut read_cursor)?;
let tentative_default_htlc_maximum_msat: u64 = Readable::read(&mut read_cursor)?;
let default_htlc_maximum_msat = if tentative_default_htlc_maximum_msat == u64::max_value() {
OptionalField::Absent
} else {
OptionalField::Present(tentative_default_htlc_maximum_msat)
};
let default_htlc_maximum_msat: u64 = Readable::read(&mut read_cursor)?;

for _ in 0..update_count {
let scid_delta: BigSize = Readable::read(read_cursor)?;
Expand All @@ -147,7 +142,7 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
flags: standard_channel_flags,
cltv_expiry_delta: default_cltv_expiry_delta,
htlc_minimum_msat: default_htlc_minimum_msat,
htlc_maximum_msat: default_htlc_maximum_msat.clone(),
htlc_maximum_msat: default_htlc_maximum_msat,
fee_base_msat: default_fee_base_msat,
fee_proportional_millionths: default_fee_proportional_millionths,
excess_data: vec![],
Expand All @@ -170,21 +165,14 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
action: ErrorAction::IgnoreError,
})?;

let htlc_maximum_msat =
if let Some(htlc_maximum_msat) = directional_info.htlc_maximum_msat {
OptionalField::Present(htlc_maximum_msat)
} else {
OptionalField::Absent
};

UnsignedChannelUpdate {
chain_hash,
short_channel_id,
timestamp: backdated_timestamp,
flags: standard_channel_flags,
cltv_expiry_delta: directional_info.cltv_expiry_delta,
htlc_minimum_msat: directional_info.htlc_minimum_msat,
htlc_maximum_msat,
htlc_maximum_msat: directional_info.htlc_maximum_msat,
fee_base_msat: directional_info.fees.base_msat,
fee_proportional_millionths: directional_info.fees.proportional_millionths,
excess_data: vec![],
Expand Down Expand Up @@ -212,13 +200,8 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
}

if channel_flags & 0b_0000_0100 > 0 {
let tentative_htlc_maximum_msat: u64 = Readable::read(read_cursor)?;
synthetic_update.htlc_maximum_msat = if tentative_htlc_maximum_msat == u64::max_value()
{
OptionalField::Absent
} else {
OptionalField::Present(tentative_htlc_maximum_msat)
};
let htlc_maximum_msat: u64 = Readable::read(read_cursor)?;
synthetic_update.htlc_maximum_msat = htlc_maximum_msat;
}

network_graph.update_channel_unsigned(&synthetic_update)?;
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/channel.rs
View file
Expand Up @@ -6572,7 +6572,7 @@ mod tests {
use ln::channel::{Channel, InboundHTLCOutput, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator};
use ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS};
use ln::features::{InitFeatures, ChannelTypeFeatures};
use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate};
use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate, MAX_VALUE_MSAT};
use ln::script::ShutdownScript;
use ln::chan_utils;
use ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight};
Expand Down Expand Up @@ -6988,7 +6988,7 @@ mod tests {
flags: 0,
cltv_expiry_delta: 100,
htlc_minimum_msat: 5,
htlc_maximum_msat: OptionalField::Absent,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 110,
fee_proportional_millionths: 11,
excess_data: Vec::new(),
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/channelmanager.rs
View file
Expand Up @@ -48,7 +48,7 @@ use routing::router::{PaymentParameters, Route, RouteHop, RoutePath, RouteParame
use ln::msgs;
use ln::msgs::NetAddress;
use ln::onion_utils;
use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT, OptionalField};
use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
use ln::wire::Encode;
use chain::keysinterface::{Sign, KeysInterface, KeysManager, InMemorySigner, Recipient};
use util::config::{UserConfig, ChannelConfig};
Expand Down Expand Up @@ -2397,7 +2397,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1),
cltv_expiry_delta: chan.get_cltv_expiry_delta(),
htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(),
htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()),
htlc_maximum_msat: chan.get_announced_htlc_max_msat(),
fee_base_msat: chan.get_outbound_forwarding_fee_base_msat(),
fee_proportional_millionths: chan.get_fee_proportional_millionths(),
excess_data: Vec::new(),
Expand Down
10 changes: 5 additions & 5 deletions lightning/src/ln/functional_tests.rs
View file
Expand Up @@ -28,7 +28,7 @@ use routing::gossip::NetworkGraph;
use routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, find_route, get_route};
use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
use ln::msgs;
use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, OptionalField, ErrorAction};
use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
use util::enforcing_trait_impls::EnforcingSigner;
use util::{byte_utils, test_utils};
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose, ClosureReason};
Expand Down Expand Up @@ -8312,19 +8312,19 @@ fn test_channel_update_has_correct_htlc_maximum_msat() {

// Assert that `node[0]`'s `ChannelUpdate` is capped at 50 percent of the `channel_value`, as
// that's the value of `node[1]`'s `holder_max_htlc_value_in_flight_msat`.
assert_eq!(node_0_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_50_percent_msat));
assert_eq!(node_0_chan_update.contents.htlc_maximum_msat, channel_value_50_percent_msat);
// Assert that `node[1]`'s `ChannelUpdate` is capped at 30 percent of the `channel_value`, as
// that's the value of `node[0]`'s `holder_max_htlc_value_in_flight_msat`.
assert_eq!(node_1_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_30_percent_msat));
assert_eq!(node_1_chan_update.contents.htlc_maximum_msat, channel_value_30_percent_msat);

// Assert that `node[2]`'s `ChannelUpdate` is capped at 90 percent of the `channel_value`, as
// the value of `node[3]`'s `holder_max_htlc_value_in_flight_msat` (100%), exceeds 90% of the
// `channel_value`.
assert_eq!(node_2_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_90_percent_msat));
assert_eq!(node_2_chan_update.contents.htlc_maximum_msat, channel_value_90_percent_msat);
// Assert that `node[3]`'s `ChannelUpdate` is capped at 90 percent of the `channel_value`, as
// the value of `node[2]`'s `holder_max_htlc_value_in_flight_msat` (95%), exceeds 90% of the
// `channel_value`.
assert_eq!(node_3_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_90_percent_msat));
assert_eq!(node_3_chan_update.contents.htlc_maximum_msat, channel_value_90_percent_msat);
}

#[test]
Expand Down
50 changes: 19 additions & 31 deletions lightning/src/ln/msgs.rs
View file
Expand Up @@ -647,8 +647,8 @@ pub struct UnsignedChannelUpdate {
pub cltv_expiry_delta: u16,
/// The minimum HTLC size incoming to sender, in milli-satoshi
pub htlc_minimum_msat: u64,
/// Optionally, the maximum HTLC value incoming to sender, in milli-satoshi
pub htlc_maximum_msat: OptionalField<u64>,
/// The maximum HTLC value incoming to sender, in milli-satoshi. Used to be optional.
pub htlc_maximum_msat: u64,
/// The base HTLC fee charged by sender, in milli-satoshi
pub fee_base_msat: u32,
/// The amount to fee multiplier, in micro-satoshi
Expand Down Expand Up @@ -1514,14 +1514,12 @@ impl_writeable!(ChannelAnnouncement, {

impl Writeable for UnsignedChannelUpdate {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
let mut message_flags: u8 = 0;
if let OptionalField::Present(_) = self.htlc_maximum_msat {
message_flags = 1;
}
// `message_flags` used to indicate presence of `htlc_maximum_msat`, but was deprecated in the spec.
const MESSAGE_FLAGS: u8 = 1;
self.chain_hash.write(w)?;
self.short_channel_id.write(w)?;
self.timestamp.write(w)?;
let all_flags = self.flags as u16 | ((message_flags as u16) << 8);
let all_flags = self.flags as u16 | ((MESSAGE_FLAGS as u16) << 8);
all_flags.write(w)?;
self.cltv_expiry_delta.write(w)?;
self.htlc_minimum_msat.write(w)?;
Expand All @@ -1535,22 +1533,20 @@ impl Writeable for UnsignedChannelUpdate {

impl Readable for UnsignedChannelUpdate {
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
let has_htlc_maximum_msat;
Ok(Self {
chain_hash: Readable::read(r)?,
short_channel_id: Readable::read(r)?,
timestamp: Readable::read(r)?,
flags: {
let flags: u16 = Readable::read(r)?;
let message_flags = flags >> 8;
has_htlc_maximum_msat = (message_flags as i32 & 1) == 1;
// Note: we ignore the `message_flags` for now, since it was deprecated by the spec.
flags as u8
},
cltv_expiry_delta: Readable::read(r)?,
htlc_minimum_msat: Readable::read(r)?,
fee_base_msat: Readable::read(r)?,
fee_proportional_millionths: Readable::read(r)?,
htlc_maximum_msat: if has_htlc_maximum_msat { Readable::read(r)? } else { OptionalField::Absent },
htlc_maximum_msat: Readable::read(r)?,
excess_data: read_to_end(r)?,
})
}
Expand Down Expand Up @@ -2103,7 +2099,7 @@ mod tests {
do_encoding_node_announcement(false, false, true, false, true, false, false, false);
}

fn do_encoding_channel_update(direction: bool, disable: bool, htlc_maximum_msat: bool, excess_data: bool) {
fn do_encoding_channel_update(direction: bool, disable: bool, excess_data: bool) {
let secp_ctx = Secp256k1::new();
let (privkey_1, _) = get_keys_from!("0101010101010101010101010101010101010101010101010101010101010101", secp_ctx);
let sig_1 = get_sig_on!(privkey_1, secp_ctx, String::from("01010101010101010101010101010101"));
Expand All @@ -2114,7 +2110,7 @@ mod tests {
flags: if direction { 1 } else { 0 } | if disable { 1 << 1 } else { 0 },
cltv_expiry_delta: 144,
htlc_minimum_msat: 1000000,
htlc_maximum_msat: if htlc_maximum_msat { OptionalField::Present(131355275467161) } else { OptionalField::Absent },
htlc_maximum_msat: 131355275467161,
fee_base_msat: 10000,
fee_proportional_millionths: 20,
excess_data: if excess_data { vec![0, 0, 0, 0, 59, 154, 202, 0] } else { Vec::new() }
Expand All @@ -2127,11 +2123,7 @@ mod tests {
let mut target_value = hex::decode("d977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a").unwrap();
target_value.append(&mut hex::decode("000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f").unwrap());
target_value.append(&mut hex::decode("00083a840000034d013413a7").unwrap());
if htlc_maximum_msat {
target_value.append(&mut hex::decode("01").unwrap());
} else {
target_value.append(&mut hex::decode("00").unwrap());
}
target_value.append(&mut hex::decode("01").unwrap());
target_value.append(&mut hex::decode("00").unwrap());
if direction {
let flag = target_value.last_mut().unwrap();
Expand All @@ -2142,9 +2134,7 @@ mod tests {
*flag = *flag | 1 << 1;
}
target_value.append(&mut hex::decode("009000000000000f42400000271000000014").unwrap());
if htlc_maximum_msat {
target_value.append(&mut hex::decode("0000777788889999").unwrap());
}
target_value.append(&mut hex::decode("0000777788889999").unwrap());
if excess_data {
target_value.append(&mut hex::decode("000000003b9aca00").unwrap());
}
Expand All @@ -2153,16 +2143,14 @@ mod tests {

#[test]
fn encoding_channel_update() {
do_encoding_channel_update(false, false, false, false);
do_encoding_channel_update(false, false, false, true);
do_encoding_channel_update(true, false, false, false);
do_encoding_channel_update(true, false, false, true);
do_encoding_channel_update(false, true, false, false);
do_encoding_channel_update(false, true, false, true);
do_encoding_channel_update(false, false, true, false);
do_encoding_channel_update(false, false, true, true);
do_encoding_channel_update(true, true, true, false);
do_encoding_channel_update(true, true, true, true);
do_encoding_channel_update(false, false, false);
do_encoding_channel_update(false, false, true);
do_encoding_channel_update(true, false, false);
do_encoding_channel_update(true, false, true);
do_encoding_channel_update(false, true, false);
do_encoding_channel_update(false, true, true);
do_encoding_channel_update(true, true, false);
do_encoding_channel_update(true, true, true);
}

fn do_encoding_open_channel(random_bit: bool, shutdown: bool, incl_chan_type: bool) {
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/onion_route_tests.rs
View file
Expand Up @@ -21,7 +21,7 @@ use routing::gossip::{NetworkUpdate, RoutingFees, NodeId};
use routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop};
use ln::features::{InitFeatures, InvoiceFeatures, NodeFeatures};
use ln::msgs;
use ln::msgs::{ChannelMessageHandler, ChannelUpdate, OptionalField};
use ln::msgs::{ChannelMessageHandler, ChannelUpdate};
use ln::wire::Encode;
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
use util::ser::{ReadableArgs, Writeable, Writer};
Expand Down Expand Up @@ -227,7 +227,7 @@ impl msgs::ChannelUpdate {
flags: 0,
cltv_expiry_delta: 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: OptionalField::Absent,
htlc_maximum_msat: msgs::MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: vec![],
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/priv_short_conf_tests.rs
View file
Expand Up @@ -19,7 +19,7 @@ use routing::gossip::RoutingFees;
use routing::router::{PaymentParameters, RouteHint, RouteHintHop};
use ln::features::{InitFeatures, InvoiceFeatures, ChannelTypeFeatures};
use ln::msgs;
use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, OptionalField, ChannelUpdate, ErrorAction};
use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ChannelUpdate, ErrorAction};
use ln::wire::Encode;
use util::enforcing_trait_impls::EnforcingSigner;
use util::events::{ClosureReason, Event, MessageSendEvent, MessageSendEventsProvider};
Expand Down Expand Up @@ -523,7 +523,7 @@ fn test_scid_alias_returned() {
flags: 1,
cltv_expiry_delta: accept_forward_cfg.channel_config.cltv_expiry_delta,
htlc_minimum_msat: 1_000,
htlc_maximum_msat: OptionalField::Present(1_000_000), // Defaults to 10% of the channel value
htlc_maximum_msat: 1_000_000, // Defaults to 10% of the channel value
fee_base_msat: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_base_msat,
fee_proportional_millionths: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_proportional_millionths,
excess_data: Vec::new(),
Expand Down

0 comments on commit 1988cb2

Please sign in to comment.