Don't pass a latest-block-time to `Channel` unless we have one
What changed, and why it matters
This commit is a code-cleanup change in a Bitcoin Lightning Network library. It makes the 'latest block timestamp' parameter optional so that callers only provide a timestamp when they actually know it, rather than passing a placeholder or stale value. The underlying function already ignored stale values, so behavior is essentially unchanged. There is no direct evidence this fixes an exploitable security bug.
Treat as a maintainability and API-clarity improvement. No urgent security action is indicated by the commit itself. If this commit is part of a larger release, review the release notes for any related security advisory, but the diff alone does not warrant an incident response.
Security signals we found
API contract clarification for timestamp handling
Avoids passing placeholder/stale timestamps during reorgs
No new bounds checks or cryptographic fixes
Behavioral change is minimal because max() previously ignored stale values
Evidence from the diff
The patch changes Channel::best_block_updated and its helper do_best_block_updated to accept highest_header_time as Option
Changed components
lightning/src/ln/channel.rslightning/src/ln/channelmanager.rsChannel::best_block_updatedChannel::do_best_block_updatedChannel update_time_counter trackingInspect captured patch +28 / −12
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 3d25934..95f7683 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -10550,8 +10550,8 @@ where
/// May return some HTLCs (and their payment_hash) which have timed out and should be failed
/// back.
pub fn best_block_updated<NS: Deref, L: Deref>(
- &mut self, height: u32, highest_header_time: u32, chain_hash: ChainHash, node_signer: &NS,
- user_config: &UserConfig, logger: &L,
+ &mut self, height: u32, highest_header_time: Option<u32>, chain_hash: ChainHash,
+ node_signer: &NS, user_config: &UserConfig, logger: &L,
) -> Result<BestBlockUpdatedRes, ClosureReason>
where
NS::Target: NodeSigner,
@@ -10567,7 +10567,7 @@ where
#[rustfmt::skip]
fn do_best_block_updated<NS: Deref, L: Deref>(
- &mut self, height: u32, highest_header_time: u32,
+ &mut self, height: u32, highest_header_time: Option<u32>,
chain_node_signer: Option<(ChainHash, &NS, &UserConfig)>, logger: &L
) -> Result<(Option<FundingConfirmedMessage>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), ClosureReason>
where
@@ -10591,7 +10591,9 @@ where
}
});
- self.context.update_time_counter = cmp::max(self.context.update_time_counter, highest_header_time);
+ if let Some(time) = highest_header_time {
+ self.context.update_time_counter = cmp::max(self.context.update_time_counter, time);
+ }
// Check if the funding transaction was unconfirmed
let funding_tx_confirmations = self.funding.get_funding_tx_confirmations(height);
@@ -10747,12 +10749,9 @@ where
// We handle the funding disconnection by calling best_block_updated with a height one
// below where our funding was connected, implying a reorg back to conf_height - 1.
let reorg_height = funding.funding_tx_confirmation_height - 1;
- // We use the time field to bump the current time we set on channel updates if its
- // larger. If we don't know that time has moved forward, we can just set it to the last
- // time we saw and it will be ignored.
- let best_time = self.context.update_time_counter;
- match self.do_best_block_updated(reorg_height, best_time, None::<(ChainHash, &&dyn NodeSigner, &UserConfig)>, logger) {
+ let signer_config = None::<(ChainHash, &&dyn NodeSigner, &UserConfig)>;
+ match self.do_best_block_updated(reorg_height, None, signer_config, logger) {
Ok((channel_ready, timed_out_htlcs, announcement_sigs)) => {
assert!(channel_ready.is_none(), "We can't generate a funding with 0 confirmations?");
assert!(timed_out_htlcs.is_empty(), "We can't have accepted HTLCs with a timeout before our funding confirmation?");
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 78bd2fa..6c8b742 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -13302,7 +13302,7 @@ where
self.do_chain_event(Some(fork_point.height), |channel| {
channel.best_block_updated(
fork_point.height,
- 0,
+ None,
self.chain_hash,
&self.node_signer,
&self.config.read().unwrap(),
@@ -13352,7 +13352,17 @@ where
let last_best_block_height = self.best_block.read().unwrap().height;
if height < last_best_block_height {
let timestamp = self.highest_seen_timestamp.load(Ordering::Acquire);
- self.do_chain_event(Some(last_best_block_height), |channel| channel.best_block_updated(last_best_block_height, timestamp as u32, self.chain_hash, &self.node_signer, &self.config.read().unwrap(), &&WithChannelContext::from(&self.logger, &channel.context, None)));
+ let do_update = |channel: &mut FundedChannel<SP>| {
+ channel.best_block_updated(
+ last_best_block_height,
+ Some(timestamp as u32),
+ self.chain_hash,
+ &self.node_signer,
+ &self.config.read().unwrap(),
+ &&WithChannelContext::from(&self.logger, &channel.context, None),
+ )
+ };
+ self.do_chain_event(Some(last_best_block_height), do_update);
}
}
@@ -13412,7 +13422,14 @@ where
}
}
- channel.best_block_updated(height, header.time, self.chain_hash, &self.node_signer, &self.config.read().unwrap(), &&WithChannelContext::from(&self.logger, &channel.context, None))
+ channel.best_block_updated(
+ height,
+ Some(header.time),
+ self.chain_hash,
+ &self.node_signer,
+ &self.config.read().unwrap(),
+ &&WithChannelContext::from(&self.logger, &channel.context, None),
+ )
});
macro_rules! max_time {
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.