Skip to content

feat(consensus): Implement and wire halt-height and halt-time - #3147

Merged
fridrik01 merged 5 commits into
mainfrom
fix-halt-height
Jul 21, 2026
Merged

feat(consensus): Implement and wire halt-height and halt-time#3147
fridrik01 merged 5 commits into
mainfrom
fix-halt-height

Conversation

@fridrik01

@fridrik01 fridrik01 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

This PR implements and wires the --halt-height and --halt-time flags so a network can perform a coordinated halt for binary-swap upgrades. I noticed these were missing while working on the cometbft re-fork (berachain/cometbft#51).

This is the upgrade path for releases where old and new binaries cannot safely coexist in one validator set.

With every validator configured to the same halt point, the network commits the halt block and shuts itself down. Operators then swap the beacond binary and restart without the halt flags, and the chain resumes at the next height from the same data directories, with no replay or rollback.

The implementation makes this safe in three ways:

  • The halt runs only after the block is fully committed, and waits a 5s grace period before exiting so slower peers can finish committing the halt block before gossip goes quiet.
  • A node can never commit a block past the halt point. A node restarted with the halt flags still set refuses to start, and the ABCI handlers neither propose nor finalize past the halt point, so the shutdown stays graceful instead of escalating into a CometBFT consensus-failure panic.
  • Commit now records the real block height and time in the commit header, which also fixes min-retain-blocks pruning, silently a no-op until now (see notes below).

Testing

  • Unit tests for the halt predicate and restart refusal (commit_internal_test.go).
  • New e2e harness (testing/upgrade/halt-swap-resume-test.sh): a local multi-validator devnet halts on the old binary, one node must refuse to restart with the flag set, then all nodes resume from the same data dirs on the new binary with tx load asserted on both sides of the swap. Runs in two flavors:
    • make test-halt-swap-resume (halt at a fixed height, --halt-height)
    • make test-halt-swap-resume-time (halt at a wall-clock time, --halt-time)
  • This is a bash harness rather than a kurtosis e2e test because the subject under test is process lifecycle, halting nodes, swapping the binary underneath them, and restarting from the same data directories, which containers built from a fixed image make awkward. Bare processes also keep the run Docker-free and fast enough for nightly CI.
  • Both run in nightly CI, with logs uploaded on failure.

Notes for release

  • The min-retain-blocks setting now takes effect. It has been silently ignored until now, so operators who have it configured will see CometBFT begin pruning old blocks after upgrading. Anyone relying on it having no effect should unset it before upgrading.
  • Forgotten halt-height or halt-time values in existing configs become live. A non-zero value the old binary ignored will now halt the node, so configs are worth checking before rolling this out.
  • The e2e test uncovered an existing cometbft bug that this PR does not fix, though I tried to mitigate it:
    1. When validators exit right after committing the halt block, a slower validator can be left still waiting for the final precommits it needs to commit that block itself, stuck one block short with no peers left to send them.
    2. After the swap it stays stuck as long as it lags by exactly one block, because restarted peers only hold the aggregated commit and vote gossip needs the individual signatures that aggregation discards.
    3. If the stuck validators hold less than 1/3 of the power the rest of the network resumes without them and they recover as soon as the next block is committed, but past 1/3 the network cannot produce that block and the restart deadlocks.
    4. The 5s grace period exists to prevent the stranding in the first place.
    5. The correct solution to this is fixing this in our cometbft forl (tracked in https://berachain.clickup.com/t/9014124274/86bayuecy)

@fridrik01 fridrik01 self-assigned this Jul 16, 2026
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.00000% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.07%. Comparing base (c6af391) to head (8e93d5f).

Files with missing lines Patch % Lines
consensus/cometbft/service/service.go 45.00% 11 Missing ⚠️
consensus/cometbft/service/commit.go 73.52% 8 Missing and 1 partial ⚠️
consensus/cometbft/service/options.go 60.00% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3147      +/-   ##
==========================================
+ Coverage   63.04%   63.07%   +0.02%     
==========================================
  Files         371      371              
  Lines       15461    15533      +72     
==========================================
+ Hits         9748     9798      +50     
- Misses       4757     4777      +20     
- Partials      956      958       +2     
Files with missing lines Coverage Δ
consensus/cometbft/service/abci.go 35.00% <100.00%> (+9.28%) ⬆️
consensus/cometbft/service/finalize_block.go 46.20% <100.00%> (+0.75%) ⬆️
node-core/builder/baseapp_options.go 63.04% <100.00%> (+1.67%) ⬆️
consensus/cometbft/service/options.go 62.85% <60.00%> (-1.15%) ⬇️
consensus/cometbft/service/commit.go 52.17% <73.52%> (+15.58%) ⬆️
consensus/cometbft/service/service.go 38.41% <45.00%> (+0.83%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fridrik01
fridrik01 marked this pull request as ready for review July 16, 2026 15:56
@fridrik01
fridrik01 requested a review from a team as a code owner July 16, 2026 15:56
Copilot AI review requested due to automatic review settings July 16, 2026 15:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Implements and wires CometBFT-side coordinated halting for binary-swap upgrades via --halt-height and --halt-time, plus adds CI coverage and an end-to-end upgrade smoke test to validate halt → swap → resume behavior.

Changes:

  • Wire halt-height / halt-time from CLI/app options into the CometBFT service and enforce halting semantics across Start/ABCI/Commit.
  • Populate commit headers with real height/time (enabling min-retain-blocks pruning and seeding halt-time state across restarts).
  • Add an e2e upgrade harness + Make targets + nightly CI job to exercise halt/swap/resume under tx load.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
testing/upgrade/halt-swap-resume-test.sh New local multi-validator bash harness to halt, swap beacond binaries, and resume while asserting safety + tx flow.
scripts/build/halt-upgrade-test.mk Make targets to run the halt/swap/resume test and fetch a bera-reth binary dependency.
node-core/builder/baseapp_options.go Wires halt flags into CometBFT service options.
Makefile Includes the new halt upgrade test make targets.
consensus/cometbft/service/service.go Seeds finalized height/time on startup and refuses Start if halt point already reached.
consensus/cometbft/service/options.go Adds service options for haltHeight / haltTime.
consensus/cometbft/service/finalize_block.go Publishes finalized height/time after successful FinalizeBlock completion.
consensus/cometbft/service/commit.go Sets commit header height/time and triggers graceful halt after commit; adds shared halt predicate and shutdown parking helper.
consensus/cometbft/service/commit_internal_test.go Adds unit tests for halt predicate, restart refusal, and ABCI gating behavior.
consensus/cometbft/service/abci.go Gates proposal/finalization past the halt point to avoid CometBFT consensus-failure panics.
.github/workflows/nightly.yml Adds nightly job to run the halt/swap/resume tests and upload logs on failure.

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

Comment thread testing/upgrade/halt-swap-resume-test.sh Outdated
Comment thread consensus/cometbft/service/service.go Outdated
Comment thread scripts/build/halt-upgrade-test.mk Outdated
Comment thread scripts/build/halt-upgrade-test.mk

@bar-bera bar-bera left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

super good

Comment thread consensus/cometbft/service/commit.go Outdated
Comment thread testing/upgrade/halt-swap-resume-test.sh
@fridrik01
fridrik01 merged commit 06c6980 into main Jul 21, 2026
22 checks passed
@fridrik01
fridrik01 deleted the fix-halt-height branch July 21, 2026 11:11
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.

4 participants