Skip to content

Commit 7078bba

Browse files
wpaulinoAditya Sharma
wpaulino
authored and
Aditya Sharma
committed
Remove funding script return value from ChannelMonitor::get_funding_txo
It's not needed except for one place where we can just access the field directly, so we can avoid the allocation on each call. For API consumers, they may still access the funding script via `ChannelMonitor::get_funding_script`.
1 parent e86a907 commit 7078bba

File tree

4 files changed

+38
-29
lines changed

4 files changed

+38
-29
lines changed

‎lightning/src/chain/chainmonitor.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ where C::Target: chain::Filter,
342342
// `ChannelMonitorUpdate` after a channel persist for a channel with the same
343343
// `latest_update_id`.
344344
let _pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
345-
let funding_txo = monitor.get_funding_txo().0;
345+
let funding_txo = monitor.get_funding_txo();
346346
match self.persister.update_persisted_channel(funding_txo, None, monitor) {
347347
ChannelMonitorUpdateStatus::Completed =>
348348
log_trace!(logger, "Finished syncing Channel Monitor for channel {} for block-data",
@@ -516,7 +516,7 @@ where C::Target: chain::Filter,
516516
// Completed event.
517517
return Ok(());
518518
}
519-
let funding_txo = monitor_data.monitor.get_funding_txo().0;
519+
let funding_txo = monitor_data.monitor.get_funding_txo();
520520
self.pending_monitor_events.lock().unwrap().push((funding_txo, channel_id, vec![MonitorEvent::Completed {
521521
funding_txo,
522522
channel_id,
@@ -535,7 +535,7 @@ where C::Target: chain::Filter,
535535
let monitors = self.monitors.read().unwrap();
536536
let monitor = &monitors.get(&channel_id).unwrap().monitor;
537537
let counterparty_node_id = monitor.get_counterparty_node_id();
538-
let funding_txo = monitor.get_funding_txo().0;
538+
let funding_txo = monitor.get_funding_txo();
539539
self.pending_monitor_events.lock().unwrap().push((funding_txo, channel_id, vec![MonitorEvent::Completed {
540540
funding_txo,
541541
channel_id,
@@ -642,7 +642,7 @@ where C::Target: chain::Filter,
642642
have_monitors_to_prune = true;
643643
}
644644
if needs_persistence {
645-
self.persister.update_persisted_channel(monitor_holder.monitor.get_funding_txo().0, None, &monitor_holder.monitor);
645+
self.persister.update_persisted_channel(monitor_holder.monitor.get_funding_txo(), None, &monitor_holder.monitor);
646646
}
647647
}
648648
if have_monitors_to_prune {
@@ -655,7 +655,7 @@ where C::Target: chain::Filter,
655655
"Archiving fully resolved ChannelMonitor for channel ID {}",
656656
channel_id
657657
);
658-
self.persister.archive_persisted_channel(monitor_holder.monitor.get_funding_txo().0);
658+
self.persister.archive_persisted_channel(monitor_holder.monitor.get_funding_txo());
659659
false
660660
} else {
661661
true
@@ -769,7 +769,7 @@ where C::Target: chain::Filter,
769769
log_trace!(logger, "Got new ChannelMonitor for channel {}", log_funding_info!(monitor));
770770
let update_id = monitor.get_latest_update_id();
771771
let mut pending_monitor_updates = Vec::new();
772-
let persist_res = self.persister.persist_new_channel(monitor.get_funding_txo().0, &monitor);
772+
let persist_res = self.persister.persist_new_channel(monitor.get_funding_txo(), &monitor);
773773
match persist_res {
774774
ChannelMonitorUpdateStatus::InProgress => {
775775
log_info!(logger, "Persistence of new ChannelMonitor for channel {} in progress", log_funding_info!(monitor));
@@ -825,7 +825,7 @@ where C::Target: chain::Filter,
825825
let update_res = monitor.update_monitor(update, &self.broadcaster, &self.fee_estimator, &self.logger);
826826

827827
let update_id = update.update_id;
828-
let funding_txo = monitor.get_funding_txo().0;
828+
let funding_txo = monitor.get_funding_txo();
829829
let persist_res = if update_res.is_err() {
830830
// Even if updating the monitor returns an error, the monitor's state will
831831
// still be changed. Therefore, we should persist the updated monitor despite the error.
@@ -878,7 +878,7 @@ where C::Target: chain::Filter,
878878
for monitor_state in self.monitors.read().unwrap().values() {
879879
let monitor_events = monitor_state.monitor.get_and_clear_pending_monitor_events();
880880
if monitor_events.len() > 0 {
881-
let monitor_funding_txo = monitor_state.monitor.get_funding_txo().0;
881+
let monitor_funding_txo = monitor_state.monitor.get_funding_txo();
882882
let monitor_channel_id = monitor_state.monitor.channel_id();
883883
let counterparty_node_id = monitor_state.monitor.get_counterparty_node_id();
884884
pending_monitor_events.push((monitor_funding_txo, monitor_channel_id, monitor_events, counterparty_node_id));

‎lightning/src/chain/channelmonitor.rs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,8 +1574,13 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
15741574
}
15751575

15761576
/// Gets the funding transaction outpoint of the channel this ChannelMonitor is monitoring for.
1577-
pub fn get_funding_txo(&self) -> (OutPoint, ScriptBuf) {
1578-
self.inner.lock().unwrap().get_funding_txo().clone()
1577+
pub fn get_funding_txo(&self) -> OutPoint {
1578+
self.inner.lock().unwrap().get_funding_txo()
1579+
}
1580+
1581+
/// Gets the funding script of the channel this ChannelMonitor is monitoring for.
1582+
pub fn get_funding_script(&self) -> ScriptBuf {
1583+
self.inner.lock().unwrap().get_funding_script()
15791584
}
15801585

15811586
/// Gets the channel_id of the channel this ChannelMonitor is monitoring for.
@@ -1599,8 +1604,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
15991604
{
16001605
let lock = self.inner.lock().unwrap();
16011606
let logger = WithChannelMonitor::from_impl(logger, &*lock, None);
1602-
log_trace!(&logger, "Registering funding outpoint {}", &lock.get_funding_txo().0);
1603-
filter.register_tx(&lock.get_funding_txo().0.txid, &lock.get_funding_txo().1);
1607+
log_trace!(&logger, "Registering funding outpoint {}", &lock.get_funding_txo());
1608+
filter.register_tx(&lock.funding_info.0.txid, &lock.funding_info.1);
16041609
for (txid, outputs) in lock.get_outputs_to_watch().iter() {
16051610
for (index, script_pubkey) in outputs.iter() {
16061611
assert!(*index <= u16::MAX as u32);
@@ -2061,7 +2066,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
20612066
"Thought we were done claiming funds, but claimable_balances now has entries");
20622067
log_error!(logger,
20632068
"WARNING: LDK thought it was done claiming all the available funds in the ChannelMonitor for channel {}, but later decided it had more to claim. This is potentially an important bug in LDK, please report it at https://github.com/lightningdevkit/rust-lightning/issues/new",
2064-
inner.get_funding_txo().0);
2069+
inner.get_funding_txo());
20652070
inner.balances_empty_height = None;
20662071
(false, true)
20672072
},
@@ -2070,7 +2075,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
20702075
// None. It is set to the current block height.
20712076
log_debug!(logger,
20722077
"ChannelMonitor funded at {} is now fully resolved. It will become archivable in {} blocks",
2073-
inner.get_funding_txo().0, ARCHIVAL_DELAY_BLOCKS);
2078+
inner.get_funding_txo(), ARCHIVAL_DELAY_BLOCKS);
20742079
inner.balances_empty_height = Some(current_height);
20752080
(false, true)
20762081
},
@@ -3296,8 +3301,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
32963301
self.latest_update_id
32973302
}
32983303

3299-
fn get_funding_txo(&self) -> &(OutPoint, ScriptBuf) {
3300-
&self.funding_info
3304+
fn get_funding_txo(&self) -> OutPoint {
3305+
self.funding_info.0
3306+
}
3307+
3308+
fn get_funding_script(&self) -> ScriptBuf {
3309+
self.funding_info.1.clone()
33013310
}
33023311

33033312
pub fn channel_id(&self) -> ChannelId {

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8131,7 +8131,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
81318131
},
81328132
hash_map::Entry::Vacant(e) => {
81338133
let mut outpoint_to_peer_lock = self.outpoint_to_peer.lock().unwrap();
8134-
match outpoint_to_peer_lock.entry(monitor.get_funding_txo().0) {
8134+
match outpoint_to_peer_lock.entry(monitor.get_funding_txo()) {
81358135
hash_map::Entry::Occupied(_) => {
81368136
fail_chan!("The funding_created message had the same funding_txid as an existing channel - funding is not possible");
81378137
},
@@ -13365,7 +13365,7 @@ where
1336513365
updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast: true }],
1336613366
channel_id: Some(monitor.channel_id()),
1336713367
};
13368-
let funding_txo = monitor.get_funding_txo().0;
13368+
let funding_txo = monitor.get_funding_txo();
1336913369
if let Some(counterparty_node_id) = monitor.get_counterparty_node_id() {
1337013370
let update = BackgroundEvent::MonitorUpdateRegeneratedOnStartup {
1337113371
counterparty_node_id,
@@ -13576,7 +13576,7 @@ where
1357613576
) => { {
1357713577
let mut max_in_flight_update_id = 0;
1357813578
$chan_in_flight_upds.retain(|upd| upd.update_id > $monitor.get_latest_update_id());
13579-
let funding_txo = $monitor.get_funding_txo().0;
13579+
let funding_txo = $monitor.get_funding_txo();
1358013580
for update in $chan_in_flight_upds.iter() {
1358113581
log_trace!($logger, "Replaying ChannelMonitorUpdate {} for {}channel {}",
1358213582
update.update_id, $channel_info_log, &$monitor.channel_id());
@@ -13741,7 +13741,7 @@ where
1374113741
// We only rebuild the pending payments map if we were most recently serialized by
1374213742
// 0.0.102+
1374313743
for (_, monitor) in args.channel_monitors.iter() {
13744-
let counterparty_opt = outpoint_to_peer.get(&monitor.get_funding_txo().0);
13744+
let counterparty_opt = outpoint_to_peer.get(&monitor.get_funding_txo());
1374513745
if counterparty_opt.is_none() {
1374613746
for (htlc_source, (htlc, _)) in monitor.get_pending_or_resolved_outbound_htlcs() {
1374713747
let logger = WithChannelMonitor::from(&args.logger, monitor, Some(htlc.payment_hash));
@@ -13821,7 +13821,7 @@ where
1382113821
// `ChannelMonitor` is removed.
1382213822
let compl_action =
1382313823
EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
13824-
channel_funding_outpoint: monitor.get_funding_txo().0,
13824+
channel_funding_outpoint: monitor.get_funding_txo(),
1382513825
channel_id: monitor.channel_id(),
1382613826
counterparty_node_id: path.hops[0].pubkey,
1382713827
};
@@ -13934,7 +13934,7 @@ where
1393413934
// channel_id -> peer map entry).
1393513935
counterparty_opt.is_none(),
1393613936
counterparty_opt.cloned().or(monitor.get_counterparty_node_id()),
13937-
monitor.get_funding_txo().0, monitor.channel_id()))
13937+
monitor.get_funding_txo(), monitor.channel_id()))
1393813938
} else { None }
1393913939
} else {
1394013940
// If it was an outbound payment, we've handled it above - if a preimage

‎lightning/src/util/persist.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,8 @@ where
356356
(&*entropy_source, &*signer_provider),
357357
) {
358358
Ok((block_hash, channel_monitor)) => {
359-
if channel_monitor.get_funding_txo().0.txid != txid
360-
|| channel_monitor.get_funding_txo().0.index != index
359+
if channel_monitor.get_funding_txo().txid != txid
360+
|| channel_monitor.get_funding_txo().index != index
361361
{
362362
return Err(io::Error::new(
363363
io::ErrorKind::InvalidData,
@@ -617,8 +617,8 @@ where
617617
(&*self.entropy_source, &*self.signer_provider),
618618
) {
619619
Ok((blockhash, channel_monitor)) => {
620-
if channel_monitor.get_funding_txo().0.txid != outpoint.txid
621-
|| channel_monitor.get_funding_txo().0.index != outpoint.index
620+
if channel_monitor.get_funding_txo().txid != outpoint.txid
621+
|| channel_monitor.get_funding_txo().index != outpoint.index
622622
{
623623
log_error!(
624624
self.logger,
@@ -1219,7 +1219,7 @@ mod tests {
12191219
// check that when we read it, we got the right update id
12201220
assert_eq!(mon.get_latest_update_id(), $expected_update_id);
12211221

1222-
let monitor_name = MonitorName::from(mon.get_funding_txo().0);
1222+
let monitor_name = MonitorName::from(mon.get_funding_txo());
12231223
assert_eq!(
12241224
persister_0
12251225
.kv_store
@@ -1238,7 +1238,7 @@ mod tests {
12381238
assert_eq!(persisted_chan_data_1.len(), 1);
12391239
for (_, mon) in persisted_chan_data_1.iter() {
12401240
assert_eq!(mon.get_latest_update_id(), $expected_update_id);
1241-
let monitor_name = MonitorName::from(mon.get_funding_txo().0);
1241+
let monitor_name = MonitorName::from(mon.get_funding_txo());
12421242
assert_eq!(
12431243
persister_1
12441244
.kv_store
@@ -1433,7 +1433,7 @@ mod tests {
14331433
// Get the monitor and make a fake stale update at update_id=1 (lowest height of an update possible)
14341434
let persisted_chan_data = persister_0.read_all_channel_monitors_with_updates().unwrap();
14351435
let (_, monitor) = &persisted_chan_data[0];
1436-
let monitor_name = MonitorName::from(monitor.get_funding_txo().0);
1436+
let monitor_name = MonitorName::from(monitor.get_funding_txo());
14371437
persister_0
14381438
.kv_store
14391439
.write(

0 commit comments

Comments
 (0)