LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

refactor(core)!: clean-up core and simplify preblock (#19672)

Browse Source
This commit is contained in:
Julien Robert
2024-03-11 14:39:37 +00:00
committed by GitHub
parent defab1a1a1
commit fea88d13c5
21 changed files with 110 additions and 177 deletions
Download Patch File Download Diff File Expand all files Collapse all files
x/upgrade/CHANGELOG.md
+5 -1
View File
@@ -25,13 +25,17 @@ Ref: https://keepachangelog.com/en/1.0.0/
## [Unreleased]
### Improvements
* [#19672](https://github.com/cosmos/cosmos-sdk/pull/19672) Follow latest `cosmossdk.io/core` `PreBlock` simplification.
### State Machine Breaking
* (x/upgrade) [#16244](https://github.com/cosmos/cosmos-sdk/pull/16244) Upgrade module no longer stores the app version but gets and sets the app version stored in the `ParamStore` of baseapp.
### API Breaking Changes
* [#19443](https://github.com/cosmos/cosmos-sdk/pull/19443) Creation of upgrade module receives `appmodule.Environment` instead of individual services
* [#19443](https://github.com/cosmos/cosmos-sdk/pull/19443) `NewKeeper` takes an `appmodule.Environment` instead of individual services.
## [v0.1.1](https://github.com/cosmos/cosmos-sdk/releases/tag/x/upgrade/v0.1.1) - 2023-12-11
x/upgrade/keeper/abci.go
+13 -24
View File
@@ -6,7 +6,6 @@ import (
"fmt"
"time"
"cosmossdk.io/core/appmodule"
storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/upgrade/types"
@@ -22,13 +21,13 @@ import (
// The purpose is to ensure the binary is switched EXACTLY at the desired block, and to allow
// a migration to be executed if needed upon this switch (migration defined in the new binary)
// skipUpgradeHeightArray is a set of block heights for which the upgrade must be skipped
func (k Keeper) PreBlocker(ctx context.Context) (appmodule.ResponsePreBlock, error) {
func (k Keeper) PreBlocker(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
blockHeight := k.environment.HeaderService.GetHeaderInfo(ctx).Height
plan, err := k.GetUpgradePlan(ctx)
if err != nil && !errors.Is(err, types.ErrNoUpgradePlanFound) {
return nil, err
return err
}
found := err == nil
@@ -43,7 +42,7 @@ func (k Keeper) PreBlocker(ctx context.Context) (appmodule.ResponsePreBlock, err
if !found || !plan.ShouldExecute(blockHeight) || (plan.ShouldExecute(blockHeight) && k.IsSkipHeight(blockHeight)) {
lastAppliedPlan, _, err := k.GetLastCompletedUpgrade(ctx)
if err != nil {
return nil, err
return err
}
if lastAppliedPlan != "" && !k.HasHandler(lastAppliedPlan) {
@@ -54,15 +53,13 @@ func (k Keeper) PreBlocker(ctx context.Context) (appmodule.ResponsePreBlock, err
appVersion = cp.Version.App
}
return nil, fmt.Errorf("wrong app version %d, upgrade handler is missing for %s upgrade plan", appVersion, lastAppliedPlan)
return fmt.Errorf("wrong app version %d, upgrade handler is missing for %s upgrade plan", appVersion, lastAppliedPlan)
}
}
}
if !found {
return &sdk.ResponsePreBlock{
ConsensusParamsChanged: false,
}, nil
return nil
}
logger := k.Logger(ctx)
@@ -76,11 +73,9 @@ func (k Keeper) PreBlocker(ctx context.Context) (appmodule.ResponsePreBlock, err
// Clear the upgrade plan at current height
if err := k.ClearUpgradePlan(ctx); err != nil {
return nil, err
return err
}
return &sdk.ResponsePreBlock{
ConsensusParamsChanged: false,
}, nil
return nil
}
// Prepare shutdown if we don't have an upgrade handler for this upgrade name (meaning this software is out of date)
@@ -89,27 +84,23 @@ func (k Keeper) PreBlocker(ctx context.Context) (appmodule.ResponsePreBlock, err
// store migrations.
err := k.DumpUpgradeInfoToDisk(blockHeight, plan)
if err != nil {
return nil, fmt.Errorf("unable to write upgrade info to filesystem: %w", err)
return fmt.Errorf("unable to write upgrade info to filesystem: %w", err)
}
upgradeMsg := BuildUpgradeNeededMsg(plan)
logger.Error(upgradeMsg)
// Returning an error will end up in a panic
return nil, errors.New(upgradeMsg)
return errors.New(upgradeMsg)
}
// We have an upgrade handler for this upgrade name, so apply the upgrade
logger.Info(fmt.Sprintf("applying upgrade \"%s\" at %s", plan.Name, plan.DueAt()))
sdkCtx = sdkCtx.WithBlockGasMeter(storetypes.NewInfiniteGasMeter())
if err := k.ApplyUpgrade(sdkCtx, plan); err != nil {
return nil, err
return err
}
return &sdk.ResponsePreBlock{
// the consensus parameters might be modified in the migration,
// refresh the consensus parameters in context.
ConsensusParamsChanged: true,
}, nil
return nil
}
// if we have a pending upgrade, but it is not yet time, make sure we did not
@@ -119,11 +110,9 @@ func (k Keeper) PreBlocker(ctx context.Context) (appmodule.ResponsePreBlock, err
logger.Error(downgradeMsg)
// Returning an error will end up in a panic
return nil, errors.New(downgradeMsg)
return errors.New(downgradeMsg)
}
return &sdk.ResponsePreBlock{
ConsensusParamsChanged: false,
}, nil
return nil
}
// BuildUpgradeNeededMsg prints the message that notifies that an upgrade is needed.
x/upgrade/keeper/abci_test.go
+17 -17
View File
@@ -45,7 +45,7 @@ func (s *TestSuite) VerifyDoUpgrade(t *testing.T) {
t.Log("Verify that a panic happens at the upgrade height")
newCtx := s.ctx.WithHeaderInfo(header.Info{Height: s.ctx.HeaderInfo().Height + 1, Time: time.Now()})
_, err := s.preModule.PreBlock(newCtx)
err := s.preModule.PreBlock(newCtx)
require.ErrorContains(t, err, "UPGRADE \"test\" NEEDED at height: 11: ")
t.Log("Verify that the upgrade can be successfully applied with a handler")
@@ -53,7 +53,7 @@ func (s *TestSuite) VerifyDoUpgrade(t *testing.T) {
return vm, nil
})
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.NoError(t, err)
s.VerifyCleared(t, newCtx)
@@ -63,7 +63,7 @@ func (s *TestSuite) VerifyDoUpgradeWithCtx(t *testing.T, newCtx sdk.Context, pro
t.Helper()
t.Log("Verify that a panic happens at the upgrade height")
_, err := s.preModule.PreBlock(newCtx)
err := s.preModule.PreBlock(newCtx)
require.ErrorContains(t, err, "UPGRADE \""+proposalName+"\" NEEDED at height: ")
t.Log("Verify that the upgrade can be successfully applied with a handler")
@@ -71,7 +71,7 @@ func (s *TestSuite) VerifyDoUpgradeWithCtx(t *testing.T, newCtx sdk.Context, pro
return vm, nil
})
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.NoError(t, err)
s.VerifyCleared(t, newCtx)
@@ -175,21 +175,21 @@ func TestHaltIfTooNew(t *testing.T) {
})
newCtx := s.ctx.WithHeaderInfo(header.Info{Height: s.ctx.HeaderInfo().Height + 1, Time: time.Now()})
_, err := s.preModule.PreBlock(newCtx)
err := s.preModule.PreBlock(newCtx)
require.NoError(t, err)
require.Equal(t, 0, called)
t.Log("Verify we error if we have a registered handler ahead of time")
err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "future", Height: s.ctx.HeaderInfo().Height + 3})
require.NoError(t, err)
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.EqualError(t, err, "BINARY UPDATED BEFORE TRIGGER! UPGRADE \"future\" - in binary but not executed on chain. Downgrade your binary")
require.Equal(t, 0, called)
t.Log("Verify we no longer panic if the plan is on time")
futCtx := s.ctx.WithHeaderInfo(header.Info{Height: s.ctx.HeaderInfo().Height + 3, Time: time.Now()})
_, err = s.preModule.PreBlock(futCtx)
err = s.preModule.PreBlock(futCtx)
require.NoError(t, err)
require.Equal(t, 1, called)
@@ -223,7 +223,7 @@ func TestCantApplySameUpgradeTwice(t *testing.T) {
func TestNoSpuriousUpgrades(t *testing.T) {
s := setupTest(t, 10, map[int64]bool{})
t.Log("Verify that no upgrade panic is triggered in the BeginBlocker when we haven't scheduled an upgrade")
_, err := s.preModule.PreBlock(s.ctx)
err := s.preModule.PreBlock(s.ctx)
require.NoError(t, err)
}
@@ -260,7 +260,7 @@ func TestSkipUpgradeSkippingAll(t *testing.T) {
s.VerifySet(t, map[int64]bool{skipOne: true, skipTwo: true})
newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipOne})
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.NoError(t, err)
t.Log("Verify a second proposal also is being cleared")
@@ -268,7 +268,7 @@ func TestSkipUpgradeSkippingAll(t *testing.T) {
require.NoError(t, err)
newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipTwo})
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.NoError(t, err)
// To ensure verification is being done only after both upgrades are cleared
@@ -295,7 +295,7 @@ func TestUpgradeSkippingOne(t *testing.T) {
// Setting block height of proposal test
newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipOne})
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.NoError(t, err)
t.Log("Verify the second proposal is not skipped")
@@ -328,7 +328,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) {
// Setting block height of proposal test
newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipOne})
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.NoError(t, err)
// A new proposal with height in skipUpgradeHeights
@@ -336,7 +336,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) {
require.NoError(t, err)
// Setting block height of proposal test2
newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipTwo})
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.NoError(t, err)
t.Log("Verify a new proposal is not skipped")
@@ -357,7 +357,7 @@ func TestUpgradeWithoutSkip(t *testing.T) {
err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1})
require.NoError(t, err)
t.Log("Verify if upgrade happens without skip upgrade")
_, err = s.preModule.PreBlock(newCtx)
err = s.preModule.PreBlock(newCtx)
require.ErrorContains(t, err, "UPGRADE \"test\" NEEDED at height:")
s.VerifyDoUpgrade(t)
@@ -447,7 +447,7 @@ func TestBinaryVersion(t *testing.T) {
for _, tc := range testCases {
ctx := tc.preRun()
_, err := s.preModule.PreBlock(ctx)
err := s.preModule.PreBlock(ctx)
if tc.expectError {
require.Error(t, err)
} else {
@@ -486,7 +486,7 @@ func TestDowngradeVerification(t *testing.T) {
})
// successful upgrade.
_, err = m.PreBlock(ctx)
err = m.PreBlock(ctx)
require.NoError(t, err)
ctx = ctx.WithHeaderInfo(header.Info{Height: ctx.HeaderInfo().Height + 1})
@@ -536,7 +536,7 @@ func TestDowngradeVerification(t *testing.T) {
tc.preRun(k, ctx, name)
}
_, err = m.PreBlock(ctx)
err = m.PreBlock(ctx)
if tc.expectError {
require.Error(t, err, name)
} else {
x/upgrade/module.go
+1 -1
View File
@@ -151,6 +151,6 @@ func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
// PreBlock calls the upgrade module hooks
//
// CONTRACT: this is called *before* all other modules' BeginBlock functions
func (am AppModule) PreBlock(ctx context.Context) (appmodule.ResponsePreBlock, error) {
func (am AppModule) PreBlock(ctx context.Context) error {
return am.keeper.PreBlocker(ctx)
}