Skip to content

Fix pruner service not triggering when not running the persister#367

Open
oskarszoon wants to merge 1 commit intobsv-blockchain:mainfrom
oskarszoon:bugfix/pruner-without-persister
Open

Fix pruner service not triggering when not running the persister#367
oskarszoon wants to merge 1 commit intobsv-blockchain:mainfrom
oskarszoon:bugfix/pruner-without-persister

Conversation

@oskarszoon
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2026

🤖 Claude Code Review

Status: Complete


This PR correctly fixes a timing issue in the pruner service's fallback trigger mechanism. The change is well-reasoned and properly documented.

Summary:

The pruner service has two triggering mechanisms:

  1. Primary: BlockPersisted notifications (when block persister is running)
  2. Fallback: Now uses BlockSubtreesSet instead of Block notification

The Problem:
The original fallback used NotificationType_Block with a manual GetBlockIsMined check. However, Block notifications are sent when a block is added to the chain (services/blockchain/Server.go:787), which happens before block validation completes. This created a race condition where the pruner would check mined_set before it was set to true, causing it to skip blocks.

The Solution:
Switch to NotificationType_BlockSubtreesSet, which is sent after block validation completes in updateSubtreesDAH() (services/blockvalidation/BlockValidation.go:2002). This ensures the block is fully validated before pruning is triggered.

Code Quality:

  • Removes unnecessary import (chainhash) and ~30 lines of timing-dependent logic
  • Simplifies the code by eliminating the GetBlockIsMined check
  • Comments clearly explain the timing guarantees
  • Aligns with existing documentation in blockvalidation/BlockValidation.go:329-346

No issues found.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
11.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

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