From acd1cf9dfea1aa1da3d75264905045ac3d520ce3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 20 Jan 2022 21:25:05 +0100 Subject: [PATCH 1/4] Require --no-admin flag if no admin set --- x/wasm/client/cli/tx.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/x/wasm/client/cli/tx.go b/x/wasm/client/cli/tx.go index 1bcc334d6a..a11c7ddc6c 100644 --- a/x/wasm/client/cli/tx.go +++ b/x/wasm/client/cli/tx.go @@ -22,6 +22,7 @@ const ( flagAmount = "amount" flagLabel = "label" flagAdmin = "admin" + flagNoAdmin = "no-admin" flagRunAs = "run-as" flagInstantiateByEverybody = "instantiate-everybody" flagInstantiateByAddress = "instantiate-only-address" @@ -157,6 +158,7 @@ func InstantiateContractCmd() *cobra.Command { 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 of an admin") + cmd.Flags().Bool(flagNoAdmin, false, "You must set this explicitly if you don't want an admin") flags.AddTxFlagsToCmd(cmd) return cmd } @@ -188,6 +190,16 @@ func parseInstantiateArgs(rawCodeID, initMsg string, sender sdk.AccAddress, flag return types.MsgInstantiateContract{}, fmt.Errorf("admin: %s", err) } + if adminStr == "" { + noAdmin, err := flags.GetBool(flagNoAdmin) + if err != nil { + return types.MsgInstantiateContract{}, fmt.Errorf("no-admin: %s", err) + } + if !noAdmin { + return types.MsgInstantiateContract{}, fmt.Errorf("You must set an admin or explicitly pass --no-admin to make it immutible (wasmd issue #719)") + } + } + // build and sign the transaction, then broadcast to Tendermint msg := types.MsgInstantiateContract{ Sender: sender.String(), From 332f4cc9a85fc3fd09212d23994cb25e82bd0a27 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 20 Jan 2022 21:36:59 +0100 Subject: [PATCH 2/4] Fix tests, handle genesis init as well --- x/wasm/client/cli/genesis_msg.go | 1 + x/wasm/client/cli/genesis_msg_test.go | 60 +++++++++++++++++++++++++++ x/wasm/client/cli/tx.go | 18 ++++---- 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/x/wasm/client/cli/genesis_msg.go b/x/wasm/client/cli/genesis_msg.go index 1795db1299..725773b32b 100644 --- a/x/wasm/client/cli/genesis_msg.go +++ b/x/wasm/client/cli/genesis_msg.go @@ -138,6 +138,7 @@ func GenesisInstantiateContractCmd(defaultNodeHome string, genesisMutator Genesi 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 of an admin") + cmd.Flags().Bool(flagNoAdmin, false, "You must set this explicitly if you don't want an admin") cmd.Flags().String(flagRunAs, "", "The address that pays the init funds. It is the creator of the contract.") cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory") diff --git a/x/wasm/client/cli/genesis_msg_test.go b/x/wasm/client/cli/genesis_msg_test.go index b11572ee13..08b3f15208 100644 --- a/x/wasm/client/cli/genesis_msg_test.go +++ b/x/wasm/client/cli/genesis_msg_test.go @@ -142,6 +142,7 @@ func TestInstantiateContractCmd(t *testing.T) { flagSet := cmd.Flags() flagSet.Set("label", "testing") flagSet.Set("run-as", myWellFundedAccount) + flagSet.Set("no-admin", "true") }, expMsgCount: 1, }, @@ -157,6 +158,7 @@ func TestInstantiateContractCmd(t *testing.T) { flagSet := cmd.Flags() flagSet.Set("label", "testing") flagSet.Set("run-as", myWellFundedAccount) + flagSet.Set("admin", myWellFundedAccount) }, expMsgCount: 2, }, @@ -175,6 +177,7 @@ func TestInstantiateContractCmd(t *testing.T) { flagSet := cmd.Flags() flagSet.Set("label", "testing") flagSet.Set("run-as", myWellFundedAccount) + flagSet.Set("no-admin", "true") }, expMsgCount: 2, }, @@ -185,6 +188,7 @@ func TestInstantiateContractCmd(t *testing.T) { flagSet := cmd.Flags() flagSet.Set("label", "testing") flagSet.Set("run-as", myWellFundedAccount) + flagSet.Set("no-admin", "true") }, expError: true, }, @@ -202,6 +206,59 @@ func TestInstantiateContractCmd(t *testing.T) { flagSet := cmd.Flags() flagSet.Set("label", "testing") flagSet.Set("run-as", myWellFundedAccount) + flagSet.Set("no-admin", "true") + }, + expError: true, + }, + "fails if no explicit --no-admin passed": { + srcGenesis: types.GenesisState{ + Params: types.DefaultParams(), + Codes: []types.Code{ + { + CodeID: 1, + CodeInfo: types.CodeInfo{ + CodeHash: []byte("a-valid-code-hash"), + Creator: keeper.RandomBech32AccountAddress(t), + InstantiateConfig: types.AccessConfig{ + Permission: types.AccessTypeEverybody, + }, + }, + CodeBytes: wasmIdent, + }, + }, + }, + mutator: func(cmd *cobra.Command) { + cmd.SetArgs([]string{"1", `{}`}) + flagSet := cmd.Flags() + flagSet.Set("label", "testing") + flagSet.Set("run-as", myWellFundedAccount) + }, + expError: true, + }, + "fails if both --admin and --no-admin passed": { + srcGenesis: types.GenesisState{ + Params: types.DefaultParams(), + Codes: []types.Code{ + { + CodeID: 1, + CodeInfo: types.CodeInfo{ + CodeHash: []byte("a-valid-code-hash"), + Creator: keeper.RandomBech32AccountAddress(t), + InstantiateConfig: types.AccessConfig{ + Permission: types.AccessTypeEverybody, + }, + }, + CodeBytes: wasmIdent, + }, + }, + }, + mutator: func(cmd *cobra.Command) { + cmd.SetArgs([]string{"1", `{}`}) + flagSet := cmd.Flags() + flagSet.Set("label", "testing") + flagSet.Set("run-as", myWellFundedAccount) + flagSet.Set("no-admin", "true") + flagSet.Set("admin", myWellFundedAccount) }, expError: true, }, @@ -227,6 +284,7 @@ func TestInstantiateContractCmd(t *testing.T) { flagSet := cmd.Flags() flagSet.Set("label", "testing") flagSet.Set("run-as", keeper.RandomBech32AccountAddress(t)) + flagSet.Set("no-admin", "true") }, expMsgCount: 1, }, @@ -253,6 +311,7 @@ func TestInstantiateContractCmd(t *testing.T) { flagSet.Set("label", "testing") flagSet.Set("run-as", myWellFundedAccount) flagSet.Set("amount", "100stake") + flagSet.Set("no-admin", "true") }, expMsgCount: 1, }, @@ -279,6 +338,7 @@ func TestInstantiateContractCmd(t *testing.T) { flagSet.Set("label", "testing") flagSet.Set("run-as", keeper.RandomBech32AccountAddress(t)) flagSet.Set("amount", "10stake") + flagSet.Set("no-admin", "true") }, expError: true, }, diff --git a/x/wasm/client/cli/tx.go b/x/wasm/client/cli/tx.go index a11c7ddc6c..1940dfb278 100644 --- a/x/wasm/client/cli/tx.go +++ b/x/wasm/client/cli/tx.go @@ -189,15 +189,17 @@ func parseInstantiateArgs(rawCodeID, initMsg string, sender sdk.AccAddress, flag if err != nil { return types.MsgInstantiateContract{}, fmt.Errorf("admin: %s", err) } + noAdmin, err := flags.GetBool(flagNoAdmin) + if err != nil { + return types.MsgInstantiateContract{}, fmt.Errorf("no-admin: %s", err) + } - if adminStr == "" { - noAdmin, err := flags.GetBool(flagNoAdmin) - if err != nil { - return types.MsgInstantiateContract{}, fmt.Errorf("no-admin: %s", err) - } - if !noAdmin { - return types.MsgInstantiateContract{}, fmt.Errorf("You must set an admin or explicitly pass --no-admin to make it immutible (wasmd issue #719)") - } + // ensure sensible admin is set (or explicitly immutable) + if adminStr == "" && !noAdmin { + return types.MsgInstantiateContract{}, fmt.Errorf("You must set an admin or explicitly pass --no-admin to make it immutible (wasmd issue #719)") + } + if adminStr != "" && noAdmin { + return types.MsgInstantiateContract{}, fmt.Errorf("You set an admin and passed --no-admin, those cannot both be true") } // build and sign the transaction, then broadcast to Tendermint From 5cd0cbf073b6b2452e24aab5500aad72af314a34 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 21 Jan 2022 10:38:43 +0100 Subject: [PATCH 3/4] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f7d740b25..e7c019fab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Fixed circleci by removing the golang executor from a docker build - Go 1.17 provides a much clearer go.mod file [\#679](https://github.com/CosmWasm/wasmd/pull/679) ([faddat](https://github.com/faddat)) - Autopin wasm code uploaded by gov proposal [\#726](https://github.com/CosmWasm/wasmd/pull/726) ([ethanfrey](https://github.com/ethanfrey)) +- You must explicitly declare --no-admin on cli instantiate if that is what you want [\#727](https://github.com/CosmWasm/wasmd/pull/727) ([ethanfrey](https://github.com/ethanfrey)) [Full Changelog](https://github.com/CosmWasm/wasmd/compare/v0.22.0...v0.21.0) From 1e78fbccf2b4186e642c3c08947c08b5cdd4ebaf Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 21 Jan 2022 10:46:55 +0100 Subject: [PATCH 4/4] Fixed error message capitalization --- x/wasm/client/cli/tx.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/wasm/client/cli/tx.go b/x/wasm/client/cli/tx.go index 1940dfb278..fac7d621b9 100644 --- a/x/wasm/client/cli/tx.go +++ b/x/wasm/client/cli/tx.go @@ -196,10 +196,10 @@ func parseInstantiateArgs(rawCodeID, initMsg string, sender sdk.AccAddress, flag // ensure sensible admin is set (or explicitly immutable) if adminStr == "" && !noAdmin { - return types.MsgInstantiateContract{}, fmt.Errorf("You must set an admin or explicitly pass --no-admin to make it immutible (wasmd issue #719)") + return types.MsgInstantiateContract{}, fmt.Errorf("you must set an admin or explicitly pass --no-admin to make it immutible (wasmd issue #719)") } if adminStr != "" && noAdmin { - return types.MsgInstantiateContract{}, fmt.Errorf("You set an admin and passed --no-admin, those cannot both be true") + return types.MsgInstantiateContract{}, fmt.Errorf("you set an admin and passed --no-admin, those cannot both be true") } // build and sign the transaction, then broadcast to Tendermint