From bcd2a9fb84688be87a2e54202e1ff303d8a082cd Mon Sep 17 00:00:00 2001 From: yihuang Date: Tue, 27 Jun 2023 21:41:01 +0800 Subject: [PATCH] fix: halt-height behavior is not deterministic (#16639) Co-authored-by: Aleksandr Bezobchuk (cherry picked from commit c0e9e8ce85ecfc35967afeca7e0ebb7ff28f14f3) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 23 ++++++++++++++++ baseapp/abci.go | 63 ++++++++++++++++---------------------------- baseapp/abci_test.go | 40 ++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d867ae25de17..431f368b3306 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,29 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/staking) [#16324](https://github.com/cosmos/cosmos-sdk/pull/16324) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error`. Notable changes: * `Validator` method now returns `types.ErrNoValidatorFound` instead of `nil` when not found. +<<<<<<< HEAD +======= +* (x/distribution) [#16440](https://github.com/cosmos/cosmos-sdk/pull/16440) use collections for `DelegatorWithdrawAddresState`: + * remove `Keeper`: `SetDelegatorWithdrawAddr`, `DeleteDelegatorWithdrawAddr`, `IterateDelegatorWithdrawAddrs`. +* (x/distribution) [#16459](https://github.com/cosmos/cosmos-sdk/pull/16459) use collections for `ValidatorCurrentRewards` state management: + * remove `Keeper`: `IterateValidatorCurrentRewards`, `GetValidatorCurrentRewards`, `SetValidatorCurrentRewards`, `DeleteValidatorCurrentRewards` +* (x/authz) [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`. +* (x/distribution) [#16483](https://github.com/cosmos/cosmos-sdk/pull/16483) use collections for `DelegatorStartingInfo` state management: + * remove `Keeper`: `IterateDelegatorStartingInfo`, `GetDelegatorStartingInfo`, `SetDelegatorStartingInfo`, `DeleteDelegatorStartingInfo`, `HasDelegatorStartingInfo` +* (x/distribution) [#16571](https://github.com/cosmos/cosmos-sdk/pull/16571) use collections for `ValidatorAccumulatedCommission` state management: + * remove `Keeper`: `IterateValidatorAccumulatedCommission`, `GetValidatorAccumulatedCommission`, `SetValidatorAccumulatedCommission`, `DeleteValidatorAccumulatedCommission` +* (x/distribution) [#16590](https://github.com/cosmos/cosmos-sdk/pull/16590) use collections for `ValidatorOutstandingRewards` state management: + * remove `Keeper`: `IterateValidatorOutstandingRewards`, `GetValidatorOutstandingRewards`, `SetValidatorOutstandingRewards`, `DeleteValidatorOutstandingRewards` +* (x/distribution) [#16607](https://github.com/cosmos/cosmos-sdk/pull/16607) use collections for `ValidatorHistoricalRewards` state management: + * remove `Keeper`: `IterateValidatorHistoricalRewards`, `GetValidatorHistoricalRewards`, `SetValidatorHistoricalRewards`, `DeleteValidatorHistoricalRewards`, `DeleteValidatorHistoricalReward`, `DeleteAllValidatorHistoricalRewards` + +### Bug Fixes + +* [#16547](https://github.com/cosmos/cosmos-sdk/pull/16547) Ensure a transaction's gas limit cannot exceed the block gas limit. +* (x/auth/types) [#16554](https://github.com/cosmos/cosmos-sdk/pull/16554) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. +* (baseapp) [#16613](https://github.com/cosmos/cosmos-sdk/pull/16613) Ensure each message in a transaction has a registered handler, otherwise `CheckTx` will fail. +* [#16639](https://github.com/cosmos/cosmos-sdk/pull/16639) Make sure we don't execute blocks beyond the halt height. +>>>>>>> c0e9e8ce8 (fix: halt-height behavior is not deterministic (#16639)) ## [v0.50.0-alpha.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-alpha.0) - 2023-06-07 diff --git a/baseapp/abci.go b/baseapp/abci.go index ded22c971933..4edef67455fb 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -4,10 +4,8 @@ import ( "context" "crypto/sha256" "fmt" - "os" "sort" "strings" - "syscall" "time" coreheader "cosmossdk.io/core/header" @@ -648,6 +646,10 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) { var events []abci.Event + if err := app.checkHalt(req.Height, req.Time); err != nil { + return nil, err + } + if err := app.validateFinalizeBlockHeight(req); err != nil { return nil, err } @@ -747,6 +749,24 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons }, nil } +// checkHalt checkes if height or time exceeds halt-height or halt-time respectively. +func (app *BaseApp) checkHalt(height int64, time time.Time) error { + 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 { + return fmt.Errorf("halt per configuration height %d time %d", app.haltHeight, app.haltTime) + } + + return nil +} + // 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 @@ -798,23 +818,6 @@ func (app *BaseApp) Commit() (*abci.ResponseCommit, error) { app.prepareCheckStater(app.checkState.ctx) } - 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 CometBFT 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() - } - go app.snapshotManager.SnapshotIfApplicable(header.Height) return resp, nil @@ -838,28 +841,6 @@ func (app *BaseApp) workingHash() []byte { return commitHash } -// 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) -} - func handleQueryApp(app *BaseApp, path []string, req *abci.RequestQuery) *abci.ResponseQuery { if len(path) >= 2 { switch path[1] { diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index a67c5d1fb8bf..d337ff95775c 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" "testing" + "time" errorsmod "cosmossdk.io/errors" "cosmossdk.io/log" @@ -1465,3 +1466,42 @@ func TestABCI_Proposal_Reset_State_Between_Calls(t *testing.T) { require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status) } } + +func TestABCI_HaltChain(t *testing.T) { + 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) { + suite := NewBaseAppSuite(t, baseapp.SetHaltHeight(tc.haltHeight), baseapp.SetHaltTime(tc.haltTime)) + suite.baseApp.InitChain(&abci.RequestInitChain{ + ConsensusParams: &cmtproto.ConsensusParams{}, + InitialHeight: tc.blockHeight, + }) + + app := suite.baseApp + _, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{ + Height: tc.blockHeight, + Time: time.Unix(tc.blockTime, 0), + }) + if !tc.expHalt { + require.NoError(t, err) + } else { + require.Error(t, err) + require.True(t, strings.HasPrefix(err.Error(), "halt per configuration")) + } + }) + } +}