Skip to content

Commit

Permalink
Disallow only address permission (#1163)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
alpe and webmaster128 authored Jan 25, 2023
1 parent 957b38e commit 8991633
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 29 deletions.
11 changes: 2 additions & 9 deletions x/wasm/client/cli/gov_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
5 changes: 1 addition & 4 deletions x/wasm/client/cli/new_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
21 changes: 10 additions & 11 deletions x/wasm/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -66,6 +65,7 @@ func GetTxCmd() *cobra.Command {
UpdateContractAdminCmd(),
ClearContractAdminCmd(),
GrantAuthorizationCmd(),
UpdateInstantiateConfigCmd(),
)
return txCmd
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion x/wasm/client/cli/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
2 changes: 1 addition & 1 deletion x/wasm/keeper/proposal_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions x/wasm/types/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
14 changes: 11 additions & 3 deletions x/wasm/types/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down

0 comments on commit 8991633

Please sign in to comment.