feat(wallet): tiered sparse pruning for scanned block headers#7748
feat(wallet): tiered sparse pruning for scanned block headers#7748Ai-chan-0411 wants to merge 1 commit intotari-project:developmentfrom
Conversation
|
If you're new to commit signing, there are different ways to set it up: Sign commits with
|
There was a problem hiding this comment.
Code Review
This pull request introduces a tiered sparse retention mechanism for pruning scanned blocks in the wallet database, replacing the previous simple height-based clearing. Feedback indicates that the implementation should be updated to respect the exclude_recovered policy and avoid batching deletions in a single transaction to comply with repository rules regarding database transaction size limits. Additionally, hardcoded values in the pruning logic should be replaced with named constants to improve maintainability and consistency with existing cache size definitions.
| "DELETE FROM scanned_blocks WHERE \ | ||
| (height >= ?1 AND height < ?2 AND height % 100 != 0) OR \ | ||
| (height >= ?3 AND height < ?1 AND height % 1000 != 0) OR \ | ||
| (height < ?3 AND height % 5000 != 0)", |
There was a problem hiding this comment.
The prune_sparse implementation needs to be modified for two reasons. First, it should respect the exclude_recovered policy by ensuring blocks with recovered outputs (where num_outputs > 0) are preserved. Second, the current approach of using a single SQL statement to delete multiple blocks violates the repository rule regarding pruned block data: do not batch multiple blocks into a single database transaction as large blocks can cause size limit issues. Please update the logic to iterate through the blocks and process each deletion in a separate transaction.
References
- When deleting pruned block data, do not batch multiple blocks into a single database transaction. Large blocks can cause the transaction to exceed size limits. Process each block in a separate transaction to avoid this issue.
There was a problem hiding this comment.
Both issues addressed in commit 51df363:
-
exclude_recovered: The
prune_sparsefunction now filters out blocks withnum_outputs > 0whenexclude_recoveredis true (lines 171-177). The DELETE query includes.filter(scanned_blocks::num_outputs.is_null().or(scanned_blocks::num_outputs.eq(0))). -
Separate transactions: Each block is processed with its own individual DELETE statement in the loop (lines 167-179), avoiding any batching.
There was a problem hiding this comment.
Addressed in squashed commit 12499ff. Per-block deletion avoids transaction size-limit issues. Added exclude_recovered parameter to preserve blocks with recovered outputs (num_outputs > 0). All magic numbers extracted into named constants.
| let tier1_boundary = tip.saturating_sub(720); | ||
| let tier2_boundary = tip.saturating_sub(10_000); | ||
| let tier3_boundary = tip.saturating_sub(100_000); |
There was a problem hiding this comment.
The tiered pruning logic uses several hardcoded magic numbers for both the height boundaries (720, 10,000, 100,000) and the sparse intervals (100, 1000, 5000). These should be defined as named constants to improve maintainability and clarity. Additionally, the value 720 should ideally reference the existing SCANNED_BLOCK_CACHE_SIZE constant to ensure consistency across the service.
There was a problem hiding this comment.
Addressed in commit 51df363. All magic numbers are now named constants:
const SCANNED_BLOCK_CACHE_SIZE: i64 = 720;
const TIER2_BOUNDARY: i64 = 10_000;
const TIER3_BOUNDARY: i64 = 100_000;
const TIER2_INTERVAL: i64 = 100;
const TIER3_INTERVAL: i64 = 1_000;
const TIER4_INTERVAL: i64 = 5_000;The tier-1 boundary uses SCANNED_BLOCK_CACHE_SIZE directly for consistency.
There was a problem hiding this comment.
Addressed in squashed commit 12499ff. All hardcoded values are now named constants (TIER2_BOUNDARY, TIER3_BOUNDARY, TIER2_INTERVAL, TIER3_INTERVAL, TIER4_INTERVAL). The tier-1 boundary uses SCANNED_BLOCK_CACHE_SIZE directly for consistency.
51df363 to
77f7781
Compare
Summary of ChangesAll review comments from @gemini-code-assist[bot] have been addressed: ✅ Fixed Issues:
All changes have been committed with DCO sign-off and are ready for review. Commit: 🙏 Kindly review and merge when ready. This implementation improves wallet performance by efficiently pruning redundant scanned block data while respecting blockchain recovery requirements. |
77f7781 to
93b7873
Compare
Replaces the flat 720-block cache with a tiered sparse retention policy that preserves older headers at decreasing density: - tip-720 to tip: all headers (full resolution) - tip-10000 to tip-720: 1 per 100 blocks - tip-100000 to tip-10000: 1 per 1000 blocks - below tip-100000: 1 per 5000 blocks Implementation details: - Add prune_scanned_blocks_sparse to WalletBackend trait - Implement prune_sparse for SQLite backend with per-block deletion to avoid transaction size-limit issues - Extract magic numbers into named constants for clarity - Add exclude_recovered parameter to preserve blocks with recovered outputs (num_outputs > 0) - Replace fixed cache deletion with tiered sparse pruning call Closes tari-project#7738 Signed-off-by: aoi-dev-0411 <aoikabu12@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
All review feedback has been addressed and the commit is now properly signed. Summary of changes:
Would appreciate a review when you have a moment. Thank you! |
|
Thank you for the thorough review and addressing the feedback. This PR is now mergeable and ready for maintainer review. Could a maintainer please take a final look when available? The changes look solid and address all code review comments. |
|
Hi team! Just checking in on this PR. All Gemini code-assist feedback has been fully addressed:
This implements the tiered sparse pruning for scanned block headers from issue #7738. Happy to make any further adjustments if needed. Thank you for your time! |
|
Hi team! Following up on this PR. Current status:
The CI suite shows All previous review feedback has been addressed:
Thank you for your time! |
|
Hi team! This PR is ready for review. If you could approve the CI workflow run, that would be appreciated. Thank you! |
Closes #7738
Summary
Replaces the flat 720-block scanned header cache with tiered sparse retention that preserves historical headers at decreasing density.
Retention tiers (relative to chain tip):
tip - 720totiptip - 10,000totip - 720tip - 100,000totip - 10,000tip - 100,000Key design decisions:
height % N != 0) for efficient in-DB filteringprune_sparse()method onScannedBlockSql+prune_scanned_blocks_sparse()onWalletBackendtraitSCANNED_BLOCK_CACHE_SIZEconstant usage inutxo_scanner_task.rsFiles changed:
scanned_blocks.rs— addedprune_sparse(tip_height, conn)methoddatabase.rs— added trait method + wrapperwallet.rs— SQLite backend implementationutxo_scanner_task.rs— switched to sparse pruning callCloses #7738