Skip to content

Move supported-feature-set logic into the supporting modules #1707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Sep 15, 2022
Merged

Move supported-feature-set logic into the supporting modules #1707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6b1f867
List supported/required feature bits explicitly in ChannelManager
TheBlueMatt Sep 12, 2022
1a32737
Stop relying on the `*Features::known` method in `routing` tests
TheBlueMatt Sep 8, 2022
3b3713f
Stop relying on the `*Features::known` method in functional tests
TheBlueMatt Sep 8, 2022
c1d8cb9
Stop relying on `*Features::known` in functional test utils
TheBlueMatt Sep 8, 2022
32b5d84
Stop relying on `*Features::known` in channel{,manager}.rs
TheBlueMatt Sep 8, 2022
4846570
Stop relying on the `*Features::known` method in lightning-invoice
TheBlueMatt Sep 12, 2022
10b471d
Stop relying on the `*Features::known` method in net-tokio
TheBlueMatt Sep 9, 2022
d33f726
Stop relying on `*Features::known` in BP and persister tests
TheBlueMatt Sep 9, 2022
f6fa624
Stop relying on `*Features::known` in fuzzing tests
TheBlueMatt Sep 9, 2022
b93fe32
Remove all remaining references to `*Features::known`
TheBlueMatt Sep 14, 2022
bec8bf1
Remove the `*Features::known` constructor
TheBlueMatt Sep 14, 2022
b83e93c
Stop tracking feature bits as known or required in features.rs
TheBlueMatt Sep 9, 2022
9170804
Assert that all defined features are in the known features set
TheBlueMatt Sep 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 19 additions & 20 deletions fuzz/src/chanmon_consistency.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ use lightning::chain::transaction::OutPoint;
use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
use lightning::chain::keysinterface::{KeyMaterial, KeysInterface, InMemorySigner, Recipient};
use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
use lightning::ln::channelmanager::{ChainParameters, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs};
use lightning::ln::channelmanager::{self, ChainParameters, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs};
use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
use lightning::ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, DecodeError, UpdateAddHTLC, Init};
use lightning::ln::script::ShutdownScript;
use lightning::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
Expand Down Expand Up @@ -315,9 +314,9 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
if let Err(err) = source.send_payment(&Route {
paths: vec![vec![RouteHop {
pubkey: dest.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: dest_chan_id,
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: amt,
cltv_expiry_delta: 200,
}]],
Expand All @@ -334,16 +333,16 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
if let Err(err) = source.send_payment(&Route {
paths: vec![vec![RouteHop {
pubkey: middle.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: middle_chan_id,
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: 50000,
cltv_expiry_delta: 100,
},RouteHop {
pubkey: dest.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: dest_chan_id,
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: amt,
cltv_expiry_delta: 200,
}]],
Expand Down Expand Up @@ -422,8 +421,8 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
let mut channel_txn = Vec::new();
macro_rules! make_channel {
($source: expr, $dest: expr, $chan_id: expr) => { {
$source.peer_connected(&$dest.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
$dest.peer_connected(&$source.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
$source.peer_connected(&$dest.get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
$dest.peer_connected(&$source.get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();

$source.create_channel($dest.get_our_node_id(), 100_000, 42, 0, None).unwrap();
let open_channel = {
Expand All @@ -434,7 +433,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
} else { panic!("Wrong event type"); }
};

$dest.handle_open_channel(&$source.get_our_node_id(), InitFeatures::known(), &open_channel);
$dest.handle_open_channel(&$source.get_our_node_id(), channelmanager::provided_init_features(), &open_channel);
let accept_channel = {
let events = $dest.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
Expand All @@ -443,7 +442,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
} else { panic!("Wrong event type"); }
};

$source.handle_accept_channel(&$dest.get_our_node_id(), InitFeatures::known(), &accept_channel);
$source.handle_accept_channel(&$dest.get_our_node_id(), channelmanager::provided_init_features(), &accept_channel);
let funding_output;
{
let events = $source.get_and_clear_pending_events();
Expand Down Expand Up @@ -940,15 +939,15 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
},
0x0e => {
if chan_a_disconnected {
nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
chan_a_disconnected = false;
}
},
0x0f => {
if chan_b_disconnected {
nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
chan_b_disconnected = false;
}
},
Expand Down Expand Up @@ -1143,13 +1142,13 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {

// Next, make sure peers are all connected to each other
if chan_a_disconnected {
nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
chan_a_disconnected = false;
}
if chan_b_disconnected {
nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
chan_b_disconnected = false;
}

Expand Down
5 changes: 2 additions & 3 deletions fuzz/src/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ use bitcoin::hash_types::BlockHash;

use lightning::chain;
use lightning::chain::transaction::OutPoint;
use lightning::ln::channelmanager::{ChannelDetails, ChannelCounterparty};
use lightning::ln::features::InitFeatures;
use lightning::ln::channelmanager::{self, ChannelDetails, ChannelCounterparty};
use lightning::ln::msgs;
use lightning::routing::gossip::{NetworkGraph, RoutingFees};
use lightning::routing::router::{find_route, PaymentParameters, RouteHint, RouteHintHop, RouteParameters};
Expand Down Expand Up @@ -211,7 +210,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
channel_id: [0; 32],
counterparty: ChannelCounterparty {
node_id: *rnid,
features: InitFeatures::known(),
features: channelmanager::provided_init_features(),
unspendable_punishment_reserve: 0,
forwarding_info: None,
outbound_htlc_minimum_msat: None,
Expand Down
12 changes: 6 additions & 6 deletions lightning-background-processor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,8 @@ mod tests {
use lightning::chain::keysinterface::{InMemorySigner, Recipient, KeysInterface, KeysManager};
use lightning::chain::transaction::OutPoint;
use lightning::get_event_msg;
use lightning::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChainParameters, ChannelManager, SimpleArcChannelManager};
use lightning::ln::features::{ChannelFeatures, InitFeatures};
use lightning::ln::channelmanager::{self, BREAKDOWN_TIMEOUT, ChainParameters, ChannelManager, SimpleArcChannelManager};
use lightning::ln::features::ChannelFeatures;
use lightning::ln::msgs::{ChannelMessageHandler, Init};
use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler};
use lightning::routing::gossip::{NetworkGraph, P2PGossipSync};
Expand Down Expand Up @@ -754,8 +754,8 @@ mod tests {

for i in 0..num_nodes {
for j in (i+1)..num_nodes {
nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &Init { features: channelmanager::provided_init_features(), remote_network_address: None }).unwrap();
}
}

Expand All @@ -776,8 +776,8 @@ mod tests {
macro_rules! begin_open_channel {
($node_a: expr, $node_b: expr, $channel_value: expr) => {{
$node_a.node.create_channel($node_b.node.get_our_node_id(), $channel_value, 100, 42, None).unwrap();
$node_b.node.handle_open_channel(&$node_a.node.get_our_node_id(), InitFeatures::known(), &get_event_msg!($node_a, MessageSendEvent::SendOpenChannel, $node_b.node.get_our_node_id()));
$node_a.node.handle_accept_channel(&$node_b.node.get_our_node_id(), InitFeatures::known(), &get_event_msg!($node_b, MessageSendEvent::SendAcceptChannel, $node_a.node.get_our_node_id()));
$node_b.node.handle_open_channel(&$node_a.node.get_our_node_id(), channelmanager::provided_init_features(), &get_event_msg!($node_a, MessageSendEvent::SendOpenChannel, $node_b.node.get_our_node_id()));
$node_a.node.handle_accept_channel(&$node_b.node.get_our_node_id(), channelmanager::provided_init_features(), &get_event_msg!($node_b, MessageSendEvent::SendAcceptChannel, $node_a.node.get_our_node_id()));
}}
}

Expand Down
14 changes: 11 additions & 3 deletions lightning-invoice/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1712,11 +1712,14 @@ mod test {
}.unwrap();
assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::InvalidFeatures));

let mut payment_secret_features = InvoiceFeatures::empty();
payment_secret_features.set_payment_secret_required();

// Including payment secret and feature bits
let invoice = {
let mut invoice = invoice_template.clone();
invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into());
invoice.data.tagged_fields.push(Features(InvoiceFeatures::known()).into());
invoice.data.tagged_fields.push(Features(payment_secret_features.clone()).into());
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key)))
}.unwrap();
assert!(Invoice::from_signed(invoice).is_ok());
Expand All @@ -1739,7 +1742,7 @@ mod test {
// Missing payment secret
let invoice = {
let mut invoice = invoice_template.clone();
invoice.data.tagged_fields.push(Features(InvoiceFeatures::known()).into());
invoice.data.tagged_fields.push(Features(payment_secret_features).into());
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key)))
}.unwrap();
assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::NoPaymentSecret));
Expand Down Expand Up @@ -1944,7 +1947,12 @@ mod test {
);
assert_eq!(invoice.payment_hash(), &sha256::Hash::from_slice(&[21;32][..]).unwrap());
assert_eq!(invoice.payment_secret(), &PaymentSecret([42; 32]));
assert_eq!(invoice.features(), Some(&InvoiceFeatures::known()));

let mut expected_features = InvoiceFeatures::empty();
expected_features.set_variable_length_onion_required();
expected_features.set_payment_secret_required();
expected_features.set_basic_mpp_optional();
assert_eq!(invoice.features(), Some(&expected_features));

let raw_invoice = builder.build_raw().unwrap();
assert_eq!(raw_invoice, *invoice.into_signed_raw().raw_invoice())
Expand Down
43 changes: 22 additions & 21 deletions lightning-invoice/src/payment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,8 @@ mod tests {
use utils::create_invoice_from_channelmanager_and_duration_since_epoch;
use bitcoin_hashes::sha256::Hash as Sha256;
use lightning::ln::PaymentPreimage;
use lightning::ln::features::{ChannelFeatures, NodeFeatures, InitFeatures};
use lightning::ln::channelmanager;
use lightning::ln::features::{ChannelFeatures, NodeFeatures};
use lightning::ln::functional_test_utils::*;
use lightning::ln::msgs::{ChannelMessageHandler, ErrorAction, LightningError};
use lightning::routing::gossip::{EffectiveCapacity, NodeId};
Expand Down Expand Up @@ -2160,24 +2161,24 @@ mod tests {
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, channelmanager::provided_init_features(), channelmanager::provided_init_features());
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, channelmanager::provided_init_features(), channelmanager::provided_init_features());
let chans = nodes[0].node.list_usable_channels();
let mut route = Route {
paths: vec![
vec![RouteHop {
pubkey: nodes[1].node.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: chans[0].short_channel_id.unwrap(),
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: 10_000,
cltv_expiry_delta: 100,
}],
vec![RouteHop {
pubkey: nodes[1].node.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: chans[1].short_channel_id.unwrap(),
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: 100_000_001, // Our default max-HTLC-value is 10% of the channel value, which this is one more than
cltv_expiry_delta: 100,
}],
Expand Down Expand Up @@ -2212,16 +2213,16 @@ mod tests {
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, channelmanager::provided_init_features(), channelmanager::provided_init_features());
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, channelmanager::provided_init_features(), channelmanager::provided_init_features());
let chans = nodes[0].node.list_usable_channels();
let mut route = Route {
paths: vec![
vec![RouteHop {
pubkey: nodes[1].node.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: chans[0].short_channel_id.unwrap(),
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: 100_000_001, // Our default max-HTLC-value is 10% of the channel value, which this is one more than
cltv_expiry_delta: 100,
}],
Expand Down Expand Up @@ -2271,38 +2272,38 @@ mod tests {
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);

let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;
let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;

let mut route = Route {
paths: vec![
vec![RouteHop {
pubkey: nodes[1].node.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: chan_1_scid,
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: 0,
cltv_expiry_delta: 100,
}, RouteHop {
pubkey: nodes[2].node.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: chan_2_scid,
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: 100_000_000,
cltv_expiry_delta: 100,
}],
vec![RouteHop {
pubkey: nodes[1].node.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: chan_1_scid,
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: 0,
cltv_expiry_delta: 100,
}, RouteHop {
pubkey: nodes[2].node.get_our_node_id(),
node_features: NodeFeatures::known(),
node_features: channelmanager::provided_node_features(),
short_channel_id: chan_2_scid,
channel_features: ChannelFeatures::known(),
channel_features: channelmanager::provided_channel_features(),
fee_msat: 100_000_000,
cltv_expiry_delta: 100,
}]
Expand Down
Loading