Skip to content

Conversation

@drahnr
Copy link
Contributor

@drahnr drahnr commented Feb 6, 2026

Important

Targeting main

We recently started tracking historical account state storage ( #1227 ) but that punted on the cleanup, causing unbounded growth of the DB.

The PR adds a cleanup routine to apply_block.

Pro:

  • simple, all writing DB transactions happen in one place
  • no risk of sqlite locked errors popping up

Con:

  • prolonged time the mutex is being held
  • does not exploit all possible parallelism of apply_block

@drahnr drahnr force-pushed the miden-node-bernhard-cleanup-old-db-entries-main branch from 2cd6161 to eb4daa5 Compare February 6, 2026 14:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements database cleanup functionality for historical account state data to prevent unbounded database growth. The implementation adds a cleanup routine that runs during apply_block to delete old vault asset and storage map entries while preserving the latest state and recent history.

Changes:

  • Added cleanup_all_accounts function that deletes historical entries older than 50 blocks (excluding is_latest=true entries)
  • Created database migration to add partial indices on account_vault_assets and account_storage_map_values tables for efficient cleanup queries
  • Integrated cleanup into the apply_block transaction in the store component

Reviewed changes

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

Show a summary per file
File Description
crates/store/src/db/models/queries/accounts.rs Adds cleanup_all_accounts function with 50-block retention policy for vault assets and storage map values
crates/store/src/db/mod.rs Integrates cleanup call into apply_block transaction after block data is written
crates/store/src/db/migrations/2026020600000_cleanup_indices/up.sql Creates partial indices for efficient cleanup queries on non-latest entries
crates/store/src/db/migrations/2026020600000_cleanup_indices/down.sql Provides rollback script to remove cleanup indices
crates/store/src/errors.rs Adds QueryTimeout error variant (unused in this PR)
crates/store/src/inner_forest/mod.rs Adds TODO comment about tying in-memory forest retention to DB pruning policy
crates/store/src/db/tests.rs Renames test function for consistency
CHANGELOG.md Adds changelog entry (duplicate of existing v0.13.0 entry)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@drahnr drahnr force-pushed the miden-node-bernhard-cleanup-old-db-entries-main branch from d333137 to b78fcb5 Compare February 6, 2026 16:39
@drahnr
Copy link
Contributor Author

drahnr commented Feb 7, 2026

After self review, this still has some implications to be addressed, particularly around storage map root calculation of account updates using ad-hoc smt forests.

@drahnr drahnr marked this pull request as draft February 7, 2026 10:15
};
use loader::{load_mmr, load_smt_forest, verify_tree_consistency};

mod apply_block;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes it harder to review, but if we want further changes to apply_block - which we do, we should keep the sync on the extra file with next to avoid more pain when merging.

@drahnr drahnr marked this pull request as ready for review February 11, 2026 16:02
@drahnr drahnr force-pushed the miden-node-bernhard-cleanup-old-db-entries-main branch from 32d068d to 3506452 Compare February 11, 2026 16:19
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.

1 participant