Add migration for AccessConfig (#1395)

* Add migration for AccessConfig

* Add more tests

* Make migration more resilient and remove "old" types

* cleanup legacy types

* fix comments

docs/proto/proto-docs.md

@@ -310,7 +310,6 @@ AccessConfig access control type.
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
 | `permission` | [AccessType](#cosmwasm.wasm.v1.AccessType) |  |  |
-| `address` | [string](#string) |  | Address Deprecated: replaced by addresses |
 | `addresses` | [string](#string) | repeated |  |
 
 
@@ -432,7 +431,6 @@ AccessType permission types
 | ---- | ------ | ----------- |
 | ACCESS_TYPE_UNSPECIFIED | 0 | AccessTypeUnspecified placeholder for empty value |
 | ACCESS_TYPE_NOBODY | 1 | AccessTypeNobody forbidden |
-| ACCESS_TYPE_ONLY_ADDRESS | 2 | AccessTypeOnlyAddress restricted to a single address Deprecated: use AccessTypeAnyOfAddresses instead |
 | ACCESS_TYPE_EVERYBODY | 3 | AccessTypeEverybody unrestricted |
 | ACCESS_TYPE_ANY_OF_ADDRESSES | 4 | AccessTypeAnyOfAddresses allow any of the addresses |
 

proto/cosmwasm/wasm/v1/types.proto

@@ -20,10 +20,9 @@ enum AccessType {
   // AccessTypeNobody forbidden
   ACCESS_TYPE_NOBODY = 1
       [ (gogoproto.enumvalue_customname) = "AccessTypeNobody" ];
-  // AccessTypeOnlyAddress restricted to a single address
-  // Deprecated: use AccessTypeAnyOfAddresses instead
-  ACCESS_TYPE_ONLY_ADDRESS = 2
-      [ (gogoproto.enumvalue_customname) = "AccessTypeOnlyAddress" ];
+
+  reserved 2; // was AccessTypeOnlyAddress
+
   // AccessTypeEverybody unrestricted
   ACCESS_TYPE_EVERYBODY = 3
       [ (gogoproto.enumvalue_customname) = "AccessTypeEverybody" ];
@@ -43,9 +42,8 @@ message AccessConfig {
   option (gogoproto.goproto_stringer) = true;
   AccessType permission = 1 [ (gogoproto.moretags) = "yaml:\"permission\"" ];
 
-  // Address
-  // Deprecated: replaced by addresses
-  string address = 2 [ (gogoproto.moretags) = "yaml:\"address\"" ];
+  reserved 2; // was address
+
   repeated string addresses = 3 [ (gogoproto.moretags) = "yaml:\"addresses\"" ];
 }
 

x/wasm/keeper/authz_policy_test.go

@@ -29,16 +29,6 @@ func TestDefaultAuthzPolicyCanCreateCode(t *testing.T) {
 			contractInstConf: types.AllowEverybody,
 			exp:              true,
 		},
-		"upload only address - same": {
-			chainConfigs:     NewChainAccessConfigs(types.AccessTypeOnlyAddress.With(myActorAddress), types.AllowEverybody),
-			contractInstConf: types.AllowEverybody,
-			exp:              true,
-		},
-		"upload only address - different": {
-			chainConfigs:     NewChainAccessConfigs(types.AccessTypeOnlyAddress.With(otherAddress), types.AllowEverybody),
-			contractInstConf: types.AllowEverybody,
-			exp:              false,
-		},
 		"upload any address - included": {
 			chainConfigs:     NewChainAccessConfigs(types.AccessTypeAnyOfAddresses.With(otherAddress, myActorAddress), types.AllowEverybody),
 			contractInstConf: types.AllowEverybody,
@@ -97,14 +87,6 @@ func TestDefaultAuthzPolicyCanInstantiateContract(t *testing.T) {
 			config: types.AllowEverybody,
 			exp:    true,
 		},
-		"only address - same": {
-			config: types.AccessTypeOnlyAddress.With(myActorAddress),
-			exp:    true,
-		},
-		"only address - different": {
-			config: types.AccessTypeOnlyAddress.With(otherAddress),
-			exp:    false,
-		},
 		"any address - included": {
 			config: types.AccessTypeAnyOfAddresses.With(otherAddress, myActorAddress),
 			exp:    true,
@@ -214,14 +196,6 @@ func TestGovAuthzPolicyCanCreateCode(t *testing.T) {
 			chainConfigs:     NewChainAccessConfigs(types.AllowEverybody, types.AllowEverybody),
 			contractInstConf: types.AllowEverybody,
 		},
-		"upload only address - same": {
-			chainConfigs:     NewChainAccessConfigs(types.AccessTypeOnlyAddress.With(myActorAddress), types.AllowEverybody),
-			contractInstConf: types.AllowEverybody,
-		},
-		"upload only address - different": {
-			chainConfigs:     NewChainAccessConfigs(types.AccessTypeOnlyAddress.With(otherAddress), types.AllowEverybody),
-			contractInstConf: types.AllowEverybody,
-		},
 		"upload any address - included": {
 			chainConfigs:     NewChainAccessConfigs(types.AccessTypeAnyOfAddresses.With(otherAddress, myActorAddress), types.AllowEverybody),
 			contractInstConf: types.AllowEverybody,
@@ -265,12 +239,6 @@ func TestGovAuthzPolicyCanInstantiateContract(t *testing.T) {
 		"everybody": {
 			config: types.AllowEverybody,
 		},
-		"only address - same": {
-			config: types.AccessTypeOnlyAddress.With(myActorAddress),
-		},
-		"only address - different": {
-			config: types.AccessTypeOnlyAddress.With(otherAddress),
-		},
 		"any address - included": {
 			config: types.AccessTypeAnyOfAddresses.With(otherAddress, myActorAddress),
 		},

x/wasm/keeper/genesis_test.go

@@ -495,8 +495,8 @@ func TestImportContractWithCodeHistoryPreserved(t *testing.T) {
         "code_hash": %q,
         "creator": "cosmos1qtu5n0cnhfkjj6l2rq97hmky9fd89gwca9yarx",
         "instantiate_config": {
-          "permission": "OnlyAddress",
-          "address": "cosmos1qtu5n0cnhfkjj6l2rq97hmky9fd89gwca9yarx"
+          "permission": "AnyOfAddresses",
+          "addresses": ["cosmos1qtu5n0cnhfkjj6l2rq97hmky9fd89gwca9yarx"]
         }
       },
       "code_bytes": %q
@@ -575,8 +575,8 @@ func TestImportContractWithCodeHistoryPreserved(t *testing.T) {
 		CodeHash: wasmCodeHash[:],
 		Creator:  codeCreatorAddr,
 		InstantiateConfig: types.AccessConfig{
-			Permission: types.AccessTypeOnlyAddress,
-			Address:    codeCreatorAddr,
+			Permission: types.AccessTypeAnyOfAddresses,
+			Addresses:  []string{codeCreatorAddr},
 		},
 	}
 	assert.Equal(t, expCodeInfo, *gotCodeInfo)

x/wasm/keeper/keeper_test.go

@@ -115,9 +115,9 @@ func TestCreateStoresInstantiatePermission(t *testing.T) {
 			srcPermission: types.AccessTypeNobody,
 			expInstConf:   types.AllowNobody,
 		},
-		"onlyAddress with matching address": {
-			srcPermission: types.AccessTypeOnlyAddress,
-			expInstConf:   types.AccessConfig{Permission: types.AccessTypeOnlyAddress, Address: myAddr.String()},
+		"anyAddress with matching address": {
+			srcPermission: types.AccessTypeAnyOfAddresses,
+			expInstConf:   types.AccessConfig{Permission: types.AccessTypeAnyOfAddresses, Addresses: []string{myAddr.String()}},
 		},
 	}
 	for msg, spec := range specs {
@@ -165,13 +165,13 @@ func TestCreateWithParamPermissions(t *testing.T) {
 			chainUpload: types.AllowNobody,
 			expError:    sdkerrors.ErrUnauthorized,
 		},
-		"onlyAddress with matching address": {
+		"anyAddress with matching address": {
 			policy:      DefaultAuthorizationPolicy{},
-			chainUpload: types.AccessTypeOnlyAddress.With(creator),
+			chainUpload: types.AccessTypeAnyOfAddresses.With(creator),
 		},
-		"onlyAddress with non matching address": {
+		"anyAddress with non matching address": {
 			policy:      DefaultAuthorizationPolicy{},
-			chainUpload: types.AccessTypeOnlyAddress.With(otherAddr),
+			chainUpload: types.AccessTypeAnyOfAddresses.With(otherAddr),
 			expError:    sdkerrors.ErrUnauthorized,
 		},
 		"gov: always allowed": {
@@ -206,8 +206,8 @@ func TestEnforceValidPermissionsOnCreate(t *testing.T) {
 	creator := keepers.Faucet.NewFundedRandomAccount(ctx, deposit...)
 	other := keepers.Faucet.NewFundedRandomAccount(ctx, deposit...)
 
-	onlyCreator := types.AccessTypeOnlyAddress.With(creator)
-	onlyOther := types.AccessTypeOnlyAddress.With(other)
+	onlyCreator := types.AccessTypeAnyOfAddresses.With(creator)
+	onlyOther := types.AccessTypeAnyOfAddresses.With(other)
 
 	specs := map[string]struct {
 		defaultPermssion    types.AccessType
@@ -243,17 +243,17 @@ func TestEnforceValidPermissionsOnCreate(t *testing.T) {
 			grantedPermission:   types.AccessConfig{Permission: types.AccessTypeNobody},
 		},
 		"only defaults to code creator": {
-			defaultPermssion:    types.AccessTypeOnlyAddress,
+			defaultPermssion:    types.AccessTypeAnyOfAddresses,
 			requestedPermission: nil,
 			grantedPermission:   onlyCreator,
 		},
 		"can explicitly set to code creator": {
-			defaultPermssion:    types.AccessTypeOnlyAddress,
+			defaultPermssion:    types.AccessTypeAnyOfAddresses,
 			requestedPermission: &onlyCreator,
 			grantedPermission:   onlyCreator,
 		},
 		"cannot override which address in only": {
-			defaultPermssion:    types.AccessTypeOnlyAddress,
+			defaultPermssion:    types.AccessTypeAnyOfAddresses,
 			requestedPermission: &onlyOther,
 			expError:            sdkerrors.ErrUnauthorized,
 		},
@@ -530,13 +530,13 @@ func TestInstantiateWithPermissions(t *testing.T) {
 			srcActor:      myAddr,
 			expError:      sdkerrors.ErrUnauthorized,
 		},
-		"onlyAddress with matching address": {
-			srcPermission: types.AccessTypeOnlyAddress.With(myAddr),
+		"anyAddress with matching address": {
+			srcPermission: types.AccessTypeAnyOfAddresses.With(myAddr),
 			srcActor:      myAddr,
 		},
-		"onlyAddress with non matching address": {
+		"anyAddress with non matching address": {
 			srcActor:      myAddr,
-			srcPermission: types.AccessTypeOnlyAddress.With(otherAddr),
+			srcPermission: types.AccessTypeAnyOfAddresses.With(otherAddr),
 			expError:      sdkerrors.ErrUnauthorized,
 		},
 	}
@@ -2112,17 +2112,6 @@ func TestSetAccessConfig(t *testing.T) {
 				"code_permission": "Nobody",
 			},
 		},
-		"gov with new permissions > chain permissions": {
-			authz:           GovAuthorizationPolicy{},
-			chainPermission: types.AccessTypeNobody,
-			newConfig:       types.AccessTypeOnlyAddress.With(creatorAddr),
-			caller:          creatorAddr,
-			expEvts: map[string]string{
-				"code_id":              "1",
-				"code_permission":      "OnlyAddress",
-				"authorized_addresses": creatorAddr.String(),
-			},
-		},
 		"gov with new permissions > chain permissions - multiple addresses": {
 			authz:           GovAuthorizationPolicy{},
 			chainPermission: types.AccessTypeNobody,

x/wasm/keeper/migrations.go

@@ -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"
 )
@@ -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, m.keeper.storeKey, m.keeper.cdc)
+}

x/wasm/keeper/migrations_integration_test.go

@@ -18,6 +18,7 @@ import (
 
 func TestModuleMigrations(t *testing.T) {
 	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)
 	}
@@ -63,10 +64,78 @@ 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()
+
+	// any address permission
+	code1, err := storeCode(ctx, wasmApp, types.AccessTypeAnyOfAddresses.With(address))
+	require.NoError(t, err)
+
+	// allow everybody permission
+	code2, err := storeCode(ctx, wasmApp, types.AllowEverybody)
+	require.NoError(t, err)
+
+	// allow nobody permission
+	code3, 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])
+
+	// any address was not migrated
+	assert.Equal(t, types.AccessTypeAnyOfAddresses.With(address), wasmApp.WasmKeeper.GetCodeInfo(ctx, code1).InstantiateConfig)
+
+	// allow everybody was not migrated
+	assert.Equal(t, types.AllowEverybody, wasmApp.WasmKeeper.GetCodeInfo(ctx, code2).InstantiateConfig)
+
+	// allow nodoby was not migrated
+	assert.Equal(t, types.AllowNobody, wasmApp.WasmKeeper.GetCodeInfo(ctx, code3).InstantiateConfig)
+}
+
+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
+}

x/wasm/keeper/msg_server_integration_test.go

@@ -58,7 +58,7 @@ func TestUpdateParams(t *testing.T) {
 
 	var (
 		myAddress              sdk.AccAddress = make([]byte, types.ContractAddrLen)
-		oneAddressAccessConfig                = types.AccessTypeOnlyAddress.With(myAddress)
+		oneAddressAccessConfig                = types.AccessTypeAnyOfAddresses.With(myAddress)
 		govAuthority                          = wasmApp.WasmKeeper.GetAuthority()
 	)
 

x/wasm/keeper/querier_test.go

@@ -717,7 +717,7 @@ func TestQueryCodeInfo(t *testing.T) {
 		},
 		"with_address": {
 			codeID:       20,
-			accessConfig: types.AccessTypeOnlyAddress.With(anyAddress),
+			accessConfig: types.AccessTypeAnyOfAddresses.With(anyAddress),
 		},
 	}
 	for msg, spec := range specs {
@@ -782,7 +782,7 @@ func TestQueryCodeInfoList(t *testing.T) {
 		{
 			name:     "with_address",
 			codeID:   20,
-			codeInfo: codeInfoWithConfig(types.AccessTypeOnlyAddress.With(anyAddress)),
+			codeInfo: codeInfoWithConfig(types.AccessTypeAnyOfAddresses.With(anyAddress)),
 		},
 	}