Skip to content

feat(hot)!: add table for journal hashes#70

Open
Fraser999 wants to merge 1 commit into
mainfrom
fraser/eng-2017/journal-hashes
Open

feat(hot)!: add table for journal hashes#70
Fraser999 wants to merge 1 commit into
mainfrom
fraser/eng-2017/journal-hashes

Conversation

@Fraser999
Copy link
Copy Markdown
Contributor

@Fraser999 Fraser999 commented May 26, 2026

Summary

Adds a JournalHashes<BlockNumber => B256> hot table so nodes can re-seed the rolling previous-journal hash across restarts and reorgs (ENG-2017). Hot-only, opt-in per block.

Changes

  • New table JournalHashes<BlockNumber => B256> plus HotDbRead::get_journal_hash and UnsafeDbWrite::put_journal_hash.
  • ExecutedBlock.journal_hash: Option<B256> + builder setter. UnifiedStorage::append_blocks persists it in the same hot transaction as headers/bundle.
  • HistoryWrite::unwind_above deletes journal hashes alongside change-sets/headers, independent of last_block_number().
  • New STANDARD_TABLES const in signet-hot as the single source of truth for table creation and MDBX FSI cache seeding. queue_db_init and read_known_fsi both iterate it.
  • Pre-upgrade DBs: read_known_fsi falls back to compile-time-expected FSI (via FixedSizeInfo::from_create_args) when an on-disk entry is missing, so RO/RW opens against an older DB succeed.
  • Fix: unwind_above(u64::MAX) now short-circuits via checked_add instead of overflowing block + 1 (wraps to 0 in release, panics in debug).
  • From<ExecutedBlock> for BlockData destructures explicitly so new fields trip a compile error on the cold side.

API breakages

  • ExecutedBlock::new() removed - use ExecutedBlockBuilder instead.
  • ExecutedBlock gains a new public journal_hash field. Struct-literal construction (ExecutedBlock { header, .. }) and exhaustive destructuring without .. will no longer compile.

Test plan

  • cargo t -p signet-hot -p signet-hot-mdbx -p signet-storage --all-features
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo clippy --workspace --all-targets --no-default-features -- -D warnings
  • cargo +nightly fmt -- --check
  • New tests: test_journal_hash_roundtrip (conformance), open_tolerates_pre_upgrade_db (FSI fallback), append_blocks_persists_journal_hashes / unwind_above_drops_journal_hashes (integration), unwind_above_u64_max_is_noop / unwind_above_below_u64_max_deletes_max_entry (overflow boundary). test_unwind_conformance updated to cover JournalHashes.

Notes

  • put_journal_hash has no consistent-trait wrapper; callers are responsible for pairing writes with a header. unwind_above cleans both regardless.
  • replay_to_cold silently discards journal_hash (hot-only).

PR description drafted by Claude (Opus 4.7, 1M context) via Claude Code.

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Fraser999 Fraser999 changed the title add table to hot db for journal hashes feat(hot)!: add table for journal hashes May 26, 2026
@Fraser999 Fraser999 requested a review from prestwich May 26, 2026 11:45
Copy link
Copy Markdown
Member

@prestwich prestwich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main thing i think is

table = table.name,
?expected,
"FSI metadata entry missing for known table; falling back to compile-time \
expected value. Fires once per open per missing table until a RW open \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this info belongs in a comment? 🤔

                     creates the table and persists its FSI; RO-only consumers of a \
                     pre-upgrade database will see this on every open."```

let mut known = [("", FixedSizeInfo::None); NUM_TABLES];
for (i, &name) in KNOWN_TABLE_NAMES.iter().enumerate() {
known[i] = (name, tx.read_fsi_from_table(name)?);
for (index, table) in STANDARD_TABLES.iter().enumerate() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could rewrite this loop as a try_for_each() ?

Comment thread crates/hot-mdbx/src/tx.rs
/// Removes a table from the environment as if it had never been created:
/// drops the named sub-database and erases its FSI metadata entry. Used
/// by tests to simulate a database written by an older binary that
/// pre-dates the table. Does not invalidate the in-memory `FsiCache`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we still control all significant running copies of the binaries, so we could choose not to implement migration logic/tests here. worth discussing whether we need this complexity yet

/// Set the journal hash (keccak256 of the wire-encoded `Journal::V1`).
/// Leave the method uncalled for block-only nodes that do not produce
/// a journal - the field defaults to `None`.
pub const fn journal_hash(mut self, hash: B256) -> Self {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: usually with_journal_hash or some other verb that indicates mutating

/// [`queue_raw_create`](crate::model::HotKvWrite::queue_raw_create) so the
/// same record drives both table creation and backend-side FSI inference.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct StandardTable {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind this complexity addition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants