Skip to content

Commit 5930c56

Browse files
dunxendunxen
committed
f revert unnecessary channel_id args
1 parent 284f101 commit 5930c56

File tree

4 files changed

+27
-31
lines changed

4 files changed

+27
-31
lines changed

‎fuzz/src/utils/test_persister.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ 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;
65
use lightning::util::test_channel_signer::TestChannelSigner;
76

87
use std::sync::Mutex;
@@ -11,11 +10,11 @@ pub struct TestPersister {
1110
pub update_ret: Mutex<chain::ChannelMonitorUpdateStatus>,
1211
}
1312
impl chainmonitor::Persist<TestChannelSigner> for TestPersister {
14-
fn persist_new_channel(&self, _funding_txo: OutPoint, _channel_id: ChannelId, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
13+
fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
1514
self.update_ret.lock().unwrap().clone()
1615
}
1716

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 {
17+
fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
1918
self.update_ret.lock().unwrap().clone()
2019
}
2120
}

‎lightning/src/chain/chainmonitor.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ pub trait Persist<ChannelSigner: WriteableEcdsaChannelSigner> {
159159
///
160160
/// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
161161
/// [`Writeable::write`]: crate::util::ser::Writeable::write
162-
fn persist_new_channel(&self, channel_funding_outpoint: OutPoint, channel_id: ChannelId, data: &ChannelMonitor<ChannelSigner>, update_id: MonitorUpdateId) -> ChannelMonitorUpdateStatus;
162+
fn persist_new_channel(&self, channel_funding_outpoint: OutPoint, data: &ChannelMonitor<ChannelSigner>, update_id: MonitorUpdateId) -> ChannelMonitorUpdateStatus;
163163

164164
/// Update one channel's data. The provided [`ChannelMonitor`] has already applied the given
165165
/// update.
@@ -194,7 +194,7 @@ pub trait Persist<ChannelSigner: WriteableEcdsaChannelSigner> {
194194
/// [`ChannelMonitorUpdateStatus`] for requirements when returning errors.
195195
///
196196
/// [`Writeable::write`]: crate::util::ser::Writeable::write
197-
fn update_persisted_channel(&self, channel_funding_outpoint: OutPoint, channel_id: ChannelId, update: Option<&ChannelMonitorUpdate>, data: &ChannelMonitor<ChannelSigner>, update_id: MonitorUpdateId) -> ChannelMonitorUpdateStatus;
197+
fn update_persisted_channel(&self, channel_funding_outpoint: OutPoint, update: Option<&ChannelMonitorUpdate>, data: &ChannelMonitor<ChannelSigner>, update_id: MonitorUpdateId) -> ChannelMonitorUpdateStatus;
198198
}
199199

200200
struct MonitorHolder<ChannelSigner: WriteableEcdsaChannelSigner> {
@@ -322,8 +322,7 @@ where C::Target: chain::Filter,
322322
for funding_outpoint in funding_outpoints.iter() {
323323
let monitor_lock = self.monitors.read().unwrap();
324324
if let Some(monitor_state) = monitor_lock.get(funding_outpoint) {
325-
let channel_id = monitor_state.monitor.get_channel_id();
326-
if self.update_monitor_with_chain_data(header, best_height, txdata, &process, funding_outpoint, channel_id, &monitor_state).is_err() {
325+
if self.update_monitor_with_chain_data(header, best_height, txdata, &process, funding_outpoint, &monitor_state).is_err() {
327326
// Take the monitors lock for writing so that we poison it and any future
328327
// operations going forward fail immediately.
329328
core::mem::drop(monitor_lock);
@@ -338,8 +337,7 @@ where C::Target: chain::Filter,
338337
let monitor_states = self.monitors.write().unwrap();
339338
for (funding_outpoint, monitor_state) in monitor_states.iter() {
340339
if !funding_outpoints.contains(funding_outpoint) {
341-
let channel_id = monitor_state.monitor.get_channel_id();
342-
if self.update_monitor_with_chain_data(header, best_height, txdata, &process, funding_outpoint, channel_id, &monitor_state).is_err() {
340+
if self.update_monitor_with_chain_data(header, best_height, txdata, &process, funding_outpoint, &monitor_state).is_err() {
343341
log_error!(self.logger, "{}", err_str);
344342
panic!("{}", err_str);
345343
}
@@ -359,7 +357,7 @@ where C::Target: chain::Filter,
359357

360358
fn update_monitor_with_chain_data<FN>(
361359
&self, header: &Header, best_height: Option<u32>, txdata: &TransactionData,
362-
process: FN, funding_outpoint: &OutPoint, channel_id: ChannelId, monitor_state: &MonitorHolder<ChannelSigner>
360+
process: FN, funding_outpoint: &OutPoint, monitor_state: &MonitorHolder<ChannelSigner>
363361
) -> Result<(), ()> where FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs> {
364362
let monitor = &monitor_state.monitor;
365363
let logger = WithChannelMonitor::from(&self.logger, &monitor);
@@ -380,7 +378,7 @@ where C::Target: chain::Filter,
380378
}
381379

382380
log_trace!(logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
383-
match self.persister.update_persisted_channel(*funding_outpoint, channel_id, None, monitor, update_id) {
381+
match self.persister.update_persisted_channel(*funding_outpoint, None, monitor, update_id) {
384382
ChannelMonitorUpdateStatus::Completed =>
385383
log_trace!(logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
386384
ChannelMonitorUpdateStatus::InProgress => {
@@ -733,7 +731,7 @@ where C::Target: chain::Filter,
733731
log_trace!(logger, "Got new ChannelMonitor for channel {}", log_funding_info!(monitor));
734732
let update_id = MonitorUpdateId::from_new_monitor(&monitor);
735733
let mut pending_monitor_updates = Vec::new();
736-
let persist_res = self.persister.persist_new_channel(funding_outpoint, monitor.get_channel_id(), &monitor, update_id);
734+
let persist_res = self.persister.persist_new_channel(funding_outpoint, &monitor, update_id);
737735
match persist_res {
738736
ChannelMonitorUpdateStatus::InProgress => {
739737
log_info!(logger, "Persistence of new ChannelMonitor for channel {} in progress", log_funding_info!(monitor));
@@ -790,9 +788,9 @@ where C::Target: chain::Filter,
790788
// while reading `channel_monitor` with updates from storage. Instead, we should persist
791789
// the entire `channel_monitor` here.
792790
log_warn!(logger, "Failed to update ChannelMonitor for channel {}. Going ahead and persisting the entire ChannelMonitor", log_funding_info!(monitor));
793-
self.persister.update_persisted_channel(funding_txo, channel_id, None, monitor, update_id)
791+
self.persister.update_persisted_channel(funding_txo, None, monitor, update_id)
794792
} else {
795-
self.persister.update_persisted_channel(funding_txo, channel_id, Some(update), monitor, update_id)
793+
self.persister.update_persisted_channel(funding_txo, Some(update), monitor, update_id)
796794
};
797795
match persist_res {
798796
ChannelMonitorUpdateStatus::InProgress => {

‎lightning/src/util/persist.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use core::ops::Deref;
1414
use core::str::FromStr;
1515
use bitcoin::{BlockHash, Txid};
1616

17-
use crate::ln::ChannelId;
1817
use crate::{io, log_error};
1918
use crate::alloc::string::ToString;
2019
use crate::prelude::*;
@@ -194,7 +193,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner, K: KVStore> Persist<ChannelSign
194193
// Then we should return InProgress rather than UnrecoverableError, implying we should probably
195194
// just shut down the node since we're not retrying persistence!
196195

197-
fn persist_new_channel(&self, funding_txo: OutPoint, _channel_id: ChannelId, monitor: &ChannelMonitor<ChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
196+
fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
198197
let key = format!("{}_{}", funding_txo.txid.to_string(), funding_txo.index);
199198
match self.write(
200199
CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE,
@@ -206,7 +205,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner, K: KVStore> Persist<ChannelSign
206205
}
207206
}
208207

209-
fn update_persisted_channel(&self, funding_txo: OutPoint, _channel_id: ChannelId, _update: Option<&ChannelMonitorUpdate>, monitor: &ChannelMonitor<ChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
208+
fn update_persisted_channel(&self, funding_txo: OutPoint, _update: Option<&ChannelMonitorUpdate>, monitor: &ChannelMonitor<ChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
210209
let key = format!("{}_{}", funding_txo.txid.to_string(), funding_txo.index);
211210
match self.write(
212211
CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE,
@@ -607,7 +606,7 @@ where
607606
/// Persists a new channel. This means writing the entire monitor to the
608607
/// parametrized [`KVStore`].
609608
fn persist_new_channel(
610-
&self, funding_txo: OutPoint, _channel_id: ChannelId, monitor: &ChannelMonitor<ChannelSigner>,
609+
&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChannelSigner>,
611610
_monitor_update_call_id: MonitorUpdateId,
612611
) -> chain::ChannelMonitorUpdateStatus {
613612
// Determine the proper key for this monitor
@@ -651,7 +650,7 @@ where
651650
/// `update` is `None`.
652651
/// - The update is at [`CLOSED_CHANNEL_UPDATE_ID`]
653652
fn update_persisted_channel(
654-
&self, funding_txo: OutPoint, channel_id: ChannelId, update: Option<&ChannelMonitorUpdate>,
653+
&self, funding_txo: OutPoint, update: Option<&ChannelMonitorUpdate>,
655654
monitor: &ChannelMonitor<ChannelSigner>, monitor_update_call_id: MonitorUpdateId,
656655
) -> chain::ChannelMonitorUpdateStatus {
657656
// IMPORTANT: monitor_update_call_id: MonitorUpdateId is not to be confused with
@@ -691,7 +690,7 @@ where
691690
};
692691

693692
// We could write this update, but it meets criteria of our design that calls for a full monitor write.
694-
let monitor_update_status = self.persist_new_channel(funding_txo, channel_id, monitor, monitor_update_call_id);
693+
let monitor_update_status = self.persist_new_channel(funding_txo, monitor, monitor_update_call_id);
695694

696695
if let chain::ChannelMonitorUpdateStatus::Completed = monitor_update_status {
697696
let cleanup_range = if monitor.get_latest_update_id() == CLOSED_CHANNEL_UPDATE_ID {
@@ -720,7 +719,7 @@ where
720719
}
721720
} else {
722721
// There is no update given, so we must persist a new monitor.
723-
self.persist_new_channel(funding_txo, channel_id, monitor, monitor_update_call_id)
722+
self.persist_new_channel(funding_txo, monitor, monitor_update_call_id)
724723
}
725724
}
726725
}
@@ -1066,7 +1065,7 @@ mod tests {
10661065
entropy_source: node_cfgs[0].keys_manager,
10671066
signer_provider: node_cfgs[0].keys_manager,
10681067
};
1069-
match ro_persister.persist_new_channel(test_txo, channel_id, &added_monitors[0].1, update_id.2) {
1068+
match ro_persister.persist_new_channel(test_txo, &added_monitors[0].1, update_id.2) {
10701069
ChannelMonitorUpdateStatus::UnrecoverableError => {
10711070
// correct result
10721071
}
@@ -1077,7 +1076,7 @@ mod tests {
10771076
panic!("Returned InProgress when shouldn't have")
10781077
}
10791078
}
1080-
match ro_persister.update_persisted_channel(test_txo, channel_id, Some(cmu), &added_monitors[0].1, update_id.2) {
1079+
match ro_persister.update_persisted_channel(test_txo, Some(cmu), &added_monitors[0].1, update_id.2) {
10811080
ChannelMonitorUpdateStatus::UnrecoverableError => {
10821081
// correct result
10831082
}

‎lightning/src/util/test_utils.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -399,10 +399,10 @@ impl WatchtowerPersister {
399399
}
400400

401401
impl<Signer: sign::ecdsa::WriteableEcdsaChannelSigner> chainmonitor::Persist<Signer> for WatchtowerPersister {
402-
fn persist_new_channel(&self, funding_txo: OutPoint, channel_id: ChannelId,
402+
fn persist_new_channel(&self, funding_txo: OutPoint,
403403
data: &channelmonitor::ChannelMonitor<Signer>, id: MonitorUpdateId
404404
) -> chain::ChannelMonitorUpdateStatus {
405-
let res = self.persister.persist_new_channel(funding_txo, channel_id, data, id);
405+
let res = self.persister.persist_new_channel(funding_txo, data, id);
406406

407407
assert!(self.unsigned_justice_tx_data.lock().unwrap()
408408
.insert(funding_txo, VecDeque::new()).is_none());
@@ -421,10 +421,10 @@ impl<Signer: sign::ecdsa::WriteableEcdsaChannelSigner> chainmonitor::Persist<Sig
421421
}
422422

423423
fn update_persisted_channel(
424-
&self, funding_txo: OutPoint, channel_id: ChannelId, update: Option<&channelmonitor::ChannelMonitorUpdate>,
424+
&self, funding_txo: OutPoint, update: Option<&channelmonitor::ChannelMonitorUpdate>,
425425
data: &channelmonitor::ChannelMonitor<Signer>, update_id: MonitorUpdateId
426426
) -> chain::ChannelMonitorUpdateStatus {
427-
let res = self.persister.update_persisted_channel(funding_txo, channel_id, update, data, update_id);
427+
let res = self.persister.update_persisted_channel(funding_txo, update, data, update_id);
428428

429429
if let Some(update) = update {
430430
let commitment_txs = data.counterparty_commitment_txs_from_update(update);
@@ -479,23 +479,23 @@ impl TestPersister {
479479
}
480480
}
481481
impl<Signer: sign::ecdsa::WriteableEcdsaChannelSigner> chainmonitor::Persist<Signer> for TestPersister {
482-
fn persist_new_channel(&self, _funding_txo: OutPoint, _channel_id: ChannelId, _data: &channelmonitor::ChannelMonitor<Signer>, _id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
482+
fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<Signer>, _id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
483483
if let Some(update_ret) = self.update_rets.lock().unwrap().pop_front() {
484484
return update_ret
485485
}
486486
chain::ChannelMonitorUpdateStatus::Completed
487487
}
488488

489-
fn update_persisted_channel(&self, funding_txo: OutPoint, channel_id: ChannelId, _update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<Signer>, update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
489+
fn update_persisted_channel(&self, funding_txo: OutPoint, _update: Option<&channelmonitor::ChannelMonitorUpdate>, data: &channelmonitor::ChannelMonitor<Signer>, update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
490490
let mut ret = chain::ChannelMonitorUpdateStatus::Completed;
491491
if let Some(update_ret) = self.update_rets.lock().unwrap().pop_front() {
492492
ret = update_ret;
493493
}
494494
let is_chain_sync = if let UpdateOrigin::ChainSync(_) = update_id.contents { true } else { false };
495495
if is_chain_sync {
496-
self.chain_sync_monitor_persistences.lock().unwrap().entry(funding_txo).or_insert((channel_id, HashSet::new())).1.insert(update_id);
496+
self.chain_sync_monitor_persistences.lock().unwrap().entry(funding_txo).or_insert((data.get_channel_id(), HashSet::new())).1.insert(update_id);
497497
} else {
498-
self.offchain_monitor_updates.lock().unwrap().entry(funding_txo).or_insert((channel_id, HashSet::new())).1.insert(update_id);
498+
self.offchain_monitor_updates.lock().unwrap().entry(funding_txo).or_insert((data.get_channel_id(), HashSet::new())).1.insert(update_id);
499499
}
500500
ret
501501
}

0 commit comments

Comments
 (0)