From ed5fb2dd581389e1a51d1452ef1c5b0686470b1b Mon Sep 17 00:00:00 2001 From: Fridrik Asmundsson Date: Thu, 16 Jul 2026 14:55:02 +0000 Subject: [PATCH 1/4] feat(consensus): implement halt-height and halt-time --- consensus/cometbft/service/abci.go | 33 +++- consensus/cometbft/service/commit.go | 92 ++++++++++- .../cometbft/service/commit_internal_test.go | 148 ++++++++++++++++++ consensus/cometbft/service/finalize_block.go | 11 +- consensus/cometbft/service/options.go | 22 +++ consensus/cometbft/service/service.go | 47 ++++++ node-core/builder/baseapp_options.go | 2 + 7 files changed, 343 insertions(+), 12 deletions(-) create mode 100644 consensus/cometbft/service/commit_internal_test.go diff --git a/consensus/cometbft/service/abci.go b/consensus/cometbft/service/abci.go index 7a8f2ea26e..3e2f752a4a 100644 --- a/consensus/cometbft/service/abci.go +++ b/consensus/cometbft/service/abci.go @@ -55,6 +55,14 @@ func (s *Service) PrepareProposal( _ context.Context, req *cmtabci.PrepareProposalRequest, ) (*cmtabci.PrepareProposalResponse, error) { + // Once halted, propose nothing so consensus cannot decide a block past the halt point while the halt + // shutdown completes. + if s.ensureNotHalted() != nil { + s.logger.Info("halt point reached, returning an empty proposal", "height", req.Height) + //nolint:nilerr // a halted node proposes nothing instead of erroring + return &cmtabci.PrepareProposalResponse{Txs: [][]byte{}}, nil + } + // Check if ctx is still good. CometBFT does not check this. if s.ctx.Err() != nil { // If the context is getting cancelled, we are shutting down. @@ -94,6 +102,14 @@ func (s *Service) ProcessProposal( _ context.Context, req *cmtabci.ProcessProposalRequest, ) (*cmtabci.ProcessProposalResponse, error) { + // Once halted, prevote nil on every proposal so no block past the halt point can be decided. An error + // return would make CometBFT panic. + if s.ensureNotHalted() != nil { + s.logger.Info("halt point reached, rejecting proposal", "height", req.Height) + //nolint:nilerr // a halted node votes nil instead of erroring + return &cmtabci.ProcessProposalResponse{Status: cmtabci.PROCESS_PROPOSAL_STATUS_REJECT}, nil + } + // Check if ctx is still good. CometBFT does not check this. if s.ctx.Err() != nil { // Node will panic on context cancel with "CONSENSUS FAILURE!!!" due to @@ -109,6 +125,16 @@ func (s *Service) FinalizeBlock( _ context.Context, req *cmtabci.FinalizeBlockRequest, ) (*cmtabci.FinalizeBlockResponse, error) { + // Never execute a block past the halt point. An error return would make CometBFT panic with CONSENSUS + // FAILURE (consensus and blocksync both escalate FinalizeBlock errors), so park until the halt shutdown + // exits the process underneath this call. Checked before the ctx guard below so a post-halt block arriving + // mid-shutdown parks instead of panicking. + if err := s.ensureNotHalted(); err != nil { + s.logger.Info("halt point reached, refusing to finalize block", "height", req.Height) + s.waitForHaltShutdown() + return nil, err + } + // Check if ctx is still good. CometBFT does not check this. if s.ctx.Err() != nil { // Node will panic on context cancel with "CONSENSUS FAILURE!!!" due to error. @@ -122,10 +148,9 @@ func (s *Service) FinalizeBlock( // Commit implements the ABCI interface. It will commit all state that exists in // the deliver state's multi-store and includes the resulting commit ID in the // returned cmtabci.ResponseCommit. Commit will set the check state based on the -// latest header and reset the deliver state. Also, if a non-zero halt height is -// defined in config, Commit will execute a deferred function call to check -// against that height and gracefully halt if it matches the latest committed -// height. +// latest header and reset the deliver state. Also, if a non-zero halt height +// or halt time is configured, Commit gracefully shuts the node down once the +// committed block reaches it. func (s *Service) Commit( _ context.Context, req *cmtabci.CommitRequest, ) (*cmtabci.CommitResponse, error) { diff --git a/consensus/cometbft/service/commit.go b/consensus/cometbft/service/commit.go index 51025973bb..4f8ee01b52 100644 --- a/consensus/cometbft/service/commit.go +++ b/consensus/cometbft/service/commit.go @@ -23,43 +23,123 @@ package cometbft import ( "fmt" + "os" + "syscall" + "time" "cosmossdk.io/store/rootmulti" cmtabci "github.com/cometbft/cometbft/abci/types" + cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" ) func (s *Service) commit( *cmtabci.CommitRequest, ) (*cmtabci.CommitResponse, error) { - _, finalState, err := s.cachedStates.GetFinal() - if err != nil { + if _, _, err := s.cachedStates.GetFinal(); err != nil { // This is unexpected since CometBFT should call Commit only // after FinalizeBlock has been called. Panic appeases nilaway. panic(fmt.Errorf("commit: %w", err)) } - header := finalState.Context().BlockHeader() - retainHeight := s.GetBlockRetentionHeight(header.Height) + // The cached state context carries an empty block header, so use the height and time captured from FinalizeBlock instead. + retainHeight := s.GetBlockRetentionHeight(s.finalizedHeight) rms, ok := s.sm.GetCommitMultiStore().(*rootmulti.Store) if ok { - rms.SetCommitHeader(header) + rms.SetCommitHeader(cmtproto.Header{ChainID: s.chainID, Height: s.finalizedHeight, Time: s.finalizedTime}) } s.sm.GetCommitMultiStore().Commit() s.cachedStates.Reset() if s.blockDelay != nil { - if err = s.sm.SaveBlockDelay(s.blockDelay.ToBytes()); err != nil { + if err := s.sm.SaveBlockDelay(s.blockDelay.ToBytes()); err != nil { panic(fmt.Errorf("failed to save block delay: %w", err)) } } + s.haltIfReached() + return &cmtabci.CommitResponse{ RetainHeight: retainHeight, }, nil } +// haltPointReached reports whether a block at the given height and time has reached the configured halt-height +// or halt-time. It is the single halt predicate, applied to the last finalized block by ensureNotHalted and haltIfReached. +func haltPointReached(haltHeight, haltTime uint64, height int64, blockTime time.Time) bool { + unixTime := blockTime.Unix() + switch { + case haltHeight > 0 && height >= 0 && uint64(height) >= haltHeight: + return true + case haltTime > 0 && unixTime >= 0 && uint64(unixTime) >= haltTime: + return true + default: + return false + } +} + +// ensureNotHalted returns an error once the last finalized block has reached the halt point. Service start +// refuses to run with it and the ABCI handlers gate on it (see abci.go), so a node with the halt flags still +// set neither advances state past the halt block nor creeps one block per restart. +func (s *Service) ensureNotHalted() error { + if !haltPointReached(s.haltHeight, s.haltTime, s.finalizedHeight, s.finalizedTime) { + return nil + } + return fmt.Errorf( + "chain reached the configured halt point (halt-height %d, halt-time %d) at committed height %d, unset the halt flags to resume", + s.haltHeight, s.haltTime, s.finalizedHeight, + ) +} + +// haltShutdownSlack bounds how long a parked ABCI call waits after the app context is cancelled. The halt +// shutdown normally exits the process within it, so the caller's error return (and the CometBFT panic it +// causes) only happens on a wedged shutdown. +// +//nolint:gochecknoglobals // var instead of const so tests can shorten it +var haltShutdownSlack = 30 * time.Second + +// waitForHaltShutdown parks an ABCI call that must not proceed past the halt point while the halt shutdown +// brings the process down. +func (s *Service) waitForHaltShutdown() { + <-s.ctx.Done() + time.Sleep(haltShutdownSlack) +} + +// haltGracePeriod keeps Commit blocked after the halt block so vote gossip can deliver the halt-block +// precommits to validators still one short. Exiting immediately can wedge those peers at the previous height, +// and a wedged set larger than 1/3 cannot recover after the restart (individual precommit signatures do not +// survive commit aggregation). +const haltGracePeriod = 5 * time.Second + +// haltIfReached gracefully shuts down the node once the committed block reaches the configured halt-height or +// halt-time. It runs after the block has been fully committed, so a restarted node resumes consensus at the +// next height with no replay needed. +func (s *Service) haltIfReached() { + if !haltPointReached(s.haltHeight, s.haltTime, s.finalizedHeight, s.finalizedTime) { + return + } + + s.logger.Info("halting node per configuration", + "halt_height", s.haltHeight, "halt_time", s.haltTime, "committed_height", s.finalizedHeight, "grace_period", haltGracePeriod) + + // Sleeping here blocks the consensus state machine inside Commit, so no halting node can advance to the + // next height while its peer gossip routines keep serving the halt-block precommits from the live vote set. + time.Sleep(haltGracePeriod) + + // Signal our own process so the node's regular shutdown path runs, the same mechanism a cosmos-sdk baseapp + // uses for halt-height. + p, err := os.FindProcess(os.Getpid()) + if err != nil { + os.Exit(0) + } + if err = p.Signal(syscall.SIGINT); err != nil { + if err = p.Signal(syscall.SIGTERM); err != nil { + os.Exit(0) + } + } +} + // GetBlockRetentionHeight returns the height for which all blocks below this // height // are pruned from CometBFT. Given a commitment height and a non-zero local diff --git a/consensus/cometbft/service/commit_internal_test.go b/consensus/cometbft/service/commit_internal_test.go new file mode 100644 index 0000000000..906a3b2609 --- /dev/null +++ b/consensus/cometbft/service/commit_internal_test.go @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: BUSL-1.1 +// +// Copyright (C) 2025, Berachain Foundation. All rights reserved. +// Use of this software is governed by the Business Source License included +// in the LICENSE file of this repository and at www.mariadb.com/bsl11. +// +// ANY USE OF THE LICENSED WORK IN VIOLATION OF THIS LICENSE WILL AUTOMATICALLY +// TERMINATE YOUR RIGHTS UNDER THIS LICENSE FOR THE CURRENT AND ALL OTHER +// VERSIONS OF THE LICENSED WORK. +// +// THIS LICENSE DOES NOT GRANT YOU ANY RIGHT IN ANY TRADEMARK OR LOGO OF +// LICENSOR OR ITS AFFILIATES (PROVIDED THAT YOU MAY USE A TRADEMARK OR LOGO OF +// LICENSOR AS EXPRESSLY REQUIRED BY THIS LICENSE). +// +// TO THE EXTENT PERMITTED BY APPLICABLE LAW, THE LICENSED WORK IS PROVIDED ON +// AN “AS IS” BASIS. LICENSOR HEREBY DISCLAIMS ALL WARRANTIES AND CONDITIONS, +// EXPRESS OR IMPLIED, INCLUDING (WITHOUT LIMITATION) WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE, NON-INFRINGEMENT, AND +// TITLE. +// + +package cometbft + +import ( + "context" + "io" + "testing" + "time" + + "github.com/berachain/beacon-kit/log/phuslu" + cmtabci "github.com/cometbft/cometbft/abci/types" + "github.com/stretchr/testify/require" +) + +func TestHaltPointReached(t *testing.T) { + t.Parallel() + + blockTime := time.Unix(1700000000, 0) + + tests := []struct { + name string + haltHeight uint64 + haltTime uint64 + height int64 + blockTime time.Time + want bool + }{ + {name: "disabled", height: 100, blockTime: blockTime, want: false}, + {name: "below halt height", haltHeight: 100, height: 99, blockTime: blockTime, want: false}, + {name: "at halt height", haltHeight: 100, height: 100, blockTime: blockTime, want: true}, + {name: "past halt height", haltHeight: 100, height: 101, blockTime: blockTime, want: true}, + {name: "before halt time", haltTime: 1700000001, height: 100, blockTime: blockTime, want: false}, + {name: "at halt time", haltTime: 1700000000, height: 100, blockTime: blockTime, want: true}, + {name: "past halt time", haltTime: 1699999999, height: 100, blockTime: blockTime, want: true}, + {name: "zero block time does not halt", haltTime: 1700000000, height: 100, blockTime: time.Time{}, want: false}, + {name: "halt time reached before halt height", haltHeight: 200, haltTime: 1700000000, height: 100, blockTime: blockTime, want: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tt.want, haltPointReached(tt.haltHeight, tt.haltTime, tt.height, tt.blockTime)) + }) + } +} + +// TestEnsureNotHalted pins ensureNotHalted to the last finalized block. FinalizeBlock at height N runs it before +// updating the finalized fields, so state held at N-1 means "may block N be finalized". +func TestEnsureNotHalted(t *testing.T) { + t.Parallel() + + s := &Service{} + require.NoError(t, s.ensureNotHalted(), "halt disabled") + + s.haltHeight = 10 + s.finalizedHeight = 9 + require.NoError(t, s.ensureNotHalted(), "halt block itself must finalize") + + s.finalizedHeight = 10 + require.Error(t, s.ensureNotHalted(), "block past halt height must be refused") + + s = &Service{haltTime: 1700000000} + require.NoError(t, s.ensureNotHalted(), "unseeded finalized time must not refuse") + + s.finalizedTime = time.Unix(1700000000, 0) + require.Error(t, s.ensureNotHalted(), "block after halt time must be refused") +} + +// haltedService returns a Service whose halt point has been reached. +func haltedService(ctx context.Context) *Service { + return &Service{ + logger: phuslu.NewLogger(io.Discard, nil), + ctx: ctx, + haltHeight: 10, + finalizedHeight: 10, + } +} + +// TestFinalizeBlockParksAtHaltPoint pins the FinalizeBlock halt gate. A block past the halt point must park +// until shutdown rather than return an error, which CometBFT escalates to a CONSENSUS FAILURE panic. +func TestFinalizeBlockParksAtHaltPoint(t *testing.T) { + t.Parallel() // safe, no other test reads haltShutdownSlack + + restore := haltShutdownSlack + haltShutdownSlack = 50 * time.Millisecond + t.Cleanup(func() { haltShutdownSlack = restore }) + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + s := haltedService(ctx) + + done := make(chan error, 1) + go func() { + _, err := s.FinalizeBlock(ctx, &cmtabci.FinalizeBlockRequest{Height: 11}) + done <- err + }() + + select { + case err := <-done: + t.Fatalf("FinalizeBlock returned instead of parking until shutdown: %v", err) + case <-time.After(250 * time.Millisecond): + } + + cancel() + select { + case err := <-done: + require.ErrorContains(t, err, "halt point") + case <-time.After(5 * time.Second): + t.Fatal("FinalizeBlock did not return after context cancel plus slack") + } +} + +// TestProposalHandlersGateAtHaltPoint pins the non-panicking proposal gates, +// which keep a halting validator from helping decide a post-halt block. +func TestProposalHandlersGateAtHaltPoint(t *testing.T) { + t.Parallel() + + ctx := t.Context() + s := haltedService(ctx) + + prepResp, err := s.PrepareProposal(ctx, &cmtabci.PrepareProposalRequest{Height: 11}) + require.NoError(t, err) + require.Empty(t, prepResp.Txs, "halted node must propose nothing") + + procResp, err := s.ProcessProposal(ctx, &cmtabci.ProcessProposalRequest{Height: 11}) + require.NoError(t, err) + require.Equal(t, cmtabci.PROCESS_PROPOSAL_STATUS_REJECT, procResp.Status) +} diff --git a/consensus/cometbft/service/finalize_block.go b/consensus/cometbft/service/finalize_block.go index d8d880653d..fc0f552f8b 100644 --- a/consensus/cometbft/service/finalize_block.go +++ b/consensus/cometbft/service/finalize_block.go @@ -284,11 +284,18 @@ func (s *Service) calculateFinalizeBlockResponse( s.syncingToHeight = req.SyncingToHeight cp := s.cmtConsensusParams.ToProto() - return &cmtabci.FinalizeBlockResponse{ + response := &cmtabci.FinalizeBlockResponse{ TxResults: txResults, ValidatorUpdates: formattedValUpdates, ConsensusParamUpdates: &cp, AppHash: s.workingHash(), NextBlockDelay: nextBlockTime, - }, nil + } + + // Only publish the block metadata used by Commit after FinalizeBlock has completed successfully. Otherwise, + // a failed attempt at the halt point could be mistaken for an already committed block on a subsequent call. + s.finalizedHeight = req.Height + s.finalizedTime = req.Time + + return response, nil } diff --git a/consensus/cometbft/service/options.go b/consensus/cometbft/service/options.go index ef687eee80..7e2611585e 100644 --- a/consensus/cometbft/service/options.go +++ b/consensus/cometbft/service/options.go @@ -55,6 +55,28 @@ func SetMinRetainBlocks(minRetainBlocks uint64) func(*Service) { return func(bs *Service) { bs.setMinRetainBlocks(minRetainBlocks) } } +// SetHaltHeight returns a Service option function that sets the block height at which the node gracefully +// shuts down after committing. +func SetHaltHeight(haltHeight uint64) func(*Service) { + return func(bs *Service) { + if haltHeight > 0 { + bs.logger.Info("halt height configured, node will shut down after committing it", "halt_height", haltHeight) + } + bs.setHaltHeight(haltHeight) + } +} + +// SetHaltTime returns a Service option function that sets the minimum block time, in Unix seconds, at which +// the node gracefully shuts down after committing. +func SetHaltTime(haltTime uint64) func(*Service) { + return func(bs *Service) { + if haltTime > 0 { + bs.logger.Info("halt time configured, node will shut down at the first block at or after it", "halt_time", haltTime) + } + bs.setHaltTime(haltTime) + } +} + // SetIAVLCacheSize provides a Service option function that sets the size of // IAVL cache. func SetIAVLCacheSize(size int) func(*Service) { diff --git a/consensus/cometbft/service/service.go b/consensus/cometbft/service/service.go index 6f869af308..3609dd6d4c 100644 --- a/consensus/cometbft/service/service.go +++ b/consensus/cometbft/service/service.go @@ -24,7 +24,9 @@ import ( "context" "errors" "fmt" + "time" + "cosmossdk.io/store/rootmulti" storetypes "cosmossdk.io/store/types" "github.com/berachain/beacon-kit/beacon/blockchain" "github.com/berachain/beacon-kit/beacon/validator" @@ -88,6 +90,16 @@ type Service struct { initialHeight int64 minRetainBlocks uint64 + // haltHeight and haltTime, when non-zero, cause the node to gracefully shut down once the committed block + // height, respectively block time in Unix seconds, reaches them. + haltHeight uint64 + haltTime uint64 + + // finalizedHeight and finalizedTime describe the block most recently processed by FinalizeBlock. commit() + // uses them for the halt checks, since the cached state context does not carry a populated block header. + finalizedHeight int64 + finalizedTime time.Time + chainID string // ctx is the context passed in for the service. CometBFT currently does @@ -108,6 +120,7 @@ type Service struct { syncingToHeight int64 } +//nolint:funlen // node assembly requires many sequential setup steps func NewService( logger *phuslu.Logger, db dbm.DB, @@ -161,6 +174,26 @@ func NewService( lastBlockHeight := s.lastBlockHeight() s.syncingToHeight = lastBlockHeight + // Seed the finalized-block tracking from the last committed block so the halt checks hold across restarts. + // Blocks committed by binaries that predate the populated commit header carry a zero timestamp, which + // leaves the halt-time check unseeded until the next commit. + s.finalizedHeight = lastBlockHeight + if lastBlockHeight > 0 { + rms, ok := s.sm.GetCommitMultiStore().(*rootmulti.Store) + if !ok { + panic("failed loading last committed block time: unexpected commit multi-store type") + } + + ci, ciErr := rms.GetCommitInfo(lastBlockHeight) + if ciErr != nil { + panic(fmt.Errorf("failed loading commit info at height %d: %w", lastBlockHeight, ciErr)) + } + if ci == nil { + panic(fmt.Errorf("failed loading commit info at height %d: empty commit info", lastBlockHeight)) + } + s.finalizedTime = ci.Timestamp + } + // Make sure that SBT consensus parameters are duly set when the node restart. // Note that we can't rely on genesis.json having these parameters set right // because we introduced stable block time post (mainnet) genesis. @@ -195,6 +228,12 @@ func NewService( func (s *Service) Start( ctx context.Context, ) error { + // Refuse to enter consensus when the halt point was already reached, so a node restarted with the halt + // flags still set exits cleanly instead of proposing or replaying blocks past the halt and crash-looping. + if err := s.ensureNotHalted(); err != nil { + return err + } + cfg := s.cmtCfg nodeKey, err := p2p.LoadOrGenNodeKey(cfg.NodeKeyFile()) if err != nil { @@ -351,6 +390,14 @@ func (s *Service) setMinRetainBlocks(minRetainBlocks uint64) { s.minRetainBlocks = minRetainBlocks } +func (s *Service) setHaltHeight(haltHeight uint64) { + s.haltHeight = haltHeight +} + +func (s *Service) setHaltTime(haltTime uint64) { + s.haltTime = haltTime +} + func (s *Service) setInterBlockCache( cache storetypes.MultiStorePersistentCache, ) { diff --git a/node-core/builder/baseapp_options.go b/node-core/builder/baseapp_options.go index 1c6ae5932d..2598408640 100644 --- a/node-core/builder/baseapp_options.go +++ b/node-core/builder/baseapp_options.go @@ -70,6 +70,8 @@ func DefaultServiceOptions( cometbft.SetMinRetainBlocks( cast.ToUint64(appOpts.Get(server.FlagMinRetainBlocks)), ), + cometbft.SetHaltHeight(cast.ToUint64(appOpts.Get(server.FlagHaltHeight))), + cometbft.SetHaltTime(cast.ToUint64(appOpts.Get(server.FlagHaltTime))), cometbft.SetInterBlockCache(cache), cometbft.SetIAVLCacheSize( cast.ToInt(appOpts.Get(server.FlagIAVLCacheSize)), From 435de1ab5657d25baa8375da1890884e6ffc1aed Mon Sep 17 00:00:00 2001 From: Fridrik Asmundsson Date: Thu, 16 Jul 2026 14:55:10 +0000 Subject: [PATCH 2/4] test(upgrade): add halt/swap/resume e2e test and nightly job --- .github/workflows/nightly.yml | 34 +- Makefile | 1 + scripts/build/halt-upgrade-test.mk | 47 ++ testing/upgrade/halt-swap-resume-test.sh | 522 +++++++++++++++++++++++ 4 files changed, 602 insertions(+), 2 deletions(-) create mode 100644 scripts/build/halt-upgrade-test.mk create mode 100755 testing/upgrade/halt-swap-resume-test.sh diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 5bd75f572b..ca7160113f 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -77,9 +77,39 @@ jobs: env: GOPATH: /home/runner/go + # Halt/swap/resume upgrade smoke test: a multi-validator devnet halts (by + # height, then by time in a second run), restarts from the same data dirs, + # and must resume with tx load flowing. The local build serves as both the + # old and new beacond, gating the halt/swap/restart mechanics. + test-halt-swap-resume: + runs-on: + labels: ubuntu-24.04-beacon-kit + permissions: + contents: read + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + submodules: recursive + - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 + with: + go-version-file: go.mod + check-latest: true + cache-dependency-path: "**/*.sum" + - name: Install Foundry (cast drives the tx load) + uses: foundry-rs/foundry-toolchain@c7450ba673e133f5ee30098b3b54f444d3a2ca2d # v1.8.0 + - run: make test-halt-swap-resume test-halt-swap-resume-time + env: + GOPATH: /home/runner/go + - name: Upload test logs + if: failure() + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + with: + name: upgrade-test-logs + path: .tmp/upgrade-test/logs + # Post to Slack only when the nightly run fails. notify-slack: - needs: [pipeline, test-deps] + needs: [pipeline, test-deps, test-halt-swap-resume] if: failure() runs-on: labels: ubuntu-24.04-beacon-kit @@ -89,7 +119,7 @@ jobs: - name: Post failure to Slack env: CORE_SLACK_WEBHOOK_URL: ${{ secrets.CORE_SLACK_WEBHOOK_URL }} - TEXT: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} nightly CI failed (pipeline=${{ needs.pipeline.result }}, test-deps=${{ needs.test-deps.result }}) + TEXT: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} nightly CI failed (pipeline=${{ needs.pipeline.result }}, test-deps=${{ needs.test-deps.result }}, test-halt-swap-resume=${{ needs.test-halt-swap-resume.result }}) run: | if [ -z "$CORE_SLACK_WEBHOOK_URL" ]; then echo "CORE_SLACK_WEBHOOK_URL not set; skipping Slack notification." diff --git a/Makefile b/Makefile index 9a13a281dc..8f01191328 100755 --- a/Makefile +++ b/Makefile @@ -8,6 +8,7 @@ include scripts/build/linting.mk include scripts/build/protobuf.mk include scripts/build/release.mk include scripts/build/testing.mk +include scripts/build/halt-upgrade-test.mk include contracts/Makefile include kurtosis/Makefile include scripts/build/help.mk diff --git a/scripts/build/halt-upgrade-test.mk b/scripts/build/halt-upgrade-test.mk new file mode 100644 index 0000000000..f9bf615678 --- /dev/null +++ b/scripts/build/halt-upgrade-test.mk @@ -0,0 +1,47 @@ +#!/usr/bin/make -f + +# Tests a coordinated network upgrade end to end: a local multi-validator devnet halts itself at a configured +# halt point, the beacond binary is swapped, and the chain must resume from the same data directories. Only +# beacond is swapped, each validator's bera-reth execution client keeps running throughout. The test flow is +# documented in testing/upgrade/halt-swap-resume-test.sh. +# +# 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) +# +# Both targets run the local build as both the old and the new beacond, gating the halt/swap/restart mechanics +# rather than cross-version compatibility. Extra script flags pass through HALT_SWAP_RESUME_ARGS, e.g. +# HALT_SWAP_RESUME_ARGS=--no-load to skip the cast-driven tx load. + +# Extra arguments passed through to halt-swap-resume-test.sh, e.g. HALT_SWAP_RESUME_ARGS="--keep --num-vals 7". +HALT_SWAP_RESUME_ARGS ?= + +# The bera-reth release used as each validator's execution client. +BERA_RETH_VERSION ?= v1.4.2 +BERA_RETH_BIN_DIR = /tmp/.halt-upgrade-test +BERA_RETH_PLATFORM := $(shell uname -m | sed 's/arm64/aarch64/')-$(if $(filter Darwin,$(shell uname)),apple-darwin,unknown-linux-gnu) +BERA_RETH_ASSET = bera-reth-$(BERA_RETH_VERSION)-$(BERA_RETH_PLATFORM).tar.gz +BERA_RETH_URL = https://github.com/berachain/bera-reth/releases/download/$(BERA_RETH_VERSION)/$(BERA_RETH_ASSET) +BERA_RETH_BIN = $(BERA_RETH_BIN_DIR)/bera-reth-$(BERA_RETH_VERSION) + +# The tarball comes from the GitHub release over HTTPS and is trusted as-is. The version-stamped mv at the end +# is the commit step, so an interrupted download never masquerades as a valid cached binary. +$(BERA_RETH_BIN): + @mkdir -p $(BERA_RETH_BIN_DIR) + curl -fsSL -o $(BERA_RETH_BIN_DIR)/$(BERA_RETH_ASSET) $(BERA_RETH_URL) + @tar xzf $(BERA_RETH_BIN_DIR)/$(BERA_RETH_ASSET) -C $(BERA_RETH_BIN_DIR) + @mv $(BERA_RETH_BIN_DIR)/bera-reth $@ && chmod +x $@ + @rm -f $(BERA_RETH_BIN_DIR)/$(BERA_RETH_ASSET) + +test-halt-swap-resume: build $(BERA_RETH_BIN) ## halt the devnet at a fixed height, swap beacond, verify it resumes + @OLD_BIN=$(CURDIR)/build/bin/beacond \ + NEW_BIN=$(CURDIR)/build/bin/beacond \ + RETH_BIN=$(BERA_RETH_BIN) \ + ./testing/upgrade/halt-swap-resume-test.sh $(HALT_SWAP_RESUME_ARGS) + +test-halt-swap-resume-time: build $(BERA_RETH_BIN) ## halt on halt-time instead of halt-height, swap, resume + @OLD_BIN=$(CURDIR)/build/bin/beacond \ + NEW_BIN=$(CURDIR)/build/bin/beacond \ + RETH_BIN=$(BERA_RETH_BIN) \ + ./testing/upgrade/halt-swap-resume-test.sh --halt-time-offset 25 $(HALT_SWAP_RESUME_ARGS) + +.PHONY: test-halt-swap-resume test-halt-swap-resume-time diff --git a/testing/upgrade/halt-swap-resume-test.sh b/testing/upgrade/halt-swap-resume-test.sh new file mode 100755 index 0000000000..4749c4488b --- /dev/null +++ b/testing/upgrade/halt-swap-resume-test.sh @@ -0,0 +1,522 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MIT +# +# Tests a coordinated network upgrade end to end: a local devnet of NUM_VALS validators halts itself at a +# configured halt point, every beacond is swapped for a new binary, and the chain must resume from the same +# data directories. Only beacond is swapped, each validator's bera-reth execution client runs throughout. +# +# The test asserts that no validator ever commits past the halt point, that a halted node restarted with the +# halt flag still set refuses to start, and that transactions keep landing both before the halt and after the +# swap. See main() at the bottom for the phase-by-phase flow. +# +# Usage: +# halt-swap-resume-test.sh [--num-vals 4] [--halt-height 8] [--halt-time-offset 0] [--keep] [--no-load] +# +# --halt-time-offset N halts on --halt-time (now + N seconds) instead of --halt-height. +# +# Required environment: +# OLD_BIN beacond the chain starts and halts on +# NEW_BIN beacond the chain resumes on, may equal OLD_BIN to smoke-test the halt/restart mechanics alone +# RETH_BIN bera-reth used as every validator's execution client + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" + +# --------------------------------------------------------------------------------------------------------------- +# Configuration +# --------------------------------------------------------------------------------------------------------------- + +NUM_VALS=7 # validators in the devnet, each paired with its own execution client +HALT_HEIGHT=10 # halt point for --halt-height mode (ignored when HALT_TIME_OFFSET > 0) +HALT_TIME_OFFSET=0 # when > 0, halt on --halt-time now+offset instead; the halt height is derived afterwards +KEEP=0 # keep RUN_DIR after a passing run instead of deleting it +LOAD=1 # run the background tx load and assert txs landed on both sides of the swap +RESUME_BLOCKS=5 # blocks the network must produce past the halt height after the swap + +while [[ $# -gt 0 ]]; do + case "$1" in + --num-vals) NUM_VALS="$2"; shift 2 ;; + --halt-height) HALT_HEIGHT="$2"; shift 2 ;; + --halt-time-offset) HALT_TIME_OFFSET="$2"; shift 2 ;; + --keep) KEEP=1; shift ;; + --no-load) LOAD=0; shift ;; + *) echo "unknown argument: $1" >&2; exit 2 ;; + esac +done + +: "${OLD_BIN:?set OLD_BIN to the current-fork beacond binary}" +: "${NEW_BIN:?set NEW_BIN to the upgraded beacond binary}" +: "${RETH_BIN:?set RETH_BIN to a bera-reth binary}" + +CHAIN_SPEC_ARG="--beacon-kit.chain-spec devnet" # chain spec passed to every beacond invocation +CHAIN_ID="beacond-2061" # CometBFT chain id of the devnet +DEPOSIT_AMOUNT=32000000000 # premined deposit per validator, in gwei +WITHDRAWAL_ADDRESS=0x20f33ce90a13a4b5e7697e3544c3083b8f8a51d4 # withdrawal credentials for those deposits + +# Prefunded devnet account (same keys as kurtosis/src/constants.star). +LOAD_ADDR=0x20f33ce90a13a4b5e7697e3544c3083b8f8a51d4 # tx-load sender +LOAD_KEY=fffdbb37105441e14b0ee6330d855d8504ff39e705c3afa8f859ac9865f99306 # its private key +LOAD_RECIPIENT=0x000000000000000000000000000000000000dEaD # transfer recipient +LOAD_INTERVAL=0.4 # seconds between transfers + +ETH_GENESIS="$REPO_ROOT/testing/files/eth-genesis.json" # execution genesis every EL is initialized from +JWT="$REPO_ROOT/testing/files/jwt.hex" # engine-API JWT secret shared by all CL/EL pairs +KZG="$REPO_ROOT/testing/files/kzg-trusted-setup.json" # KZG trusted setup for beacond +RUN_DIR="${RUN_DIR:-$REPO_ROOT/.tmp/upgrade-test}" # node homes and datadirs, overridable via env +LOG_DIR="$RUN_DIR/logs" # per-node and per-phase log files +RUN_DIR_MARKER=".halt-swap-resume-test" # marks RUN_DIR as safe for this harness to delete + +# Budget for the network to reach the halt point: fixed startup slack plus consensus time. +if ((HALT_TIME_OFFSET > 0)); then + HALT_TIMEOUT_SECS=$((120 + HALT_TIME_OFFSET)) +else + HALT_TIMEOUT_SECS=$((120 + HALT_HEIGHT * 5)) +fi +RESUME_TIMEOUT_SECS=240 # budget for all nodes to reach TARGET_HEIGHT after the swap + +# Per-node port layout, node i: +cl_rpc_port() { echo $((26657 + $1 * 10)); } +cl_p2p_port() { echo $((26656 + $1 * 10)); } +el_http_port() { echo $((8545 + $1 * 10)); } +el_auth_port() { echo $((8551 + $1 * 10)); } +el_p2p_port() { echo $((30303 + $1)); } + +# --------------------------------------------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------------------------------------------- + +log() { printf '\n==> %s\n' "$*"; } +fail() { echo "$*" >&2; exit 1; } + +# wait_for +wait_for() { + local deadline=$1 desc=$2; shift 2 + local start elapsed + start=$(date +%s) + until "$@"; do + elapsed=$(( $(date +%s) - start )) + if (( elapsed > deadline )); then + echo "timeout after ${deadline}s waiting for: $desc" >&2 + return 1 + fi + sleep 1 + done +} + +port_free() { ! (exec 3<>"/dev/tcp/127.0.0.1/$1") 2>/dev/null; } +proc_gone() { ! kill -0 "$1" 2>/dev/null; } + +# el_rpc [params-json] +el_rpc() { + curl -sf "http://127.0.0.1:$(el_http_port "$1")" -H 'Content-Type: application/json' \ + -d "{\"jsonrpc\":\"2.0\",\"method\":\"$2\",\"params\":${3:-[]},\"id\":1}" +} +el_ready() { el_rpc "$1" eth_chainId >/dev/null 2>&1; } + +cl_status() { curl -sf "http://127.0.0.1:$(cl_rpc_port "$1")/status"; } +node_height() { cl_status "$1" | jq -r '.result.sync_info.latest_block_height' 2>/dev/null || echo 0; } +node_p2p_version() { cl_status "$1" | jq -r '.result.node_info.protocol_version.p2p' 2>/dev/null || echo "?"; } +node_resumed() { + local h + h="$(node_height "$1")" + [[ "$h" =~ ^[0-9]+$ ]] && (( h >= TARGET_HEIGHT )) +} + +# start_cl [extra args...] — starts a validator, prints its pid. +start_cl() { + local i=$1 bin=$2 logfile=$3; shift 3 + "$bin" start $CHAIN_SPEC_ARG --home "$RUN_DIR/cl$i" \ + --beacon-kit.engine.rpc-dial-url "http://127.0.0.1:$(el_auth_port "$i")" \ + --beacon-kit.engine.jwt-secret-path "$JWT" --beacon-kit.kzg.trusted-setup-path "$KZG" \ + --beacon-kit.logger.log-level info "$@" >"$LOG_DIR/$logfile" 2>&1 & + echo $! +} + +# Validators that exited after logging the halt line, respectively ones still running. +halted_count() { + local n=0 k + for ((k = 0; k < NUM_VALS; k++)); do + if ! kill -0 "${CL_PIDS[k]}" 2>/dev/null && + grep -q "halting node per configuration" "$LOG_DIR/cl$k.old.log"; then + n=$((n + 1)) + fi + done + echo "$n" +} +any_halted() { (($(halted_count) >= 1)); } +alive_count() { + local n=0 k + for ((k = 0; k < NUM_VALS; k++)); do + if kill -0 "${CL_PIDS[k]}" 2>/dev/null; then n=$((n + 1)); fi + done + echo "$n" +} + +last_committed_height() { + # `|| true` so a log without a match yields "" under pipefail; the caller fails loudly on an empty result. + grep -o "Committed state.*height=[0-9]*" "$LOG_DIR/cl$1.old.log" | grep -o "[0-9]*$" | tail -1 || true +} + +# el_tx_count_range — load txs in el0 blocks first..last, filtered by sender since every block +# also carries a system transaction. +el_tx_count_range() { + local total=0 h n + for ((h = $1; h <= $2; h++)); do + n="$(el_rpc 0 eth_getBlockByNumber "[\"$(printf '0x%x' "$h")\",true]" | jq -r --arg from "$LOAD_ADDR" \ + '[.result.transactions[] | select(.from == $from)] | length' 2>/dev/null)" || n=0 + total=$((total + ${n:-0})) + done + echo "$total" +} + +PIDS=() +cleanup() { + local code=$? + for pid in "${PIDS[@]:-}"; do + kill "$pid" >/dev/null 2>&1 || true + done + # Bounded shutdown, escalating to SIGKILL so one wedged child cannot hang the run (or its CI job). + local waited=0 alive=1 + while ((alive && waited < 15)); do + alive=0 + for pid in "${PIDS[@]:-}"; do + if kill -0 "$pid" 2>/dev/null; then alive=1; fi + done + if ((alive)); then + sleep 1 + waited=$((waited + 1)) + fi + done + for pid in "${PIDS[@]:-}"; do + kill -9 "$pid" >/dev/null 2>&1 || true + done + wait >/dev/null 2>&1 || true + if [[ $code -ne 0 ]]; then + echo "FAILED (exit $code). Logs are in $LOG_DIR" >&2 + elif [[ $KEEP -eq 0 ]]; then + remove_run_dir + fi +} + +# --------------------------------------------------------------------------------------------------------------- +# Setup +# --------------------------------------------------------------------------------------------------------------- + +check_preconditions() { + local bin tool + for bin in "$OLD_BIN" "$NEW_BIN" "$RETH_BIN"; do + [[ -x "$bin" ]] || fail "not executable: $bin" + done + for tool in jq curl; do + command -v "$tool" >/dev/null || fail "missing tool: $tool" + done + if ((LOAD)) && ! command -v cast >/dev/null; then + fail "missing tool: cast (foundry) is required for tx load, or pass --no-load" + fi + + # Node 0 uses the standard devnet ports, so a leftover devnet (or --keep run) would answer our readiness + # probes and the test would silently run against the wrong chain. Fail fast if any port is already taken. + local busy="" i port + for ((i = 0; i < NUM_VALS; i++)); do + for port in "$(cl_rpc_port "$i")" "$(cl_p2p_port "$i")" \ + "$(el_http_port "$i")" "$(el_auth_port "$i")" "$(el_p2p_port "$i")"; do + port_free "$port" || busy+=" $port" + done + done + [[ -z "$busy" ]] || fail "ports already in use:$busy (stop the running devnet or previous --keep run)" +} + +# RUN_DIR is env-overridable and gets wiped, so only delete a directory this harness marked as its own, +# the default location under the repo's .tmp, or an empty one. Anything else is not ours to destroy. +remove_run_dir() { + [[ -e "$RUN_DIR" ]] || return 0 + if [[ ! -f "$RUN_DIR/$RUN_DIR_MARKER" && "$RUN_DIR" != "$REPO_ROOT/.tmp/"* ]] && + [[ -n "$(ls -A "$RUN_DIR" 2>/dev/null)" ]]; then + fail "refusing to delete non-empty $RUN_DIR: no $RUN_DIR_MARKER marker, was it created by this harness?" + fi + rm -rf "$RUN_DIR" +} + +prepare_run_dir() { + log "Preparing run directory $RUN_DIR" + remove_run_dir + mkdir -p "$RUN_DIR" "$LOG_DIR" + touch "$RUN_DIR/$RUN_DIR_MARKER" +} + +# Every validator adds a premined deposit, cl0 aggregates them into the final genesis.json (copied back to +# every home) and writes the eth-genesis with the deposit storage set. +run_genesis_ceremony() { + log "Genesis ceremony for $NUM_VALS validators" + local i + for ((i = 0; i < NUM_VALS; i++)); do + "$OLD_BIN" init "val$i" --chain-id "$CHAIN_ID" --home "$RUN_DIR/cl$i" \ + $CHAIN_SPEC_ARG >"$LOG_DIR/init-cl$i.log" 2>&1 + "$OLD_BIN" genesis add-premined-deposit "$DEPOSIT_AMOUNT" "$WITHDRAWAL_ADDRESS" \ + --home "$RUN_DIR/cl$i" $CHAIN_SPEC_ARG >>"$LOG_DIR/init-cl$i.log" 2>&1 + done + { + for ((i = 1; i < NUM_VALS; i++)); do + cp "$RUN_DIR/cl$i/config/premined-deposits/premined-deposit"* "$RUN_DIR/cl0/config/premined-deposits/" + done + "$OLD_BIN" genesis collect-premined-deposits --home "$RUN_DIR/cl0" $CHAIN_SPEC_ARG + "$OLD_BIN" genesis set-deposit-storage "$ETH_GENESIS" --home "$RUN_DIR/cl0" $CHAIN_SPEC_ARG + "$OLD_BIN" genesis execution-payload "$RUN_DIR/cl0/eth-genesis.json" --home "$RUN_DIR/cl0" $CHAIN_SPEC_ARG + for ((i = 1; i < NUM_VALS; i++)); do + cp "$RUN_DIR/cl0/config/genesis.json" "$RUN_DIR/cl$i/config/genesis.json" + done + } >"$LOG_DIR/genesis-ceremony.log" 2>&1 +} + +wire_cometbft_configs() { + log "Wiring CometBFT configs (ports and persistent peers)" + local i j peers cfg node_ids=() + for ((i = 0; i < NUM_VALS; i++)); do + node_ids[i]="$("$OLD_BIN" comet show-node-id --home "$RUN_DIR/cl$i")" + done + for ((i = 0; i < NUM_VALS; i++)); do + peers="" + for ((j = 0; j < NUM_VALS; j++)); do + [[ $i -eq $j ]] && continue + peers+="${peers:+,}${node_ids[j]}@127.0.0.1:$(cl_p2p_port "$j")" + done + cfg="$RUN_DIR/cl$i/config/config.toml" + sed -i.bak \ + -e "s|^laddr = \"tcp://127.0.0.1:26657\"|laddr = \"tcp://127.0.0.1:$(cl_rpc_port "$i")\"|" \ + -e "s|^laddr = \"tcp://0.0.0.0:26656\"|laddr = \"tcp://127.0.0.1:$(cl_p2p_port "$i")\"|" \ + -e "s|^persistent_peers = \"\"|persistent_peers = \"$peers\"|" \ + -e 's|^addr_book_strict = true|addr_book_strict = false|' \ + -e 's|^allow_duplicate_ip = false|allow_duplicate_ip = true|' \ + -e 's|^pprof_laddr = .*|pprof_laddr = ""|' \ + "$cfg" + rm -f "$cfg.bak" + done +} + +start_execution_clients() { + log "Starting $NUM_VALS execution clients (bera-reth)" + local i + for ((i = 0; i < NUM_VALS; i++)); do + "$RETH_BIN" init --datadir "$RUN_DIR/el$i" --chain "$RUN_DIR/cl0/eth-genesis.json" >"$LOG_DIR/el$i.log" 2>&1 + "$RETH_BIN" node --datadir "$RUN_DIR/el$i" --chain "$RUN_DIR/cl0/eth-genesis.json" \ + --http --http.addr 127.0.0.1 --http.port "$(el_http_port "$i")" --http.api admin,eth,net,web3 \ + --authrpc.addr 127.0.0.1 --authrpc.port "$(el_auth_port "$i")" --authrpc.jwtsecret "$JWT" \ + --port "$(el_p2p_port "$i")" --discovery.port "$(el_p2p_port "$i")" \ + --txpool.max-account-slots 1000 --ipcdisable >>"$LOG_DIR/el$i.log" 2>&1 & + PIDS+=($!) + done + for ((i = 0; i < NUM_VALS; i++)); do + wait_for 60 "el$i JSON-RPC endpoint" el_ready "$i" + done + + log "Peering execution clients" + local enode0 + enode0="$(el_rpc 0 admin_nodeInfo | jq -r '.result.enode' | sed -E 's/@[^:@?]+:/@127.0.0.1:/')" + for ((i = 1; i < NUM_VALS; i++)); do + el_rpc "$i" admin_addPeer "[\"$enode0\"]" >/dev/null + done +} + +# Submits a steady stream of transfers to el0 for the rest of the run. The pending nonce is re-fetched per send +# so the stream self-heals after a rejected or dropped transaction. The execution clients stay up across the +# halt, so the load also exercises EL mempool carryover across the consensus binary swap. +start_tx_load() { + log "Starting tx load (transfers to el0 every ${LOAD_INTERVAL}s)" + local rpc="http://127.0.0.1:$(el_http_port 0)" + ( + while :; do + nonce="$(cast nonce --block pending "$LOAD_ADDR" --rpc-url "$rpc" 2>/dev/null)" || nonce="" + if [[ -n "$nonce" ]]; then + cast send "$LOAD_RECIPIENT" --value 1 --private-key "$LOAD_KEY" --nonce "$nonce" \ + --gas-limit 21000 --gas-price 10gwei --async --rpc-url "$rpc" \ + >/dev/null 2>>"$LOG_DIR/load.log" || true + fi + sleep "$LOAD_INTERVAL" + done + ) & + PIDS+=($!) + # Drop the job from bash's job table so cleanup's SIGTERM does not print a "Terminated: 15" job notice. + disown +} + +# --------------------------------------------------------------------------------------------------------------- +# Phases +# --------------------------------------------------------------------------------------------------------------- + +phase1_halt_on_old() { + # Halt time is computed here rather than at flag parsing so genesis and EL setup time does not eat the offset. + if ((HALT_TIME_OFFSET > 0)); then + HALT_FLAGS=(--halt-time $(($(date +%s) + HALT_TIME_OFFSET))) + else + HALT_FLAGS=(--halt-height "$HALT_HEIGHT") + fi + + log "Phase 1: starting $NUM_VALS validators on OLD binary with ${HALT_FLAGS[*]}" + local i + CL_PIDS=() + for ((i = 0; i < NUM_VALS; i++)); do + CL_PIDS[i]="$(start_cl "$i" "$OLD_BIN" "cl$i.old.log" "${HALT_FLAGS[@]}")" + PIDS+=("${CL_PIDS[i]}") + done + if ((LOAD)); then start_tx_load; fi + + log "Waiting for the first validator to self-halt (timeout ${HALT_TIMEOUT_SECS}s)" + wait_for "$HALT_TIMEOUT_SECS" "a validator to exit at the halt point" any_halted + stop_stragglers + derive_and_check_halt_height +} + +# Committing the halt block requires >2/3 precommits, but a validator exits ~immediately after its own commit, +# racing the gossip of the final precommits. So any subset of validators can wedge one block short of the halt +# height with their peers gone. That mirrors a real coordinated halt: the chain stops at the halt height, some +# nodes self-halt, and operators stop the wedged rest before swapping binaries. Success means at least one node +# self-halted and NO node committed past the halt height (asserted in derive_and_check_halt_height). +stop_stragglers() { + # Validators only ever exit here, so instead of a fixed sleep, wait until the set of live processes has + # been stable for a while: fast runs move on as soon as everyone halted, slow runners get more room. + log "Waiting for the remaining validators to self-halt" + local i prev_alive alive stable_since + prev_alive="$(alive_count)" + stable_since=$SECONDS + while ((prev_alive > 0 && SECONDS - stable_since < 15)); do + sleep 1 + alive="$(alive_count)" + if ((alive != prev_alive)); then + prev_alive=$alive + stable_since=$SECONDS + fi + done + + STRAGGLERS=() + for ((i = 0; i < NUM_VALS; i++)); do + if kill -0 "${CL_PIDS[i]}" 2>/dev/null; then + STRAGGLERS+=("cl$i") + kill "${CL_PIDS[i]}" 2>/dev/null || true + fi + done + for ((i = 0; i < NUM_VALS; i++)); do + wait_for 30 "cl$i process to exit" proc_gone "${CL_PIDS[i]}" + done +} + +derive_and_check_halt_height() { + local i h + + # In halt-time mode the halt block is wherever the chain was when the halt time passed. Every self-halted + # validator must have stopped at the same height, which becomes the halt height for the remaining phases. + if ((HALT_TIME_OFFSET > 0)); then + HALT_HEIGHT="" + for ((i = 0; i < NUM_VALS; i++)); do + grep -q "halting node per configuration" "$LOG_DIR/cl$i.old.log" || continue + h="$(last_committed_height "$i")" + [[ -n "$h" ]] || fail "could not read a committed height for self-halted cl$i" + if [[ -n "$HALT_HEIGHT" && "$h" != "$HALT_HEIGHT" ]]; then + fail "self-halted nodes disagree on the halt height ($h vs $HALT_HEIGHT)" + fi + HALT_HEIGHT="$h" + done + log "Halt time reached at height $HALT_HEIGHT" + fi + + for ((i = 0; i < NUM_VALS; i++)); do + h="$(last_committed_height "$i")" + [[ -n "$h" ]] || fail "could not read a committed height for cl$i from its log," \ + "did the 'Committed state' log line change format?" + ((h <= HALT_HEIGHT)) || fail "cl$i committed height $h past the halt height $HALT_HEIGHT" + done + + if ((${#STRAGGLERS[@]} > 0)); then + log "$(halted_count)/$NUM_VALS validators self-halted at height $HALT_HEIGHT," \ + "stopped stragglers: ${STRAGGLERS[*]} (they catch up after the swap)" + else + log "All $NUM_VALS validators halted after committing height $HALT_HEIGHT" + fi +} + +# A node that halted and is restarted with the halt flag still set must refuse to enter consensus instead of +# advancing or crash-looping. +phase2_restart_with_flag_refused() { + local i node="" pid + for ((i = 0; i < NUM_VALS; i++)); do + if grep -q "halting node per configuration" "$LOG_DIR/cl$i.old.log"; then + node=$i + break + fi + done + log "Phase 2: restarting cl$node with ${HALT_FLAGS[*]} still set (must refuse to start)" + pid="$(start_cl "$node" "$OLD_BIN" "cl$node.haltcheck.log" "${HALT_FLAGS[@]}")" + PIDS+=("$pid") + wait_for 90 "cl$node to refuse startup at the halt point" proc_gone "$pid" + grep -q "reached the configured halt point" "$LOG_DIR/cl$node.haltcheck.log" || + fail "cl$node restarted past the halt point instead of refusing, see $LOG_DIR/cl$node.haltcheck.log" +} + +phase3_resume_on_new() { + log "Phase 3: restarting all validators on NEW binary (same data dirs, no halt flags)" + local i + CL_PIDS=() + for ((i = 0; i < NUM_VALS; i++)); do + CL_PIDS[i]="$(start_cl "$i" "$NEW_BIN" "cl$i.new.log")" + PIDS+=("${CL_PIDS[i]}") + done + + TARGET_HEIGHT=$((HALT_HEIGHT + RESUME_BLOCKS)) + log "Waiting for all nodes to advance past height $TARGET_HEIGHT (timeout ${RESUME_TIMEOUT_SECS}s)" + for ((i = 0; i < NUM_VALS; i++)); do + wait_for "$RESUME_TIMEOUT_SECS" "cl$i to reach height $TARGET_HEIGHT" node_resumed "$i" + done + for ((i = 0; i < NUM_VALS; i++)); do + if grep -q "CONSENSUS FAILURE" "$LOG_DIR/cl$i.new.log"; then + fail "cl$i hit a consensus failure after the swap, see $LOG_DIR/cl$i.new.log" + fi + done + + EL0_HEIGHT="$(($(el_rpc 0 eth_blockNumber | jq -r '.result')))" + if ((LOAD)); then + PRE_SWAP_TXS="$(el_tx_count_range 1 "$HALT_HEIGHT")" + POST_SWAP_TXS="$(el_tx_count_range $((HALT_HEIGHT + 1)) "$EL0_HEIGHT")" + ((PRE_SWAP_TXS > 0)) || fail "no transactions were included before the halt, see $LOG_DIR/load.log" + ((POST_SWAP_TXS > 0)) || fail "no transactions were included after the swap, see $LOG_DIR/load.log" + fi +} + +print_result() { + log "RESULT" + local i mined + for ((i = 0; i < NUM_VALS; i++)); do + echo " cl$i: height=$(node_height "$i") p2p_version=$(node_p2p_version "$i")" + done + echo " el0: block=$EL0_HEIGHT" + if ((LOAD)); then + mined="$(($(el_rpc 0 eth_getTransactionCount "[\"$LOAD_ADDR\",\"latest\"]" | jq -r '.result')))" + echo " load: $mined txs mined in total: $PRE_SWAP_TXS in blocks 1..$HALT_HEIGHT (old binary)," \ + "$POST_SWAP_TXS in blocks $((HALT_HEIGHT + 1))..$EL0_HEIGHT (new binary)" + fi + echo + echo "PASS: the network halted at height $HALT_HEIGHT on the old binary (${#STRAGGLERS[@]} straggler(s))," \ + "resumed from the same data dirs on the new binary, and all $NUM_VALS validators advanced past" \ + "height $TARGET_HEIGHT." +} + +# --------------------------------------------------------------------------------------------------------------- +# Main +# --------------------------------------------------------------------------------------------------------------- + +main() { + check_preconditions + trap cleanup EXIT + + prepare_run_dir + run_genesis_ceremony + wire_cometbft_configs + start_execution_clients + + phase1_halt_on_old + phase2_restart_with_flag_refused + phase3_resume_on_new + + print_result +} + +main From 571e1db2f735a3c8ccf6eee7a94dae525cee0a76 Mon Sep 17 00:00:00 2001 From: Fridrik Asmundsson Date: Thu, 16 Jul 2026 15:55:13 +0000 Subject: [PATCH 3/4] chore(lint): fix lint --- consensus/cometbft/service/service.go | 46 +++++++++++++----------- scripts/build/halt-upgrade-test.mk | 2 +- testing/upgrade/halt-swap-resume-test.sh | 2 +- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/consensus/cometbft/service/service.go b/consensus/cometbft/service/service.go index 3609dd6d4c..3fbe7689cf 100644 --- a/consensus/cometbft/service/service.go +++ b/consensus/cometbft/service/service.go @@ -120,7 +120,6 @@ type Service struct { syncingToHeight int64 } -//nolint:funlen // node assembly requires many sequential setup steps func NewService( logger *phuslu.Logger, db dbm.DB, @@ -174,25 +173,7 @@ func NewService( lastBlockHeight := s.lastBlockHeight() s.syncingToHeight = lastBlockHeight - // Seed the finalized-block tracking from the last committed block so the halt checks hold across restarts. - // Blocks committed by binaries that predate the populated commit header carry a zero timestamp, which - // leaves the halt-time check unseeded until the next commit. - s.finalizedHeight = lastBlockHeight - if lastBlockHeight > 0 { - rms, ok := s.sm.GetCommitMultiStore().(*rootmulti.Store) - if !ok { - panic("failed loading last committed block time: unexpected commit multi-store type") - } - - ci, ciErr := rms.GetCommitInfo(lastBlockHeight) - if ciErr != nil { - panic(fmt.Errorf("failed loading commit info at height %d: %w", lastBlockHeight, ciErr)) - } - if ci == nil { - panic(fmt.Errorf("failed loading commit info at height %d: empty commit info", lastBlockHeight)) - } - s.finalizedTime = ci.Timestamp - } + s.seedFinalizedBlock(lastBlockHeight) // Make sure that SBT consensus parameters are duly set when the node restart. // Note that we can't rely on genesis.json having these parameters set right @@ -224,6 +205,31 @@ func NewService( return s } +// seedFinalizedBlock seeds the finalized-block tracking from the last committed block so the halt checks hold +// across restarts. The commit-info load is skipped unless a halt flag is set, the first FinalizeBlock refreshes +// these fields before anything else reads them. Blocks committed by binaries that predate the populated commit +// header carry a zero timestamp, which leaves the halt-time check unseeded until the next commit. +func (s *Service) seedFinalizedBlock(lastBlockHeight int64) { + s.finalizedHeight = lastBlockHeight + if lastBlockHeight <= 0 || (s.haltHeight == 0 && s.haltTime == 0) { + return + } + + rms, ok := s.sm.GetCommitMultiStore().(*rootmulti.Store) + if !ok { + panic("failed loading last committed block time: unexpected commit multi-store type") + } + + ci, ciErr := rms.GetCommitInfo(lastBlockHeight) + if ciErr != nil { + panic(fmt.Errorf("failed loading commit info at height %d: %w", lastBlockHeight, ciErr)) + } + if ci == nil { + panic(fmt.Errorf("failed loading commit info at height %d: empty commit info", lastBlockHeight)) + } + s.finalizedTime = ci.Timestamp +} + // TODO: Move nodeKey into being created within the function. func (s *Service) Start( ctx context.Context, diff --git a/scripts/build/halt-upgrade-test.mk b/scripts/build/halt-upgrade-test.mk index f9bf615678..2a87ba8f1c 100644 --- a/scripts/build/halt-upgrade-test.mk +++ b/scripts/build/halt-upgrade-test.mk @@ -21,7 +21,7 @@ BERA_RETH_BIN_DIR = /tmp/.halt-upgrade-test BERA_RETH_PLATFORM := $(shell uname -m | sed 's/arm64/aarch64/')-$(if $(filter Darwin,$(shell uname)),apple-darwin,unknown-linux-gnu) BERA_RETH_ASSET = bera-reth-$(BERA_RETH_VERSION)-$(BERA_RETH_PLATFORM).tar.gz BERA_RETH_URL = https://github.com/berachain/bera-reth/releases/download/$(BERA_RETH_VERSION)/$(BERA_RETH_ASSET) -BERA_RETH_BIN = $(BERA_RETH_BIN_DIR)/bera-reth-$(BERA_RETH_VERSION) +BERA_RETH_BIN = $(BERA_RETH_BIN_DIR)/bera-reth-$(BERA_RETH_VERSION)-$(BERA_RETH_PLATFORM) # The tarball comes from the GitHub release over HTTPS and is trusted as-is. The version-stamped mv at the end # is the commit step, so an interrupted download never masquerades as a valid cached binary. diff --git a/testing/upgrade/halt-swap-resume-test.sh b/testing/upgrade/halt-swap-resume-test.sh index 4749c4488b..fca51219f8 100755 --- a/testing/upgrade/halt-swap-resume-test.sh +++ b/testing/upgrade/halt-swap-resume-test.sh @@ -337,7 +337,7 @@ start_tx_load() { ) & PIDS+=($!) # Drop the job from bash's job table so cleanup's SIGTERM does not print a "Terminated: 15" job notice. - disown + disown || true } # --------------------------------------------------------------------------------------------------------------- From 8e93d5f89adfe2c95fb9abd2d63fcfad3da04fce Mon Sep 17 00:00:00 2001 From: Fridrik Asmundsson Date: Tue, 21 Jul 2026 11:02:49 +0000 Subject: [PATCH 4/4] Address PR comments --- consensus/cometbft/service/commit.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/consensus/cometbft/service/commit.go b/consensus/cometbft/service/commit.go index 4f8ee01b52..25126a82f0 100644 --- a/consensus/cometbft/service/commit.go +++ b/consensus/cometbft/service/commit.go @@ -128,12 +128,9 @@ func (s *Service) haltIfReached() { time.Sleep(haltGracePeriod) // Signal our own process so the node's regular shutdown path runs, the same mechanism a cosmos-sdk baseapp - // uses for halt-height. - p, err := os.FindProcess(os.Getpid()) - if err != nil { - os.Exit(0) - } - if err = p.Signal(syscall.SIGINT); err != nil { + // uses for halt-height. FindProcess never fails on Unix. + p, _ := os.FindProcess(os.Getpid()) + if err := p.Signal(syscall.SIGINT); err != nil { if err = p.Signal(syscall.SIGTERM); err != nil { os.Exit(0) }