From 476d7ae9948a44a5df448d8f22939074a5b63cd9 Mon Sep 17 00:00:00 2001 From: moul <94029+moul@users.noreply.github.com> Date: Thu, 26 Mar 2026 15:47:15 +0100 Subject: [PATCH 1/8] feat(node): add GovDAO-based halt height via r/sys/params Enables GovDAO to propose a coordinated chain halt at a specific block height without requiring every operator to pass a CLI flag. This is the governance-driven counterpart to the --halt-height CLI flag. Changes: - Add `NewSetHaltHeightRequest(height int64)` to `r/sys/params` realm, allowing GovDAO to vote on halting the chain at a target block. - Add `nodeParamsKeeper` to validate `node:p:halt_height` params. - Register the "node" module in the params keeper so halt_height can be set via governance proposals. - Extend `EndBlocker` to read `node:p:halt_height` from the params store and call `osm.Kill()` when the halt height is reached. Usage: // Create and submit a GovDAO proposal to halt at block 100000 pr := params.NewSetHaltHeightRequest(100_000) id := dao.MustCreateProposal(cross, pr) // After approval and execution, all nodes will halt at block 100000 Generated with [Claude Code](https://claude.com/claude-code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy --- examples/gno.land/r/sys/params/halt.gno | 26 ++++++++ .../gno.land/r/sys/params/params_test.gno | 14 +++++ gno.land/pkg/gnoland/app.go | 24 ++++++- gno.land/pkg/gnoland/app_test.go | 63 ++++++++++++++++--- gno.land/pkg/gnoland/node_params.go | 30 +++++++++ 5 files changed, 148 insertions(+), 9 deletions(-) create mode 100644 examples/gno.land/r/sys/params/halt.gno create mode 100644 gno.land/pkg/gnoland/node_params.go diff --git a/examples/gno.land/r/sys/params/halt.gno b/examples/gno.land/r/sys/params/halt.gno new file mode 100644 index 00000000000..ccc1322cd54 --- /dev/null +++ b/examples/gno.land/r/sys/params/halt.gno @@ -0,0 +1,26 @@ +package params + +import ( + prms "sys/params" + + "gno.land/r/gov/dao" +) + +const ( + nodeModulePrefix = "node" + haltHeightKey = "halt_height" +) + +// NewSetHaltHeightRequest creates a GovDAO proposal to halt all chain nodes at the given block height. +// Once the proposal is approved and executed, all nodes reading the halt_height parameter will +// gracefully stop after committing the specified block, enabling coordinated chain upgrades. +// +// Use height=0 to cancel a previously scheduled halt. +func NewSetHaltHeightRequest(height int64) dao.ProposalRequest { + key := syskey(nodeModulePrefix, "p", haltHeightKey) + return newPropRequest( + key, + func() { prms.SetSysParamInt64(nodeModulePrefix, "p", haltHeightKey, height) }, + "Set node halt height", + ) +} diff --git a/examples/gno.land/r/sys/params/params_test.gno b/examples/gno.land/r/sys/params/params_test.gno index e0e901e2dbb..109e8ae444f 100644 --- a/examples/gno.land/r/sys/params/params_test.gno +++ b/examples/gno.land/r/sys/params/params_test.gno @@ -15,3 +15,17 @@ func TestNewStringPropRequest(t *testing.T) { t.Errorf("executor shouldn't be nil") } } + +func TestNewSetHaltHeightRequest(t *testing.T) { + pr := NewSetHaltHeightRequest(100_000) + if pr.Title() == "" { + t.Errorf("proposal title shouldn't be empty") + } +} + +func TestNewSetHaltHeightRequestZero(t *testing.T) { + pr := NewSetHaltHeightRequest(0) + if pr.Title() == "" { + t.Errorf("proposal title shouldn't be empty") + } +} diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index af8b35e4179..6cdfc05efc7 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -13,6 +13,7 @@ import ( "github.com/gnolang/gno/gno.land/pkg/sdk/vm" "github.com/gnolang/gno/gnovm/pkg/gnoenv" "github.com/gnolang/gno/tm2/pkg/amino" + osm "github.com/gnolang/gno/tm2/pkg/os" abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types" "github.com/gnolang/gno/tm2/pkg/bft/config" bft "github.com/gnolang/gno/tm2/pkg/bft/types" @@ -114,6 +115,7 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { prmk.Register(auth.ModuleName, acck) prmk.Register(bank.ModuleName, bankk) prmk.Register(vm.ModuleName, vmk) + prmk.Register("node", nodeParamsKeeper{}) // Set InitChainer icc := cfg.InitChainerConfig @@ -188,6 +190,7 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { acck, gpk, vmk, + prmk, baseApp, ), ) @@ -448,18 +451,19 @@ type endBlockerApp interface { // EndBlocker defines the logic executed after every block. // Currently, it parses events that happened during execution to calculate -// validator set changes +// validator set changes, and checks for a governance-requested chain halt. func EndBlocker( collector *collector[validatorUpdate], acck auth.AccountKeeperI, gpk auth.GasPriceKeeperI, vmk vm.VMKeeperI, + prmk params.ParamsKeeperI, app endBlockerApp, ) func( ctx sdk.Context, req abci.RequestEndBlock, ) abci.ResponseEndBlock { - return func(ctx sdk.Context, _ abci.RequestEndBlock) abci.ResponseEndBlock { + return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { // set the auth params value in the ctx. The EndBlocker will use InitialGasPrice in // the params to calculate the updated gas price. if acck != nil { @@ -469,6 +473,22 @@ func EndBlocker( auth.EndBlocker(ctx, gpk) } + // Check if GovDAO has requested a halt at this height. + if prmk != nil { + var haltHeight int64 + prmk.GetInt64(ctx, "node:p:halt_height", &haltHeight) + if haltHeight > 0 && req.Height >= haltHeight { + app.Logger().Info( + "GovDAO halt height reached, shutting down", + "height", req.Height, + "halt_height", haltHeight, + ) + if err := osm.Kill(); err != nil { + app.Logger().Error("Failed to halt node at requested height", "err", err) + } + } + } + // Check if there was a valset change if len(collector.getEvents()) == 0 { // No valset updates diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 8ccfb0de6c3..5aa8198bdc8 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -533,7 +533,7 @@ func TestEndBlocker(t *testing.T) { c := newCollector[validatorUpdate](&mockEventSwitch{}, noFilter) // Create the EndBlocker - eb := EndBlocker(c, nil, nil, nil, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, nil, nil, &mockEndBlockerApp{}) // Run the EndBlocker res := eb(sdk.Context{}.WithConsensusParams(&abci.ConsensusParams{ @@ -577,7 +577,7 @@ func TestEndBlocker(t *testing.T) { mockEventSwitch.FireEvent(chain.Event{}) // Create the EndBlocker - eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, nil, &mockEndBlockerApp{}) // Run the EndBlocker res := eb(sdk.Context{}.WithConsensusParams(&abci.ConsensusParams{ @@ -624,7 +624,7 @@ func TestEndBlocker(t *testing.T) { mockEventSwitch.FireEvent(chain.Event{}) // Create the EndBlocker - eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, nil, &mockEndBlockerApp{}) // Run the EndBlocker res := eb(sdk.Context{}.WithConsensusParams(&abci.ConsensusParams{ @@ -696,7 +696,7 @@ func TestEndBlocker(t *testing.T) { mockEventSwitch.FireEvent(txEvent) // Create the EndBlocker - eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, nil, &mockEndBlockerApp{}) // Run the EndBlocker res := eb(sdk.Context{}.WithConsensusParams(&abci.ConsensusParams{ @@ -770,7 +770,7 @@ func TestEndBlocker(t *testing.T) { c := newCollector[validatorUpdate](mockEventSwitch, validatorEventFilter) mockEventSwitch.FireEvent(txEvent) - eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, nil, &mockEndBlockerApp{}) res := eb(sdk.Context{}.WithConsensusParams(&abci.ConsensusParams{ Validator: &abci.ValidatorParams{ PubKeyTypeURLs: []string{"/tm.PubKeySecp256k1"}, @@ -835,7 +835,7 @@ func TestEndBlocker(t *testing.T) { c := newCollector[validatorUpdate](mockEventSwitch, validatorEventFilter) mockEventSwitch.FireEvent(txEvent) - eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, nil, &mockEndBlockerApp{}) res := eb(sdk.Context{}.WithConsensusParams(&abci.ConsensusParams{ Validator: &abci.ValidatorParams{ PubKeyTypeURLs: []string{"/tm.PubKeySecp256k1"}, @@ -890,7 +890,7 @@ func TestEndBlocker(t *testing.T) { c := newCollector[validatorUpdate](mockEventSwitch, validatorEventFilter) mockEventSwitch.FireEvent(txEvent) - eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, nil, &mockEndBlockerApp{}) res := eb(sdk.Context{}.WithConsensusParams(&abci.ConsensusParams{ Validator: &abci.ValidatorParams{ PubKeyTypeURLs: []string{"/tm.PubKeyEd25519"}, @@ -1120,6 +1120,7 @@ func newGasPriceTestApp(t *testing.T) abci.Application { acck, gpk, nil, + nil, baseApp, ), ) @@ -1315,3 +1316,51 @@ func TestPruneStrategyNothing(t *testing.T) { err = db.Close() require.NoError(t, err) } + +func TestNodeParamsKeeperWillSetParam(t *testing.T) { + t.Parallel() + + npk := nodeParamsKeeper{} + + t.Run("valid halt_height", func(t *testing.T) { + t.Parallel() + assert.NotPanics(t, func() { + npk.WillSetParam(sdk.Context{}, "p:halt_height", int64(100)) + }) + }) + + t.Run("halt_height zero is allowed", func(t *testing.T) { + t.Parallel() + assert.NotPanics(t, func() { + npk.WillSetParam(sdk.Context{}, "p:halt_height", int64(0)) + }) + }) + + t.Run("negative halt_height panics", func(t *testing.T) { + t.Parallel() + assert.Panics(t, func() { + npk.WillSetParam(sdk.Context{}, "p:halt_height", int64(-1)) + }) + }) + + t.Run("wrong type panics", func(t *testing.T) { + t.Parallel() + assert.Panics(t, func() { + npk.WillSetParam(sdk.Context{}, "p:halt_height", "not-an-int64") + }) + }) + + t.Run("unknown p: key panics", func(t *testing.T) { + t.Parallel() + assert.Panics(t, func() { + npk.WillSetParam(sdk.Context{}, "p:unknown_key", int64(0)) + }) + }) + + t.Run("non-p: key is allowed", func(t *testing.T) { + t.Parallel() + assert.NotPanics(t, func() { + npk.WillSetParam(sdk.Context{}, "other:key", "value") + }) + }) +} diff --git a/gno.land/pkg/gnoland/node_params.go b/gno.land/pkg/gnoland/node_params.go new file mode 100644 index 00000000000..29644b049b1 --- /dev/null +++ b/gno.land/pkg/gnoland/node_params.go @@ -0,0 +1,30 @@ +package gnoland + +import ( + "fmt" + "strings" + + "github.com/gnolang/gno/tm2/pkg/sdk" +) + +// nodeParamsKeeper implements a minimal ParamfulKeeper for the "node" module. +// It validates node-level parameters set through governance proposals. +type nodeParamsKeeper struct{} + +// WillSetParam validates node parameters before they are written to the params store. +func (nodeParamsKeeper) WillSetParam(_ sdk.Context, key string, value any) { + switch key { + case "p:halt_height": + h, ok := value.(int64) + if !ok { + panic(fmt.Sprintf("halt_height must be an int64, got %T", value)) + } + if h < 0 { + panic(fmt.Sprintf("halt_height must be non-negative, got %d", h)) + } + default: + if strings.HasPrefix(key, "p:") { + panic(fmt.Sprintf("unknown node param key: %q", key)) + } + } +} From 8ec0b2c2dccd5d220ec721daec3e3d04c8f4caac Mon Sep 17 00:00:00 2001 From: moul <94029+moul@users.noreply.github.com> Date: Thu, 26 Mar 2026 16:09:56 +0100 Subject: [PATCH 2/8] feat(node): add halt_min_version guard against old-binary resume Extends the GovDAO halt proposal with a mandatory minimum binary version field. When set, nodes refuse to restart unless their version satisfies the requirement, preventing an old binary from accidentally resuming a chain that was halted for an upgrade. - `NewSetHaltRequest(height, minVersion)` sets both `node:p:halt_height` and `node:p:halt_min_version` atomically in one GovDAO proposal. - `checkNodeStartupParams` runs at node startup (after state is loaded) and compares `version.Version` against the stored `halt_min_version`. - `meetsMinVersion` / `parseGnolandVersion` handle the "chain/gnolandX.Y" version format used for gno.land chain releases, with a string-equality fallback for other formats. Example: setting minVersion="chain/gnoland1.1" will allow 1.1 and newer to start, but reject 1.0 ("develop" also rejected unless it matches). Generated with [Claude Code](https://claude.com/claude-code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy --- examples/gno.land/r/sys/params/halt.gno | 49 +++++++--- .../gno.land/r/sys/params/params_test.gno | 15 ++- gno.land/pkg/gnoland/app.go | 7 ++ gno.land/pkg/gnoland/app_test.go | 97 ++++++++++++++++++- gno.land/pkg/gnoland/node_params.go | 81 ++++++++++++++++ 5 files changed, 232 insertions(+), 17 deletions(-) diff --git a/examples/gno.land/r/sys/params/halt.gno b/examples/gno.land/r/sys/params/halt.gno index ccc1322cd54..f9dccaac0ea 100644 --- a/examples/gno.land/r/sys/params/halt.gno +++ b/examples/gno.land/r/sys/params/halt.gno @@ -1,26 +1,51 @@ package params import ( + "strconv" + + "chain" prms "sys/params" "gno.land/r/gov/dao" ) const ( - nodeModulePrefix = "node" - haltHeightKey = "halt_height" + nodeModulePrefix = "node" + haltHeightKey = "halt_height" + haltMinVersionKey = "halt_min_version" ) -// NewSetHaltHeightRequest creates a GovDAO proposal to halt all chain nodes at the given block height. -// Once the proposal is approved and executed, all nodes reading the halt_height parameter will -// gracefully stop after committing the specified block, enabling coordinated chain upgrades. +// NewSetHaltRequest creates a GovDAO proposal to halt all chain nodes at the given block height. +// Once approved and executed, nodes will gracefully stop after committing the specified block, +// enabling coordinated chain upgrades. +// +// minVersion, if non-empty, sets the minimum binary version required to resume after the halt. +// Nodes will refuse to restart unless their version satisfies the minimum requirement, +// preventing old binaries from accidentally resuming a chain halted for an upgrade. +// Example: minVersion="chain/gnoland1.1" prevents gnoland1.0 from resuming. // // Use height=0 to cancel a previously scheduled halt. -func NewSetHaltHeightRequest(height int64) dao.ProposalRequest { - key := syskey(nodeModulePrefix, "p", haltHeightKey) - return newPropRequest( - key, - func() { prms.SetSysParamInt64(nodeModulePrefix, "p", haltHeightKey, height) }, - "Set node halt height", - ) +func NewSetHaltRequest(height int64, minVersion string) dao.ProposalRequest { + callback := func(cur realm) error { + prms.SetSysParamInt64(nodeModulePrefix, "p", haltHeightKey, height) + prms.SetSysParamString(nodeModulePrefix, "p", haltMinVersionKey, minVersion) + chain.Emit("set_halt", + "height", strconv.FormatInt(height, 10), + "min_version", minVersion, + ) + return nil + } + + var desc string + if height == 0 { + desc = "Cancel the scheduled chain halt and clear the minimum version requirement." + } else { + desc = "Halt the chain at block " + strconv.FormatInt(height, 10) + "." + if minVersion != "" { + desc += " Requires binary version >= " + minVersion + " to resume." + } + } + + e := dao.NewSimpleExecutor(callback, "") + return dao.NewProposalRequest("Set node halt height", desc, e) } diff --git a/examples/gno.land/r/sys/params/params_test.gno b/examples/gno.land/r/sys/params/params_test.gno index 109e8ae444f..f82b2451a19 100644 --- a/examples/gno.land/r/sys/params/params_test.gno +++ b/examples/gno.land/r/sys/params/params_test.gno @@ -16,15 +16,22 @@ func TestNewStringPropRequest(t *testing.T) { } } -func TestNewSetHaltHeightRequest(t *testing.T) { - pr := NewSetHaltHeightRequest(100_000) +func TestNewSetHaltRequest(t *testing.T) { + pr := NewSetHaltRequest(100_000, "chain/gnoland1.1") if pr.Title() == "" { t.Errorf("proposal title shouldn't be empty") } } -func TestNewSetHaltHeightRequestZero(t *testing.T) { - pr := NewSetHaltHeightRequest(0) +func TestNewSetHaltRequestNoVersion(t *testing.T) { + pr := NewSetHaltRequest(100_000, "") + if pr.Title() == "" { + t.Errorf("proposal title shouldn't be empty") + } +} + +func TestNewSetHaltRequestCancel(t *testing.T) { + pr := NewSetHaltRequest(0, "") if pr.Title() == "" { t.Errorf("proposal title shouldn't be empty") } diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index 6cdfc05efc7..0dd7f2c9bb7 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -211,6 +211,13 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { vmk.Initialize(cfg.Logger, ms) ms.MultiWrite() // XXX why was't this needed? + // Verify that the running binary meets the minimum version requirement + // set by a previous governance halt proposal, preventing old binaries + // from accidentally resuming a chain halted for an upgrade. + if err := checkNodeStartupParams(prmk, baseApp.GetCacheMultiStore()); err != nil { + return nil, err + } + return baseApp, nil } diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 5aa8198bdc8..354bee2c93a 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -1343,13 +1343,34 @@ func TestNodeParamsKeeperWillSetParam(t *testing.T) { }) }) - t.Run("wrong type panics", func(t *testing.T) { + t.Run("halt_height wrong type panics", func(t *testing.T) { t.Parallel() assert.Panics(t, func() { npk.WillSetParam(sdk.Context{}, "p:halt_height", "not-an-int64") }) }) + t.Run("valid halt_min_version", func(t *testing.T) { + t.Parallel() + assert.NotPanics(t, func() { + npk.WillSetParam(sdk.Context{}, "p:halt_min_version", "chain/gnoland1.1") + }) + }) + + t.Run("empty halt_min_version is allowed", func(t *testing.T) { + t.Parallel() + assert.NotPanics(t, func() { + npk.WillSetParam(sdk.Context{}, "p:halt_min_version", "") + }) + }) + + t.Run("halt_min_version wrong type panics", func(t *testing.T) { + t.Parallel() + assert.Panics(t, func() { + npk.WillSetParam(sdk.Context{}, "p:halt_min_version", int64(1)) + }) + }) + t.Run("unknown p: key panics", func(t *testing.T) { t.Parallel() assert.Panics(t, func() { @@ -1364,3 +1385,77 @@ func TestNodeParamsKeeperWillSetParam(t *testing.T) { }) }) } + +func TestMeetsMinVersion(t *testing.T) { + t.Parallel() + + cases := []struct { + binary string + minVer string + want bool + }{ + // Empty minVersion always passes + {"chain/gnoland1.0", "", true}, + {"develop", "", true}, + + // Same version passes + {"chain/gnoland1.0", "chain/gnoland1.0", true}, + {"chain/gnoland1.1", "chain/gnoland1.1", true}, + + // Newer binary passes + {"chain/gnoland1.1", "chain/gnoland1.0", true}, + {"chain/gnoland2.0", "chain/gnoland1.0", true}, + {"chain/gnoland1.2", "chain/gnoland1.1", true}, + + // Older binary fails + {"chain/gnoland1.0", "chain/gnoland1.1", false}, + {"chain/gnoland1.0", "chain/gnoland2.0", false}, + + // Non-gnoland format: requires exact match + {"develop", "chain/gnoland1.1", false}, + {"v1.0.0", "v1.0.0", true}, + {"v1.0.0", "v1.1.0", false}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.binary+">="+tc.minVer, func(t *testing.T) { + t.Parallel() + got := meetsMinVersion(tc.binary, tc.minVer) + assert.Equal(t, tc.want, got, + "meetsMinVersion(%q, %q)", tc.binary, tc.minVer) + }) + } +} + +func TestParseGnolandVersion(t *testing.T) { + t.Parallel() + + cases := []struct { + input string + major int + minor int + ok bool + }{ + {"chain/gnoland1.0", 1, 0, true}, + {"chain/gnoland1.1", 1, 1, true}, + {"chain/gnoland2.3", 2, 3, true}, + {"develop", 0, 0, false}, + {"v1.0.0", 0, 0, false}, + {"chain/gnoland", 0, 0, false}, + {"chain/gnolandX.Y", 0, 0, false}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.input, func(t *testing.T) { + t.Parallel() + major, minor, ok := parseGnolandVersion(tc.input) + assert.Equal(t, tc.ok, ok) + if tc.ok { + assert.Equal(t, tc.major, major) + assert.Equal(t, tc.minor, minor) + } + }) + } +} diff --git a/gno.land/pkg/gnoland/node_params.go b/gno.land/pkg/gnoland/node_params.go index 29644b049b1..fc5374f594b 100644 --- a/gno.land/pkg/gnoland/node_params.go +++ b/gno.land/pkg/gnoland/node_params.go @@ -2,9 +2,18 @@ package gnoland import ( "fmt" + "strconv" "strings" "github.com/gnolang/gno/tm2/pkg/sdk" + sdkparams "github.com/gnolang/gno/tm2/pkg/sdk/params" + "github.com/gnolang/gno/tm2/pkg/store" + tmver "github.com/gnolang/gno/tm2/pkg/version" +) + +const ( + nodeParamHaltHeight = "node:p:halt_height" + nodeParamHaltMinVersion = "node:p:halt_min_version" ) // nodeParamsKeeper implements a minimal ParamfulKeeper for the "node" module. @@ -22,9 +31,81 @@ func (nodeParamsKeeper) WillSetParam(_ sdk.Context, key string, value any) { if h < 0 { panic(fmt.Sprintf("halt_height must be non-negative, got %d", h)) } + case "p:halt_min_version": + _, ok := value.(string) + if !ok { + panic(fmt.Sprintf("halt_min_version must be a string, got %T", value)) + } default: if strings.HasPrefix(key, "p:") { panic(fmt.Sprintf("unknown node param key: %q", key)) } } } + +// checkNodeStartupParams reads halt-related params from the committed state and verifies +// that the running binary satisfies any minimum version requirement set by governance. +// This prevents an old binary from accidentally resuming a chain that was halted for an upgrade. +func checkNodeStartupParams(prmk sdkparams.ParamsKeeperI, ms store.MultiStore) error { + // Build a minimal read-only context with just the multistore and a placeholder chain ID. + // We only need store access to read params; no block execution context is required. + ctx := sdk.Context{}.WithMultiStore(ms).WithChainID("_") + + var minVersion string + prmk.GetString(ctx, nodeParamHaltMinVersion, &minVersion) + if minVersion == "" { + return nil + } + + binaryVersion := tmver.Version + if !meetsMinVersion(binaryVersion, minVersion) { + return fmt.Errorf( + "binary version %q does not meet the minimum version %q required by governance; "+ + "please upgrade to a compatible binary before restarting", + binaryVersion, minVersion, + ) + } + + return nil +} + +// meetsMinVersion reports whether binaryVersion satisfies the minVersion requirement. +// Versions are expected to follow the "chain/gnolandX.Y" format used for gno.land chain releases. +// If either version cannot be parsed in that format, an exact string match is required. +func meetsMinVersion(binaryVersion, minVersion string) bool { + if minVersion == "" { + return true + } + + bMajor, bMinor, bOK := parseGnolandVersion(binaryVersion) + mMajor, mMinor, mOK := parseGnolandVersion(minVersion) + + if bOK && mOK { + if bMajor != mMajor { + return bMajor > mMajor + } + return bMinor >= mMinor + } + + // Fall back to exact match if versions are not in the recognized format. + return binaryVersion == minVersion +} + +// parseGnolandVersion parses a version string like "chain/gnoland1.2" into its major and minor parts. +func parseGnolandVersion(v string) (major, minor int, ok bool) { + const prefix = "chain/gnoland" + if !strings.HasPrefix(v, prefix) { + return 0, 0, false + } + rest := v[len(prefix):] + dot := strings.IndexByte(rest, '.') + if dot < 0 { + return 0, 0, false + } + maj, err1 := strconv.Atoi(rest[:dot]) + min, err2 := strconv.Atoi(rest[dot+1:]) + if err1 != nil || err2 != nil { + return 0, 0, false + } + return maj, min, true +} From 530e60bbd2460065174d8dc6529980d01519d2fa Mon Sep 17 00:00:00 2001 From: moul <94029+moul@users.noreply.github.com> Date: Tue, 7 Apr 2026 16:42:42 +0000 Subject: [PATCH 3/8] refactor: use BaseApp.SetHaltHeight instead of osm.Kill for GovDAO halt Aligns with #5334's approach: GovDAO EndBlocker now sets the halt height on BaseApp, which panics in BeginBlock of the next block. This is deterministic (no async signals) and ensures the halted block is fully committed. --- gno.land/pkg/gnoland/app.go | 12 +++++++----- gno.land/pkg/gnoland/mock_test.go | 2 ++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index 0dd7f2c9bb7..b6df537c539 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -13,7 +13,6 @@ import ( "github.com/gnolang/gno/gno.land/pkg/sdk/vm" "github.com/gnolang/gno/gnovm/pkg/gnoenv" "github.com/gnolang/gno/tm2/pkg/amino" - osm "github.com/gnolang/gno/tm2/pkg/os" abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types" "github.com/gnolang/gno/tm2/pkg/bft/config" bft "github.com/gnolang/gno/tm2/pkg/bft/types" @@ -454,6 +453,9 @@ type endBlockerApp interface { // Logger returns the logger reference Logger() *slog.Logger + + // SetHaltHeight sets the block height at which the node will halt. + SetHaltHeight(uint64) } // EndBlocker defines the logic executed after every block. @@ -481,18 +483,18 @@ func EndBlocker( } // Check if GovDAO has requested a halt at this height. + // If so, propagate to BaseApp so BeginBlock panics on the next block, + // ensuring this block is fully committed before the node stops. if prmk != nil { var haltHeight int64 prmk.GetInt64(ctx, "node:p:halt_height", &haltHeight) if haltHeight > 0 && req.Height >= haltHeight { app.Logger().Info( - "GovDAO halt height reached, shutting down", + "GovDAO halt height reached, will halt after this block", "height", req.Height, "halt_height", haltHeight, ) - if err := osm.Kill(); err != nil { - app.Logger().Error("Failed to halt node at requested height", "err", err) - } + app.SetHaltHeight(uint64(haltHeight)) } } diff --git a/gno.land/pkg/gnoland/mock_test.go b/gno.land/pkg/gnoland/mock_test.go index 9732d2a2f56..f480aa1bb4c 100644 --- a/gno.land/pkg/gnoland/mock_test.go +++ b/gno.land/pkg/gnoland/mock_test.go @@ -219,3 +219,5 @@ func (m *mockEndBlockerApp) Logger() *slog.Logger { return log.NewNoopLogger() } + +func (m *mockEndBlockerApp) SetHaltHeight(_ uint64) {} From c143f2a9c81b2943a51755a1fb15fb9908c70684 Mon Sep 17 00:00:00 2001 From: moul <94029+moul@users.noreply.github.com> Date: Tue, 7 Apr 2026 20:25:25 +0000 Subject: [PATCH 4/8] fix: use nodeParamHaltHeight constant instead of raw string --- gno.land/pkg/gnoland/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index b6df537c539..fc9fd3671e8 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -487,7 +487,7 @@ func EndBlocker( // ensuring this block is fully committed before the node stops. if prmk != nil { var haltHeight int64 - prmk.GetInt64(ctx, "node:p:halt_height", &haltHeight) + prmk.GetInt64(ctx, nodeParamHaltHeight, &haltHeight) if haltHeight > 0 && req.Height >= haltHeight { app.Logger().Info( "GovDAO halt height reached, will halt after this block", From e13a09a87950f24893725556608eefc909087c88 Mon Sep 17 00:00:00 2001 From: moul <94029+moul@users.noreply.github.com> Date: Tue, 7 Apr 2026 20:34:04 +0000 Subject: [PATCH 5/8] feat: prevent premature upgrade binary startup + add skip_upgrade_height config Addresses tbruyelle's review feedback: 1. Panic if new binary runs before the chain has halted at halt_height 2. Add skip_upgrade_height config field to bypass the check when the validator has already migrated state --- gno.land/cmd/gnoland/start.go | 1 + gno.land/pkg/gnoland/app.go | 9 +++--- gno.land/pkg/gnoland/app_test.go | 3 +- gno.land/pkg/gnoland/node_params.go | 45 +++++++++++++++++++++++------ tm2/pkg/bft/config/config.go | 4 +++ 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/gno.land/cmd/gnoland/start.go b/gno.land/cmd/gnoland/start.go index 5c015c79033..738d6fe343d 100644 --- a/gno.land/cmd/gnoland/start.go +++ b/gno.land/cmd/gnoland/start.go @@ -264,6 +264,7 @@ func execStart(ctx context.Context, c *startCfg, io commands.IO) error { cfg.Application, evsw, logger, + cfg.BaseConfig.SkipUpgradeHeight, ) if err != nil { return fmt.Errorf("unable to create the Gnoland app, %w", err) diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index fc9fd3671e8..72ef7a83265 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -41,6 +41,7 @@ type AppOptions struct { EventSwitch events.EventSwitch // required VMOutput io.Writer // optional SkipGenesisSigVerification bool // default to verify genesis transactions + SkipUpgradeHeight int64 // if set, skip the halt_min_version check at this height InitChainerConfig // options related to InitChainer MinGasPrices string // optional PruneStrategy types.PruneStrategy @@ -210,10 +211,8 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { vmk.Initialize(cfg.Logger, ms) ms.MultiWrite() // XXX why was't this needed? - // Verify that the running binary meets the minimum version requirement - // set by a previous governance halt proposal, preventing old binaries - // from accidentally resuming a chain halted for an upgrade. - if err := checkNodeStartupParams(prmk, baseApp.GetCacheMultiStore()); err != nil { + // Verify node startup constraints set by governance halt proposals. + if err := checkNodeStartupParams(prmk, baseApp.GetCacheMultiStore(), baseApp.LastBlockHeight(), cfg.SkipUpgradeHeight); err != nil { return nil, err } @@ -242,6 +241,7 @@ func NewApp( appCfg *sdkCfg.AppConfig, evsw events.EventSwitch, logger *slog.Logger, + skipUpgradeHeight int64, ) (abci.Application, error) { var err error @@ -254,6 +254,7 @@ func NewApp( }, MinGasPrices: appCfg.MinGasPrices, SkipGenesisSigVerification: genesisCfg.SkipSigVerification, + SkipUpgradeHeight: skipUpgradeHeight, PruneStrategy: appCfg.PruneStrategy, } if genesisCfg.SkipFailingTxs { diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 354bee2c93a..4d0165d94bb 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -147,7 +147,7 @@ func TestNewApp(t *testing.T) { // NewApp should have good defaults and manage to run InitChain. td := t.TempDir() - app, err := NewApp(td, NewTestGenesisAppConfig(), config.DefaultAppConfig(), events.NewEventSwitch(), log.NewNoopLogger()) + app, err := NewApp(td, NewTestGenesisAppConfig(), config.DefaultAppConfig(), events.NewEventSwitch(), log.NewNoopLogger(), 0) require.NoError(t, err, "NewApp should be successful") resp := app.InitChain(abci.RequestInitChain{ @@ -1262,6 +1262,7 @@ func TestPruneStrategyNothing(t *testing.T) { appCfg, events.NewEventSwitch(), log.NewNoopLogger(), + 0, ) require.NoError(t, err) diff --git a/gno.land/pkg/gnoland/node_params.go b/gno.land/pkg/gnoland/node_params.go index fc5374f594b..183635a583d 100644 --- a/gno.land/pkg/gnoland/node_params.go +++ b/gno.land/pkg/gnoland/node_params.go @@ -43,26 +43,53 @@ func (nodeParamsKeeper) WillSetParam(_ sdk.Context, key string, value any) { } } -// checkNodeStartupParams reads halt-related params from the committed state and verifies -// that the running binary satisfies any minimum version requirement set by governance. -// This prevents an old binary from accidentally resuming a chain that was halted for an upgrade. -func checkNodeStartupParams(prmk sdkparams.ParamsKeeperI, ms store.MultiStore) error { +// checkNodeStartupParams reads halt-related params from the committed state and verifies: +// 1. The running binary meets the minimum version requirement set by governance. +// 2. A new (upgraded) binary is not started before the chain has actually halted. +// +// skipUpgradeHeight, if non-zero, skips all upgrade checks at that specific height. +func checkNodeStartupParams(prmk sdkparams.ParamsKeeperI, ms store.MultiStore, lastBlockHeight, skipUpgradeHeight int64) error { // Build a minimal read-only context with just the multistore and a placeholder chain ID. // We only need store access to read params; no block execution context is required. ctx := sdk.Context{}.WithMultiStore(ms).WithChainID("_") + var haltHeight int64 + prmk.GetInt64(ctx, nodeParamHaltHeight, &haltHeight) + var minVersion string prmk.GetString(ctx, nodeParamHaltMinVersion, &minVersion) - if minVersion == "" { + + // Nothing to check if no governance halt is configured. + if haltHeight == 0 || minVersion == "" { + return nil + } + + // Allow skipping upgrade checks at a specific height (e.g., validator already migrated). + if skipUpgradeHeight > 0 && skipUpgradeHeight == haltHeight { return nil } binaryVersion := tmver.Version - if !meetsMinVersion(binaryVersion, minVersion) { + + // Check 1: Prevent old binaries from resuming after a halt. + if lastBlockHeight >= haltHeight { + if !meetsMinVersion(binaryVersion, minVersion) { + return fmt.Errorf( + "binary version %q does not meet the minimum version %q required by governance; "+ + "please upgrade to a compatible binary before restarting", + binaryVersion, minVersion, + ) + } + return nil + } + + // Check 2: Prevent new (upgraded) binaries from running before the halt height. + if meetsMinVersion(binaryVersion, minVersion) && binaryVersion != minVersion { return fmt.Errorf( - "binary version %q does not meet the minimum version %q required by governance; "+ - "please upgrade to a compatible binary before restarting", - binaryVersion, minVersion, + "binary version %q is an upgrade intended for halt height %d, "+ + "but the chain is at height %d; please use the previous binary until the halt, "+ + "or set skip_upgrade_height = %d in config.toml if you have already migrated", + binaryVersion, haltHeight, lastBlockHeight, haltHeight, ) } diff --git a/tm2/pkg/bft/config/config.go b/tm2/pkg/bft/config/config.go index 4793860391b..25b243a339f 100644 --- a/tm2/pkg/bft/config/config.go +++ b/tm2/pkg/bft/config/config.go @@ -328,6 +328,10 @@ type BaseConfig struct { // If non-zero, the node will halt after committing this block height. // Useful for coordinated chain upgrades. HaltHeight int64 `toml:"halt_height" comment:"If non-zero, the node will halt after committing this block height.\n Useful for coordinated chain upgrades."` + + // If non-zero, skip the governance upgrade check at this height. + // Use when the validator has already migrated state but the chain is still at halt height. + SkipUpgradeHeight int64 `toml:"skip_upgrade_height" comment:"If non-zero, skip the governance upgrade check at this height.\n Use when the validator has already migrated but the chain is still at halt height."` } // DefaultBaseConfig returns a default base configuration for a Tendermint node From 842e80550fc3cae6eb1cba172bc0b6e4bdc3400f Mon Sep 17 00:00:00 2001 From: moul <94029+moul@users.noreply.github.com> Date: Thu, 9 Apr 2026 17:00:54 +0200 Subject: [PATCH 6/8] fix(node): address all 5 tbruyelle review issues in GovDAO halt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All issues found by Thomas during manual testing are now fixed: **1. Infinite loop after upgrade** Change EndBlocker condition from `>=` to `==`: only fires at the exact halt height, not on every subsequent block. After restart the chain is at halt_height+1 so the condition never re-fires — no more infinite loop. **2. New binary guard before halt height** Remove the erroneous `&& binaryVersion != minVersion` from `checkNodeStartupParams`. The intent is to block *any* binary that already satisfies the minimum version from running before the halt; the extra condition was accidentally allowing the exact minimum version through. **3. Cancel proposal now works** `halt_height=0` (cancel sentinel) combined with the `==` condition means `req.Height == 0` is never true, so cancellation is immediately effective. **4. Deterministic single-fire halt** Using `==` instead of `>=` ensures the halt triggers exactly once at the scheduled height. BeginBlock of the next block panics, leaving the chain cleanly stopped with halt_height as the last committed block. **5. Validate halt_height is in the future** `WillSetParam` now rejects a `halt_height` that is <= the current block height (except the cancel sentinel 0). Uses `safeBlockHeight()` to handle genesis / test contexts where the block header may not be set. New tests: - `TestCheckNodeStartupParams`: old binary guard, new binary guard, skip_upgrade_height bypass - `TestEndBlockerHalt`: exact-match halt, no re-halt after height, cancel sentinel, before-halt no-op - `TestNodeParamsKeeperWillSetParam`: past/present height rejection --- gno.land/pkg/gnoland/app.go | 8 +- gno.land/pkg/gnoland/app_test.go | 189 +++++++++++++++++++++++++++- gno.land/pkg/gnoland/mock_test.go | 42 ++++++- gno.land/pkg/gnoland/node_params.go | 18 ++- 4 files changed, 249 insertions(+), 8 deletions(-) diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index 72ef7a83265..549ec061a09 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -484,12 +484,14 @@ func EndBlocker( } // Check if GovDAO has requested a halt at this height. - // If so, propagate to BaseApp so BeginBlock panics on the next block, - // ensuring this block is fully committed before the node stops. + // Use == (not >=) so we only trigger once: at the exact halt height. + // SetHaltHeight causes BeginBlock of the *next* block to panic, ensuring + // this block is fully committed before the node stops. + // On restart, req.Height > halt_height, so == never re-fires — no infinite loop. if prmk != nil { var haltHeight int64 prmk.GetInt64(ctx, nodeParamHaltHeight, &haltHeight) - if haltHeight > 0 && req.Height >= haltHeight { + if haltHeight > 0 && req.Height == haltHeight { app.Logger().Info( "GovDAO halt height reached, will halt after this block", "height", req.Height, diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 4d0165d94bb..1c5698ba1d3 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -1323,20 +1323,45 @@ func TestNodeParamsKeeperWillSetParam(t *testing.T) { npk := nodeParamsKeeper{} - t.Run("valid halt_height", func(t *testing.T) { + t.Run("valid halt_height (no block context)", func(t *testing.T) { t.Parallel() + // Without a block header, safeBlockHeight returns 0, so no future check. assert.NotPanics(t, func() { npk.WillSetParam(sdk.Context{}, "p:halt_height", int64(100)) }) }) - t.Run("halt_height zero is allowed", func(t *testing.T) { + t.Run("halt_height zero is allowed (cancel sentinel)", func(t *testing.T) { t.Parallel() assert.NotPanics(t, func() { npk.WillSetParam(sdk.Context{}, "p:halt_height", int64(0)) }) }) + t.Run("halt_height in the future is valid when block height is known", func(t *testing.T) { + t.Parallel() + ctx := sdk.Context{}.WithBlockHeader(&bft.Header{Height: 50}) + assert.NotPanics(t, func() { + npk.WillSetParam(ctx, "p:halt_height", int64(100)) + }) + }) + + t.Run("halt_height equal to current block height panics", func(t *testing.T) { + t.Parallel() + ctx := sdk.Context{}.WithBlockHeader(&bft.Header{Height: 100}) + assert.Panics(t, func() { + npk.WillSetParam(ctx, "p:halt_height", int64(100)) + }) + }) + + t.Run("halt_height in the past panics", func(t *testing.T) { + t.Parallel() + ctx := sdk.Context{}.WithBlockHeader(&bft.Header{Height: 200}) + assert.Panics(t, func() { + npk.WillSetParam(ctx, "p:halt_height", int64(100)) + }) + }) + t.Run("negative halt_height panics", func(t *testing.T) { t.Parallel() assert.Panics(t, func() { @@ -1460,3 +1485,163 @@ func TestParseGnolandVersion(t *testing.T) { }) } } + +// newTestParamsKeeper creates a minimal ParamsKeeper with an in-memory store +// and pre-seeds it with the given halt params. +func newTestParamsKeeper(t *testing.T, haltHeight int64, minVersion string) (params.ParamsKeeper, store.MultiStore) { + t.Helper() + + db := memdb.NewMemDB() + mainKey := store.NewStoreKey("main") + + cms := store.NewCommitMultiStore(db) + cms.MountStoreWithDB(mainKey, iavl.StoreConstructor, db) + require.NoError(t, cms.LoadLatestVersion()) + + prmk := params.NewParamsKeeper(mainKey) + prmk.Register("node", nodeParamsKeeper{}) + + ms := cms.MultiCacheWrap() + ctx := sdk.Context{}.WithMultiStore(ms).WithChainID("_") + + prmk.SetInt64(ctx, nodeParamHaltHeight, haltHeight) + prmk.SetString(ctx, nodeParamHaltMinVersion, minVersion) + ms.MultiWrite() + cms.Commit() + + return prmk, cms.MultiCacheWrap() +} + +func TestCheckNodeStartupParams(t *testing.T) { + t.Parallel() + + t.Run("no halt configured", func(t *testing.T) { + t.Parallel() + prmk, ms := newTestParamsKeeper(t, 0, "") + require.NoError(t, checkNodeStartupParams(prmk, ms, 50, 0)) + }) + + t.Run("halt with no version passes", func(t *testing.T) { + t.Parallel() + prmk, ms := newTestParamsKeeper(t, 100, "") + require.NoError(t, checkNodeStartupParams(prmk, ms, 100, 0)) + }) + + t.Run("binary meets version after halt", func(t *testing.T) { + t.Parallel() + prmk, ms := newTestParamsKeeper(t, 100, "develop") + // binary "develop" == "develop" -> meetsMinVersion (exact match), lastBlock >= haltHeight + require.NoError(t, checkNodeStartupParams(prmk, ms, 100, 0)) + }) + + t.Run("old binary rejected after halt", func(t *testing.T) { + t.Parallel() + prmk, ms := newTestParamsKeeper(t, 100, "chain/gnoland9.9") + // binary "develop" doesn't meet "chain/gnoland9.9" -> rejected + err := checkNodeStartupParams(prmk, ms, 100, 0) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not meet the minimum version") + }) + + t.Run("new binary rejected before halt height", func(t *testing.T) { + t.Parallel() + prmk, ms := newTestParamsKeeper(t, 100, "develop") + // binary "develop" == "develop" -> meetsMinVersion, but chain hasn't halted yet + err := checkNodeStartupParams(prmk, ms, 50, 0) + require.Error(t, err) + assert.Contains(t, err.Error(), "upgrade intended for halt height") + }) + + t.Run("old binary allowed before halt height", func(t *testing.T) { + t.Parallel() + prmk, ms := newTestParamsKeeper(t, 100, "chain/gnoland9.9") + // binary "develop" doesn't meet "chain/gnoland9.9", chain hasn't halted -> old binary, OK + require.NoError(t, checkNodeStartupParams(prmk, ms, 50, 0)) + }) + + t.Run("skip_upgrade_height bypasses check", func(t *testing.T) { + t.Parallel() + prmk, ms := newTestParamsKeeper(t, 100, "develop") + // Even though binary meets version before halt, skip_upgrade_height=100 bypasses + require.NoError(t, checkNodeStartupParams(prmk, ms, 50, 100)) + }) +} + +func TestEndBlockerHalt(t *testing.T) { + t.Parallel() + + noFilter := func(_ events.Event) []validatorUpdate { return nil } + + t.Run("halts at exact height", func(t *testing.T) { + t.Parallel() + + var haltSet uint64 + mockApp := &mockEndBlockerApp{ + setHaltHeightFn: func(h uint64) { haltSet = h }, + } + mockPrmk := &mockConfigurableParamsKeeper{ + int64s: map[string]int64{nodeParamHaltHeight: 100}, + } + + c := newCollector[validatorUpdate](&mockEventSwitch{}, noFilter) + eb := EndBlocker(c, nil, nil, nil, mockPrmk, mockApp) + eb(sdk.Context{}, abci.RequestEndBlock{Height: 100}) + + assert.Equal(t, uint64(100), haltSet, "SetHaltHeight should be called with halt_height") + }) + + t.Run("does not halt before halt height", func(t *testing.T) { + t.Parallel() + + var haltSet uint64 + mockApp := &mockEndBlockerApp{ + setHaltHeightFn: func(h uint64) { haltSet = h }, + } + mockPrmk := &mockConfigurableParamsKeeper{ + int64s: map[string]int64{nodeParamHaltHeight: 100}, + } + + c := newCollector[validatorUpdate](&mockEventSwitch{}, noFilter) + eb := EndBlocker(c, nil, nil, nil, mockPrmk, mockApp) + eb(sdk.Context{}, abci.RequestEndBlock{Height: 99}) + + assert.Equal(t, uint64(0), haltSet, "SetHaltHeight should NOT be called before halt height") + }) + + t.Run("does not re-halt after halt height (no infinite loop)", func(t *testing.T) { + t.Parallel() + + var haltSet uint64 + mockApp := &mockEndBlockerApp{ + setHaltHeightFn: func(h uint64) { haltSet = h }, + } + mockPrmk := &mockConfigurableParamsKeeper{ + int64s: map[string]int64{nodeParamHaltHeight: 100}, + } + + c := newCollector[validatorUpdate](&mockEventSwitch{}, noFilter) + eb := EndBlocker(c, nil, nil, nil, mockPrmk, mockApp) + // After restart at height 101, halt_height=100 still in params but == doesn't re-fire + eb(sdk.Context{}, abci.RequestEndBlock{Height: 101}) + + assert.Equal(t, uint64(0), haltSet, "SetHaltHeight must NOT be called after halt height (prevents infinite loop)") + }) + + t.Run("cancel: halt_height zero never halts", func(t *testing.T) { + t.Parallel() + + var haltSet uint64 + mockApp := &mockEndBlockerApp{ + setHaltHeightFn: func(h uint64) { haltSet = h }, + } + mockPrmk := &mockConfigurableParamsKeeper{ + int64s: map[string]int64{nodeParamHaltHeight: 0}, + } + + c := newCollector[validatorUpdate](&mockEventSwitch{}, noFilter) + eb := EndBlocker(c, nil, nil, nil, mockPrmk, mockApp) + eb(sdk.Context{}, abci.RequestEndBlock{Height: 100}) + + assert.Equal(t, uint64(0), haltSet, "SetHaltHeight should NOT be called when halt_height=0 (cancelled)") + }) +} diff --git a/gno.land/pkg/gnoland/mock_test.go b/gno.land/pkg/gnoland/mock_test.go index f480aa1bb4c..09231d564ba 100644 --- a/gno.land/pkg/gnoland/mock_test.go +++ b/gno.land/pkg/gnoland/mock_test.go @@ -197,11 +197,13 @@ func (m *mockGasPriceKeeper) UpdateGasPrice(ctx sdk.Context) {} type ( lastBlockHeightDelegate func() int64 loggerDelegate func() *slog.Logger + setHaltHeightDelegate func(uint64) ) type mockEndBlockerApp struct { lastBlockHeightFn lastBlockHeightDelegate loggerFn loggerDelegate + setHaltHeightFn setHaltHeightDelegate } func (m *mockEndBlockerApp) LastBlockHeight() int64 { @@ -220,4 +222,42 @@ func (m *mockEndBlockerApp) Logger() *slog.Logger { return log.NewNoopLogger() } -func (m *mockEndBlockerApp) SetHaltHeight(_ uint64) {} +func (m *mockEndBlockerApp) SetHaltHeight(height uint64) { + if m.setHaltHeightFn != nil { + m.setHaltHeightFn(height) + } +} + +// mockConfigurableParamsKeeper is a ParamsKeeperI that returns values from pre-seeded maps. +type mockConfigurableParamsKeeper struct { + int64s map[string]int64 + strings map[string]string +} + +func (m *mockConfigurableParamsKeeper) GetInt64(ctx sdk.Context, key string, ptr *int64) { + if v, ok := m.int64s[key]; ok { + *ptr = v + } +} +func (m *mockConfigurableParamsKeeper) GetString(ctx sdk.Context, key string, ptr *string) { + if v, ok := m.strings[key]; ok { + *ptr = v + } +} +func (m *mockConfigurableParamsKeeper) GetUint64(ctx sdk.Context, key string, ptr *uint64) {} +func (m *mockConfigurableParamsKeeper) GetBool(ctx sdk.Context, key string, ptr *bool) {} +func (m *mockConfigurableParamsKeeper) GetBytes(ctx sdk.Context, key string, ptr *[]byte) {} +func (m *mockConfigurableParamsKeeper) GetStrings(ctx sdk.Context, key string, ptr *[]string) { +} +func (m *mockConfigurableParamsKeeper) SetString(ctx sdk.Context, key, value string) {} +func (m *mockConfigurableParamsKeeper) SetInt64(ctx sdk.Context, key string, value int64) {} +func (m *mockConfigurableParamsKeeper) SetUint64(ctx sdk.Context, key string, value uint64) {} +func (m *mockConfigurableParamsKeeper) SetBool(ctx sdk.Context, key string, value bool) {} +func (m *mockConfigurableParamsKeeper) SetBytes(ctx sdk.Context, key string, value []byte) {} +func (m *mockConfigurableParamsKeeper) SetStrings(ctx sdk.Context, key string, value []string) { +} +func (m *mockConfigurableParamsKeeper) Has(ctx sdk.Context, key string) bool { return false } +func (m *mockConfigurableParamsKeeper) GetStruct(ctx sdk.Context, key string, strctPtr any) {} +func (m *mockConfigurableParamsKeeper) SetStruct(ctx sdk.Context, key string, strct any) {} +func (m *mockConfigurableParamsKeeper) GetAny(ctx sdk.Context, key string) any { return nil } +func (m *mockConfigurableParamsKeeper) SetAny(ctx sdk.Context, key string, value any) {} diff --git a/gno.land/pkg/gnoland/node_params.go b/gno.land/pkg/gnoland/node_params.go index 183635a583d..7fb0f04d66e 100644 --- a/gno.land/pkg/gnoland/node_params.go +++ b/gno.land/pkg/gnoland/node_params.go @@ -21,7 +21,7 @@ const ( type nodeParamsKeeper struct{} // WillSetParam validates node parameters before they are written to the params store. -func (nodeParamsKeeper) WillSetParam(_ sdk.Context, key string, value any) { +func (nodeParamsKeeper) WillSetParam(ctx sdk.Context, key string, value any) { switch key { case "p:halt_height": h, ok := value.(int64) @@ -31,6 +31,12 @@ func (nodeParamsKeeper) WillSetParam(_ sdk.Context, key string, value any) { if h < 0 { panic(fmt.Sprintf("halt_height must be non-negative, got %d", h)) } + // Reject halt heights that are in the past or present. + // h == 0 is the cancel sentinel and is always allowed. + // safeBlockHeight handles genesis/test contexts where the block header may not be set. + if curHeight := safeBlockHeight(ctx); h > 0 && curHeight > 0 && h <= curHeight { + panic(fmt.Sprintf("halt_height %d must be greater than the current block height %d", h, curHeight)) + } case "p:halt_min_version": _, ok := value.(string) if !ok { @@ -84,7 +90,8 @@ func checkNodeStartupParams(prmk sdkparams.ParamsKeeperI, ms store.MultiStore, l } // Check 2: Prevent new (upgraded) binaries from running before the halt height. - if meetsMinVersion(binaryVersion, minVersion) && binaryVersion != minVersion { + // Any binary that meets the minimum version is rejected until the halt occurs. + if meetsMinVersion(binaryVersion, minVersion) { return fmt.Errorf( "binary version %q is an upgrade intended for halt height %d, "+ "but the chain is at height %d; please use the previous binary until the halt, "+ @@ -96,6 +103,13 @@ func checkNodeStartupParams(prmk sdkparams.ParamsKeeperI, ms store.MultiStore, l return nil } +// safeBlockHeight returns ctx.BlockHeight() or 0 if the context has no block header. +// This handles genesis and test contexts where the header may not be initialized. +func safeBlockHeight(ctx sdk.Context) (h int64) { + defer func() { recover() }() //nolint:errcheck + return ctx.BlockHeight() +} + // meetsMinVersion reports whether binaryVersion satisfies the minVersion requirement. // Versions are expected to follow the "chain/gnolandX.Y" format used for gno.land chain releases. // If either version cannot be parsed in that format, an exact string match is required. From 5eec2ff295203abea3c3a2fa29742793fee0ee2b Mon Sep 17 00:00:00 2001 From: moul <94029+moul@users.noreply.github.com> Date: Thu, 9 Apr 2026 17:36:24 +0200 Subject: [PATCH 7/8] test(node): improve test coverage for EndBlocker and app options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - TestNewAppWithOptions_ErrNoLogger / ErrNoEventSwitch: cover nil-logger and nil-event-switch validation paths in AppOptions.validate(). - TestEndBlocker/extract_updates_error: cover the path where the VM returns a regex-matching response with an unparseable address, causing extractUpdatesFromResponse to return an error and EndBlocker to return an empty response (lines 524-528 in app.go). - TestExtractUpdatesFromResponse: directly test all reachable error paths (empty response, no regex match, invalid bech32 address, invalid pubkey). EndBlocker coverage: 87.9% → 93.9% extractUpdatesFromResponse coverage: 83.3% → 94.4% node_params.go remains at 100%. The two remaining uncovered lines (u.Power < 0 check and strconv.ParseInt error in extractUpdatesFromResponse) are unreachable via normal flow: the regex pattern (\d+) only captures non-negative digits, so parsed powers are always ≥ 0 and ParseInt always succeeds. --- gno.land/pkg/gnoland/app_test.go | 89 ++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 1c5698ba1d3..d6e22562b5d 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -143,6 +143,24 @@ func TestNewAppWithOptions_ErrNoDB(t *testing.T) { assert.ErrorContains(t, err, "no db provided") } +func TestNewAppWithOptions_ErrNoLogger(t *testing.T) { + t.Parallel() + + opts := TestAppOptions(memdb.NewMemDB()) + opts.Logger = nil + _, err := NewAppWithOptions(opts) + assert.ErrorContains(t, err, "no logger provided") +} + +func TestNewAppWithOptions_ErrNoEventSwitch(t *testing.T) { + t.Parallel() + + opts := TestAppOptions(memdb.NewMemDB()) + opts.EventSwitch = nil + _, err := NewAppWithOptions(opts) + assert.ErrorContains(t, err, "no event switch provided") +} + func TestNewApp(t *testing.T) { // NewApp should have good defaults and manage to run InitChain. td := t.TempDir() @@ -900,6 +918,40 @@ func TestEndBlocker(t *testing.T) { // Verify only the valid update is returned require.Len(t, res.ValidatorUpdates, 0) }) + + t.Run("extract updates error", func(t *testing.T) { + t.Parallel() + + var ( + noFilter = func(_ events.Event) []validatorUpdate { + return make([]validatorUpdate, 1) // 1 update + } + + mockEventSwitchInner = newCommonEvSwitch() + + mockVMKeeperInner = &mockVMKeeper{ + queryFn: func(_ sdk.Context, pkgPath, expr string) (string, error) { + require.Equal(t, valRealm, pkgPath) + // Return a response that matches the regex but has an invalid bech32 address. + // This causes extractUpdatesFromResponse to return an error. + return `{("notabech32" std.Address),("notapubkey" string),(1 uint64)}`, nil + }, + } + ) + + c := newCollector[validatorUpdate](mockEventSwitchInner, noFilter) + mockEventSwitchInner.FireEvent(chain.Event{}) + + eb := EndBlocker(c, nil, nil, mockVMKeeperInner, nil, &mockEndBlockerApp{}) + res := eb(sdk.Context{}.WithConsensusParams(&abci.ConsensusParams{ + Validator: &abci.ValidatorParams{ + PubKeyTypeURLs: []string{"/tm.PubKeySecp256k1"}, + }, + }), abci.RequestEndBlock{}) + + // Error from extractUpdatesFromResponse → EndBlocker returns empty response + assert.Equal(t, abci.ResponseEndBlock{}, res) + }) } func TestGasPriceUpdate(t *testing.T) { @@ -1645,3 +1697,40 @@ func TestEndBlockerHalt(t *testing.T) { assert.Equal(t, uint64(0), haltSet, "SetHaltHeight should NOT be called when halt_height=0 (cancelled)") }) } + +func TestExtractUpdatesFromResponse(t *testing.T) { + t.Parallel() + + t.Run("empty response returns nil", func(t *testing.T) { + t.Parallel() + updates, err := extractUpdatesFromResponse("") + require.NoError(t, err) + assert.Nil(t, updates) + }) + + t.Run("no regex match returns nil", func(t *testing.T) { + t.Parallel() + updates, err := extractUpdatesFromResponse("some random string with no validator data") + require.NoError(t, err) + assert.Nil(t, updates) + }) + + t.Run("invalid address", func(t *testing.T) { + t.Parallel() + // The regex captures any quoted string as the address, so we can inject an invalid bech32. + response := `{("notabech32" std.Address),("notapubkey" string),(1 uint64)}` + _, err := extractUpdatesFromResponse(response) + require.Error(t, err) + assert.Contains(t, err.Error(), "unable to parse address") + }) + + t.Run("invalid pubkey", func(t *testing.T) { + t.Parallel() + // Valid bech32 address, but invalid pubkey string. + addr := crypto.AddressFromPreimage([]byte("test")) + response := fmt.Sprintf(`{(%q std.Address),("notapubkey" string),(1 uint64)}`, addr) + _, err := extractUpdatesFromResponse(response) + require.Error(t, err) + assert.Contains(t, err.Error(), "unable to parse public key") + }) +} From 5a5f026b6e7967a345909166474343079a98d229 Mon Sep 17 00:00:00 2001 From: moul <94029+moul@users.noreply.github.com> Date: Mon, 27 Apr 2026 15:07:19 +0000 Subject: [PATCH 8/8] fix(gnoland): gofmt + rename predeclared min in parseGnolandVersion --- gno.land/pkg/gnoland/app_test.go | 6 +++--- gno.land/pkg/gnoland/mock_test.go | 10 +++++----- gno.land/pkg/gnoland/node_params.go | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 6f3cfeca6b1..cead9445e5c 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -1467,9 +1467,9 @@ func TestMeetsMinVersion(t *testing.T) { t.Parallel() cases := []struct { - binary string - minVer string - want bool + binary string + minVer string + want bool }{ // Empty minVersion always passes {"chain/gnoland1.0", "", true}, diff --git a/gno.land/pkg/gnoland/mock_test.go b/gno.land/pkg/gnoland/mock_test.go index eef2d12cef4..65a786067bb 100644 --- a/gno.land/pkg/gnoland/mock_test.go +++ b/gno.land/pkg/gnoland/mock_test.go @@ -266,11 +266,11 @@ func (m *mockConfigurableParamsKeeper) GetBool(ctx sdk.Context, key string, ptr func (m *mockConfigurableParamsKeeper) GetBytes(ctx sdk.Context, key string, ptr *[]byte) {} func (m *mockConfigurableParamsKeeper) GetStrings(ctx sdk.Context, key string, ptr *[]string) { } -func (m *mockConfigurableParamsKeeper) SetString(ctx sdk.Context, key, value string) {} -func (m *mockConfigurableParamsKeeper) SetInt64(ctx sdk.Context, key string, value int64) {} -func (m *mockConfigurableParamsKeeper) SetUint64(ctx sdk.Context, key string, value uint64) {} -func (m *mockConfigurableParamsKeeper) SetBool(ctx sdk.Context, key string, value bool) {} -func (m *mockConfigurableParamsKeeper) SetBytes(ctx sdk.Context, key string, value []byte) {} +func (m *mockConfigurableParamsKeeper) SetString(ctx sdk.Context, key, value string) {} +func (m *mockConfigurableParamsKeeper) SetInt64(ctx sdk.Context, key string, value int64) {} +func (m *mockConfigurableParamsKeeper) SetUint64(ctx sdk.Context, key string, value uint64) {} +func (m *mockConfigurableParamsKeeper) SetBool(ctx sdk.Context, key string, value bool) {} +func (m *mockConfigurableParamsKeeper) SetBytes(ctx sdk.Context, key string, value []byte) {} func (m *mockConfigurableParamsKeeper) SetStrings(ctx sdk.Context, key string, value []string) { } func (m *mockConfigurableParamsKeeper) Has(ctx sdk.Context, key string) bool { return false } diff --git a/gno.land/pkg/gnoland/node_params.go b/gno.land/pkg/gnoland/node_params.go index 7fb0f04d66e..9b373278553 100644 --- a/gno.land/pkg/gnoland/node_params.go +++ b/gno.land/pkg/gnoland/node_params.go @@ -144,9 +144,9 @@ func parseGnolandVersion(v string) (major, minor int, ok bool) { return 0, 0, false } maj, err1 := strconv.Atoi(rest[:dot]) - min, err2 := strconv.Atoi(rest[dot+1:]) + mnr, err2 := strconv.Atoi(rest[dot+1:]) if err1 != nil || err2 != nil { return 0, 0, false } - return maj, min, true + return maj, mnr, true }