Skip to content

fix(network): enforce checkpoint-safe block apply budget#292

Merged
p0mvn merged 3 commits into
feat/pre-release-mainfrom
codex/block-sync-checkpoint-byte-floor-pr
Jun 28, 2026
Merged

fix(network): enforce checkpoint-safe block apply budget#292
p0mvn merged 3 commits into
feat/pre-release-mainfrom
codex/block-sync-checkpoint-byte-floor-pr

Conversation

@evan-forbes

Copy link
Copy Markdown

Motivation

The post-stack audit found one portable runtime safety gap from evan/perf-plus-download-fixes: the block-sync apply window and byte budget need to be large enough to submit one full checkpoint range plus the resolving checkpoint block. If the configured budget cannot hold that range, checkpoint verification can stall below the boundary because no submitted body becomes durable and no body bytes are released.

Solution

  • Raise DEFAULT_BS_MAX_SUBMITTED_BLOCK_APPLIES from MAX_CHECKPOINT_HEIGHT_GAP to MAX_CHECKPOINT_HEIGHT_GAP + 1.
  • Clamp the effective submitted-apply limit to that checkpoint-safe floor even when serialized config provides a smaller value.
  • Add BS_CHECKPOINT_RANGE_BYTE_FLOOR validation for max_inflight_block_bytes.
  • Refresh the v5.0.0-rc.4 config fixture to serialize max_submitted_block_applies = 401.

Scope boundary: this intentionally does not port the later Committer/apply-queue split or benchmark tooling.

Tests

Passed locally on codex/block-sync-checkpoint-byte-floor-pr:

  • cargo fmt --all -- --check
  • cargo test -p zebra-network zakura::block_sync --lib (146 passed)
  • cargo test -p zebra-network p2p_v2_block_sync_config_validation_rejects_degenerate_values --lib

Attempted but blocked before test execution by the local RocksDB C++ build issue:

  • cargo test -p zebrad --test acceptance latest_config_is_stored -- --nocapture
  • Blocker: librocksdb-sys fails compiling RocksDB headers where uint64_t is used without <cstdint>.

Specifications & References

Stack context:

AI Disclosure

  • No AI tools were used in this PR
  • AI tools were used: Codex was used to audit the remaining branch gap, port this focused follow-up, run validation commands, and draft this PR description.

PR Checklist

  • The PR title follows conventional commits format: type(scope): description
  • The PR follows the contribution guidelines.
  • This change was discussed in the block-sync follow-up audit context.
  • The solution is tested as described above.
  • The documentation and changelogs are up to date.

@evan-forbes evan-forbes marked this pull request as ready for review June 28, 2026 00:18
@evan-forbes evan-forbes changed the base branch from review/blocksync-throughput-defaults to feat/pre-release-main June 28, 2026 00:19
@evan-forbes evan-forbes marked this pull request as draft June 28, 2026 00:19
@evan-forbes evan-forbes force-pushed the codex/block-sync-checkpoint-byte-floor-pr branch from 2eef6a1 to c7e9164 Compare June 28, 2026 00:25

Copy link
Copy Markdown
Author

Pre-split backup note before rewriting this PR:

#295 is intentionally left unchanged.

Copy link
Copy Markdown
Author

Additional pre-split backup note: #292 advanced from cfef29dac5011f121429afbfc36ae8ed350839d4 to 7a3f1860142447ae18d41382b7d88a4c4504e0ec before the force-update attempt. That newer tip is now backed up at:

  • backup/pr292-before-split-20260628-7a3f186

The original cfef29d... backup remains at backup/pr292-before-split-20260628.

A configured `max_inflight_block_bytes` below the checkpoint-range floor
(`MIN_BS_CHECKPOINT_SUBMITTED_BLOCK_APPLIES * BS_PER_BLOCK_WORST_CASE_BYTES`,
~802 MB) cannot hold one full worst-case checkpoint range, so checkpoint sync
would deadlock. Previously the config validation rejected such a value, which
made zebrad refuse to start on older Zakura configs (e.g. the stored
`v4.5.0-zakura-blocksync.toml`, 256 MiB) and broke `config_tests`.

Instead of rejecting, clamp a positive-but-too-small budget up to the floor and
warn once, so older configs keep starting while checkpoint sync stays
deadlock-free. Zero is still rejected by `validate` as an explicit
misconfiguration. Adds unit + deserialize regression tests.
@evan-forbes evan-forbes marked this pull request as ready for review June 28, 2026 03:00
@p0mvn p0mvn merged commit 07f3fd1 into feat/pre-release-main Jun 28, 2026
54 checks passed
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.

2 participants