Skip to content

BlockValidation - regenerate missing subtreemeta#468

Open
freemans13 wants to merge 6 commits intobsv-blockchain:mainfrom
freemans13:stu/block-validation-re-generate-subtree-meta
Open

BlockValidation - regenerate missing subtreemeta#468
freemans13 wants to merge 6 commits intobsv-blockchain:mainfrom
freemans13:stu/block-validation-re-generate-subtree-meta

Conversation

@freemans13
Copy link
Collaborator

@freemans13 freemans13 commented Feb 3, 2026

Problem

When getSubtreeMetaSlice() fails to find subtree meta files during block validation, blocks are incorrectly marked invalid even though the underlying subtree data exists to regenerate them. This occurs when:

  • Blocks are validated during catchup (where meta creation is intentionally skipped for performance)
  • Meta files have been pruned but subtree data still exists
  • Operators manually revalidate old blocks after pruning

Solution

Add on-demand subtree meta regeneration that attempts to rebuild missing meta files from subtree data (local store first, then peer fallback).

Changes

  1. Deleted model/tx_fetcher.go (~200 lines removed)

    • Merged subtreedata fetching logic directly into the regenerator
    • Removed unproven batch transaction API
  2. Simplified model/subtree_meta_regenerator.go

    • Inlined local subtreedata fetching + peer HTTP fallback
    • Removed TxFetcherI interface dependency
    • Constructor: NewSubtreeMetaRegenerator(logger, subtreeStore, peerURLs, apiPrefix)
  3. Simplified Block.Valid() signature

    • Removed functional options pattern (ValidationOptions, ValidationOption, WithMetaRegenerator)
    • Added metaRegenerator SubtreeMetaRegeneratorI as a regular nil-able parameter
    • Callers pass nil when regeneration not needed (e.g., block assembly)
  4. Updated callers

    • BlockValidation.go / Server.go (blockvalidation): pass regenerator
    • Server.go (blockassembly): pass nil (just created the subtrees, no regeneration needed)
    • Test files: pass nil

Regeneration Flow

RegenerateMeta(subtreeHash, subtree)
    ├── Try local subtreedata store
    │   └── Success → build meta, store it, return
    ├── Try each peer URL
    │   └── GET /api/v1/subtree_data/{hash}
    │       └── Success → build meta, store it, return
    └── All failed → return error

Testing

  • go test ./model/... -run SubtreeMeta - regenerator unit tests
  • go test ./services/blockvalidation/... - integration tests
  • make lint - passes

@freemans13 freemans13 self-assigned this Feb 3, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2026

🤖 Claude Code Review

Status: Complete

No issues found. All previously reported issues have been resolved:

Fixed Issues:

  • HTTP client timeout reduced from 5 minutes to 30 seconds (line 51)
  • Error handling now differentiates between 404 and other HTTP errors (lines 113-117)

Review Summary:
This PR adds on-demand regeneration of missing subtree meta files during block validation. The implementation is well-structured with:

  • Clean separation of concerns (local store → peer fallback)
  • Comprehensive test coverage including local, peer, and failure scenarios
  • Proper error handling and logging
  • Non-blocking meta storage to avoid validation delays

The changes follow project conventions and integrate cleanly with existing validation flows.

- Reduce HTTP client timeout from 5 minutes to 30 seconds for faster fail
- Differentiate HTTP errors: 404 returns NotFoundError, others return ServiceError

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2026

🤖 Claude Code Review

Status: Complete


Current Review:

All previous issues have been addressed:

  • ✅ HTTP timeout reduced from 5 minutes to 30 seconds
  • ✅ Error handling improved to differentiate between 404 and other HTTP errors

Minor Observation:

One potential defensive programming improvement (not a blocker):

  • In buildMetaFromSubtreeData (model/subtree_meta_regenerator.go:146), accessing subtree.Nodes[0] without checking len(subtree.Nodes) > 0 could theoretically panic if called with an empty subtree. While this may never occur in practice (subtrees always have at least a coinbase placeholder), adding a length check would make the code more defensive.

Summary: The implementation is solid. The regeneration logic is well-structured with local-first fallback to peers, comprehensive test coverage (7 test cases), and proper error handling. The design correctly uses warn-level logging for regeneration events since this is an exceptional but valid scenario.

History:

  • ✅ Fixed: HTTP timeout reduced to 30s (was 5 minutes)
  • ✅ Fixed: Error handling now distinguishes 404 from other HTTP errors

freemans13 and others added 4 commits February 3, 2026 13:55
getSubtreeMetaSlice just reads a file from blob store - no need to retry.
If the file is missing, regeneration already handles recovery.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
…o stu/block-validation-re-generate-subtree-meta
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 3, 2026

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