From c006bf27b3064516849859d1b42be85ae92098b4 Mon Sep 17 00:00:00 2001 From: Mathieu Hofman <86499+mhofman@users.noreply.github.com> Date: Tue, 19 Sep 2023 16:58:53 -0700 Subject: [PATCH] fix: halt-height behavior is not deterministic (#305) Port of cosmos/cosmos-sdk#16639 Co-authored-by: yihuang --- baseapp/abci.go | 63 +++++++++++++++----------------------------- baseapp/abci_test.go | 59 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 42 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 9b20e687fdee..92947aab419d 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -5,10 +5,8 @@ import ( "encoding/json" "errors" "fmt" - "os" "sort" "strings" - "syscall" "time" abcitypes "github.com/cometbft/cometbft/abci/types" @@ -180,6 +178,8 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg )) } + app.checkHalt(req.Header.Height, req.Header.Time) + if err := app.validateHeight(req); err != nil { panic(err) } @@ -355,6 +355,23 @@ func (app *BaseApp) deliverTxWithoutEventHistory(req abci.RequestDeliverTx) (res } } +// checkHalt checkes if height or time exceeds halt-height or halt-time respectively. +func (app *BaseApp) checkHalt(height int64, time time.Time) { + var halt bool + switch { + case app.haltHeight > 0 && uint64(height) > app.haltHeight: + halt = true + + case app.haltTime > 0 && time.Unix() > int64(app.haltTime): + halt = true + } + + if halt { + app.logger.Info("halt per configuration", "height", app.haltHeight, "time", app.haltTime) + panic(errors.New("halt application")) + } +} + // 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 abci.ResponseCommit. Commit will set the check state based on the @@ -411,51 +428,13 @@ func (app *BaseApp) CommitWithoutSnapshot() (_res abci.ResponseCommit, snapshotH // empty/reset the deliver state app.deliverState = nil - var halt bool - - switch { - case app.haltHeight > 0 && uint64(header.Height) >= app.haltHeight: - halt = true - - case app.haltTime > 0 && header.Time.Unix() >= int64(app.haltTime): - halt = true - } - - if halt { - // Halt the binary and allow Tendermint to receive the ResponseCommit - // response with the commit ID hash. This will allow the node to successfully - // restart and process blocks assuming the halt configuration has been - // reset or moved to a more distant value. - app.halt() + if app.snapshotManager.ShouldTakeSnapshot(header.Height) { + snapshotHeight = header.Height } - go app.snapshotManager.SnapshotIfApplicable(header.Height) - return res } -// halt attempts to gracefully shutdown the node via SIGINT and SIGTERM falling -// back on os.Exit if both fail. -func (app *BaseApp) halt() { - app.logger.Info("halting node per configuration", "height", app.haltHeight, "time", app.haltTime) - - p, err := os.FindProcess(os.Getpid()) - if err == nil { - // attempt cascading signals in case SIGINT fails (os dependent) - sigIntErr := p.Signal(syscall.SIGINT) - sigTermErr := p.Signal(syscall.SIGTERM) - - if sigIntErr == nil || sigTermErr == nil { - return - } - } - - // Resort to exiting immediately if the process could not be found or killed - // via SIGINT/SIGTERM signals. - app.logger.Info("failed to send SIGINT/SIGTERM; exiting...") - os.Exit(0) -} - // Snapshot takes a snapshot of the current state and prunes any old snapshottypes. // It should be started as a goroutine func (app *BaseApp) Snapshot(height int64) { diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 8c401daea279..2f1e24fdffb2 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -2,7 +2,10 @@ package baseapp_test import ( "encoding/json" + "fmt" + "strings" "testing" + "time" abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1" @@ -2804,3 +2807,59 @@ func (ps *paramStore) Get(_ sdk.Context, key []byte, ptr interface{}) { panic(err) } } + +func TestABCI_HaltChain(t *testing.T) { + logger := defaultLogger() + db := dbm.NewMemDB() + name := t.Name() + + testCases := []struct { + name string + haltHeight uint64 + haltTime uint64 + blockHeight int64 + blockTime int64 + expHalt bool + }{ + {"default", 0, 0, 10, 0, false}, + {"halt-height-edge", 10, 0, 10, 0, false}, + {"halt-height", 10, 0, 11, 0, true}, + {"halt-time-edge", 0, 10, 1, 10, false}, + {"halt-time", 0, 10, 1, 11, true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + defer func() { + rec := recover() + var err error + if rec != nil { + err = rec.(error) + } + if !tc.expHalt { + require.NoError(t, err) + } else { + require.Error(t, err) + require.True(t, strings.HasPrefix(err.Error(), "halt application")) + } + }() + + app := NewBaseApp( + name, logger, db, nil, + SetHaltHeight(tc.haltHeight), + SetHaltTime(tc.haltTime), + ) + + app.InitChain(abci.RequestInitChain{ + InitialHeight: tc.blockHeight, + }) + + app.BeginBlock(abci.RequestBeginBlock{ + Header: tmproto.Header{ + Height: tc.blockHeight, + Time: time.Unix(tc.blockTime, 0), + }, + }) + }) + } +}