Skip to content

execution/commitment: compact the pbin branch record and convert pre-version datadirs - #23564

Open
awskii wants to merge 33 commits into
binary-triefrom
awskii/pbin-record-compaction
Open

execution/commitment: compact the pbin branch record and convert pre-version datadirs#23564
awskii wants to merge 33 commits into
binary-triefrom
awskii/pbin-record-compaction

Conversation

@awskii

@awskii awskii commented Aug 25, 2026

Copy link
Copy Markdown
Member

Drops the touchMap/afterMap header and the per-field lengths from every pbin branch record,
omits the prefix on storage leaves, and versions the trie state blob. integration commitment convert-format rewrites a pre-version datadir into a separate output datadir; the source is
hardlinked in and never opened for writing.

snap-arb1, mainnet PBT datadir, 7 shards, 3,537,594,455 records, 2h37m:
commitment .kv 430,372,793,732 -> 369,063,989,152 B — 57.09 GiB, -14.2%
(frozen 0-8192 -14.9%; the three newest shards -22% to -24%)

integration commitment convert-format \
  --datadir=<src> --output.datadir=<out> \
  --chaindata=<any existing db: a snapshot-only datadir has none> \
  --chain=mainnet --verify.sample=1000

awskii added 23 commits August 23, 2026 12:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Compacts binary-trie commitment records, versions trie-state blobs, and adds an offline legacy-datadir converter.

Changes:

  • Removes redundant branch headers, field lengths, and storage prefixes.
  • Adds state-format validation and legacy decoding.
  • Adds staged conversion with integrity checks and resume support.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
execution/commitment/pbin_witness_context.go Uses the new cell encoder.
execution/commitment/pbin_verify_test.go Adapts verifier decoding.
execution/commitment/pbin_unfold_test.go Updates record fixtures.
execution/commitment/pbin_state.go Versions state blobs.
execution/commitment/pbin_state_test.go Tests state versioning.
execution/commitment/pbin_patricia_hashed.go Uses compact record decoding.
execution/commitment/pbin_fold_test.go Tests compact folding behavior.
execution/commitment/pbin_convert_legacy.go Implements legacy conversion.
execution/commitment/pbin_convert_legacy_test.go Tests legacy conversion.
execution/commitment/pbin_cell_test.go Expands codec validation tests.
execution/commitment/pbin_branch.go Implements compact branch encoding.
db/state/squeeze.go Validates rebuild state format.
db/state/rebuild_variant_test.go Tests legacy-state rejection.
db/state/rebuild_pbin_state_test.go Tests rebuild validation.
db/state/commitment_convert_pbin.go Converts commitment shards.
db/state/commitment_convert_pbin_test.go Tests conversion and recovery.
db/state/commitment_convert_pbin_e2e_test.go Adds end-to-end conversion coverage.
db/state/commitment_convert_export_test.go Exposes test hooks.
db/seg/decompress.go Documents platform-specific field use.
cmd/integration/Readme.md Documents conversion usage.
cmd/integration/commands/flags.go Adds converter flags.
cmd/integration/commands/commitment.go Adds the conversion command and staging.
cmd/integration/commands/commitment_output_test.go Tests output staging safeguards.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cmd/integration/commands/commitment.go Outdated
Comment thread cmd/integration/commands/commitment.go Outdated
Comment thread execution/commitment/pbin_branch.go
Comment thread db/state/commitment_convert_pbin.go
Comment thread db/state/squeeze.go Outdated

@yperbasis yperbasis left a comment

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.

Reviewed head: 9086cc8eea61799d6c5a023a6a8b6112c866cc6b

Requesting changes for three migration correctness issues:

  • [P1] Convert the reserved root record. convertPBinFile copies key 0x08 because legacy detection only recognizes the zero-leading branch header. Legacy root cells also contain per-field lengths, so a legacy hashed root such as 12 00 20 <hash> is unreadable by the new decoder, which expects 12 00 <hash>. The legacy test fixture keeps this record in the current format, masking the failure.

  • [P1] Migrate or reject commitment history. linkCommitmentSnapshots hardlinks commitment history files unchanged, while conversion only rewrites domain .kv files. On history-enabled datadirs, GetAsOf will return legacy pbin values to the new-only decoder.

  • [P2] Keep stateValue valid through validation. RebuildCommitmentFiles rolls roTx back before calling validatePBinRebuildState(stateValue). KV read slices are valid only for the transaction lifetime. Validate before rollback or clone the value.

Also update docs/pbin-encoding.md, which still specifies the retired header and length fields.

@awskii
awskii requested a review from bloxster as a code owner August 26, 2026 06:38
@awskii

awskii commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

All three confirmed against the tree, fixed in 52c5073.

Root record. Confirmed, and it had already shipped — checked the converted mainnet PBT datadir before touching anything, shard 9524-9526:

source     key=08 vlen=35 v=12 00 20 ac1aad1f...
converted  key=08 vlen=35 v=12 00 20 ac1aad1f...

Your reading of the shape is exact: 12 00 20 <hash> where pbinDecodeCell wants 12 00 <hash>. All seven shards, so that output datadir has to be rebuilt.

ConvertRootRecord decodes the legacy bare cell, re-encodes it and verifies the round-trip. Detection is the key plus PBinRootRecordIsLegacy — decode with the current decoder, call it legacy unless the body ends exactly at len(data). Every field is fixed-width now, so a legacy record always leaves its length bytes over; that also stops a resumed run re-converting an already-current file. The fixture spells the root through PBinEncodeLegacyRootRecord, and the e2e assertion moved from "the root is copied verbatim" to "the root is not legacy in the output".

Commitment history. Confirmed. I took the reject half rather than the migrate half: requireConvertFormatSource refuses a source carrying commitment files under history/, idx/ or accessor/. Migrating them means rewriting page-compressed .v values and rebuilding the .vi offsets — a larger change than this PR should carry, and one that would ship without a corpus to test it against, since the datadirs this command exists for are built without --keep.execution.proofs. If we want history conversion it deserves its own PR.

stateValue past the rollback. Confirmed, validation now runs before roTx.Rollback(). Separately the validator accepted trailing bytes and skipped the check entirely when stateLen == 0; the length test is now len(stateValue) == 18+stateLen and runs first.

docs/pbin-encoding.md. Rewritten — 5.2-5.6 and 7, against bytes produced by running the documented legacy records through the converter rather than re-derived by hand. Corpus sizes drop to 88 / 136 / 88 / 44 plus a 34-byte root, 5.5 keeps a was column, 5.4 becomes "Why there is no header", and a new 7.1 documents the state blob envelope — the only pbin structure with a version byte, which is what the root record lacks and why this had no self-describing tell.

awskii added 3 commits August 26, 2026 21:32
…uildFiles can return with mergeLoop still running
…ommitment-only trim rewrites files Windows has open
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.

3 participants