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: unify version modifier for v2 #21508

Merged
merged 12 commits into from
Sep 6, 2024
2 changes: 1 addition & 1 deletion core/server/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type TxResult struct {
}

// VersionModifier defines the interface fulfilled by BaseApp
// which allows getting and setting it's appVersion field. This
// which allows getting and setting its appVersion field. This
// in turn updates the consensus params that are sent to the
// consensus engine in EndBlock
type VersionModifier interface {
Expand Down
8 changes: 0 additions & 8 deletions runtime/v2/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"cosmossdk.io/core/comet"
"cosmossdk.io/core/legacy"
"cosmossdk.io/core/registry"
"cosmossdk.io/core/server"
"cosmossdk.io/core/store"
"cosmossdk.io/core/transaction"
"cosmossdk.io/depinject"
Expand Down Expand Up @@ -99,7 +98,6 @@ func init() {
ProvideEnvironment[transaction.Tx],
ProvideModuleManager[transaction.Tx],
ProvideCometService,
ProvideAppVersionModifier[transaction.Tx],
),
appconfig.Invoke(SetupAppBuilder),
)
Expand Down Expand Up @@ -239,9 +237,3 @@ func storeKeyOverride(config *runtimev2.Module, moduleName string) *runtimev2.St
func ProvideCometService() comet.Service {
return &services.ContextAwareCometInfoService{}
}

// ProvideAppVersionModifier returns nil, `app.VersionModifier` is a feature of BaseApp and neither used nor required for runtime/v2.
// nil is acceptable, see: https://github.com/cosmos/cosmos-sdk/blob/0a6ee406a02477ae8ccbfcbe1b51fc3930087f4c/x/upgrade/keeper/keeper.go#L438
func ProvideAppVersionModifier[T transaction.Tx](app *AppBuilder[T]) server.VersionModifier {
return nil
}
4 changes: 3 additions & 1 deletion server/v2/stf/core_branch_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (bs BranchService) ExecuteWithGasLimit(
// restore original context
gasUsed = exCtx.meter.Limit() - exCtx.meter.Remaining()
_ = originalGasMeter.Consume(gasUsed, "execute-with-gas-limit")
exCtx.setGasLimit(originalGasMeter.Limit() - originalGasMeter.Remaining())
exCtx.setGasLimit(originalGasMeter.Remaining())

return gasUsed, err
}
Expand All @@ -62,6 +62,8 @@ func (bs BranchService) execute(ctx *executionContext, f func(ctx context.Contex
branchFn: ctx.branchFn,
makeGasMeter: ctx.makeGasMeter,
makeGasMeteredStore: ctx.makeGasMeteredStore,
msgRouter: ctx.msgRouter,
queryRouter: ctx.queryRouter,
}

err := f(branchedCtx)
Expand Down
8 changes: 6 additions & 2 deletions server/v2/stf/core_branch_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import (
"errors"
"testing"

gogotypes "github.com/cosmos/gogoproto/types"

appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/server/v2/stf/branch"
"cosmossdk.io/server/v2/stf/gas"
"cosmossdk.io/server/v2/stf/mock"
gogotypes "github.com/cosmos/gogoproto/types"
)

func TestBranchService(t *testing.T) {
Expand Down Expand Up @@ -71,6 +70,7 @@ func TestBranchService(t *testing.T) {

t.Run("fail - reverts state", func(t *testing.T) {
stfCtx := makeContext()
originalGas := stfCtx.meter.Remaining()
gasUsed, err := branchService.ExecuteWithGasLimit(stfCtx, 10000, func(ctx context.Context) error {
kvSet(t, ctx, "cookies")
return errors.New("fail")
Expand All @@ -81,6 +81,10 @@ func TestBranchService(t *testing.T) {
if gasUsed == 0 {
t.Error("expected non-zero gasUsed")
}
if stfCtx.meter.Remaining() != originalGas-gasUsed {
t.Error("expected gas to be reverted")
}

stateNotHas(t, stfCtx.state, "cookies")
})

Expand Down
38 changes: 37 additions & 1 deletion x/consensus/depinject.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package consensus

import (
"context"
modulev1 "cosmossdk.io/api/cosmos/consensus/module/v1"
"cosmossdk.io/core/address"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/core/server"
"cosmossdk.io/depinject"
"cosmossdk.io/depinject/appconfig"
"cosmossdk.io/x/consensus/keeper"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/runtime"
Expand All @@ -23,6 +24,7 @@ func init() {
appconfig.RegisterModule(
&modulev1.Module{},
appconfig.Provide(ProvideModule),
appconfig.Provide(ProvideAppVersionModifier),
)
}

Expand Down Expand Up @@ -72,3 +74,37 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
BaseAppOption: baseappOpt,
Copy link
Member

Choose a reason for hiding this comment

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

Line 68, we should return a baseapp option that sets the version modifier too:

	baseappOpt := func(app *baseapp.BaseApp) {
	      app.SetParamStore(k.ParamsStore)
+             app.SetVersionModifier(versionModifier{Keeper: k})
	}

}
}

type versionModifier struct {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Keeper keeper.Keeper
}

func (v versionModifier) SetAppVersion(ctx context.Context, version uint64) error {
params, err := v.Keeper.Params(ctx, nil)
if err != nil {
return err
}

updatedParams := params.Params
updatedParams.Version.App = version

err = v.Keeper.ParamsStore.Set(ctx, *updatedParams)
if err != nil {
return err
}

return nil
}

func (v versionModifier) AppVersion(ctx context.Context) (uint64, error) {
params, err := v.Keeper.Params(ctx, nil)
if err != nil {
return 0, err
}

return params.Params.Version.GetApp(), nil
}

func ProvideAppVersionModifier(k keeper.Keeper) server.VersionModifier {
return versionModifier{Keeper: k}
}
Loading