Merge confirmed_log and preprocessed_blocks into block_hashes#6163
Conversation
Replaces `confirmed_log: LogView<CryptoHash>` and `preprocessed_blocks: MapView<BlockHeight, CryptoHash>` with a single `block_hashes: CustomMapView<BlockHeight, CryptoHash>` that holds executed blocks (height < next_block_height) and preprocessed blocks (height >= next_block_height) together. This removes the dense/sparse partition spread across multiple call sites and prepares for checkpointing, where the executed range will no longer start at zero. Adds a big-endian `CustomSerialize` impl for `BlockHeight` so that lexicographic key order matches numeric order; this also fixes a latent bug in `next_height_to_preprocess`, which relied on `indices().last()` returning the numerically highest height. Collapses three near-duplicate `WorkerError` variants (`ConfirmedLogEntryNotFound`, `PreprocessedBlocksEntryNotFound`, `ConfirmedBlockHashNotFound`) into a single `BlockHashNotFound`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Instruction Count Benchmark Results
Deterministic metrics — reproducible across runs (34 benchmarks)
Cache-dependent metrics — expect fluctuations between runs (34 benchmarks)
|
Update the GraphQL query to fetch (height, hash) pairs from `blockHashes`, and rewrite the "Confirmed Log" section in Chain.vue to iterate the entries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Can you add a test for this, similar to the on in #6162 ? |
Ports the regression test from linera-io#6162 to the merged-`block_hashes` design: heights 1 and 256 span a byte boundary, so a naive `indices().last()` under little-endian encoding would return the wrong height. With the big-endian `CustomSerialize` impl, `next_height_to_preprocess` returns the numeric maximum. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| let next_block_height = self.tip_state.get().next_block_height; | ||
| if let Some(height) = self.block_hashes.indices().await?.last() { | ||
| if *height >= next_block_height { | ||
| return Ok(height.saturating_add(BlockHeight(1))); | ||
| } |
There was a problem hiding this comment.
For chains with many executed blocks and no preprocessed ones, this is an extra cost on every call. Probably negligible since indices().last() on a sorted map should be O(log n), but worth confirming the storage backend doesn't first load the full list.
There was a problem hiding this comment.
.indices() most certainly loads the full list
There was a problem hiding this comment.
Good catch! We'd need a last_key method, I guess.
There was a problem hiding this comment.
I added a field to memoize the max height.
Later we could do something like #6171, and remove that field.
Replaces the O(N) `block_hashes.indices().last()` scan with a direct read from a new `next_height_to_preprocess: RegisterView<C, BlockHeight>` field. The register is updated on every `block_hashes.insert(..)` in `confirm_block` and `preprocess_block`, so it always reflects one past the highest height known to this chain (executed or preprocessed). Callers read the register directly (`*chain.next_height_to_preprocess.get()`), which lets us drop the asynchronous wrapper method. This is a temporary shortcut: once `linera-views` exposes an efficient `last_index` lookup on `CustomMapView`, the field can be removed and the value computed from `block_hashes` directly. The field's docstring records that future cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both call sites (`confirm_block`, `preprocess_block`) used to call `self.block_hashes.insert(...)` and then separately update `next_height_to_preprocess`. Wrapping the pair in a single `insert_block_hash(height, hash)` helper makes the invariant local — every write to `block_hashes` keeps the register in sync — and a future third write site can't accidentally skip the update. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| /// `height < next_block_height` is executed; a block at `height >= next_block_height` | ||
| /// is preprocessed (verified but not yet executed) and may not be contiguous. |
There was a problem hiding this comment.
(maybe related) Is this still the case that we eagerly execute sender blocks if they happen to be contiguous?
Motivation
We are planning to implement checkpointing for chains, which will mean that even the executed part of a chain will not necessarily start at block 0. This makes the artificial distinction between
confirmed_logandpreprocessed_blockseven more complicated, since the former now has an offset, i.e. index is not equal to block height anymore.Proposal
Replace
confirmed_log: LogView<CryptoHash>andpreprocessed_blocks: MapView<BlockHeight, CryptoHash>with a singleblock_hashes: CustomMapView<BlockHeight, CryptoHash>that holds executed blocks (height < next_block_height) and preprocessed blocks (height >= next_block_height) together.Add a big-endian
CustomSerializeimpl forBlockHeightso that lexicographic key order matches numeric order; this also fixes a latent bug innext_height_to_preprocess, which relied onindices().last()returning the numerically highest height. (Fixed by #6162 ontestnet_conway.)Collapse three near-duplicate
WorkerErrorvariants (ConfirmedLogEntryNotFound,PreprocessedBlocksEntryNotFound,ConfirmedBlockHashNotFound) into a singleBlockHashNotFound.Test Plan
CI should catch any regressions.
Release Plan
Links
confirmed_logandpreprocessed_blocks. #5069.