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

Add migration for AccessConfig #1395

Merged
merged 5 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions x/wasm/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

v1 "github.com/CosmWasm/wasmd/x/wasm/migrations/v1"
v2 "github.com/CosmWasm/wasmd/x/wasm/migrations/v2"
v3 "github.com/CosmWasm/wasmd/x/wasm/migrations/v3"

"github.com/CosmWasm/wasmd/x/wasm/exported"
)
Expand All @@ -30,3 +31,9 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error {
func (m Migrator) Migrate2to3(ctx sdk.Context) error {
return v2.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc)
}

// Migrate3to4 migrates the x/wasm module state from the consensus
// version 3 to version 4.
func (m Migrator) Migrate3to4(ctx sdk.Context) error {
return v3.NewMigrator(m.keeper, m.keeper.storeCodeInfo).Migrate3to4(ctx)
}
98 changes: 97 additions & 1 deletion x/wasm/keeper/migrations_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ import (
)

func TestModuleMigrations(t *testing.T) {
addr := "cosmos1vx8knpllrj7n963p9ttd80w47kpacrhuts497x"
address, err := sdk.AccAddressFromBech32(addr)
require.NoError(t, err)

wasmApp := app.Setup(t)

upgradeHandler := func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { //nolint:unparam
return wasmApp.ModuleManager.RunMigrations(ctx, wasmApp.Configurator(), fromVM)
}
Expand Down Expand Up @@ -47,6 +52,21 @@ func TestModuleMigrations(t *testing.T) {
setup: func(ctx sdk.Context) {},
exp: types.DefaultParams(),
},
"with legacy params and access config migrated": {
startVersion: 1,
setup: func(ctx sdk.Context) {
params := types.Params{
CodeUploadAccess: types.AccessTypeOnlyAddress.With(address),
InstantiateDefaultPermission: types.AccessTypeOnlyAddress,
}
sp, _ := wasmApp.ParamsKeeper.GetSubspace(types.ModuleName)
sp.SetParamSet(ctx, &params)
},
exp: types.Params{
CodeUploadAccess: types.AccessTypeAnyOfAddresses.With(address),
InstantiateDefaultPermission: types.AccessTypeAnyOfAddresses,
},
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
Expand All @@ -63,10 +83,86 @@ func TestModuleMigrations(t *testing.T) {

// then
require.NoError(t, err)
var expModuleVersion uint64 = 3
var expModuleVersion uint64 = 4
assert.Equal(t, expModuleVersion, gotVM[wasm.ModuleName])
gotParams := wasmApp.WasmKeeper.GetParams(ctx)
assert.Equal(t, spec.exp, gotParams)
})
}
}

func TestAccessConfigMigrations(t *testing.T) {
addr := "cosmos1vx8knpllrj7n963p9ttd80w47kpacrhuts497x"
address, err := sdk.AccAddressFromBech32(addr)
require.NoError(t, err)

wasmApp := app.Setup(t)

upgradeHandler := func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { //nolint:unparam
return wasmApp.ModuleManager.RunMigrations(ctx, wasmApp.Configurator(), fromVM)
}

ctx, _ := wasmApp.BaseApp.NewContext(false, tmproto.Header{}).CacheContext()

// only address permission cannot be stored and returns an error
code1, err := storeCode(ctx, wasmApp, types.AccessTypeOnlyAddress.With(address))
require.Error(t, err)
require.Equal(t, uint64(0), code1)

// any address permission
code2, err := storeCode(ctx, wasmApp, types.AccessTypeAnyOfAddresses.With(address))
require.NoError(t, err)

// allow everybody permission
code3, err := storeCode(ctx, wasmApp, types.AllowEverybody)
require.NoError(t, err)

// allow nobody permission
code4, err := storeCode(ctx, wasmApp, types.AllowNobody)
require.NoError(t, err)

fromVM := wasmApp.UpgradeKeeper.GetModuleVersionMap(ctx)
fromVM[wasm.ModuleName] = wasmApp.ModuleManager.GetVersionMap()[types.ModuleName]
_, err = upgradeHandler(ctx, upgradetypes.Plan{Name: "testing"}, fromVM)
require.NoError(t, err)

// when
gotVM, err := wasmApp.ModuleManager.RunMigrations(ctx, wasmApp.Configurator(), fromVM)

// then
require.NoError(t, err)
var expModuleVersion uint64 = 4
assert.Equal(t, expModuleVersion, gotVM[wasm.ModuleName])

// only address was not stored
assert.Nil(t, wasmApp.WasmKeeper.GetCodeInfo(ctx, code1))

// any address was not migrated
assert.Equal(t, types.AccessTypeAnyOfAddresses.With(address), wasmApp.WasmKeeper.GetCodeInfo(ctx, code2).InstantiateConfig)

// allow everybody was not migrated
assert.Equal(t, types.AllowEverybody, wasmApp.WasmKeeper.GetCodeInfo(ctx, code3).InstantiateConfig)

// allow nodoby was not migrated
assert.Equal(t, types.AllowNobody, wasmApp.WasmKeeper.GetCodeInfo(ctx, code4).InstantiateConfig)
}
Copy link
Contributor

@alpe alpe May 12, 2023

Choose a reason for hiding this comment

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

This looks like a subset of cases that are covered in v3/store_test already.
No need to replicate them all. Some kind of a smoke test would be good enough.
A serialized ContractInfo with legacy AccessType could be used for example.


func storeCode(ctx sdk.Context, wasmApp *app.WasmApp, instantiatePermission types.AccessConfig) (codeID uint64, err error) {
msg := types.MsgStoreCodeFixture(func(m *types.MsgStoreCode) {
m.WASMByteCode = wasmContract
m.InstantiatePermission = &instantiatePermission
})
rsp, err := wasmApp.MsgServiceRouter().Handler(msg)(ctx, msg)
if err != nil {
return
}

var result types.MsgStoreCodeResponse
err = wasmApp.AppCodec().Unmarshal(rsp.Data, &result)
if err != nil {
return
}

codeID = result.CodeID
return
}
59 changes: 59 additions & 0 deletions x/wasm/migrations/v3/store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package v3

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/CosmWasm/wasmd/x/wasm/types"
)

// StoreCodeInfoFn stores code info
type StoreCodeInfoFn func(ctx sdk.Context, codeID uint64, codeInfo types.CodeInfo)

// Keeper abstract keeper
type wasmKeeper interface {
IterateCodeInfos(ctx sdk.Context, cb func(uint64, types.CodeInfo) bool)
GetParams(ctx sdk.Context) types.Params
SetParams(ctx sdk.Context, ps types.Params) error
Copy link
Contributor

Choose a reason for hiding this comment

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

Good abstraction!

}

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
keeper wasmKeeper
storeCodeInfoFn StoreCodeInfoFn
}

// NewMigrator returns a new Migrator.
func NewMigrator(k wasmKeeper, fn StoreCodeInfoFn) Migrator {
return Migrator{keeper: k, storeCodeInfoFn: fn}
}

// Migrate3to4 migrates from version 3 to 4.
func (m Migrator) Migrate3to4(ctx sdk.Context) error {
params := m.keeper.GetParams(ctx)
if params.CodeUploadAccess.Permission == types.AccessTypeOnlyAddress {
params.CodeUploadAccess.Permission = types.AccessTypeAnyOfAddresses
params.CodeUploadAccess.Addresses = []string{params.CodeUploadAccess.Address}
params.CodeUploadAccess.Address = ""
}

if params.InstantiateDefaultPermission == types.AccessTypeOnlyAddress {
params.InstantiateDefaultPermission = types.AccessTypeAnyOfAddresses
}

err := m.keeper.SetParams(ctx, params)
if err != nil {
return err
}

m.keeper.IterateCodeInfos(ctx, func(codeID uint64, info types.CodeInfo) bool {
if info.InstantiateConfig.Permission == types.AccessTypeOnlyAddress {
info.InstantiateConfig.Permission = types.AccessTypeAnyOfAddresses
info.InstantiateConfig.Addresses = []string{info.InstantiateConfig.Address}
info.InstantiateConfig.Address = ""

m.storeCodeInfoFn(ctx, codeID, info)
}
return false
})
return nil
}
83 changes: 83 additions & 0 deletions x/wasm/migrations/v3/store_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package v3_test

import (
"bytes"
"testing"

"github.com/CosmWasm/wasmd/x/wasm/keeper"
"github.com/CosmWasm/wasmd/x/wasm/keeper/wasmtesting"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/CosmWasm/wasmd/x/wasm/types"
)

func TestMigrate3To4(t *testing.T) {
const AvailableCapabilities = "iterator,staking,stargate,cosmwasm_1_1"
ctx, keepers := keeper.CreateTestInput(t, false, AvailableCapabilities)
wasmKeeper := keepers.WasmKeeper

deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000))
creator := sdk.AccAddress(bytes.Repeat([]byte{1}, address.Len))
keepers.Faucet.Fund(ctx, creator, deposit...)

var mock wasmtesting.MockWasmer
wasmtesting.MakeInstantiable(&mock)

// contract with only address permission
onlyAddrPermission := types.AccessTypeOnlyAddress.With(creator)
contract1 := keeper.StoreRandomContractWithAccessConfig(t, ctx, keepers, &mock, &onlyAddrPermission)

// contract with any addresses permission
anyAddrsPermission := types.AccessTypeAnyOfAddresses.With(creator)
contract2 := keeper.StoreRandomContractWithAccessConfig(t, ctx, keepers, &mock, &anyAddrsPermission)

// contract with everybody permission
everybodyPermission := types.AllowEverybody
contract3 := keeper.StoreRandomContractWithAccessConfig(t, ctx, keepers, &mock, &everybodyPermission)

// contract with nobody permission
nobodyPermission := types.AllowNobody
contract4 := keeper.StoreRandomContractWithAccessConfig(t, ctx, keepers, &mock, &nobodyPermission)

// set only address permission params
params := types.Params{
CodeUploadAccess: types.AccessTypeOnlyAddress.With(creator),
InstantiateDefaultPermission: types.AccessTypeOnlyAddress,
}
err := wasmKeeper.SetParams(ctx, params)
require.NoError(t, err)

// when
err = keeper.NewMigrator(*wasmKeeper, nil).Migrate3to4(ctx)

// then
require.NoError(t, err)

expParams := types.Params{
CodeUploadAccess: types.AccessTypeAnyOfAddresses.With(creator),
InstantiateDefaultPermission: types.AccessTypeAnyOfAddresses,
}

// params are migrated
assert.Equal(t, expParams, wasmKeeper.GetParams(ctx))

// access config for only address is migrated
info1 := wasmKeeper.GetCodeInfo(ctx, contract1.CodeID)
assert.Equal(t, anyAddrsPermission, info1.InstantiateConfig)

// access config for any addresses is not migrated
info2 := wasmKeeper.GetCodeInfo(ctx, contract2.CodeID)
assert.Equal(t, anyAddrsPermission, info2.InstantiateConfig)

// access config for allow everybody is not migrated
info3 := wasmKeeper.GetCodeInfo(ctx, contract3.CodeID)
assert.Equal(t, types.AllowEverybody, info3.InstantiateConfig)

// access config for allow nobody is not migrated
info4 := wasmKeeper.GetCodeInfo(ctx, contract4.CodeID)
assert.Equal(t, types.AllowNobody, info4.InstantiateConfig)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Good test cases!
Please consider table test in the future to make it more readable but no need to change this now.

6 changes: 5 additions & 1 deletion x/wasm/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (am AppModule) IsAppModule() { // marker
// module. It should be incremented on each consensus-breaking change
// introduced by the module. To avoid wrong/empty versions, the initial version
// should be set to 1.
func (AppModule) ConsensusVersion() uint64 { return 3 }
func (AppModule) ConsensusVersion() uint64 { return 4 }
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Important


func (am AppModule) RegisterServices(cfg module.Configurator) {
types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper))
Expand All @@ -162,6 +162,10 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
if err != nil {
panic(err)
}
err = cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4)
if err != nil {
panic(err)
}
}

// RegisterInvariants registers the wasm module invariants.
Expand Down