Skip to content

Commit f9b7490

Browse files
dunxendunxen
committed
Remove Outpoint::to_channel_id method
To avoid confusion and for accuracy going forward, we remove this method as it is inconsistent with channel IDs generated during V2 channel establishment. If one wants to create a V1, funding outpoint-based channel ID, then `ChannelId::v1_from_funding_outpoint` should be used instead. A large portion of the library has always made the assumption that having the funding outpoint will always allow us to generate the channel ID. This will not be the case anymore and we need to pass the channel ID along where appropriate. All channels that could have been persisted up to this point could only have used V1 establishment, so if some structures don't store a channel ID for them they can safely fall back to the funding outpoint-based version.
1 parent 5592378 commit f9b7490

27 files changed

+436
-334
lines changed

‎.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ lightning-rapid-gossip-sync/res/full_graph.lngossip
1313
lightning-custom-message/target
1414
lightning-transaction-sync/target
1515
no-std-check/target
16+
msrv-no-dev-deps-check/target

‎fuzz/src/chanmon_consistency.rs

+26-24
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget,
4040
use lightning::sign::{KeyMaterial, InMemorySigner, Recipient, EntropySource, NodeSigner, SignerProvider};
4141
use lightning::events;
4242
use lightning::events::MessageSendEventsProvider;
43-
use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
43+
use lightning::ln::{ChannelId, PaymentHash, PaymentPreimage, PaymentSecret};
4444
use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs, PaymentId, RecipientOnionFields};
4545
use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
4646
use lightning::ln::msgs::{self, CommitmentUpdate, ChannelMessageHandler, DecodeError, UpdateAddHTLC, Init};
@@ -176,7 +176,7 @@ impl chain::Watch<TestChannelSigner> for TestChainMonitor {
176176
self.chain_monitor.watch_channel(funding_txo, monitor)
177177
}
178178

179-
fn update_channel(&self, funding_txo: OutPoint, update: &channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus {
179+
fn update_channel(&self, funding_txo: OutPoint, channel_id: ChannelId, update: &channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus {
180180
let mut map_lock = self.latest_monitors.lock().unwrap();
181181
let mut map_entry = match map_lock.entry(funding_txo) {
182182
hash_map::Entry::Occupied(entry) => entry,
@@ -188,10 +188,10 @@ impl chain::Watch<TestChannelSigner> for TestChainMonitor {
188188
let mut ser = VecWriter(Vec::new());
189189
deserialized_monitor.write(&mut ser).unwrap();
190190
map_entry.insert((update.update_id, ser.0));
191-
self.chain_monitor.update_channel(funding_txo, update)
191+
self.chain_monitor.update_channel(funding_txo, channel_id, update)
192192
}
193193

194-
fn release_pending_monitor_events(&self) -> Vec<(OutPoint, Vec<MonitorEvent>, Option<PublicKey>)> {
194+
fn release_pending_monitor_events(&self) -> Vec<(OutPoint, ChannelId, Vec<MonitorEvent>, Option<PublicKey>)> {
195195
return self.chain_monitor.release_pending_monitor_events();
196196
}
197197
}
@@ -704,7 +704,9 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
704704
lock_fundings!(nodes);
705705

706706
let chan_a = nodes[0].list_usable_channels()[0].short_channel_id.unwrap();
707+
let chan_1_id = nodes[0].list_usable_channels()[0].channel_id;
707708
let chan_b = nodes[2].list_usable_channels()[0].short_channel_id.unwrap();
709+
let chan_2_id = nodes[2].list_usable_channels()[0].channel_id;
708710

709711
let mut payment_id: u8 = 0;
710712
let mut payment_idx: u64 = 0;
@@ -1060,25 +1062,25 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
10601062

10611063
0x08 => {
10621064
if let Some((id, _)) = monitor_a.latest_monitors.lock().unwrap().get(&chan_1_funding) {
1063-
monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
1065+
monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, chan_1_id, *id);
10641066
nodes[0].process_monitor_events();
10651067
}
10661068
},
10671069
0x09 => {
10681070
if let Some((id, _)) = monitor_b.latest_monitors.lock().unwrap().get(&chan_1_funding) {
1069-
monitor_b.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
1071+
monitor_b.chain_monitor.force_channel_monitor_updated(chan_1_funding, chan_1_id, *id);
10701072
nodes[1].process_monitor_events();
10711073
}
10721074
},
10731075
0x0a => {
10741076
if let Some((id, _)) = monitor_b.latest_monitors.lock().unwrap().get(&chan_2_funding) {
1075-
monitor_b.chain_monitor.force_channel_monitor_updated(chan_2_funding, *id);
1077+
monitor_b.chain_monitor.force_channel_monitor_updated(chan_2_funding, chan_2_id, *id);
10761078
nodes[1].process_monitor_events();
10771079
}
10781080
},
10791081
0x0b => {
10801082
if let Some((id, _)) = monitor_c.latest_monitors.lock().unwrap().get(&chan_2_funding) {
1081-
monitor_c.chain_monitor.force_channel_monitor_updated(chan_2_funding, *id);
1083+
monitor_c.chain_monitor.force_channel_monitor_updated(chan_2_funding, chan_2_id, *id);
10821084
nodes[2].process_monitor_events();
10831085
}
10841086
},
@@ -1292,87 +1294,87 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
12921294
0xf0 => {
12931295
let pending_updates = monitor_a.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
12941296
if let Some(id) = pending_updates.get(0) {
1295-
monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
1297+
monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, chan_1_id, *id).unwrap();
12961298
}
12971299
nodes[0].process_monitor_events();
12981300
}
12991301
0xf1 => {
13001302
let pending_updates = monitor_a.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
13011303
if let Some(id) = pending_updates.get(1) {
1302-
monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
1304+
monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, chan_1_id, *id).unwrap();
13031305
}
13041306
nodes[0].process_monitor_events();
13051307
}
13061308
0xf2 => {
13071309
let pending_updates = monitor_a.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
13081310
if let Some(id) = pending_updates.last() {
1309-
monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
1311+
monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, chan_1_id, *id).unwrap();
13101312
}
13111313
nodes[0].process_monitor_events();
13121314
}
13131315

13141316
0xf4 => {
13151317
let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
13161318
if let Some(id) = pending_updates.get(0) {
1317-
monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
1319+
monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, chan_1_id, *id).unwrap();
13181320
}
13191321
nodes[1].process_monitor_events();
13201322
}
13211323
0xf5 => {
13221324
let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
13231325
if let Some(id) = pending_updates.get(1) {
1324-
monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
1326+
monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, chan_1_id, *id).unwrap();
13251327
}
13261328
nodes[1].process_monitor_events();
13271329
}
13281330
0xf6 => {
13291331
let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
13301332
if let Some(id) = pending_updates.last() {
1331-
monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
1333+
monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, chan_1_id, *id).unwrap();
13321334
}
13331335
nodes[1].process_monitor_events();
13341336
}
13351337

13361338
0xf8 => {
13371339
let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
13381340
if let Some(id) = pending_updates.get(0) {
1339-
monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
1341+
monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, chan_2_id, *id).unwrap();
13401342
}
13411343
nodes[1].process_monitor_events();
13421344
}
13431345
0xf9 => {
13441346
let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
13451347
if let Some(id) = pending_updates.get(1) {
1346-
monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
1348+
monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, chan_2_id, *id).unwrap();
13471349
}
13481350
nodes[1].process_monitor_events();
13491351
}
13501352
0xfa => {
13511353
let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
13521354
if let Some(id) = pending_updates.last() {
1353-
monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
1355+
monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, chan_2_id, *id).unwrap();
13541356
}
13551357
nodes[1].process_monitor_events();
13561358
}
13571359

13581360
0xfc => {
13591361
let pending_updates = monitor_c.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
13601362
if let Some(id) = pending_updates.get(0) {
1361-
monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
1363+
monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, chan_2_id, *id).unwrap();
13621364
}
13631365
nodes[2].process_monitor_events();
13641366
}
13651367
0xfd => {
13661368
let pending_updates = monitor_c.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
13671369
if let Some(id) = pending_updates.get(1) {
1368-
monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
1370+
monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, chan_2_id, *id).unwrap();
13691371
}
13701372
nodes[2].process_monitor_events();
13711373
}
13721374
0xfe => {
13731375
let pending_updates = monitor_c.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
13741376
if let Some(id) = pending_updates.last() {
1375-
monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
1377+
monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, chan_2_id, *id).unwrap();
13761378
}
13771379
nodes[2].process_monitor_events();
13781380
}
@@ -1387,19 +1389,19 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13871389
*monitor_c.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
13881390

13891391
if let Some((id, _)) = monitor_a.latest_monitors.lock().unwrap().get(&chan_1_funding) {
1390-
monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
1392+
monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, chan_1_id, *id);
13911393
nodes[0].process_monitor_events();
13921394
}
13931395
if let Some((id, _)) = monitor_b.latest_monitors.lock().unwrap().get(&chan_1_funding) {
1394-
monitor_b.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
1396+
monitor_b.chain_monitor.force_channel_monitor_updated(chan_1_funding, chan_1_id, *id);
13951397
nodes[1].process_monitor_events();
13961398
}
13971399
if let Some((id, _)) = monitor_b.latest_monitors.lock().unwrap().get(&chan_2_funding) {
1398-
monitor_b.chain_monitor.force_channel_monitor_updated(chan_2_funding, *id);
1400+
monitor_b.chain_monitor.force_channel_monitor_updated(chan_2_funding, chan_1_id, *id);
13991401
nodes[1].process_monitor_events();
14001402
}
14011403
if let Some((id, _)) = monitor_c.latest_monitors.lock().unwrap().get(&chan_2_funding) {
1402-
monitor_c.chain_monitor.force_channel_monitor_updated(chan_2_funding, *id);
1404+
monitor_c.chain_monitor.force_channel_monitor_updated(chan_2_funding, chan_1_id, *id);
14031405
nodes[2].process_monitor_events();
14041406
}
14051407

‎fuzz/src/full_stack.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ use bitcoin::consensus::encode::deserialize;
2222
use bitcoin::network::constants::Network;
2323

2424
use bitcoin::hashes::hex::FromHex;
25-
use bitcoin::hashes::Hash as TraitImport;
26-
use bitcoin::hashes::HashEngine as TraitImportEngine;
25+
use bitcoin::hashes::Hash as _;
2726
use bitcoin::hashes::sha256::Hash as Sha256;
2827
use bitcoin::hashes::sha256d::Hash as Sha256dHash;
2928
use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
@@ -651,7 +650,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
651650
if let None = loss_detector.txids_confirmed.get(&funding_txid) {
652651
let outpoint = OutPoint { txid: funding_txid, index: 0 };
653652
for chan in channelmanager.list_channels() {
654-
if chan.channel_id == outpoint.to_channel_id() {
653+
if chan.funding_txo == Some(outpoint) {
655654
tx.version += 1;
656655
continue 'search_loop;
657656
}

‎fuzz/src/utils/test_persister.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use lightning::chain;
22
use lightning::chain::{chainmonitor, channelmonitor};
33
use lightning::chain::chainmonitor::MonitorUpdateId;
44
use lightning::chain::transaction::OutPoint;
5+
use lightning::ln::ChannelId;
56
use lightning::util::test_channel_signer::TestChannelSigner;
67

78
use std::sync::Mutex;
@@ -10,11 +11,11 @@ pub struct TestPersister {
1011
pub update_ret: Mutex<chain::ChannelMonitorUpdateStatus>,
1112
}
1213
impl chainmonitor::Persist<TestChannelSigner> for TestPersister {
13-
fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
14+
fn persist_new_channel(&self, _funding_txo: OutPoint, _channel_id: ChannelId, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
1415
self.update_ret.lock().unwrap().clone()
1516
}
1617

17-
fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
18+
fn update_persisted_channel(&self, _funding_txo: OutPoint, _channel_id: ChannelId, _update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
1819
self.update_ret.lock().unwrap().clone()
1920
}
2021
}

‎lightning-background-processor/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,7 @@ mod tests {
929929
use lightning::chain::transaction::OutPoint;
930930
use lightning::events::{Event, PathFailure, MessageSendEventsProvider, MessageSendEvent};
931931
use lightning::{get_event_msg, get_event};
932-
use lightning::ln::PaymentHash;
932+
use lightning::ln::{PaymentHash, ChannelId};
933933
use lightning::ln::channelmanager;
934934
use lightning::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChainParameters, MIN_CLTV_EXPIRY_DELTA, PaymentId};
935935
use lightning::ln::features::{ChannelFeatures, NodeFeatures};
@@ -1414,7 +1414,7 @@ mod tests {
14141414
}
14151415

14161416
// Force-close the channel.
1417-
nodes[0].node.force_close_broadcasting_latest_txn(&OutPoint { txid: tx.txid(), index: 0 }.to_channel_id(), &nodes[1].node.get_our_node_id()).unwrap();
1417+
nodes[0].node.force_close_broadcasting_latest_txn(&ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.txid(), index: 0 }), &nodes[1].node.get_our_node_id()).unwrap();
14181418

14191419
// Check that the force-close updates are persisted.
14201420
check_persisted_data!(nodes[0].node, filepath.clone());

‎lightning-block-sync/src/init.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ BlockSourceResult<ValidatedBlockHeader> where B::Target: BlockSource {
4949
/// use lightning::chain::chaininterface::FeeEstimator;
5050
/// use lightning::sign;
5151
/// use lightning::sign::{EntropySource, NodeSigner, SignerProvider};
52+
/// use lightning::ln::ChannelId;
5253
/// use lightning::ln::channelmanager::{ChannelManager, ChannelManagerReadArgs};
5354
/// use lightning::routing::router::Router;
5455
/// use lightning::util::config::UserConfig;
@@ -119,7 +120,8 @@ BlockSourceResult<ValidatedBlockHeader> where B::Target: BlockSource {
119120
///
120121
/// // Allow the chain monitor to watch any channels.
121122
/// let monitor = monitor_listener.0;
122-
/// chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
123+
/// let funding_outpoint = monitor.get_funding_txo().0;
124+
/// chain_monitor.watch_channel(funding_outpoint, monitor);
123125
///
124126
/// // Create an SPV client to notify the chain monitor and channel manager of block events.
125127
/// let chain_poller = poll::ChainPoller::new(block_source, Network::Bitcoin);

‎lightning-persister/src/fs_store.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,8 @@ mod tests {
450450
check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000);
451451
let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap();
452452
let update_map = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap();
453-
let update_id = update_map.get(&added_monitors[0].0.to_channel_id()).unwrap();
453+
let channel_id = &added_monitors[0].1.get_channel_id();
454+
let update_id = update_map.get(&channel_id).unwrap();
454455

455456
// Set the store's directory to read-only, which should result in
456457
// returning an unrecoverable failure when we then attempt to persist a
@@ -464,7 +465,7 @@ mod tests {
464465
txid: Txid::from_str("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(),
465466
index: 0
466467
};
467-
match store.persist_new_channel(test_txo, &added_monitors[0].1, update_id.2) {
468+
match store.persist_new_channel(test_txo, *channel_id, &added_monitors[0].1, update_id.2) {
468469
ChannelMonitorUpdateStatus::UnrecoverableError => {},
469470
_ => panic!("unexpected result from persisting new channel")
470471
}
@@ -489,7 +490,9 @@ mod tests {
489490
check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000);
490491
let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap();
491492
let update_map = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap();
492-
let update_id = update_map.get(&added_monitors[0].0.to_channel_id()).unwrap();
493+
let funding_outpoint = &added_monitors[0].0;
494+
let channel_id = &added_monitors[0].1.get_channel_id();
495+
let update_id = update_map.get(channel_id).unwrap();
493496

494497
// Create the store with an invalid directory name and test that the
495498
// channel fails to open because the directories fail to be created. There
@@ -501,7 +504,7 @@ mod tests {
501504
txid: Txid::from_str("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(),
502505
index: 0
503506
};
504-
match store.persist_new_channel(test_txo, &added_monitors[0].1, update_id.2) {
507+
match store.persist_new_channel(test_txo, *channel_id, &added_monitors[0].1, update_id.2) {
505508
ChannelMonitorUpdateStatus::UnrecoverableError => {},
506509
_ => panic!("unexpected result from persisting new channel")
507510
}

0 commit comments

Comments
 (0)