Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: allow for hardcoded upgrade schedules #2583

Merged
merged 16 commits into from
Oct 10, 2023
Merged
48 changes: 34 additions & 14 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/celestiaorg/celestia-app/x/mint"
mintkeeper "github.com/celestiaorg/celestia-app/x/mint/keeper"
minttypes "github.com/celestiaorg/celestia-app/x/mint/types"
"github.com/celestiaorg/celestia-app/x/upgrade"
"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
nodeservice "github.com/cosmos/cosmos-sdk/client/grpc/node"
Expand Down Expand Up @@ -66,8 +67,6 @@ import (
"github.com/cosmos/cosmos-sdk/x/staking"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
sdkupgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
sdkupgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
"github.com/cosmos/ibc-go/v6/modules/apps/transfer"
ibctransferkeeper "github.com/cosmos/ibc-go/v6/modules/apps/transfer/keeper"
ibctransfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
Expand All @@ -92,7 +91,6 @@ import (
blobmoduletypes "github.com/celestiaorg/celestia-app/x/blob/types"
"github.com/celestiaorg/celestia-app/x/paramfilter"
"github.com/celestiaorg/celestia-app/x/tokenfilter"
appupgrade "github.com/celestiaorg/celestia-app/x/upgrade"

qgbmodule "github.com/celestiaorg/celestia-app/x/qgb"
qgbmodulekeeper "github.com/celestiaorg/celestia-app/x/qgb/keeper"
Expand Down Expand Up @@ -160,7 +158,7 @@ var (

// ModuleEncodingRegisters keeps track of all the module methods needed to
// register interfaces and specific type to encoding config
ModuleEncodingRegisters = extractRegisters(ModuleBasics, appupgrade.TypeRegister{})
ModuleEncodingRegisters = extractRegisters(ModuleBasics, upgrade.TypeRegister{})

// module account permissions
maccPerms = map[string][]string{
Expand Down Expand Up @@ -219,7 +217,7 @@ type App struct {
DistrKeeper distrkeeper.Keeper
GovKeeper govkeeper.Keeper
CrisisKeeper crisiskeeper.Keeper
UpgradeKeeper sdkupgradekeeper.Keeper
UpgradeKeeper upgrade.Keeper
ParamsKeeper paramskeeper.Keeper
IBCKeeper *ibckeeper.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly
EvidenceKeeper evidencekeeper.Keeper
Expand All @@ -235,6 +233,9 @@ type App struct {

// the module manager
mm *module.Manager

// module configurator
configurator module.Configurator
}

// New returns a reference to an initialized celestia app.
Expand All @@ -243,13 +244,18 @@ func New(
db dbm.DB,
traceStore io.Writer,
loadLatest bool,
skipUpgradeHeights map[int64]bool,
homePath string,
invCheckPeriod uint,
encodingConfig encoding.Config,
upgradeSchedule map[string]upgrade.Schedule,
appOpts servertypes.AppOptions,
baseAppOptions ...func(*baseapp.BaseApp),
) *App {
for _, schedule := range upgradeSchedule {
if err := schedule.ValidateVersions(supportedVersions); err != nil {
panic(err)
}
}

appCodec := encodingConfig.Codec
cdc := encodingConfig.Amino
interfaceRegistry := encodingConfig.InterfaceRegistry
Expand All @@ -262,7 +268,7 @@ func New(
keys := sdk.NewKVStoreKeys(
authtypes.StoreKey, authzkeeper.StoreKey, banktypes.StoreKey, stakingtypes.StoreKey,
minttypes.StoreKey, distrtypes.StoreKey, slashingtypes.StoreKey,
govtypes.StoreKey, paramstypes.StoreKey, sdkupgradetypes.StoreKey, feegrant.StoreKey,
govtypes.StoreKey, paramstypes.StoreKey, upgrade.StoreKey, feegrant.StoreKey,
evidencetypes.StoreKey, capabilitytypes.StoreKey,
blobmoduletypes.StoreKey,
qgbmoduletypes.StoreKey,
Expand Down Expand Up @@ -328,7 +334,7 @@ func New(
)

app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, keys[feegrant.StoreKey], app.AccountKeeper)
app.UpgradeKeeper = sdkupgradekeeper.NewKeeper(skipUpgradeHeights, keys[sdkupgradetypes.StoreKey], appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
app.UpgradeKeeper = upgrade.NewKeeper(keys[upgrade.StoreKey], upgradeSchedule)

app.QgbKeeper = *qgbmodulekeeper.NewKeeper(
appCodec,
Expand Down Expand Up @@ -463,6 +469,7 @@ func New(
paramstypes.ModuleName,
authz.ModuleName,
vestingtypes.ModuleName,
upgrade.ModuleName,
)

app.mm.SetOrderEndBlockers(
Expand All @@ -485,6 +492,7 @@ func New(
paramstypes.ModuleName,
authz.ModuleName,
vestingtypes.ModuleName,
upgrade.ModuleName,
)

// NOTE: The genutils module must occur after staking so that pools are
Expand Down Expand Up @@ -512,16 +520,16 @@ func New(
feegrant.ModuleName,
paramstypes.ModuleName,
authz.ModuleName,
sdkupgradetypes.ModuleName,
upgrade.ModuleName,
)

app.QueryRouter().AddRoute(proof.TxInclusionQueryPath, proof.QueryTxInclusionProof)
app.QueryRouter().AddRoute(proof.ShareInclusionQueryPath, proof.QueryShareInclusionProof)

app.mm.RegisterInvariants(&app.CrisisKeeper)
app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino)
configurator := module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
app.mm.RegisterServices(configurator)
app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
app.mm.RegisterServices(app.configurator)

// initialize stores
app.MountKVStores(keys)
Expand Down Expand Up @@ -565,7 +573,17 @@ func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.R

// EndBlocker application updates every end block
func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
return app.mm.EndBlock(ctx, req)
res := app.mm.EndBlock(ctx, req)
if app.UpgradeKeeper.ShouldUpgrade() {
newAppVersion := app.UpgradeKeeper.GetNextAppVersion()
app.SetProtocolVersion(newAppVersion)
_, err := app.mm.RunMigrations(ctx, app.configurator, GetModuleVersion(newAppVersion))
if err != nil {
panic(err)
}
app.UpgradeKeeper.MarkUpgradeComplete()
}
return res
}

// InitChainer application update at chain initialization
Expand All @@ -574,7 +592,9 @@ func (app *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.Res
if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil {
panic(err)
}
app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap())
if req.ConsensusParams != nil && req.ConsensusParams.Version != nil {
app.SetProtocolVersion(req.ConsensusParams.Version.AppVersion)
}
return app.mm.InitGenesis(ctx, app.appCodec, genesisState)
}

Expand Down
2 changes: 1 addition & 1 deletion app/default_overrides.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func DefaultConsensusParams() *tmproto.ConsensusParams {
Evidence: DefaultEvidenceParams(),
Validator: coretypes.DefaultValidatorParams(),
Version: tmproto.VersionParams{
AppVersion: appconsts.LatestVersion,
AppVersion: DefaultInitialVersion,
},
}
}
Expand Down
23 changes: 23 additions & 0 deletions app/deliver_tx.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package app

import (
"fmt"

"github.com/celestiaorg/celestia-app/x/upgrade"
abci "github.com/tendermint/tendermint/abci/types"
)

func (app *App) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
sdkTx, err := app.txConfig.TxDecoder()(req.Tx)
if err == nil {
if appVersion, ok := upgrade.IsUpgradeMsg(sdkTx.GetMsgs()); ok {
if !IsSupported(appVersion) {
panic(fmt.Sprintf("network has upgraded to version %d which is not supported by this node. Please upgrade and restart", appVersion))
}
app.UpgradeKeeper.PrepareUpgradeAtEndBlock(appVersion)
// TODO: we may want to emit an event for this
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved
return abci.ResponseDeliverTx{Code: abci.CodeTypeOK}
}
}
return app.BaseApp.DeliverTx(req)
}
30 changes: 30 additions & 0 deletions app/prepare_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/celestiaorg/celestia-app/pkg/da"
"github.com/celestiaorg/celestia-app/pkg/shares"
"github.com/celestiaorg/celestia-app/pkg/square"
"github.com/celestiaorg/celestia-app/x/upgrade"
"github.com/cosmos/cosmos-sdk/telemetry"
abci "github.com/tendermint/tendermint/abci/types"
core "github.com/tendermint/tendermint/proto/tendermint/types"
Expand Down Expand Up @@ -58,6 +59,27 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr
txs = make([][]byte, 0)
} else {
txs = FilterTxs(app.Logger(), sdkCtx, handler, app.txConfig, req.BlockData.Txs)

// TODO: this would be improved if we only attempted the upgrade in the first round of the
// height to still allow transactions to pass through without being delayed from trying
// to coordinate the upgrade height
if newVersion, ok := app.UpgradeKeeper.ShouldProposeUpgrade(req.ChainId, req.Height); ok && newVersion > app.GetBaseApp().AppVersion() {
upgradeTx, err := upgrade.NewMsgVersionChange(app.txConfig, newVersion)
if err != nil {
panic(err)
}
// the upgrade transaction must be the first transaction in the block
txs = append([][]byte{upgradeTx}, txs...)
cmwaters marked this conversation as resolved.
Show resolved Hide resolved

// because we are adding bytes, we need to check that we are not going over the limit
// if we are, we continually prune the last tx (the lowest paying blobTx).
size := sizeOf(txs)
for size > int(req.BlockDataSize) {
lastTx := txs[len(txs)-1]
txs = txs[:len(txs)-1]
size -= len(lastTx)
}
cmwaters marked this conversation as resolved.
Show resolved Hide resolved
}
}

// build the square from the set of valid and prioritised transactions.
Expand Down Expand Up @@ -102,3 +124,11 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr
},
}
}

func sizeOf(txs [][]byte) int {
size := 0
for _, tx := range txs {
size += len(tx)
}
return size
}
28 changes: 26 additions & 2 deletions app/process_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/celestiaorg/celestia-app/pkg/shares"
"github.com/celestiaorg/celestia-app/pkg/square"
blobtypes "github.com/celestiaorg/celestia-app/x/blob/types"
"github.com/celestiaorg/celestia-app/x/upgrade"
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
abci "github.com/tendermint/tendermint/abci/types"
Expand Down Expand Up @@ -66,19 +67,42 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp

// handle non-blob transactions first
if !isBlobTx {
_, has := hasPFB(sdkTx.GetMsgs())
msgs := sdkTx.GetMsgs()

_, has := hasPFB(msgs)
if has {
// A non blob tx has a PFB, which is invalid
logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("tx %d has PFB but is not a blob tx", idx))
return reject()
}

if appVersion, ok := upgrade.IsUpgradeMsg(msgs); ok {
if idx != 0 {
logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("upgrade message %d is not the first transaction", idx))
return reject()
}

if !IsSupported(appVersion) {
logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("block proposes an unsupported app version %d", appVersion))
return reject()
}
Comment on lines +79 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a scenario where nodes running v1 that don't know how to parse this msg or have this logic vote for the proposal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I thought about this. This was why I chose to wrap it as a tx rather than having it unparseable which would then be accepted. As a tx, it would fail the ante handler because there is no signature attached to it. Ideally I can write a test to check it out.


// app version must always increase
if appVersion <= app.GetBaseApp().AppVersion() {
logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("block proposes an app version %d that is not greater than the current app version %d", appVersion, app.GetBaseApp().AppVersion()))
return reject()
}
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved

// we don't need to pass this message through the ante handler
continue
}

// we need to increment the sequence for every transaction so that
// the signature check below is accurate. this error only gets hit
// if the account in question doens't exist.
sdkCtx, err = handler(sdkCtx, sdkTx, false)
if err != nil {
logInvalidPropBlockError(app.Logger(), req.Header, "failure to incrememnt sequence", err)
logInvalidPropBlockError(app.Logger(), req.Header, "failure to increment sequence", err)
return reject()
}

Expand Down
94 changes: 94 additions & 0 deletions app/version.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package app

import (
"fmt"

v1 "github.com/celestiaorg/celestia-app/pkg/appconsts/v1"
v2 "github.com/celestiaorg/celestia-app/pkg/appconsts/v2"
"github.com/celestiaorg/celestia-app/x/blob"
"github.com/celestiaorg/celestia-app/x/mint"
"github.com/celestiaorg/celestia-app/x/qgb"
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/cosmos/cosmos-sdk/x/auth"
"github.com/cosmos/cosmos-sdk/x/auth/vesting"
authzmodule "github.com/cosmos/cosmos-sdk/x/authz/module"
"github.com/cosmos/cosmos-sdk/x/bank"
"github.com/cosmos/cosmos-sdk/x/capability"
"github.com/cosmos/cosmos-sdk/x/crisis"
"github.com/cosmos/cosmos-sdk/x/distribution"
"github.com/cosmos/cosmos-sdk/x/evidence"
feegrantmodule "github.com/cosmos/cosmos-sdk/x/feegrant/module"
"github.com/cosmos/cosmos-sdk/x/genutil"
"github.com/cosmos/cosmos-sdk/x/gov"
"github.com/cosmos/cosmos-sdk/x/params"
"github.com/cosmos/cosmos-sdk/x/slashing"
"github.com/cosmos/cosmos-sdk/x/staking"
"github.com/cosmos/ibc-go/v6/modules/apps/transfer"
ibc "github.com/cosmos/ibc-go/v6/modules/core"
)

var (
// versions that the current state machine supports
supportedVersions = []uint64{v1.Version, v2.Version}

v1moduleVersionMap = module.VersionMap{
"bank": bank.AppModule{}.ConsensusVersion(),
"auth": auth.AppModule{}.ConsensusVersion(),
"authz": authzmodule.AppModule{}.ConsensusVersion(),
"staking": staking.AppModule{}.ConsensusVersion(),
"mint": mint.AppModule{}.ConsensusVersion(),
"distribution": distribution.AppModule{}.ConsensusVersion(),
"slashing": slashing.AppModule{}.ConsensusVersion(),
"gov": gov.AppModule{}.ConsensusVersion(),
"params": params.AppModule{}.ConsensusVersion(),
"vesting": vesting.AppModule{}.ConsensusVersion(),
"feegrant": feegrantmodule.AppModule{}.ConsensusVersion(),
"evidence": evidence.AppModule{}.ConsensusVersion(),
"crisis": crisis.AppModule{}.ConsensusVersion(),
"genutil": genutil.AppModule{}.ConsensusVersion(),
"capability": capability.AppModule{}.ConsensusVersion(),
"blob": blob.AppModule{}.ConsensusVersion(),
"qgb": qgb.AppModule{}.ConsensusVersion(),
"ibc": ibc.AppModule{}.ConsensusVersion(),
"transfer": transfer.AppModule{}.ConsensusVersion(),
}
Comment on lines +34 to +54
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way we map app versions to module versions will need to be modified later on as it doesn't quite fit to how the SDK are used to doing migrations. I will follow up with this in a later PR but for now this makes the tests pass and somewhat signals how migrations will work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can dig it


// There is currently complete parity between v1 and v2 modules, but this
// will likely change
v2moduleVersionMap = v1moduleVersionMap
)

const DefaultInitialVersion = v1.Version

// this is used as a compile time consistency check across different module
// based maps
func init() {
for moduleName := range ModuleBasics {
for _, v := range supportedVersions {
versionMap := GetModuleVersion(v)
if _, ok := versionMap[moduleName]; !ok {
panic(fmt.Sprintf("inconsistency: module %s not found in module version map for version %d", moduleName, v))
}
}
}
}

func IsSupported(version uint64) bool {
for _, v := range supportedVersions {
if v == version {
return true
}
}
return false
}

func GetModuleVersion(appVersion uint64) module.VersionMap {
switch appVersion {
case v1.Version:
return v1moduleVersionMap
case v2.Version:
return v2moduleVersionMap
default:
panic(fmt.Sprintf("unsupported app version %d", appVersion))
}
}
Loading