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
3 changes: 3 additions & 0 deletions server/v2/stf/core_branch_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

gogotypes "github.com/cosmos/gogoproto/types"
"github.com/stretchr/testify/require"

appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/server/v2/stf/branch"
Expand Down Expand Up @@ -71,6 +72,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 +83,7 @@ func TestBranchService(t *testing.T) {
if gasUsed == 0 {
t.Error("expected non-zero gasUsed")
}
require.Equal(t, int(originalGas-gasUsed), int(stfCtx.meter.Remaining()))
randygrok marked this conversation as resolved.
Show resolved Hide resolved
stateNotHas(t, stfCtx.state, "cookies")
})

Expand Down
4 changes: 4 additions & 0 deletions server/v2/stf/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ replace cosmossdk.io/core => ../../../core
require (
cosmossdk.io/core v0.11.0
github.com/cosmos/gogoproto v1.7.0
github.com/stretchr/testify v1.8.4
github.com/tidwall/btree v1.7.0
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
google.golang.org/protobuf v1.34.2 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
10 changes: 10 additions & 0 deletions server/v2/stf/go.sum
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
github.com/cosmos/gogoproto v1.7.0 h1:79USr0oyXAbxg3rspGh/m4SWNyoz/GLaAh0QlCe2fro=
github.com/cosmos/gogoproto v1.7.0/go.mod h1:yWChEv5IUEYURQasfyBW5ffkMHR/90hiHgbNgrtp4j0=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg=
github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
github.com/tidwall/btree v1.7.0 h1:L1fkJH/AuEh5zBnnBbmTwQ5Lt+bRJ5A8EWecslvo9iI=
github.com/tidwall/btree v1.7.0/go.mod h1:twD9XRA5jj9VUQGELzDO4HPQTNJsoWWfYEL+EUQ2cKY=
google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg=
google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
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