From be7bd1e5e0a1c4ca4901d0a3342c46bd786df04e Mon Sep 17 00:00:00 2001 From: jchappelow <140431406+jchappelow@users.noreply.github.com> Date: Tue, 1 Oct 2024 15:31:07 -0500 Subject: [PATCH] chain: bump appVersion to 1 (#1038) * chain: bump appVersion to 1 * remove app version from genesis doc --- cmd/kwild/server/build.go | 20 +++++++++++++++++++- cmd/kwild/server/cometbft.go | 6 ++++-- common/chain/chaincfg.go | 8 -------- extensions/consensus/consensus.go | 19 +++++++++++++++++-- internal/abci/cometbft/genesis.go | 10 ++++------ internal/abci/utils.go | 3 --- test/nodes/fork/gremlin.go | 3 +-- 7 files changed, 45 insertions(+), 24 deletions(-) diff --git a/cmd/kwild/server/build.go b/cmd/kwild/server/build.go index 874b3e429..842332526 100644 --- a/cmd/kwild/server/build.go +++ b/cmd/kwild/server/build.go @@ -52,6 +52,24 @@ import ( "github.com/kwilteam/kwil-db/internal/voting/broadcast" ) +var ( + // AppVersion is the application's "protocol version", signaling changes in + // the application's component of the state machine that could impact + // consensus. This version should be bumped whenever changes in the + // application's logic might cause nodes with different versions to come to + // different conclusions about the validity of transactions, blocks, votes, + // or the resulting state. + // + // It should NOT be bumped for changes that do not affect consensus, such as + // performance optimizations, logging updates, or bug fixes that do not + // alter how transactions are validated or blocks are processed. + // + // Custom applications, especially those use extensions to extend or modify + // the state machine's logic, should set this variable appropriately prior + // to starting the Server or executing the root command. + AppVersion uint64 = 1 +) + // initStores prepares the datastores with an atomic DB transaction. These // actions are performed outside of ABCI's DB sessions. The stores should not // keep the db after initialization. Their functions accept a DB connection. @@ -486,7 +504,7 @@ func buildAbci(d *coreDependencies, db *pg.DB, txApp abci.TxApp, snapshotter *st cfg := &abci.AbciConfig{ GenesisAppHash: d.genesisCfg.ComputeGenesisHash(), ChainID: d.genesisCfg.ChainID, - ApplicationVersion: d.genesisCfg.ConsensusParams.Version.App, + ApplicationVersion: AppVersion, GenesisAllocs: d.genesisCfg.Alloc, GasEnabled: !d.genesisCfg.ConsensusParams.WithoutGasCosts, ForkHeights: d.genesisCfg.ForkHeights, diff --git a/cmd/kwild/server/cometbft.go b/cmd/kwild/server/cometbft.go index 649ae98e7..9305f8620 100644 --- a/cmd/kwild/server/cometbft.go +++ b/cmd/kwild/server/cometbft.go @@ -141,10 +141,12 @@ func newCometConfig(cfg *config.KwildConfig) *cmtCfg.Config { // extractGenesisDoc is used by cometbft while initializing the node to extract // the genesis configuration. Note that cometbft's GenesisDoc is a subset of -// kwild's genesis file. +// kwild's genesis file. The app version set in AppVersion is used to supply +// cometbft with the application protocol version, which is determined by the +// app code rather than a configurable value in our genesis config. func extractGenesisDoc(g *chain.GenesisConfig) (*cmttypes.GenesisDoc, error) { // BaseConsensusParms => cometbft's ConsensusParms - consensusParams := cometbft.ExtractConsensusParams(&g.ConsensusParams.BaseConsensusParams) + consensusParams := cometbft.ExtractConsensusParams(&g.ConsensusParams.BaseConsensusParams, AppVersion) genDoc := &cmttypes.GenesisDoc{ ChainID: g.ChainID, diff --git a/common/chain/chaincfg.go b/common/chain/chaincfg.go index fc4d36374..94dfd04a5 100644 --- a/common/chain/chaincfg.go +++ b/common/chain/chaincfg.go @@ -71,7 +71,6 @@ type GenesisValidator struct { type BaseConsensusParams struct { Block BlockParams `json:"block"` Evidence EvidenceParams `json:"evidence"` - Version VersionParams `json:"version"` Validator ValidatorParams `json:"validator"` Votes VoteParams `json:"votes"` ABCI ABCIParams `json:"abci"` @@ -123,10 +122,6 @@ type VoteParams struct { MaxVotesPerTx int64 `json:"max_votes_per_tx"` } -type VersionParams struct { - App uint64 `json:"app"` -} - type MigrationParams struct { // StartHeight is the height from which the state from the old chain is to be migrated. StartHeight int64 `json:"start_height,omitempty"` @@ -155,9 +150,6 @@ func defaultConsensusParams() *ConsensusParams { MaxAgeDuration: 48 * time.Hour, // 2 days MaxBytes: 1024 * 1024, // 1 MiB }, - Version: VersionParams{ - App: 0, - }, Validator: ValidatorParams{ PubKeyTypes: []string{abciPubKeyTypeEd25519}, JoinExpiry: 100800, // approx 1 week considering block rate of 6 sec/blk diff --git a/extensions/consensus/consensus.go b/extensions/consensus/consensus.go index a2ae4558c..2eb1d7648 100644 --- a/extensions/consensus/consensus.go +++ b/extensions/consensus/consensus.go @@ -141,12 +141,27 @@ type Hardfork struct { type ParamUpdates struct { Block *chain.BlockParams `json:"block,omitempty"` Evidence *chain.EvidenceParams `json:"evidence,omitempty"` - Version *chain.VersionParams `json:"version,omitempty"` + Version *VersionParams `json:"version,omitempty"` Validator *chain.ValidatorParams `json:"validator,omitempty"` Votes *chain.VoteParams `json:"votes,omitempty"` ABCI *chain.ABCIParams `json:"abci,omitempty"` } +// VersionParams contains an update to the application protocol version to give +// to cometbft. While not required, this can help proactively signal a new +// application version with altered logic that affects state machine. For +// instance, a coordinated height-based upgrade to consensus logic could be +// accompanied by an update to this version to ensure all nodes made the same +// update at the same height. Canonical configurable hard fork activations on +// the other hand would not do this; only hard-coded changes. +// +// In the case of new major release that has incompatible logic, a global +// AppVersion would be bumped (compared to the old network) at genesis instead +// of using this update, which is to change a live network. +type VersionParams struct { + App uint64 `json:"app"` +} + // ResolutionMod defines a modification to a consensus resolution used by the // oracle system. A modification may be adding a new resolution, or modifying or // removing an existing resolution. @@ -237,7 +252,7 @@ func MergeConsensusUpdates(params, update *ParamUpdates) { if update.Version != nil { if update.Version.App > 0 { if params.Version == nil { - params.Version = new(chain.VersionParams) + params.Version = new(VersionParams) } params.Version.App = update.Version.App } diff --git a/internal/abci/cometbft/genesis.go b/internal/abci/cometbft/genesis.go index 82bd3028d..fec2c51c4 100644 --- a/internal/abci/cometbft/genesis.go +++ b/internal/abci/cometbft/genesis.go @@ -27,8 +27,9 @@ func AddrBookPath(chainRootDir string) string { // ExtractConsensusParams creates cometbft's ConsensusParams from kwild's, which // includes a subset of cometbft's and other parameters that pertain to the ABCI -// application (kwild) rather than the consensus engine (cometbft). -func ExtractConsensusParams(cp *chain.BaseConsensusParams) *cmtTypes.ConsensusParams { +// application (kwild) rather than the consensus engine (cometbft). The +// appVersion indicates state machine logic and it not an application parameter. +func ExtractConsensusParams(cp *chain.BaseConsensusParams, appVersion uint64) *cmtTypes.ConsensusParams { return &cmtTypes.ConsensusParams{ Block: cmtTypes.BlockParams{ MaxBytes: cp.Block.MaxBytes, @@ -40,7 +41,7 @@ func ExtractConsensusParams(cp *chain.BaseConsensusParams) *cmtTypes.ConsensusPa MaxBytes: cp.Evidence.MaxBytes, }, Version: cmtTypes.VersionParams{ - App: cp.Version.App, + App: appVersion, }, Validator: cmtTypes.ValidatorParams{ PubKeyTypes: cp.Validator.PubKeyTypes, @@ -65,9 +66,6 @@ func MergeConsensusParams(cometbftParams *cmtTypes.ConsensusParams, abciParams * MaxAgeDuration: cometbftParams.Evidence.MaxAgeDuration, MaxBytes: cometbftParams.Evidence.MaxBytes, }, - Version: chain.VersionParams{ - App: cometbftParams.Version.App, - }, Validator: chain.ValidatorParams{ PubKeyTypes: cometbftParams.Validator.PubKeyTypes, JoinExpiry: abciParams.JoinExpiry, diff --git a/internal/abci/utils.go b/internal/abci/utils.go index 0b7375532..04323c672 100644 --- a/internal/abci/utils.go +++ b/internal/abci/utils.go @@ -55,9 +55,6 @@ func updateConsensusParams(p *chain.ConsensusParams, up *consensus.ParamUpdates) p.Evidence.MaxAgeNumBlocks = up.Evidence.MaxAgeNumBlocks p.Evidence.MaxBytes = up.Evidence.MaxBytes } - if up.Version != nil { // if set, expect all set - p.Version.App = up.Version.App - } if up.Validator != nil { if pkt := up.Validator.PubKeyTypes; len(pkt) > 0 { p.Validator.PubKeyTypes = pkt diff --git a/test/nodes/fork/gremlin.go b/test/nodes/fork/gremlin.go index ce43d357d..068157e7d 100644 --- a/test/nodes/fork/gremlin.go +++ b/test/nodes/fork/gremlin.go @@ -9,7 +9,6 @@ import ( "math/big" "github.com/kwilteam/kwil-db/common" - "github.com/kwilteam/kwil-db/common/chain" "github.com/kwilteam/kwil-db/common/functions" "github.com/kwilteam/kwil-db/core/types/serialize" "github.com/kwilteam/kwil-db/core/types/transactions" @@ -53,7 +52,7 @@ func init() { }, ParamsUpdates: &consensus.ParamUpdates{ - Version: &chain.VersionParams{ + Version: &consensus.VersionParams{ App: 9876, }, },