← Watch feed
Update BestBlock to store ANTI_REORG_DELAY * 2 recent block hashes
Public commit record
85/100 · Strong
What the developer wrote
Authored by Matt Corallo
Update BestBlock to store ANTI_REORG_DELAY * 2 recent block hashes
On restart, LDK expects the chain to be replayed starting from
where it was when objects were last serialized. This is fine in the
normal case, but if there was a reorg and the node which we were
syncing from either resynced or was changed, the last block that we
were synced as of might no longer be available. As a result, it
becomes impossible to figure out where the fork point is, and thus
to replay the chain.
Luckily, changing the block source during a reorg isn't exactly
common, but we shouldn't end up with a bricked node.
To address this, `lightning-block-sync` allows the user to pass in
`Cache` which can be used to cache recent blocks and thus allow for
reorg handling in this case. However, serialization for, and a
reasonable default implementation of a `Cache` was never built.
Instead, here, we start taking a different approach. To avoid
developers having to persist yet another object, we move
`BestBlock` to storing some number of recent block hashes. This
allows us to find the fork point with just the serialized state.
In conjunction with 403dc1a48bb71ae794f6883ae0b760aad44cda39 (which
allows us to disconnect blocks without having the stored header),
this should allow us to replay chain state after a reorg even if
we no longer have access to the top few blocks of the old chain
tip.
While we only really need to store `ANTI_REORG_DELAY` blocks (as we
generally assume that any deeper reorg won't happen and thus we
don't guarantee we handle it correctly), its nice to store a few
more to be able to handle more than a six block reorg. While other
parts of the codebase may not be entirely robust against such a
reorg if the transactions confirmed change out from under us, its
entirely possible (and, indeed, common) for reorgs to contain
nearly identical transactions.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This commit changes how LDK (a Bitcoin Lightning Network library) remembers recent Bitcoin block hashes. Previously, it only remembered the single latest block. After this change, it keeps a small history of the last 12 block hashes. This helps the node recover correctly if the chain reorganizes (a 'reorg') and the block source it was syncing from has changed or resynced. Without this history, the node could get stuck after a restart because it couldn't find the point where the old chain and new chain split. The change is a robustness improvement, not a fix for an active exploit.
Recommended action
Review the reorg handling paths that consume `BestBlock::find_common_ancestor` and `get_hash_at_height` to ensure the new history is used consistently across all chain listeners. Verify that the 12-block history length is sufficient for the intended reorg scenarios and that the serialization defaults do not cause issues when loading old states. Consider whether any documentation or migration notes are needed for downstream users.
Security signals we found
01
Reorg recovery / fork-point resolution
02
Persistence of recent block hashes in serialized state
03
Backward-compatible TLV serialization additions
04
Removal of direct BestBlock::new assignments in favor of update_for_new_tip
05
New serialization helpers for [Option<BlockHash>; 12]
Evidence from the diff
The commit extends the BestBlock struct to store ANTI_REORG_DELAY * 2 (12) previous block hashes in reverse chronological order. It adds methods advance, update_for_new_tip, get_hash_at_height, and find_common_ancestor to manage and query this history. Serialization/deserialization is updated in ChannelMonitor and ChannelManager via new TLV fields (tag 39 and 23 respectively), with backward-compatible defaults of empty history arrays. The lightning-block-sync cache approach is effectively superseded by embedding recent block history directly in persisted state. This enables chain replay after a reorg even when the previous best tip is no longer available from the block source, working in conjunction with commit 403dc1a48bb71ae794f6883ae0b760aad44cda39 which allows disconnecting blocks without stored headers.
Changed components
lightning/src/chain/mod.rs (BestBlock struct and methods)lightning/src/chain/channelmonitor.rs (ChannelMonitor serialization and best block updates)lightning/src/ln/channelmanager.rs (ChannelManager serialization and best block handling)lightning/src/util/ser.rs (Option<BlockHash> array serialization)lightning/src/util/sweep.rs (Sweeper best block updates)Inspect captured patch +181 / −21
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 1eb1484..5a49c39 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -1755,6 +1755,7 @@ pub(crate) fn write_chanmon_internal<Signer: EcdsaChannelSigner, W: Writer>(
(34, channel_monitor.alternative_funding_confirmed, option),
(35, channel_monitor.is_manual_broadcast, required),
(37, channel_monitor.funding_seen_onchain, required),
+ (39, channel_monitor.best_block.previous_blocks, required),
});
Ok(())
@@ -5390,9 +5391,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
&mut self, header: &Header, txdata: &TransactionData, height: u32, broadcaster: B,
fee_estimator: F, logger: &WithContext<L>,
) -> Vec<TransactionOutputs> {
- let block_hash = header.block_hash();
- self.best_block = BestBlock::new(block_hash, height);
-
let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
self.transactions_confirmed(header, txdata, height, broadcaster, &bounded_fee_estimator, logger)
}
@@ -5409,7 +5407,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
let block_hash = header.block_hash();
if height > self.best_block.height {
- self.best_block = BestBlock::new(block_hash, height);
+ self.best_block.update_for_new_tip(block_hash, height);
log_trace!(logger, "Connecting new block {} at height {}", block_hash, height);
self.block_confirmed(height, block_hash, vec![], vec![], vec![], &broadcaster, &fee_estimator, logger)
} else if block_hash != self.best_block.block_hash {
@@ -5683,7 +5681,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
if height > self.best_block.height {
- self.best_block = BestBlock::new(block_hash, height);
+ self.best_block.update_for_new_tip(block_hash, height);
}
if should_broadcast_commitment {
@@ -6644,7 +6642,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
}
}
- let best_block = BestBlock::new(Readable::read(reader)?, Readable::read(reader)?);
+ let mut best_block = BestBlock::new(Readable::read(reader)?, Readable::read(reader)?);
let waiting_threshold_conf_len: u64 = Readable::read(reader)?;
let mut onchain_events_awaiting_threshold_conf = Vec::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128));
@@ -6694,6 +6692,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
let mut alternative_funding_confirmed = None;
let mut is_manual_broadcast = RequiredWrapper(None);
let mut funding_seen_onchain = RequiredWrapper(None);
+ let mut best_block_previous_blocks = None;
read_tlv_fields!(reader, {
(1, funding_spend_confirmed, option),
(3, htlcs_resolved_on_chain, optional_vec),
@@ -6716,7 +6715,12 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
(34, alternative_funding_confirmed, option),
(35, is_manual_broadcast, (default_value, false)),
(37, funding_seen_onchain, (default_value, true)),
+ (39, best_block_previous_blocks, option), // Added and always set in 0.3
});
+ if let Some(previous_blocks) = best_block_previous_blocks {
+ best_block.previous_blocks = previous_blocks;
+ }
+
// Note that `payment_preimages_with_info` was added (and is always written) in LDK 0.1, so
// we can use it to determine if this monitor was last written by LDK 0.1 or later.
let written_by_0_1_or_later = payment_preimages_with_info.is_some();
diff --git a/lightning/src/chain/mod.rs b/lightning/src/chain/mod.rs
index 99e184d..9692558 100644
--- a/lightning/src/chain/mod.rs
+++ b/lightning/src/chain/mod.rs
@@ -18,7 +18,9 @@ use bitcoin::network::Network;
use bitcoin::script::{Script, ScriptBuf};
use bitcoin::secp256k1::PublicKey;
-use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, MonitorEvent};
+use crate::chain::channelmonitor::{
+ ChannelMonitor, ChannelMonitorUpdate, MonitorEvent, ANTI_REORG_DELAY,
+};
use crate::chain::transaction::{OutPoint, TransactionData};
use crate::ln::types::ChannelId;
use crate::sign::ecdsa::EcdsaChannelSigner;
@@ -43,13 +45,20 @@ pub struct BestBlock {
pub block_hash: BlockHash,
/// The height at which the block was confirmed.
pub height: u32,
+ /// Previous blocks immediately before [`Self::block_hash`], in reverse chronological order.
+ ///
+ /// These ensure we can find the fork point of a reorg if our block source no longer has the
+ /// previous best tip after a restart.
+ pub previous_blocks: [Option<BlockHash>; ANTI_REORG_DELAY as usize * 2],
}
impl BestBlock {
/// Constructs a `BestBlock` that represents the genesis block at height 0 of the given
/// network.
pub fn from_network(network: Network) -> Self {
- BestBlock { block_hash: genesis_block(network).header.block_hash(), height: 0 }
+ let block_hash = genesis_block(network).header.block_hash();
+ let previous_blocks = [None; ANTI_REORG_DELAY as usize * 2];
+ BestBlock { block_hash, height: 0, previous_blocks }
}
/// Returns a `BestBlock` as identified by the given block hash and height.
@@ -57,13 +66,88 @@ impl BestBlock {
/// This is not exported to bindings users directly as the bindings auto-generate an
/// equivalent `new`.
pub fn new(block_hash: BlockHash, height: u32) -> Self {
- BestBlock { block_hash, height }
+ let previous_blocks = [None; ANTI_REORG_DELAY as usize * 2];
+ BestBlock { block_hash, height, previous_blocks }
+ }
+
+ /// Advances to a new block at height [`Self::height`] + 1.
+ pub fn advance(&mut self, new_hash: BlockHash) {
+ // Shift all block hashes to the right (making room for the old tip at index 0)
+ for i in (1..self.previous_blocks.len()).rev() {
+ self.previous_blocks[i] = self.previous_blocks[i - 1];
+ }
+
+ // The old tip becomes the new index 0 (tip-1)
+ self.previous_blocks[0] = Some(self.block_hash);
+
+ // Update to the new tip
+ self.block_hash = new_hash;
+ self.height += 1;
+ }
+
+ /// Updates this object for a new best-block, either delegating to [`Self::advance`] if the new
+ /// block is simply one higher than the current tip and wiping [`Self::previous_blocks`] if a
+ /// few blocks have been skipped.
+ pub fn update_for_new_tip(&mut self, new_tip_hash: BlockHash, new_tip_height: u32) {
+ if new_tip_height == self.height + 1 {
+ self.advance(new_tip_hash);
+ } else {
+ *self = BestBlock::new(new_tip_hash, new_tip_height);
+ }
+ }
+
+ /// Returns the block hash at the given height, if available in our history.
+ pub fn get_hash_at_height(&self, height: u32) -> Option<BlockHash> {
+ if height > self.height {
+ return None;
+ }
+ if height == self.height {
+ return Some(self.block_hash);
+ }
+
+ // offset = 1 means we want tip-1, which is block_hashes[0]
+ // offset = 2 means we want tip-2, which is block_hashes[1], etc.
+ let offset = self.height.saturating_sub(height) as usize;
+ if offset >= 1 && offset <= self.previous_blocks.len() {
+ self.previous_blocks[offset - 1]
+ } else {
+ None
+ }
+ }
+
+ /// Find the most recent common ancestor between two BestBlocks by searching their block hash
+ /// histories.
+ ///
+ /// Returns the common block hash and height, or None if no common block is found in the
+ /// available histories.
+ pub fn find_common_ancestor(&self, other: &BestBlock) -> Option<(BlockHash, u32)> {
+ // First check if either tip matches
+ if self.block_hash == other.block_hash && self.height == other.height {
+ return Some((self.block_hash, self.height));
+ }
+
+ // Check all heights covered by self's history
+ let min_height = self.height.saturating_sub(self.previous_blocks.len() as u32);
+ for check_height in (min_height..=self.height).rev() {
+ if let Some(self_hash) = self.get_hash_at_height(check_height) {
+ if let Some(other_hash) = other.get_hash_at_height(check_height) {
+ if self_hash == other_hash {
+ return Some((self_hash, check_height));
+ }
+ }
+ }
+ }
+ None
}
}
impl_writeable_tlv_based!(BestBlock, {
(0, block_hash, required),
+ // Note that any change to the previous_blocks array length will change the serialization
+ // format and thus it is specified without constants here.
+ (1, previous_blocks_read, (legacy, [Option<BlockHash>; 6 * 2], |_| Ok(()), |us: &BestBlock| Some(us.previous_blocks))),
(2, height, required),
+ (unused, previous_blocks, (static_value, previous_blocks_read.unwrap_or([None; 6 * 2]))),
});
/// The `Listen` trait is used to notify when blocks have been connected or disconnected from the
@@ -491,3 +575,45 @@ impl ClaimId {
ClaimId(Sha256::from_engine(engine).to_byte_array())
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use bitcoin::hashes::Hash;
+
+ #[test]
+ fn test_best_block() {
+ let hash1 = BlockHash::from_slice(&[1; 32]).unwrap();
+ let mut chain_a = BestBlock::new(hash1, 100);
+ let mut chain_b = BestBlock::new(hash1, 100);
+
+ // Test get_hash_at_height on initial block
+ assert_eq!(chain_a.get_hash_at_height(100), Some(hash1));
+ assert_eq!(chain_a.get_hash_at_height(101), None);
+ assert_eq!(chain_a.get_hash_at_height(99), None);
+
+ // Test find_common_ancestor with identical blocks
+ assert_eq!(chain_a.find_common_ancestor(&chain_b), Some((hash1, 100)));
+
+ let hash2 = BlockHash::from_slice(&[2; 32]).unwrap();
+ chain_a.advance(hash2);
+ assert_eq!(chain_a.height, 101);
+ assert_eq!(chain_a.block_hash, hash2);
+ assert_eq!(chain_a.previous_blocks[0], Some(hash1));
+ assert_eq!(chain_a.get_hash_at_height(101), Some(hash2));
+ assert_eq!(chain_a.get_hash_at_height(100), Some(hash1));
+
+ // Test find_common_ancestor with different heights
+ assert_eq!(chain_a.find_common_ancestor(&chain_b), Some((hash1, 100)));
+
+ // Test find_common_ancestor with diverged chains but the same height
+ let hash_b3 = BlockHash::from_slice(&[33; 32]).unwrap();
+ chain_b.advance(hash_b3);
+ assert_eq!(chain_a.find_common_ancestor(&chain_b), Some((hash1, 100)));
+
+ // Test find_common_ancestor with no common history
+ let hash_other = BlockHash::from_slice(&[99; 32]).unwrap();
+ let chain_c = BestBlock::new(hash_other, 200);
+ assert_eq!(chain_a.find_common_ancestor(&chain_c), None);
+ }
+}
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index d042a69..d63ccca 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -15887,7 +15887,7 @@ impl<
let _persistence_guard =
PersistenceNotifierGuard::optionally_notify_skipping_background_events(
self, || -> NotifyOption { NotifyOption::DoPersist });
- *self.best_block.write().unwrap() = BestBlock::new(block_hash, height);
+ self.best_block.write().unwrap().update_for_new_tip(block_hash, height);
let mut min_anchor_feerate = None;
let mut min_non_anchor_feerate = None;
@@ -18215,6 +18215,7 @@ impl<
(17, in_flight_monitor_updates, option),
(19, peer_storage_dir, optional_vec),
(21, WithoutLength(&self.flow.writeable_async_receive_offer_cache()), required),
+ (23, self.best_block.read().unwrap().previous_blocks, required),
});
// Remove the SpliceFailed and DiscardFunding events added earlier.
@@ -18284,8 +18285,7 @@ impl Readable for AmountlessClaimablePaymentHTLCOnion {
// This is an internal DTO used in the two-stage deserialization process.
pub(super) struct ChannelManagerData<SP: SignerProvider> {
chain_hash: ChainHash,
- best_block_height: u32,
- best_block_hash: BlockHash,
+ best_block: BestBlock,
channels: Vec<FundedChannel<SP>>,
claimable_payments: HashMap<PaymentHash, ClaimablePayment>,
peer_init_features: Vec<(PublicKey, InitFeatures)>,
@@ -18493,6 +18493,7 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger>
let mut inbound_payment_id_secret = None;
let mut peer_storage_dir: Option<Vec<(PublicKey, Vec<u8>)>> = None;
let mut async_receive_offer_cache: AsyncReceiveOfferCache = AsyncReceiveOfferCache::new();
+ let mut best_block_previous_blocks = None;
read_tlv_fields!(reader, {
(1, pending_outbound_payments_no_retry, option),
(2, pending_intercepted_htlcs_legacy, option),
@@ -18511,6 +18512,7 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger>
(17, in_flight_monitor_updates, option),
(19, peer_storage_dir, optional_vec),
(21, async_receive_offer_cache, (default_value, async_receive_offer_cache)),
+ (23, best_block_previous_blocks, option),
});
// Merge legacy pending_outbound_payments fields into a single HashMap.
@@ -18605,8 +18607,11 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger>
Ok(ChannelManagerData {
chain_hash,
- best_block_height,
- best_block_hash,
+ best_block: BestBlock {
+ block_hash: best_block_hash,
+ height: best_block_height,
+ previous_blocks: best_block_previous_blocks.unwrap_or([None; 12]),
+ },
channels,
forward_htlcs_legacy,
claimable_payments,
@@ -18910,8 +18915,7 @@ impl<
) -> Result<(BlockHash, Self), DecodeError> {
let ChannelManagerData {
chain_hash,
- best_block_height,
- best_block_hash,
+ best_block,
channels,
mut forward_htlcs_legacy,
claimable_payments,
@@ -19596,7 +19600,7 @@ impl<
htlc.payment_hash,
session_priv_bytes,
&path,
- best_block_height,
+ best_block.height,
&logger,
);
}
@@ -19917,7 +19921,7 @@ impl<
loop {
outbound_scid_alias = fake_scid::Namespace::OutboundAlias
.get_fake_scid(
- best_block_height,
+ best_block.height,
&chain_hash,
fake_scid_rand_bytes.as_ref().unwrap(),
&args.entropy_source,
@@ -20119,7 +20123,6 @@ impl<
}
}
- let best_block = BestBlock::new(best_block_hash, best_block_height);
let flow = OffersMessageFlow::new(
chain_hash,
best_block,
@@ -20531,7 +20534,7 @@ impl<
//TODO: Broadcast channel update for closed channels, but only after we've made a
//connection or two.
- Ok((best_block_hash, channel_manager))
+ Ok((best_block.block_hash, channel_manager))
}
}
diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs
index 7d0acac..2b02629 100644
--- a/lightning/src/util/ser.rs
+++ b/lightning/src/util/ser.rs
@@ -1439,6 +1439,33 @@ impl Readable for BlockHash {
}
}
+impl Writeable for [Option<BlockHash>; 12] {
+ fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+ for hash_opt in self {
+ match hash_opt {
+ Some(hash) => hash.write(w)?,
+ None => ([0u8; 32]).write(w)?,
+ }
+ }
+ Ok(())
+ }
+}
+
+impl Readable for [Option<BlockHash>; 12] {
+ fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+ use bitcoin::hashes::Hash;
+
+ let mut res = [None; 12];
+ for hash_opt in res.iter_mut() {
+ let buf: [u8; 32] = Readable::read(r)?;
+ if buf != [0; 32] {
+ *hash_opt = Some(BlockHash::from_slice(&buf[..]).unwrap());
+ }
+ }
+ Ok(res)
+ }
+}
+
impl Writeable for ChainHash {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
w.write_all(self.as_bytes())
diff --git a/lightning/src/util/sweep.rs b/lightning/src/util/sweep.rs
index b70eb27..bbaaf29 100644
--- a/lightning/src/util/sweep.rs
+++ b/lightning/src/util/sweep.rs
@@ -734,7 +734,7 @@ where
fn best_block_updated_internal(
&self, sweeper_state: &mut SweeperState, header: &Header, height: u32,
) {
- sweeper_state.best_block = BestBlock::new(header.block_hash(), height);
+ sweeper_state.best_block.update_for_new_tip(header.block_hash(), height);
self.prune_confirmed_outputs(sweeper_state);
sweeper_state.dirty = true;
Risk score
Our methodology →Why this scored 46/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.