Skip to content

Commit 0052b2c

Browse files
author
Antoine Riard
committed
Provide peer local_features to handle_open_channel/accept_channel
Peer may send us a shutdown_scriptpubkey in open_channel or accept_channel messages. Before to enforce this policy on channel closing, we want to be sure that our peer has opt-in to it. Extend LocalFeatures new method visibilty from crate to public for fuzz tests
1 parent 8470e60 commit 0052b2c

File tree

8 files changed

+29
-26
lines changed

8 files changed

+29
-26
lines changed

fuzz/fuzz_targets/chanmon_fail_consistency.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use lightning::ln::channelmonitor;
3636
use lightning::ln::channelmonitor::{ChannelMonitorUpdateErr, HTLCUpdate};
3737
use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage};
3838
use lightning::ln::router::{Route, RouteHop};
39-
use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, ErrorAction, HandleError, UpdateAddHTLC};
39+
use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, ErrorAction, HandleError, UpdateAddHTLC, LocalFeatures};
4040
use lightning::util::{reset_rng_state, fill_bytes, events};
4141
use lightning::util::logger::Logger;
4242
use lightning::util::config::UserConfig;
@@ -168,7 +168,7 @@ pub fn do_test(data: &[u8]) {
168168
} else { panic!("Wrong event type"); }
169169
};
170170

171-
$dest.handle_open_channel(&$source.get_our_node_id(), &open_channel).unwrap();
171+
$dest.handle_open_channel(&$source.get_our_node_id(), LocalFeatures::new(), &open_channel).unwrap();
172172
let accept_channel = {
173173
let events = $dest.get_and_clear_pending_msg_events();
174174
assert_eq!(events.len(), 1);
@@ -177,7 +177,7 @@ pub fn do_test(data: &[u8]) {
177177
} else { panic!("Wrong event type"); }
178178
};
179179

180-
$source.handle_accept_channel(&$dest.get_our_node_id(), &accept_channel).unwrap();
180+
$source.handle_accept_channel(&$dest.get_our_node_id(), LocalFeatures::new(), &accept_channel).unwrap();
181181
{
182182
let events = $source.get_and_clear_pending_events();
183183
assert_eq!(events.len(), 1);

src/ln/channel.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use secp256k1::{Secp256k1,Signature};
1616
use secp256k1;
1717

1818
use ln::msgs;
19-
use ln::msgs::{DecodeError, OptionalField};
19+
use ln::msgs::{DecodeError, OptionalField, LocalFeatures};
2020
use ln::channelmonitor::ChannelMonitor;
2121
use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingForwardHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash};
2222
use ln::chan_utils::{TxCreationKeys,HTLCOutputInCommitment,HTLC_SUCCESS_TX_WEIGHT,HTLC_TIMEOUT_TX_WEIGHT};
@@ -522,7 +522,7 @@ impl Channel {
522522

523523
/// Creates a new channel from a remote sides' request for one.
524524
/// Assumes chain_hash has already been checked and corresponds with what we expect!
525-
pub fn new_from_req(fee_estimator: &FeeEstimator, keys_provider: &Arc<KeysInterface>, their_node_id: PublicKey, msg: &msgs::OpenChannel, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel, ChannelError> {
525+
pub fn new_from_req(fee_estimator: &FeeEstimator, keys_provider: &Arc<KeysInterface>, their_node_id: PublicKey, _their_local_features: LocalFeatures, msg: &msgs::OpenChannel, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel, ChannelError> {
526526
let chan_keys = keys_provider.get_channel_keys(true);
527527
let mut local_config = (*config).channel_options.clone();
528528

@@ -1341,7 +1341,7 @@ impl Channel {
13411341

13421342
// Message handlers:
13431343

1344-
pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig) -> Result<(), ChannelError> {
1344+
pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, _their_local_features: LocalFeatures) -> Result<(), ChannelError> {
13451345
// Check sanity of message fields:
13461346
if !self.channel_outbound {
13471347
return Err(ChannelError::Close("Got an accept_channel message from an inbound peer"));

src/ln/channelmanager.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use ln::channel::{Channel, ChannelError};
3131
use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
3232
use ln::router::Route;
3333
use ln::msgs;
34+
use ln::msgs::LocalFeatures;
3435
use ln::onion_utils;
3536
use ln::msgs::{ChannelMessageHandler, DecodeError, HandleError};
3637
use chain::keysinterface::KeysInterface;
@@ -1702,12 +1703,12 @@ impl ChannelManager {
17021703
}
17031704
}
17041705

1705-
fn internal_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> {
1706+
fn internal_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> {
17061707
if msg.chain_hash != self.genesis_hash {
17071708
return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash", msg.temporary_channel_id.clone()));
17081709
}
17091710

1710-
let channel = Channel::new_from_req(&*self.fee_estimator, &self.keys_manager, their_node_id.clone(), msg, 0, Arc::clone(&self.logger), &self.default_configuration)
1711+
let channel = Channel::new_from_req(&*self.fee_estimator, &self.keys_manager, their_node_id.clone(), their_local_features, msg, 0, Arc::clone(&self.logger), &self.default_configuration)
17111712
.map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?;
17121713
let mut channel_state_lock = self.channel_state.lock().unwrap();
17131714
let channel_state = channel_state_lock.borrow_parts();
@@ -1724,7 +1725,7 @@ impl ChannelManager {
17241725
Ok(())
17251726
}
17261727

1727-
fn internal_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> {
1728+
fn internal_accept_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> {
17281729
let (value, output_script, user_id) = {
17291730
let mut channel_lock = self.channel_state.lock().unwrap();
17301731
let channel_state = channel_lock.borrow_parts();
@@ -1734,7 +1735,7 @@ impl ChannelManager {
17341735
//TODO: see issue #153, need a consistent behavior on obnoxious behavior from random node
17351736
return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.temporary_channel_id));
17361737
}
1737-
try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration), channel_state, chan);
1738+
try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration, their_local_features), channel_state, chan);
17381739
(chan.get().get_value_satoshis(), chan.get().get_funding_redeemscript().to_v0_p2wsh(), chan.get().get_user_id())
17391740
},
17401741
//TODO: same as above
@@ -2525,14 +2526,14 @@ impl ChainListener for ChannelManager {
25252526

25262527
impl ChannelMessageHandler for ChannelManager {
25272528
//TODO: Handle errors and close channel (or so)
2528-
fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), HandleError> {
2529+
fn handle_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::OpenChannel) -> Result<(), HandleError> {
25292530
let _ = self.total_consistency_lock.read().unwrap();
2530-
handle_error!(self, self.internal_open_channel(their_node_id, msg))
2531+
handle_error!(self, self.internal_open_channel(their_node_id, their_local_features, msg))
25312532
}
25322533

2533-
fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
2534+
fn handle_accept_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
25342535
let _ = self.total_consistency_lock.read().unwrap();
2535-
handle_error!(self, self.internal_accept_channel(their_node_id, msg))
2536+
handle_error!(self, self.internal_accept_channel(their_node_id, their_local_features, msg))
25362537
}
25372538

25382539
fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), HandleError> {

src/ln/functional_test_utils.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use chain::keysinterface::KeysInterface;
77
use ln::channelmanager::{ChannelManager,RAACommitmentOrder, PaymentPreimage, PaymentHash};
88
use ln::router::{Route, Router};
99
use ln::msgs;
10-
use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler};
10+
use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler, LocalFeatures};
1111
use util::test_utils;
1212
use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
1313
use util::errors::APIError;
@@ -174,8 +174,8 @@ macro_rules! get_feerate {
174174

175175
pub fn create_chan_between_nodes_with_value_init(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64) -> Transaction {
176176
node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap();
177-
node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id())).unwrap();
178-
node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id())).unwrap();
177+
node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), LocalFeatures::new(), &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id())).unwrap();
178+
node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), LocalFeatures::new(), &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id())).unwrap();
179179

180180
let chan_id = *node_a.network_chan_count.borrow();
181181
let tx;

src/ln/functional_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT};
1313
use ln::onion_utils;
1414
use ln::router::{Route, RouteHop};
1515
use ln::msgs;
16-
use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate};
16+
use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, LocalFeatures};
1717
use util::test_utils;
1818
use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
1919
use util::errors::APIError;
@@ -4861,7 +4861,7 @@ fn bolt2_open_channel_sending_node_checks_part1() { //This test needs to be on i
48614861
let push_msat=10001;
48624862
nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), channel_value_satoshis, push_msat, 42).unwrap();
48634863
let node0_to_1_send_open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
4864-
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &node0_to_1_send_open_channel).unwrap();
4864+
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), LocalFeatures::new(), &node0_to_1_send_open_channel).unwrap();
48654865

48664866
//Create a second channel with a channel_id collision
48674867
assert!(nodes[0].node.create_channel(nodes[0].node.get_our_node_id(), channel_value_satoshis, push_msat, 42).is_err());

src/ln/msgs.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ pub struct LocalFeatures {
5959
}
6060

6161
impl LocalFeatures {
62-
pub(crate) fn new() -> LocalFeatures {
62+
/// Create a blank LocalFeatures flags (visibility extended for fuzz tests)
63+
pub fn new() -> LocalFeatures {
6364
LocalFeatures {
6465
flags: Vec::new(),
6566
}
@@ -611,9 +612,9 @@ pub enum OptionalField<T> {
611612
pub trait ChannelMessageHandler : events::MessageSendEventsProvider + Send + Sync {
612613
//Channel init:
613614
/// Handle an incoming open_channel message from the given peer.
614-
fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &OpenChannel) -> Result<(), HandleError>;
615+
fn handle_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &OpenChannel) -> Result<(), HandleError>;
615616
/// Handle an incoming accept_channel message from the given peer.
616-
fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &AcceptChannel) -> Result<(), HandleError>;
617+
fn handle_accept_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &AcceptChannel) -> Result<(), HandleError>;
617618
/// Handle an incoming funding_created message from the given peer.
618619
fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &FundingCreated) -> Result<(), HandleError>;
619620
/// Handle an incoming funding_signed message from the given peer.

src/ln/peer_handler.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -659,11 +659,11 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
659659
// Channel control:
660660
32 => {
661661
let msg = try_potential_decodeerror!(msgs::OpenChannel::read(&mut reader));
662-
try_potential_handleerror!(self.message_handler.chan_handler.handle_open_channel(&peer.their_node_id.unwrap(), &msg));
662+
try_potential_handleerror!(self.message_handler.chan_handler.handle_open_channel(&peer.their_node_id.unwrap(), peer.their_local_features.clone().unwrap(), &msg));
663663
},
664664
33 => {
665665
let msg = try_potential_decodeerror!(msgs::AcceptChannel::read(&mut reader));
666-
try_potential_handleerror!(self.message_handler.chan_handler.handle_accept_channel(&peer.their_node_id.unwrap(), &msg));
666+
try_potential_handleerror!(self.message_handler.chan_handler.handle_accept_channel(&peer.their_node_id.unwrap(), peer.their_local_features.clone().unwrap(), &msg));
667667
},
668668

669669
34 => {

src/util/test_utils.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use chain::transaction::OutPoint;
44
use chain::keysinterface;
55
use ln::channelmonitor;
66
use ln::msgs;
7+
use ln::msgs::LocalFeatures;
78
use ln::msgs::{HandleError};
89
use ln::channelmonitor::HTLCUpdate;
910
use util::events;
@@ -96,10 +97,10 @@ impl TestChannelMessageHandler {
9697
}
9798

9899
impl msgs::ChannelMessageHandler for TestChannelMessageHandler {
99-
fn handle_open_channel(&self, _their_node_id: &PublicKey, _msg: &msgs::OpenChannel) -> Result<(), HandleError> {
100+
fn handle_open_channel(&self, _their_node_id: &PublicKey, _their_local_features: LocalFeatures, _msg: &msgs::OpenChannel) -> Result<(), HandleError> {
100101
Err(HandleError { err: "", action: None })
101102
}
102-
fn handle_accept_channel(&self, _their_node_id: &PublicKey, _msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
103+
fn handle_accept_channel(&self, _their_node_id: &PublicKey, _their_local_features: LocalFeatures, _msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
103104
Err(HandleError { err: "", action: None })
104105
}
105106
fn handle_funding_created(&self, _their_node_id: &PublicKey, _msg: &msgs::FundingCreated) -> Result<(), HandleError> {

0 commit comments

Comments
 (0)