Disallow only address permission (CosmWasm#1163)

* Remove AccessTypeOnlyAddress for store msg * Remove AccessTypeOnlyAddress for update config msg * Review feedback Co-authored-by: Simon Warta <2603011+webmaster128@users.noreply.github.com> Co-authored-by: Simon Warta <2603011+webmaster128@users.noreply.github.com>
notional-labs · Jan 25, 2023 · 8991633 · 8991633
1 parent 957b38e
commit 8991633
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 29 deletions.
diff --git a/x/wasm/client/cli/gov_tx.go b/x/wasm/client/cli/gov_tx.go
@@ -82,14 +82,11 @@ func ProposalStoreCodeCmd() *cobra.Command {
 	}
 
 	cmd.Flags().String(flagRunAs, "", "The address that is stored as code creator")
-	cmd.Flags().String(flagInstantiateByEverybody, "", "Everybody can instantiate a contract from the code, optional")
-	cmd.Flags().String(flagInstantiateNobody, "", "Nobody except the governance process can instantiate a contract from the code, optional")
-	cmd.Flags().String(flagInstantiateByAddress, "", "Only this address can instantiate a contract instance from the code, optional")
 	cmd.Flags().Bool(flagUnpinCode, false, "Unpin code on upload, optional")
-	cmd.Flags().StringSlice(flagInstantiateByAnyOfAddress, []string{}, "Any of the addresses can instantiate a contract from the code, optional")
 	cmd.Flags().String(flagSource, "", "Code Source URL is a valid absolute HTTPS URI to the contract's source code,")
 	cmd.Flags().String(flagBuilder, "", "Builder is a valid docker image name with tag, such as \"cosmwasm/workspace-optimizer:0.12.9\"")
 	cmd.Flags().BytesHex(flagCodeHash, nil, "CodeHash is the sha256 hash of the wasm code")
+	addInstantiatePermissionFlags(cmd)
 
 	// proposal flags
 	cmd.Flags().String(cli.FlagTitle, "", "Title of proposal")
@@ -374,19 +371,15 @@ func ProposalStoreAndInstantiateContractCmd() *cobra.Command {
 	}
 
 	cmd.Flags().String(flagRunAs, "", "The address that is stored as code creator. It is the creator of the contract and passed to the contract as sender on proposal execution")
-	cmd.Flags().String(flagInstantiateByEverybody, "", "Everybody can instantiate a contract from the code, optional")
-	cmd.Flags().String(flagInstantiateNobody, "", "Nobody except the governance process can instantiate a contract from the code, optional")
-	cmd.Flags().String(flagInstantiateByAddress, "", "Only this address can instantiate a contract instance from the code, optional")
 	cmd.Flags().Bool(flagUnpinCode, false, "Unpin code on upload, optional")
 	cmd.Flags().String(flagSource, "", "Code Source URL is a valid absolute HTTPS URI to the contract's source code,")
 	cmd.Flags().String(flagBuilder, "", "Builder is a valid docker image name with tag, such as \"cosmwasm/workspace-optimizer:0.12.9\"")
 	cmd.Flags().BytesHex(flagCodeHash, nil, "CodeHash is the sha256 hash of the wasm code")
-	cmd.Flags().StringSlice(flagInstantiateByAnyOfAddress, []string{}, "Any of the addresses can instantiate a contract from the code, optional")
 	cmd.Flags().String(flagAmount, "", "Coins to send to the contract during instantiation")
 	cmd.Flags().String(flagLabel, "", "A human-readable name for this contract in lists")
 	cmd.Flags().String(flagAdmin, "", "Address or key name of an admin")
 	cmd.Flags().Bool(flagNoAdmin, false, "You must set this explicitly if you don't want an admin")
-
+	addInstantiatePermissionFlags(cmd)
 	// proposal flags
 	cmd.Flags().String(cli.FlagTitle, "", "Title of proposal")
 	cmd.Flags().String(cli.FlagDescription, "", "Description of proposal")

diff --git a/x/wasm/client/cli/new_tx.go b/x/wasm/client/cli/new_tx.go
@@ -157,10 +157,7 @@ func UpdateInstantiateConfigCmd() *cobra.Command {
 		SilenceUsage: true,
 	}
 
-	cmd.Flags().String(flagInstantiateByEverybody, "", "Everybody can instantiate a contract from the code, optional")
-	cmd.Flags().String(flagInstantiateNobody, "", "Nobody except the governance process can instantiate a contract from the code, optional")
-	cmd.Flags().String(flagInstantiateByAddress, "", "Deprecated: Only this address can instantiate a contract from the code, optional")
-	cmd.Flags().StringSlice(flagInstantiateByAnyOfAddress, []string{}, "Any of the addresses can instantiate a contract from the code, optional")
+	addInstantiatePermissionFlags(cmd)
 	flags.AddTxFlagsToCmd(cmd)
 	return cmd
 }
diff --git a/x/wasm/client/cli/tx.go b/x/wasm/client/cli/tx.go
@@ -13,7 +13,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/tx"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/version"
 	"github.com/cosmos/cosmos-sdk/x/authz"
 	"github.com/spf13/cobra"
@@ -66,6 +65,7 @@ func GetTxCmd() *cobra.Command {
 		UpdateContractAdminCmd(),
 		ClearContractAdminCmd(),
 		GrantAuthorizationCmd(),
+		UpdateInstantiateConfigCmd(),
 	)
 	return txCmd
 }
@@ -94,10 +94,7 @@ func StoreCodeCmd() *cobra.Command {
 		SilenceUsage: true,
 	}
 
-	cmd.Flags().String(flagInstantiateByEverybody, "", "Everybody can instantiate a contract from the code, optional")
-	cmd.Flags().String(flagInstantiateNobody, "", "Nobody except the governance process can instantiate a contract from the code, optional")
-	cmd.Flags().String(flagInstantiateByAddress, "", "Deprecated: Only this address can instantiate a contract from the code, optional")
-	cmd.Flags().StringSlice(flagInstantiateByAnyOfAddress, []string{}, "Any of the addresses can instantiate a contract from the code, optional")
+	addInstantiatePermissionFlags(cmd)
 	flags.AddTxFlagsToCmd(cmd)
 	return cmd
 }
@@ -154,12 +151,7 @@ func parseAccessConfigFlags(flags *flag.FlagSet) (*types.AccessConfig, error) {
 		return nil, fmt.Errorf("instantiate by address: %s", err)
 	}
 	if onlyAddrStr != "" {
-		allowedAddr, err := sdk.AccAddressFromBech32(onlyAddrStr)
-		if err != nil {
-			return nil, sdkerrors.Wrap(err, flagInstantiateByAddress)
-		}
-		x := types.AccessTypeOnlyAddress.With(allowedAddr)
-		return &x, nil
+		return nil, fmt.Errorf("not supported anymore. Use: %s", flagInstantiateByAnyOfAddress)
 	}
 	everybodyStr, err := flags.GetString(flagInstantiateByEverybody)
 	if err != nil {
@@ -191,6 +183,13 @@ func parseAccessConfigFlags(flags *flag.FlagSet) (*types.AccessConfig, error) {
 	return nil, nil
 }
 
+func addInstantiatePermissionFlags(cmd *cobra.Command) {
+	cmd.Flags().String(flagInstantiateByEverybody, "", "Everybody can instantiate a contract from the code, optional")
+	cmd.Flags().String(flagInstantiateNobody, "", "Nobody except the governance process can instantiate a contract from the code, optional")
+	cmd.Flags().String(flagInstantiateByAddress, "", fmt.Sprintf("Removed: use %s instead", flagInstantiateByAnyOfAddress))
+	cmd.Flags().StringSlice(flagInstantiateByAnyOfAddress, []string{}, "Any of the addresses can instantiate a contract from the code, optional")
+}
+
 // InstantiateContractCmd will instantiate a contract from previously uploaded code.
 func InstantiateContractCmd() *cobra.Command {
 	cmd := &cobra.Command{

diff --git a/x/wasm/client/cli/tx_test.go b/x/wasm/client/cli/tx_test.go
@@ -25,7 +25,7 @@ func TestParseAccessConfigFlags(t *testing.T) {
 		},
 		"only address": {
 			args:   []string{"--instantiate-only-address=cosmos1vx8knpllrj7n963p9ttd80w47kpacrhuts497x"},
-			expCfg: &types.AccessConfig{Permission: types.AccessTypeOnlyAddress, Address: "cosmos1vx8knpllrj7n963p9ttd80w47kpacrhuts497x"},
+			expErr: true,
 		},
 		"only address - invalid": {
 			args:   []string{"--instantiate-only-address=foo"},

diff --git a/x/wasm/keeper/proposal_integration_test.go b/x/wasm/keeper/proposal_integration_test.go
@@ -917,7 +917,7 @@ func TestUpdateInstantiateConfigProposal(t *testing.T) {
 	anyAddress, err := sdk.AccAddressFromBech32("cosmos100dejzacpanrldpjjwksjm62shqhyss44jf5xz")
 	require.NoError(t, err)
 
-	withAddressAccessConfig := types.AccessTypeOnlyAddress.With(anyAddress)
+	withAddressAccessConfig := types.AccessTypeAnyOfAddresses.With(anyAddress)
 	var (
 		nobody      = StoreRandomContractWithAccessConfig(t, ctx, keepers, &mock, &types.AllowNobody)
 		everybody   = StoreRandomContractWithAccessConfig(t, ctx, keepers, &mock, &types.AllowEverybody)

diff --git a/x/wasm/types/tx.go b/x/wasm/types/tx.go
@@ -68,6 +68,11 @@ func (msg MsgStoreCode) ValidateBasic() error {
 		if err := msg.InstantiatePermission.ValidateBasic(); err != nil {
 			return sdkerrors.Wrap(err, "instantiate permission")
 		}
+		// AccessTypeOnlyAddress is still considered valid as legacy instantiation permission
+		// but not for new contracts
+		if msg.InstantiatePermission.Permission == AccessTypeOnlyAddress {
+			return ErrInvalid.Wrap("unsupported type, use AccessTypeAnyOfAddresses instead")
+		}
 	}
 	return nil
 }
@@ -420,6 +425,11 @@ func (msg MsgUpdateInstantiateConfig) ValidateBasic() error {
 	if err := msg.NewInstantiatePermission.ValidateBasic(); err != nil {
 		return sdkerrors.Wrap(err, "instantiate permission")
 	}
+	// AccessTypeOnlyAddress is still considered valid as legacy instantiation permission
+	// but not for new contracts
+	if msg.NewInstantiatePermission.Permission == AccessTypeOnlyAddress {
+		return ErrInvalid.Wrap("unsupported type, use AccessTypeAnyOfAddresses instead")
+	}
 
 	return nil
 }

diff --git a/x/wasm/types/tx_test.go b/x/wasm/types/tx_test.go
@@ -693,32 +693,40 @@ func TestMsgUpdateInstantiateConfig(t *testing.T) {
 		expErr bool
 	}{
 		"all good": {
+			src: MsgUpdateInstantiateConfig{
+				Sender:                   goodAddress,
+				CodeID:                   1,
+				NewInstantiatePermission: &AccessConfig{Permission: AccessTypeAnyOfAddresses, Addresses: []string{anotherGoodAddress}},
+			},
+		},
+		"retained AccessTypeOnlyAddress": {
 			src: MsgUpdateInstantiateConfig{
 				Sender:                   goodAddress,
 				CodeID:                   1,
 				NewInstantiatePermission: &AccessConfig{Permission: AccessTypeOnlyAddress, Address: anotherGoodAddress},
 			},
+			expErr: true,
 		},
 		"bad sender": {
 			src: MsgUpdateInstantiateConfig{
 				Sender:                   badAddress,
 				CodeID:                   1,
-				NewInstantiatePermission: &AccessConfig{Permission: AccessTypeOnlyAddress, Address: anotherGoodAddress},
+				NewInstantiatePermission: &AccessConfig{Permission: AccessTypeAnyOfAddresses, Addresses: []string{anotherGoodAddress}},
 			},
 			expErr: true,
 		},
 		"invalid NewInstantiatePermission": {
 			src: MsgUpdateInstantiateConfig{
 				Sender:                   goodAddress,
 				CodeID:                   1,
-				NewInstantiatePermission: &AccessConfig{Permission: AccessTypeOnlyAddress, Address: badAddress},
+				NewInstantiatePermission: &AccessConfig{Permission: AccessTypeAnyOfAddresses, Addresses: []string{badAddress}},
 			},
 			expErr: true,
 		},
 		"missing code id": {
 			src: MsgUpdateInstantiateConfig{
 				Sender:                   goodAddress,
-				NewInstantiatePermission: &AccessConfig{Permission: AccessTypeOnlyAddress, Address: anotherGoodAddress},
+				NewInstantiatePermission: &AccessConfig{Permission: AccessTypeAnyOfAddresses, Addresses: []string{anotherGoodAddress}},
 			},
 			expErr: true,
 		},