Skip to content

Commit 4f0d5ed

Browse files
committed
Remove redundant locking when creating WithChannelMonitor
The `WithChannelMonitor` log decorator redundantly locks the `ChannelMonitor` inner mutex, which we fix here, as well as add a new constructor which avoids locking at all if an inner mutex lock is already readily available.
1 parent ed75ce3 commit 4f0d5ed

File tree

1 file changed

+41
-26
lines changed

1 file changed

+41
-26
lines changed

lightning/src/chain/channelmonitor.rs

+41-26
Original file line numberDiff line numberDiff line change
@@ -1141,10 +1141,14 @@ impl<'a, L: Deref> Logger for WithChannelMonitor<'a, L> where L::Target: Logger
11411141

11421142
impl<'a, L: Deref> WithChannelMonitor<'a, L> where L::Target: Logger {
11431143
pub(crate) fn from<S: WriteableEcdsaChannelSigner>(logger: &'a L, monitor: &ChannelMonitor<S>) -> Self {
1144+
Self::from_impl(logger, &*monitor.inner.lock().unwrap())
1145+
}
1146+
1147+
pub(crate) fn from_impl<S: WriteableEcdsaChannelSigner>(logger: &'a L, monitor_impl: &ChannelMonitorImpl<S>) -> Self {
1148+
let peer_id = monitor_impl.counterparty_node_id;
1149+
let channel_id = Some(monitor_impl.funding_info.0.to_channel_id());
11441150
WithChannelMonitor {
1145-
logger,
1146-
peer_id: monitor.get_counterparty_node_id(),
1147-
channel_id: Some(monitor.get_funding_txo().0.to_channel_id()),
1151+
logger, peer_id, channel_id,
11481152
}
11491153
}
11501154
}
@@ -1282,8 +1286,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
12821286
)
12831287
where L::Target: Logger
12841288
{
1285-
let logger = WithChannelMonitor::from(logger, self);
1286-
self.inner.lock().unwrap().provide_initial_counterparty_commitment_tx(txid,
1289+
let mut inner = self.inner.lock().unwrap();
1290+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1291+
inner.provide_initial_counterparty_commitment_tx(txid,
12871292
htlc_outputs, commitment_number, their_cur_per_commitment_point, feerate_per_kw,
12881293
to_broadcaster_value_sat, to_countersignatory_value_sat, &logger);
12891294
}
@@ -1301,8 +1306,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
13011306
their_per_commitment_point: PublicKey,
13021307
logger: &L,
13031308
) where L::Target: Logger {
1304-
let logger = WithChannelMonitor::from(logger, self);
1305-
self.inner.lock().unwrap().provide_latest_counterparty_commitment_tx(
1309+
let mut inner = self.inner.lock().unwrap();
1310+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1311+
inner.provide_latest_counterparty_commitment_tx(
13061312
txid, htlc_outputs, commitment_number, their_per_commitment_point, &logger)
13071313
}
13081314

@@ -1328,8 +1334,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
13281334
F::Target: FeeEstimator,
13291335
L::Target: Logger,
13301336
{
1331-
let logger = WithChannelMonitor::from(logger, self);
1332-
self.inner.lock().unwrap().provide_payment_preimage(
1337+
let mut inner = self.inner.lock().unwrap();
1338+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1339+
inner.provide_payment_preimage(
13331340
payment_hash, payment_preimage, broadcaster, fee_estimator, &logger)
13341341
}
13351342

@@ -1349,8 +1356,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
13491356
F::Target: FeeEstimator,
13501357
L::Target: Logger,
13511358
{
1352-
let logger = WithChannelMonitor::from(logger, self);
1353-
self.inner.lock().unwrap().update_monitor(updates, broadcaster, fee_estimator, &logger)
1359+
let mut inner = self.inner.lock().unwrap();
1360+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1361+
inner.update_monitor(updates, broadcaster, fee_estimator, &logger)
13541362
}
13551363

13561364
/// Gets the update_id from the latest ChannelMonitorUpdate which was applied to this
@@ -1529,8 +1537,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
15291537
/// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
15301538
pub fn get_latest_holder_commitment_txn<L: Deref>(&self, logger: &L) -> Vec<Transaction>
15311539
where L::Target: Logger {
1532-
let logger = WithChannelMonitor::from(logger, self);
1533-
self.inner.lock().unwrap().get_latest_holder_commitment_txn(&logger)
1540+
let mut inner = self.inner.lock().unwrap();
1541+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1542+
inner.get_latest_holder_commitment_txn(&logger)
15341543
}
15351544

15361545
/// Unsafe test-only version of get_latest_holder_commitment_txn used by our test framework
@@ -1539,8 +1548,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
15391548
#[cfg(any(test, feature = "unsafe_revoked_tx_signing"))]
15401549
pub fn unsafe_get_latest_holder_commitment_txn<L: Deref>(&self, logger: &L) -> Vec<Transaction>
15411550
where L::Target: Logger {
1542-
let logger = WithChannelMonitor::from(logger, self);
1543-
self.inner.lock().unwrap().unsafe_get_latest_holder_commitment_txn(&logger)
1551+
let mut inner = self.inner.lock().unwrap();
1552+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1553+
inner.unsafe_get_latest_holder_commitment_txn(&logger)
15441554
}
15451555

15461556
/// Processes transactions in a newly connected block, which may result in any of the following:
@@ -1568,8 +1578,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
15681578
F::Target: FeeEstimator,
15691579
L::Target: Logger,
15701580
{
1571-
let logger = WithChannelMonitor::from(logger, self);
1572-
self.inner.lock().unwrap().block_connected(
1581+
let mut inner = self.inner.lock().unwrap();
1582+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1583+
inner.block_connected(
15731584
header, txdata, height, broadcaster, fee_estimator, &logger)
15741585
}
15751586

@@ -1587,8 +1598,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
15871598
F::Target: FeeEstimator,
15881599
L::Target: Logger,
15891600
{
1590-
let logger = WithChannelMonitor::from(logger, self);
1591-
self.inner.lock().unwrap().block_disconnected(
1601+
let mut inner = self.inner.lock().unwrap();
1602+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1603+
inner.block_disconnected(
15921604
header, height, broadcaster, fee_estimator, &logger)
15931605
}
15941606

@@ -1614,8 +1626,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
16141626
L::Target: Logger,
16151627
{
16161628
let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
1617-
let logger = WithChannelMonitor::from(logger, self);
1618-
self.inner.lock().unwrap().transactions_confirmed(
1629+
let mut inner = self.inner.lock().unwrap();
1630+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1631+
inner.transactions_confirmed(
16191632
header, txdata, height, broadcaster, &bounded_fee_estimator, &logger)
16201633
}
16211634

@@ -1637,8 +1650,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
16371650
L::Target: Logger,
16381651
{
16391652
let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
1640-
let logger = WithChannelMonitor::from(logger, self);
1641-
self.inner.lock().unwrap().transaction_unconfirmed(
1653+
let mut inner = self.inner.lock().unwrap();
1654+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1655+
inner.transaction_unconfirmed(
16421656
txid, broadcaster, &bounded_fee_estimator, &logger
16431657
);
16441658
}
@@ -1664,8 +1678,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
16641678
L::Target: Logger,
16651679
{
16661680
let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
1667-
let logger = WithChannelMonitor::from(logger, self);
1668-
self.inner.lock().unwrap().best_block_updated(
1681+
let mut inner = self.inner.lock().unwrap();
1682+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
1683+
inner.best_block_updated(
16691684
header, height, broadcaster, &bounded_fee_estimator, &logger
16701685
)
16711686
}
@@ -1703,8 +1718,8 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
17031718
L::Target: Logger,
17041719
{
17051720
let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
1706-
let logger = WithChannelMonitor::from(logger, self);
17071721
let mut inner = self.inner.lock().unwrap();
1722+
let logger = WithChannelMonitor::from_impl(logger, &*inner);
17081723
let current_height = inner.best_block.height;
17091724
inner.onchain_tx_handler.rebroadcast_pending_claims(
17101725
current_height, &broadcaster, &fee_estimator, &logger,

0 commit comments

Comments
 (0)