[NazariiDenha]

PR Type

Enhancement, Tests

---

Description

• Introduce support for pruning history in both deprecated and new-state layouts.

• Refactor history pruning migration into discrete, parallelizable pipeline steps (stage, restore history, restore lookups).

• Update the intermediate state persistence encoding to track multi-step resume points efficiently.

• Extend test suites to validate pruning and migration workflows against both state layouts.

---

File Walkthrough

	Relevant files
Miscellaneous	2 files copy.go Remove explicit copyScratch logic                                                +0/-115  keys.go Remove hardcoded scratch key fill functions                            +0/-149
Enhancement	5 files history.go Add staging and restoring logic for history scratch space +211/-0  layout.go Add layout detection and iterator seek logic for history  +134/-0  migrator.go Refactor migrator steps and intermediate state encoding    +183/-230 restorer.go Simplify restorer to process hash lookups instead of copying history +6/-18    stager.go Update stager to support parallel key range tasks                +70/-41
Tests	4 files migrator_test.go Update migration tests to cover multiple layouts                  +181/-118 accessors_test.go Add assertions for state history pruning in accessors tests +1/-0      pruner_test.go Verify pruned state history in rolling pruner tests            +2/-0      helpers.go Expand test utilities to generate mock blocks for different layouts +179/-78
Formatting	2 files counter.go Rename package to progresslogger                                                  +1/-1      counter_test.go Rename package to progresslogger in tests                                +1/-1
Dependencies	2 files committer.go Update imports and references for progresslogger counter  +3/-2      migrator.go Update counter initialization with progresslogger package +2/-1

---

[github-actions]

Claude is reviewing this PR

• Gather context (read CLAUDE.md, diff, changed files)
• Understand the request (PR review)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

View job run

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit f253a53)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

⚠️ Risk level: High

📂 Priority files migration/historyprunner/migrator.go migration/historyprunner/history.go

🏅 Score: 85

🧪 PR contains tests

🔒 No security concerns identified

[github-actions]

Claude finished @NazariiDenha's task in 21m 57s —— View job

---

Claude finished reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Understand the request (PR review)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Summary

Reviewed the diff between origin/fix/newstate-nits-bugs and HEAD (9 files, +571/-55). Posted 3 inline comments.

Blocker

• node/node_test.go:207-213 — TestNew_RejectsPruneWithNewState still asserts node.New rejects Prune=true+NewState=true with "--prune-mode and --new-state are mutually exclusive". This PR deletes exactly that check from node/node.go, since allowing the combination is the point of the PR. That test wasn't touched here and will now fail. (I couldn't run go test in this sandbox due to tool permissions, but go build ./node/... succeeds and the check is verifiably gone from node.go while the test is unchanged — this isn't a guess.) Note: I couldn't post this inline since node_test.go isn't part of this PR's diff.

Important

• migration/historyprunner/newstate.go:124 (restoreNewStateHistory) — no ctx parameter or cancellation check, unlike stageBucket's ctxCheckInterval pattern in the same file. On a large history this blocks graceful shutdown until the full restore finishes. Agreeing with the earlier automated review comment on this point.
• pruner/testutils/helpers.go — the new StoreNewStateBlock / NewStateHistoryAnswers test helpers are added but never called by any test in the repo. Correspondingly, the actual new logic this PR introduces — pruneNewStateHistoryFromUpdate/supersededPruner (pruner/accessors.go), stageNewStateHistory/restoreNewStateHistory (migration/historyprunner/newstate.go), and requireNewStateBeforePruning (node/migration.go) — has no test coverage in this diff, despite the PR description and PR-Agent summary both saying "PR contains tests." Given the PR title is "NOT READY YET," this is presumably known, but worth calling out explicitly since the supersededPruner.prune Seek/Prev boundary logic is subtle enough to want a direct test.

Correction to an earlier automated comment

• node/migration.go:125 — the earlier bot comment flagged a possible nil dereference on metadata.CurrentVersion if GetSchemaMetadata returns db.ErrKeyNotFound. This isn't an issue: SchemaMetadata is a plain struct (not a pointer), so on that error path metadata is just its zero value, and the applied() closure correctly treats a zero SchemaVersion as "nothing applied."

Nit / no action needed

• The migration-ordering guard logic in requireNewStateBeforePruning and the refactor to build registry once and reuse it for both the guard check and migration.NewRunner both look correct — verified registry.OptionalMigrationFlags()/SchemaVersion.Has semantics against migration/registry.go and migration/version.go.
• pruneNewStateHistoryFromUpdate / supersededPruner logic (delete the pre-block entry, keep the post-block entry since new-state reads take "latest at or before") matches the documented semantics in migration/historyprunner/newstate.go's comments and looks correct by inspection, modulo the missing test coverage noted above.

Everything else (CLI flag changes in cmd/juno/juno.go/node/node.go, pruner/pruner.go's WithNewState option, restorer.go's skip-deprecated-copy change) is straightforward plumbing consistent with the stated goal.
· branch feat/newstate-pruning → fix/newstate-nits-bugs

[github-actions]

Persistent review updated to latest commit a133c29

[github-actions]

Claude finished @NazariiDenha's task in 7m 42s —— View job

---

Claude finished reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Understand the request (PR review of latest changes vs origin/fix/newstate-nits-bugs)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Summary

Reviewed the diff between origin/fix/newstate-nits-bugs and HEAD (commit 9120bc450, 13 files, +700/-77). go build ./pruner/... ./migration/... ./node/... succeeds. I could not run go test in this sandbox (tool permission denied), so test coverage was verified by reading the test files, not by execution.

This commit addresses most of what the earlier automated reviews flagged:

• Fixed — stale test removed. node/node_test.go's TestNew_RejectsPruneWithNewState (previously flagged as a guaranteed failure) is deleted in this commit, consistent with node/node.go no longer rejecting Prune+NewState.
• Fixed — test coverage added. pruner/accessors_test.go:TestPruneUpto_NewStateHistory and migration/historyprunner/migrator_test.go:TestMigrate_NewState now exercise pruneNewStateHistoryFromUpdate/supersededPruner and stageNewStateHistory/restoreNewStateHistory end-to-end via the new testutils.StoreNewStateBlock/NewStateHistoryAnswers helpers, including a re-run-is-idempotent check. The supersededPruner.prune Seek/Prev boundary logic looks correct by inspection and is now backed by a test.
• Correction confirmed. metadata in node/migration.go:115 (migration.GetSchemaMetadata) is a value type, not a pointer — the earlier "nil dereference" finding doesn't apply; on ErrKeyNotFound it's just the zero value and applied() correctly treats it as "nothing applied."
• Batch "resource leak" findings revisited. Re-checked db.Batch implementations (db/pebble/batch.go:150, db/pebblev2/batch.go:150, db/memory/batch.go:183): Write() calls Close() internally on all backends before returning. So the batch-rotation pattern in newstate.go's scratchWriter.put and restoreNewStateHistory (write, then allocate a fresh batch without an explicit Close()) does not leak — the old batch is already closed by Write(). The earlier bot comments flagging this as a leak appear to be false positives.

Still open (not addressed in this commit):

• Important — restoreNewStateHistory (migration/historyprunner/newstate.go:124) still takes no context.Context and never checks cancellation, unlike stageBucket's ctxCheckInterval pattern in the same file. On a large new-state history this blocks graceful shutdown until the full restore finishes.
• Nit — requireNewStateBeforePruning (node/migration.go:108) still has no direct test coverage (confirmed via node/migration_test.go, which only covers fetchL1HeadIfMissing/migrateIfNeeded's remote-DB and L1-fetch paths). Given the PR title is "NOT READY YET" and the author already flagged a related follow-up (pruned-db migration support) in review, this is likely known, but the guard's own branches (no chain yet / new-state already applied / prune-mode already applied / neither applied) are cheap to unit test directly and aren't covered yet.

Everything else — the cmd/juno/juno.go flag-usage text updates, node/node.go's plumbing of pruner.WithNewState(cfg.NewState), and pruner/pruner.go's new newState field/option — is straightforward and consistent with removing the --prune-mode/--new-state mutual exclusion.
· branch feat/newstate-pruning → fix/newstate-nits-bugs

[codecov]

Codecov Report

❌ Patch coverage is 87.67908% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.78%. Comparing base (d308f99) to head (f253a53).

Files with missing lines	Patch %	Lines
migration/historyprunner/migrator.go	83.33%	15 Missing ⚠️
migration/historyprunner/history.go	83.90%	14 Missing ⚠️
migration/historyprunner/layout.go	84.61%	8 Missing ⚠️
migration/historyprunner/stager.go	85.71%	5 Missing ⚠️
migration/historyprunner/restorer.go	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4147      +/-   ##
==========================================
- Coverage   80.87%   80.78%   -0.10%     
==========================================
  Files         509      509              
  Lines       39062    39102      +40     
==========================================
- Hits        31593    31589       -4     
- Misses       7460     7507      +47     
+ Partials        9        6       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Claude finished @NazariiDenha's task in 6m 28s —— View job

---

Claude finished reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Understand the request (PR review of latest commit c0d16bda5)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Summary

Reviewed the diff between origin/main (5464fa082) and HEAD (c0d16bda5, 13 files, +700/-77 — this is the full PR, one commit). go build ./pruner/... ./migration/... ./node/... and go vet both succeed. I couldn't run go test in this sandbox (tool permission denied for test execution), so correctness of the new tests was verified by reading them, not by executing them.

This commit resolves everything flagged by the two earlier automated review rounds:

• Fixed. node/node_test.go's stale TestNew_RejectsPruneWithNewState is deleted, consistent with node/node.go no longer rejecting Prune+NewState.
• Fixed — test coverage added. pruner/accessors_test.go:TestPruneUpto_NewStateHistory and migration/historyprunner/migrator_test.go:TestMigrate_NewState now exercise pruneNewStateHistoryFromUpdate/supersededPruner and stageNewStateHistory/restoreNewStateHistory end-to-end (including a "re-run is idempotent" check and scratch-space-wiped assertion), via the new testutils.StoreNewStateBlock/NewStateHistoryAnswers helpers, which are now actually called. supersededPruner.prune's Seek/Prev boundary logic looks correct by inspection and is now backed by a test.
• False positives retracted. The earlier "resource leak" findings on scratchWriter.put and restoreNewStateHistory's batch rotation don't hold: batch.Write() calls Close() internally on every backend (db/pebble/batch.go:150, db/pebblev2/batch.go:150, db/memory/batch.go:183), so allocating a fresh batch right after Write() doesn't leak the old one. Likewise the "nil dereference" concern on node/migration.go:125 doesn't apply — migration.SchemaMetadata is a value type, so GetSchemaMetadata's ErrKeyNotFound path just returns the zero value, which applied() correctly treats as "nothing applied."

Still open (posted inline):

• Important — restoreNewStateHistory (migration/historyprunner/newstate.go:124) still takes no context.Context and never checks cancellation, unlike stageBucket's pattern in the same file. On a large new-state history this blocks graceful shutdown until the full restore completes. This has been flagged in both prior review rounds and remains unaddressed.

Nit / no action needed:

• requireNewStateBeforePruning (node/migration.go:108) still has no direct unit test (confirmed via node/migration_test.go, which only covers fetchL1HeadIfMissing/migrateIfNeeded's remote-DB and L1-fetch paths). The author already noted in review that the related pruned-db migration check will be revisited in a follow-up, so this is likely known. The guard's own branches (no chain yet / new-state already applied / prune-mode already applied / neither applied) would be cheap to cover directly.
• Everything else — cmd/juno/juno.go usage-string updates, node/node.go's plumbing of pruner.WithNewState(cfg.NewState), pruner/pruner.go's new newState option, and the pruner/running_event_filter_test.go call-site updates — is straightforward and consistent with removing the --prune-mode/--new-state mutual exclusion.

Given the PR title is still "NOT READY YET," the one open item (ctx cancellation in restoreNewStateHistory) is presumably a known follow-up, but worth resolving before merge since it affects graceful shutdown on large databases.
· branch feat/newstate-pruning

[github-actions]

Persistent review updated to latest commit c0d16bd

[github-actions]

Claude finished @NazariiDenha's task in 9m 32s —— View job

---

Claude finished reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Understand the request (PR review of latest commit)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Summary

Reviewed the diff between origin/main (5464fa082) and HEAD (8d7218f09, 15 files, +737/-78 — two commits). go build ./pruner/... ./migration/... ./node/... and go vet both succeed. I couldn't run go test in this sandbox (tool permission denied for test execution), so correctness of the new/changed tests was verified by reading them, not by executing them.

The latest commit (8d7218f09, "address bot comment and fix lint") resolves the one previously-open blocking item:

• Fixed — restoreNewStateHistory ctx cancellation. migration/historyprunner/newstate.go:123-166 now takes ctx context.Context, checks ctx.Err() every ctxCheckInterval (65536) entries, and returns (false, nil) on cancellation instead of blocking until the full restore completes. migrator.go's new restoreNewState helper (migrator.go:256-271) correctly treats done=false as "not done," persisting an intermediate state (chainHeight+1, 0, oldestBlockKept) that resumes the entire restore phase — it re-restores from scratch next run rather than from a partial offset, which is correct since restoreNewStateHistory overwrites keys idempotently from the still-intact scratch space (only wiped by the normal runRestorer completion branch in migrator.go:386-393, which only runs after restoreNewState reports done=true).
• Docs/flag-usage text (cmd/juno/juno.go, docs/docs/_config-options.md, docs/generate-config.js) correctly drops the now-false "cannot be combined with --prune-mode"/"mutually exclusive" wording, consistent with node/node.go no longer rejecting the combination.

Verified by inspection (no outstanding issues found):

• pruneNewStateHistoryFromUpdate/supersededPruner.prune (pruner/accessors.go:318-417): the Seek-then-Prev boundary logic correctly deletes only the entry a later-pruned block supersedes, keeping the entry at/after the block since new-state reads take "latest at or before." This matches core/state/state_reader.go's getHistoricalValue semantics exactly, and is now backed by TestPruneUpto_NewStateHistory (pruner/accessors_test.go) and TestMigrate_NewState (migration/historyprunner/migrator_test.go), both of which assert via NewStateHistoryAnswers/StateReader round-trips plus direct key-existence checks, including a "re-run is idempotent" pass.
• requireNewStateBeforePruning (node/migration.go:108-134): metadata from GetSchemaMetadata is a value type (migration.SchemaMetadata), not a pointer, so the earlier "nil dereference" concern on ErrKeyNotFound doesn't apply — confirmed against migration/metadata.go and migration/runner.go. The four branches (no chain yet / new-state already applied / prune-mode already applied / neither applied) look correct against migration/registry.go's OptionalMigrationFlags()/SchemaVersion.Has semantics, though still has no direct unit test (same gap noted in earlier review rounds — the author has flagged the related pruned-db-migration check as a planned follow-up).
• Batch resource-leak concerns raised by earlier automated reviews on scratchWriter.put and restoreNewStateHistory's batch rotation are false positives: every db.Batch.Write() implementation (db/pebble/batch.go:150, db/pebblev2/batch.go:150, db/memory/batch.go) calls Close() internally before returning, so allocating a fresh batch immediately after Write() doesn't leak the old one.
• migration/historyprunner/restorer.go, pruner/pruner.go's WithNewState option, and the node/node.go plumbing of pruner.WithNewState(cfg.NewState) are straightforward and consistent with removing the --prune-mode/--new-state mutual exclusion.
• Distinct bucket IDs (db.ContractStorageHistory etc. vs. db.DeprecatedContractStorageHistory etc., see db/buckets.go) mean the new-state and deprecated history/scratch paths can't collide.

Nit (unchanged from earlier rounds, non-blocking):

• requireNewStateBeforePruning still has no direct unit test covering its four branches — cheap to add, and the author already indicated a related follow-up PR will touch this function.

Given the PR title is still "NOT READY YET," this looks functionally complete and clean; the one remaining item is the test-coverage nit above, which the author may fold into the planned follow-up.
· branch feat/newstate-pruning

[github-actions]

Claude is reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Understand the request (review latest commit 6253cf813, "address bot comment and fix lint")
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

View job run

[github-actions]

Claude finished @NazariiDenha's task in 15m 50s —— View job

---

Claude finished reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Understand the request (review latest commit 80aa95c0a, "reuse same batch writer both in new state stager and restorer")
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Summary

Reviewed the diff between origin/main (5464fa082) and HEAD (80aa95c0a, 15 files, +726/-78 — three commits). go build ./migration/... ./pruner/... ./node/... succeeds and gofmt -l is clean on the touched files. I could not run go test in this sandbox (tool permission denied), so the two new tests (TestPruneUpto_NewStateHistory, TestMigrate_NewState) were verified by reading, not executing.

This commit is a pure refactor of migration/historyprunner/newstate.go plus one related cleanup in pruner/accessors.go; no behavior changes versus the previous round, which had already resolved every blocking item from earlier reviews (stale test removal, ctx-cancellation in the restore path, test coverage for pruneNewStateHistoryFromUpdate/supersededPruner and stageNewStateHistory/restoreNewStateHistory).

Verified in this commit, no issues found:

• scratchWriter and the restorer's ad-hoc batch loop were unified into one batchWriter type (newstate.go:178-212), used identically by stageBucket (writes scratchKey(key)) and the new restoreBucket (writes it.Key()[1:], i.e. with the scratch tag stripped). Both call sites still batch-rotate via database.NewBatchWithSize after Write(); as confirmed in earlier review rounds, batch.Write() calls Close() internally on every backend (pebble/pebblev2/memory), so this isn't a leak.
• restoreBucket (newstate.go:136-161) now checks ctx.Err() per iterated entry (same as stageBucket), preserving the cancellation fix from the prior commit — restoreNewStateHistory/restoreBucket correctly propagate done=false up through Migrate's restoreNewState helper (migrator.go:256-271), which persists a resumable intermediate state.
• The dropped length guards (len(key) <= blockNumberSuffixLen in stageBucket, len(key) != len(keyPrefix)+8 in pruner/accessors.go:409) are safe to remove: both buckets only ever contain keys of their fixed schema shape (addr/slot felt + 8-byte block suffix, per db/schema.go's ContractStorageHistoryKey/ContractNonceHistoryKey/ContractClassHashHistoryKey), so the checks were unreachable dead code, not validation of untrusted input.
• TestMigrate_NewState (migration/historyprunner/migrator_test.go:291-334) and TestPruneUpto_NewStateHistory (pruner/accessors_test.go:255-296) both assert full round-trip correctness (answers unchanged pre/post-prune, superseded entries deleted, kept entries retained, scratch space wiped, re-run idempotence) — good coverage of the supersededPruner.prune Seek/Prev boundary logic this PR depends on.
• No dangling references to the old scratchWriter/joinClose/ctxCheckInterval names remain anywhere in the tree.

No blockers, important issues, or nits found in this commit — it's a clean simplification that keeps the cancellation and resource-handling behavior from the previous round intact.

· branch feat/newstate-pruning

[github-actions]

Claude is reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Determine what's new since the last reviewed commit (33e6c2083)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

View job run

[github-actions]

Claude is reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Determine what's new since the last reviewed commit (33e6c2083)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Branch

[github-actions]

Claude is reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Determine what's new since the last reviewed commit (33e6c2083)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Branch

[github-actions]

Claude is reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Determine what's new since the last fully-reviewed commit (33e6c2083)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Branch

[EgeCaner]

Looks good to me, thank you!

[github-actions]

Claude encountered an error after 29s —— View job

---

Claude is reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Determine what's new since the last fully-reviewed commit (33e6c2083)
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Branch

[github-actions]

Title

(Describe updated until commit f253a53)

Feat(historyprunner): one pruning migration for old and new state

---

PR Type

Enhancement, Refactoring

---

Description

• Adds support for pruning migration across both deprecated and new state history layouts.

• Refactors the migrator into a resumable 3-step pipeline: stage history, restore history, and restore lookups.

• Parallelizes history staging using address ranges and the pipeline framework.

• Centralizes progress counting logic by moving it to the progresslogger package.

• Updates pruner tests to validate against both history layout formats.

---

File Walkthrough

	Relevant files
Refactoring	8 files copy.go Delete standalone copy logic                                                          +0/-115  keys.go Remove standalone scratch keys generator                                  +0/-149  migrator.go Refactor migrator into a 3-step pipeline architecture        +183/-230 restorer.go Rebuild hash lookups using pipeline integration                    +6/-18    stager.go Stage history ranges into scratch space via parallel pipeline +70/-41  counter.go Move progress counter to progresslogger package                    +1/-1      committer.go Update committer to use new progresslogger package              +3/-2      migrator.go Update headstate migrator to use new progresslogger package +2/-1
Enhancement	2 files history.go Add pipeline-based stage and restore history logic              +211/-0  layout.go Introduce layout definitions for deprecated and new state +134/-0
Tests	5 files migrator_test.go Update migrator tests to cover both history layouts            +181/-118 counter_test.go Update progress counter test package                                          +1/-1      accessors_test.go Add state history pruned assertions in tests                          +1/-0      pruner_test.go Assert state history is pruned in pruner tests                      +2/-0      helpers.go Add Layout abstractions to support testing multiple state formats +179/-78

---

[github-actions]

Claude is reviewing this PR

• Gather context (CLAUDE.md, diff, changed files)
• Determine what's new since the last fully-reviewed commit (33e6c2083, "lint")
• Review changed files for correctness, concurrency, resource leaks, error handling
• Post inline comments for specific issues
• Post final summary comment

Branch

[github-actions]

Persistent review updated to latest commit f253a53