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 msg update contract label #1640

Merged
merged 2 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
32 changes: 32 additions & 0 deletions docs/proto/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@
- [MsgUnpinCodesResponse](#cosmwasm.wasm.v1.MsgUnpinCodesResponse)
- [MsgUpdateAdmin](#cosmwasm.wasm.v1.MsgUpdateAdmin)
- [MsgUpdateAdminResponse](#cosmwasm.wasm.v1.MsgUpdateAdminResponse)
- [MsgUpdateContractLabel](#cosmwasm.wasm.v1.MsgUpdateContractLabel)
- [MsgUpdateContractLabelResponse](#cosmwasm.wasm.v1.MsgUpdateContractLabelResponse)
- [MsgUpdateInstantiateConfig](#cosmwasm.wasm.v1.MsgUpdateInstantiateConfig)
- [MsgUpdateInstantiateConfigResponse](#cosmwasm.wasm.v1.MsgUpdateInstantiateConfigResponse)
- [MsgUpdateParams](#cosmwasm.wasm.v1.MsgUpdateParams)
Expand Down Expand Up @@ -1845,6 +1847,33 @@ MsgUpdateAdminResponse returns empty data



<a name="cosmwasm.wasm.v1.MsgUpdateContractLabel"></a>

### MsgUpdateContractLabel
MsgUpdateContractLabel sets a new label for a smart contract


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `sender` | [string](#string) | | Sender is the that actor that signed the messages |
| `new_label` | [string](#string) | | NewLabel string to be set |
| `contract` | [string](#string) | | Contract is the address of the smart contract |






<a name="cosmwasm.wasm.v1.MsgUpdateContractLabelResponse"></a>

### MsgUpdateContractLabelResponse
MsgUpdateContractLabelResponse returns empty data






<a name="cosmwasm.wasm.v1.MsgUpdateInstantiateConfig"></a>

### MsgUpdateInstantiateConfig
Expand Down Expand Up @@ -1946,6 +1975,9 @@ Since: 0.40 | |
| `StoreAndMigrateContract` | [MsgStoreAndMigrateContract](#cosmwasm.wasm.v1.MsgStoreAndMigrateContract) | [MsgStoreAndMigrateContractResponse](#cosmwasm.wasm.v1.MsgStoreAndMigrateContractResponse) | StoreAndMigrateContract defines a governance operation for storing and migrating the contract. The authority is defined in the keeper.

Since: 0.42 | |
| `UpdateContractLabel` | [MsgUpdateContractLabel](#cosmwasm.wasm.v1.MsgUpdateContractLabel) | [MsgUpdateContractLabelResponse](#cosmwasm.wasm.v1.MsgUpdateContractLabelResponse) | UpdateContractLabel sets a new label for a smart contract

Since: 0.43 | |

<!-- end services -->

Expand Down
25 changes: 23 additions & 2 deletions proto/cosmwasm/wasm/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ service Msg {
rpc ExecuteContract(MsgExecuteContract) returns (MsgExecuteContractResponse);
// Migrate runs a code upgrade/ downgrade for a smart contract
rpc MigrateContract(MsgMigrateContract) returns (MsgMigrateContractResponse);
// UpdateAdmin sets a new admin for a smart contract
// UpdateAdmin sets a new admin for a smart contract
rpc UpdateAdmin(MsgUpdateAdmin) returns (MsgUpdateAdminResponse);
// ClearAdmin removes any admin stored for a smart contract
rpc ClearAdmin(MsgClearAdmin) returns (MsgClearAdminResponse);
Expand Down Expand Up @@ -78,6 +78,11 @@ service Msg {
// Since: 0.42
rpc StoreAndMigrateContract(MsgStoreAndMigrateContract)
returns (MsgStoreAndMigrateContractResponse);
// UpdateContractLabel sets a new label for a smart contract
//
// Since: 0.43
rpc UpdateContractLabel(MsgUpdateContractLabel)
returns (MsgUpdateContractLabelResponse);
}

// MsgStoreCode submit Wasm code to the system
Expand Down Expand Up @@ -472,4 +477,20 @@ message MsgStoreAndMigrateContractResponse {
bytes checksum = 2;
// Data contains bytes to returned from the contract
bytes data = 3;
}
}

// MsgUpdateContractLabel sets a new label for a smart contract
message MsgUpdateContractLabel {
option (amino.name) = "wasm/MsgUpdateContractLabel";
option (cosmos.msg.v1.signer) = "sender";

// Sender is the that actor that signed the messages
string sender = 1 [ (cosmos_proto.scalar) = "cosmos.AddressString" ];
// NewLabel string to be set
string new_label = 2;
// Contract is the address of the smart contract
string contract = 3 [ (cosmos_proto.scalar) = "cosmos.AddressString" ];
}

// MsgUpdateContractLabelResponse returns empty data
message MsgUpdateContractLabelResponse {}
28 changes: 28 additions & 0 deletions x/wasm/client/cli/new_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,31 @@
flags.AddTxFlagsToCmd(cmd)
return cmd
}

// UpdateContractLabelCmd sets an new label for a contract
func UpdateContractLabelCmd() *cobra.Command {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

cmd := &cobra.Command{
Use: "set-contract-label [contract_addr_bech32] [new_label]",
Short: "Set new label for a contract",
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
return err

Check warning on line 170 in x/wasm/client/cli/new_tx.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/client/cli/new_tx.go#L162-L170

Added lines #L162 - L170 were not covered by tests
}

msg := types.MsgUpdateContractLabel{
Sender: clientCtx.GetFromAddress().String(),
Contract: args[0],
NewLabel: args[1],

Check warning on line 176 in x/wasm/client/cli/new_tx.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/client/cli/new_tx.go#L173-L176

Added lines #L173 - L176 were not covered by tests
}
if err = msg.ValidateBasic(); err != nil {
return err

Check warning on line 179 in x/wasm/client/cli/new_tx.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/client/cli/new_tx.go#L178-L179

Added lines #L178 - L179 were not covered by tests
}
return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), &msg)

Check warning on line 181 in x/wasm/client/cli/new_tx.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/client/cli/new_tx.go#L181

Added line #L181 was not covered by tests
},
SilenceUsage: true,
}
flags.AddTxFlagsToCmd(cmd)
return cmd

Check warning on line 186 in x/wasm/client/cli/new_tx.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/client/cli/new_tx.go#L185-L186

Added lines #L185 - L186 were not covered by tests
}
1 change: 1 addition & 0 deletions x/wasm/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
GrantCmd(),
UpdateInstantiateConfigCmd(),
SubmitProposalCmd(),
UpdateContractLabelCmd(),

Check warning on line 74 in x/wasm/client/cli/tx.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/client/cli/tx.go#L74

Added line #L74 was not covered by tests
)
return txCmd
}
Expand Down
20 changes: 20 additions & 0 deletions x/wasm/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,26 @@ func (k Keeper) setContractAdmin(ctx context.Context, contractAddress, caller, n
return nil
}

func (k Keeper) setContractLabel(ctx context.Context, contractAddress, caller sdk.AccAddress, newLabel string, authZ types.AuthorizationPolicy) error {
sdkCtx := sdk.UnwrapSDKContext(ctx)
contractInfo := k.GetContractInfo(sdkCtx, contractAddress)
if contractInfo == nil {
return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "unknown contract")
}
if !authZ.CanModifyContract(contractInfo.AdminAddr(), caller) {
return errorsmod.Wrap(sdkerrors.ErrUnauthorized, "can not modify contract")
}
contractInfo.Label = newLabel
k.mustStoreContractInfo(sdkCtx, contractAddress, contractInfo)
sdkCtx.EventManager().EmitEvent(sdk.NewEvent(
types.EventTypeUpdateContractLabel,
sdk.NewAttribute(types.AttributeKeyContractAddr, contractAddress.String()),
sdk.NewAttribute(types.AttributeKeyNewLabel, newLabel),
))
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!


return nil
}

func (k Keeper) appendToContractHistory(ctx context.Context, contractAddr sdk.AccAddress, newEntries ...types.ContractCodeHistoryEntry) error {
store := k.storeService.OpenKVStore(ctx)
// find last element position
Expand Down
63 changes: 63 additions & 0 deletions x/wasm/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2489,6 +2489,69 @@ func TestGasConsumed(t *testing.T) {
}
}

func TestSetContractLabel(t *testing.T) {
parentCtx, keepers := CreateTestInput(t, false, AvailableCapabilities)
k := keepers.WasmKeeper
example := InstantiateReflectExampleContract(t, parentCtx, keepers)

specs := map[string]struct {
newLabel string
caller sdk.AccAddress
policy types.AuthorizationPolicy
contract sdk.AccAddress
expErr bool
}{
"update label - default policy": {
newLabel: "new label",
caller: example.CreatorAddr,
policy: DefaultAuthorizationPolicy{},
contract: example.Contract,
},
"update label - gov policy": {
newLabel: "new label",
policy: GovAuthorizationPolicy{},
caller: RandomAccountAddress(t),
contract: example.Contract,
},
"update label - unauthorized": {
newLabel: "new label",
caller: RandomAccountAddress(t),
policy: DefaultAuthorizationPolicy{},
contract: example.Contract,
expErr: true,
},
"update label - unknown contract": {
newLabel: "new label",
caller: example.CreatorAddr,
Copy link
Contributor

Choose a reason for hiding this comment

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

When you use a random caller address, then it would ensure the different behaviour of the gov policy

policy: DefaultAuthorizationPolicy{},
contract: RandomAccountAddress(t),
expErr: true,
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
ctx, _ := parentCtx.CacheContext()
em := sdk.NewEventManager()
ctx = ctx.WithEventManager(em)
gotErr := k.setContractLabel(ctx, spec.contract, spec.caller, spec.newLabel, spec.policy)
if spec.expErr {
require.Error(t, gotErr)
return
}
require.NoError(t, gotErr)
assert.Equal(t, spec.newLabel, k.GetContractInfo(ctx, spec.contract).Label)
// and event emitted
require.Len(t, em.Events(), 1)
assert.Equal(t, "update_contract_label", em.Events()[0].Type)
exp := map[string]string{
"_contract_address": spec.contract.String(),
"new_label": spec.newLabel,
}
assert.Equal(t, exp, attrsToStringMap(em.Events()[0].Attributes))
})
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Good tests! For bonus points, there is also the unknown contract address scenario that must not panic.


func attrsToStringMap(attrs []abci.EventAttribute) map[string]string {
r := make(map[string]string, len(attrs))
for _, v := range attrs {
Expand Down
23 changes: 23 additions & 0 deletions x/wasm/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,3 +462,26 @@
Data: data,
}, nil
}

func (m msgServer) UpdateContractLabel(ctx context.Context, msg *types.MsgUpdateContractLabel) (*types.MsgUpdateContractLabelResponse, error) {
if err := msg.ValidateBasic(); err != nil {
return nil, err

Check warning on line 468 in x/wasm/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/keeper/msg_server.go#L468

Added line #L468 was not covered by tests
}

senderAddr, err := sdk.AccAddressFromBech32(msg.Sender)
if err != nil {
return nil, errorsmod.Wrap(err, "sender")

Check warning on line 473 in x/wasm/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/keeper/msg_server.go#L473

Added line #L473 was not covered by tests
}
contractAddr, err := sdk.AccAddressFromBech32(msg.Contract)
if err != nil {
return nil, errorsmod.Wrap(err, "contract")

Check warning on line 477 in x/wasm/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/keeper/msg_server.go#L477

Added line #L477 was not covered by tests
}

policy := m.selectAuthorizationPolicy(ctx, msg.Sender)

if err := m.keeper.setContractLabel(ctx, contractAddr, senderAddr, msg.NewLabel, policy); err != nil {
return nil, err
}

return &types.MsgUpdateContractLabelResponse{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

straight forward 👍

}
83 changes: 83 additions & 0 deletions x/wasm/keeper/msg_server_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1144,3 +1144,86 @@ func TestStoreAndMigrateContract(t *testing.T) {
})
}
}

func TestUpdateContractLabel(t *testing.T) {
wasmApp := app.Setup(t)
ctx := wasmApp.BaseApp.NewContextLegacy(false, tmproto.Header{Time: time.Now()})

var (
myAddress sdk.AccAddress = make([]byte, types.ContractAddrLen)
authority = wasmApp.WasmKeeper.GetAuthority()
_, _, otherAddr = testdata.KeyTestPubAddr()
)

specs := map[string]struct {
addr string
newLabel string
expErr bool
}{
"authority can update contract label": {
addr: authority,
newLabel: "new label",
expErr: false,
},
"admin can update contract label": {
addr: myAddress.String(),
newLabel: "new label",
expErr: false,
},
"other address cannot update contract label": {
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 good to have the scenarios covered but there is some overlap with the keeper tests.
The integration tests give more confidence on the whole process. If you see value in both, then we can keep them.

addr: otherAddr.String(),
newLabel: "new label",
expErr: true,
},
"empty new label": {
addr: authority,
expErr: true,
},
"invalid new label": {
addr: authority,
newLabel: " start with space ",
expErr: true,
},
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check for invalid data

for name, spec := range specs {
t.Run(name, func(t *testing.T) {
// setup
msg := &types.MsgStoreAndInstantiateContract{
Authority: spec.addr,
WASMByteCode: wasmContract,
InstantiatePermission: &types.AllowEverybody,
Admin: myAddress.String(),
UnpinCode: false,
Label: "old label",
Msg: []byte(`{}`),
Funds: sdk.Coins{},
}
rsp, err := wasmApp.MsgServiceRouter().Handler(msg)(ctx, msg)
require.NoError(t, err)
var storeAndInstantiateResponse types.MsgStoreAndInstantiateContractResponse
require.NoError(t, wasmApp.AppCodec().Unmarshal(rsp.Data, &storeAndInstantiateResponse))

contract := storeAndInstantiateResponse.Address
contractAddr, err := sdk.AccAddressFromBech32(contract)
require.NoError(t, err)
require.Equal(t, "old label", wasmApp.WasmKeeper.GetContractInfo(ctx, contractAddr).Label)

// when
msgUpdateLabel := &types.MsgUpdateContractLabel{
Sender: spec.addr,
NewLabel: spec.newLabel,
Contract: storeAndInstantiateResponse.Address,
}
_, err = wasmApp.MsgServiceRouter().Handler(msgUpdateLabel)(ctx, msgUpdateLabel)

// then
if spec.expErr {
require.Error(t, err)
require.Equal(t, "old label", wasmApp.WasmKeeper.GetContractInfo(ctx, contractAddr).Label)
} else {
require.NoError(t, err)
require.Equal(t, spec.newLabel, wasmApp.WasmKeeper.GetContractInfo(ctx, contractAddr).Label)
}
})
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice test!

2 changes: 2 additions & 0 deletions x/wasm/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
cdc.RegisterConcrete(&MsgAddCodeUploadParamsAddresses{}, "wasm/MsgAddCodeUploadParamsAddresses", nil)
cdc.RegisterConcrete(&MsgRemoveCodeUploadParamsAddresses{}, "wasm/MsgRemoveCodeUploadParamsAddresses", nil)
cdc.RegisterConcrete(&MsgStoreAndMigrateContract{}, "wasm/MsgStoreAndMigrateContract", nil)
cdc.RegisterConcrete(&MsgUpdateContractLabel{}, "wasm/MsgUpdateContractLabel", nil)

Check warning on line 30 in x/wasm/types/codec.go

View check run for this annotation

Codecov / codecov/patch

x/wasm/types/codec.go#L30

Added line #L30 was not covered by tests

cdc.RegisterConcrete(&PinCodesProposal{}, "wasm/PinCodesProposal", nil)
cdc.RegisterConcrete(&UnpinCodesProposal{}, "wasm/UnpinCodesProposal", nil)
Expand Down Expand Up @@ -79,6 +80,7 @@
&MsgAddCodeUploadParamsAddresses{},
&MsgRemoveCodeUploadParamsAddresses{},
&MsgStoreAndMigrateContract{},
&MsgUpdateContractLabel{},
Copy link
Contributor

Choose a reason for hiding this comment

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

Important!

)
registry.RegisterImplementations(
(*v1beta1.Content)(nil),
Expand Down
2 changes: 2 additions & 0 deletions x/wasm/types/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const (
EventTypeReply = "reply"
EventTypeGovContractResult = "gov_contract_result"
EventTypeUpdateContractAdmin = "update_contract_admin"
EventTypeUpdateContractLabel = "update_contract_label"
EventTypeUpdateCodeAccessConfig = "update_code_access_config"
EventTypePacketRecv = "ibc_packet_received"
// add new types to IsAcceptedEventOnRecvPacketErrorAck
Expand Down Expand Up @@ -61,6 +62,7 @@ const (
AttributeKeyResultDataHex = "result"
AttributeKeyRequiredCapability = "required_capability"
AttributeKeyNewAdmin = "new_admin_address"
AttributeKeyNewLabel = "new_label"
AttributeKeyCodePermission = "code_permission"
AttributeKeyAuthorizedAddresses = "authorized_addresses"
AttributeKeyAckSuccess = "success"
Expand Down
Loading