From 9a16d583d3c0e5208133f8ce614d11aae7b8b1b8 Mon Sep 17 00:00:00 2001 From: Alexander Peters Date: Fri, 5 Jun 2020 14:02:01 +0200 Subject: [PATCH] Update contract admin (#124) * Add update administrator * Review comments --- CHANGELOG.md | 3 +- x/wasm/alias.go | 1 + x/wasm/handler.go | 21 ++++++++ x/wasm/internal/keeper/genesis.go | 2 +- x/wasm/internal/keeper/keeper.go | 21 +++++++- x/wasm/internal/keeper/keeper_test.go | 78 +++++++++++++++++++++++++++ x/wasm/internal/types/codec.go | 1 + x/wasm/internal/types/msg.go | 40 ++++++++++++++ x/wasm/internal/types/msg_test.go | 69 ++++++++++++++++++++++++ 9 files changed, 232 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a94a4f1..0b77ed8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,8 @@ Base64 encoded transactions. - `raw-bytes` convert raw-bytes to hex * (wasmcli) [\#191](https://github.com/cosmwasm/wasmd/pull/191) Add cmd `decode-tx`, decodes a tx from hex or base64 * (wasmd) [\#9](https://github.com/cosmwasm/wasmd/pull/9) Allow gzip data in tx body on Create - +* (wasmd)[\#124](https://github.com/CosmWasm/wasmd/pull/124) Update contract admin + ## [v2.0.3] - 2019-11-04 ### Improvements diff --git a/x/wasm/alias.go b/x/wasm/alias.go index a4482c36..ae870f27 100644 --- a/x/wasm/alias.go +++ b/x/wasm/alias.go @@ -94,6 +94,7 @@ type ( MsgInstantiateContract = types.MsgInstantiateContract MsgExecuteContract = types.MsgExecuteContract MsgMigrateContract = types.MsgMigrateContract + MsgUpdateAdministrator = types.MsgUpdateAdministrator Model = types.Model CodeInfo = types.CodeInfo ContractInfo = types.ContractInfo diff --git a/x/wasm/handler.go b/x/wasm/handler.go index f1c5d822..980950e1 100644 --- a/x/wasm/handler.go +++ b/x/wasm/handler.go @@ -39,6 +39,11 @@ func NewHandler(k Keeper) sdk.Handler { case MsgMigrateContract: return handleMigration(ctx, k, &msg) + case *MsgUpdateAdministrator: + return handleUpdateContractAdmin(ctx, k, msg) + case MsgUpdateAdministrator: + return handleUpdateContractAdmin(ctx, k, &msg) + default: errMsg := fmt.Sprintf("unrecognized wasm message type: %T", msg) return nil, sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, errMsg) @@ -139,3 +144,19 @@ func handleMigration(ctx sdk.Context, k Keeper, msg *MsgMigrateContract) (*sdk.R res.Events = append(events, ourEvent) return res, nil } + +func handleUpdateContractAdmin(ctx sdk.Context, k Keeper, msg *MsgUpdateAdministrator) (*sdk.Result, error) { + if err := k.UpdateContractAdmin(ctx, msg.Contract, msg.Sender, msg.NewAdmin); err != nil { + return nil, err + } + events := ctx.EventManager().Events() + ourEvent := sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(sdk.AttributeKeyModule, ModuleName), + sdk.NewAttribute(AttributeSigner, msg.Sender.String()), + sdk.NewAttribute(AttributeKeyContract, msg.Contract.String()), + ) + return &sdk.Result{ + Events: append(events, ourEvent), + }, nil +} diff --git a/x/wasm/internal/keeper/genesis.go b/x/wasm/internal/keeper/genesis.go index 4bd9c214..d3341de5 100644 --- a/x/wasm/internal/keeper/genesis.go +++ b/x/wasm/internal/keeper/genesis.go @@ -25,7 +25,7 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) { } for _, contract := range data.Contracts { - keeper.setContractInfo(ctx, contract.ContractAddress, contract.ContractInfo) + keeper.setContractInfo(ctx, contract.ContractAddress, &contract.ContractInfo) keeper.setContractState(ctx, contract.ContractAddress, contract.ContractState) } diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index b09d4bc0..774a6745 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -246,7 +246,7 @@ func (k Keeper) Migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller value.Events = nil contractInfo.UpdateCodeID(ctx, newCodeID) - k.setContractInfo(ctx, contractAddress, *contractInfo) + k.setContractInfo(ctx, contractAddress, contractInfo) if err := k.dispatchMessages(ctx, contractAddress, res.Messages); err != nil { return nil, sdkerrors.Wrap(err, "dispatch") @@ -255,6 +255,23 @@ func (k Keeper) Migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller return &value, nil } +// UpdateContractAdmin sets the admin value on the ContractInfo. New admin can be nil to disable further migrations/ updates. +func (k Keeper) UpdateContractAdmin(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newAdmin sdk.AccAddress) error { + contractInfo := k.GetContractInfo(ctx, contractAddress) + if contractInfo == nil { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "unknown contract") + } + if contractInfo.Admin == nil { + return sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "migration not supported by this contract") + } + if !contractInfo.Admin.Equals(caller) { + return sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "no permission") + } + contractInfo.Admin = newAdmin + k.setContractInfo(ctx, contractAddress, contractInfo) + return nil +} + // QuerySmart queries the smart contract itself. func (k Keeper) QuerySmart(ctx sdk.Context, contractAddr sdk.AccAddress, req []byte) ([]byte, error) { ctx = ctx.WithGasMeter(sdk.NewGasMeter(k.queryGasLimit)) @@ -326,7 +343,7 @@ func (k Keeper) GetContractInfo(ctx sdk.Context, contractAddress sdk.AccAddress) return &contract } -func (k Keeper) setContractInfo(ctx sdk.Context, contractAddress sdk.AccAddress, contract types.ContractInfo) { +func (k Keeper) setContractInfo(ctx sdk.Context, contractAddress sdk.AccAddress, contract *types.ContractInfo) { store := ctx.KVStore(k.storeKey) store.Set(types.GetContractAddressKey(contractAddress), k.cdc.MustMarshalBinaryBare(contract)) } diff --git a/x/wasm/internal/keeper/keeper_test.go b/x/wasm/internal/keeper/keeper_test.go index ea290b55..3c9b84f3 100644 --- a/x/wasm/internal/keeper/keeper_test.go +++ b/x/wasm/internal/keeper/keeper_test.go @@ -568,6 +568,84 @@ func TestMigrate(t *testing.T) { } } +func TestUpdateContractAdmin(t *testing.T) { + tempDir, err := ioutil.TempDir("", "wasm") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) + accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper + + deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) + topUp := sdk.NewCoins(sdk.NewInt64Coin("denom", 5000)) + creator := createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit...)) + fred := createFakeFundedAccount(ctx, accKeeper, topUp) + + wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + + originalContractID, err := keeper.Create(ctx, creator, wasmCode, "", "") + require.NoError(t, err) + + _, _, anyAddr := keyPubAddr() + initMsg := InitMsg{ + Verifier: fred, + Beneficiary: anyAddr, + } + initMsgBz, err := json.Marshal(initMsg) + require.NoError(t, err) + specs := map[string]struct { + instAdmin sdk.AccAddress + newAdmin sdk.AccAddress + overrideContractAddr sdk.AccAddress + caller sdk.AccAddress + expErr *sdkerrors.Error + }{ + "all good with admin set": { + instAdmin: fred, + newAdmin: anyAddr, + caller: fred, + }, + "all good with new admin empty": { + instAdmin: fred, + newAdmin: nil, + caller: fred, + }, + "prevent update when admin was not set on instantiate": { + caller: creator, + newAdmin: fred, + expErr: sdkerrors.ErrUnauthorized, + }, + "prevent updates from non admin address": { + instAdmin: creator, + newAdmin: fred, + caller: fred, + expErr: sdkerrors.ErrUnauthorized, + }, + "fail with non existing contract addr": { + instAdmin: creator, + caller: creator, + overrideContractAddr: anyAddr, + expErr: sdkerrors.ErrInvalidRequest, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + addr, err := keeper.Instantiate(ctx, originalContractID, creator, spec.instAdmin, initMsgBz, "demo contract", nil) + require.NoError(t, err) + if spec.overrideContractAddr != nil { + addr = spec.overrideContractAddr + } + err = keeper.UpdateContractAdmin(ctx, addr, spec.caller, spec.newAdmin) + require.True(t, spec.expErr.Is(err), "expected %v but got %+v", spec.expErr, err) + if spec.expErr != nil { + return + } + cInfo := keeper.GetContractInfo(ctx, addr) + assert.Equal(t, spec.newAdmin, cInfo.Admin) + }) + } +} + type InitMsg struct { Verifier sdk.AccAddress `json:"verifier"` Beneficiary sdk.AccAddress `json:"beneficiary"` diff --git a/x/wasm/internal/types/codec.go b/x/wasm/internal/types/codec.go index b31d9086..bf56bd35 100644 --- a/x/wasm/internal/types/codec.go +++ b/x/wasm/internal/types/codec.go @@ -11,6 +11,7 @@ func RegisterCodec(cdc *codec.Codec) { cdc.RegisterConcrete(&MsgInstantiateContract{}, "wasm/instantiate", nil) cdc.RegisterConcrete(&MsgExecuteContract{}, "wasm/execute", nil) cdc.RegisterConcrete(&MsgMigrateContract{}, "wasm/migrate", nil) + cdc.RegisterConcrete(&MsgUpdateAdministrator{}, "wasm/update-contract-admin", nil) } // ModuleCdc generic sealed codec to be used throughout module diff --git a/x/wasm/internal/types/msg.go b/x/wasm/internal/types/msg.go index a6e49ab9..664856bd 100644 --- a/x/wasm/internal/types/msg.go +++ b/x/wasm/internal/types/msg.go @@ -227,3 +227,43 @@ func (msg MsgMigrateContract) GetSignBytes() []byte { func (msg MsgMigrateContract) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{msg.Sender} } + +type MsgUpdateAdministrator struct { + Sender sdk.AccAddress `json:"sender" yaml:"sender"` + NewAdmin sdk.AccAddress `json:"new_admin,omitempty" yaml:"new_admin"` + Contract sdk.AccAddress `json:"contract" yaml:"contract"` +} + +func (msg MsgUpdateAdministrator) Route() string { + return RouterKey +} + +func (msg MsgUpdateAdministrator) Type() string { + return "update-contract-admin" +} + +func (msg MsgUpdateAdministrator) ValidateBasic() error { + if err := sdk.VerifyAddressFormat(msg.Sender); err != nil { + return sdkerrors.Wrap(err, "sender") + } + if err := sdk.VerifyAddressFormat(msg.Contract); err != nil { + return sdkerrors.Wrap(err, "contract") + } + if len(msg.NewAdmin) != 0 { + if err := sdk.VerifyAddressFormat(msg.NewAdmin); err != nil { + return sdkerrors.Wrap(err, "new admin") + } + if msg.Sender.Equals(msg.NewAdmin) { + return sdkerrors.Wrap(ErrInvalidMsg, "new admin is the same as the old") + } + } + return nil +} + +func (msg MsgUpdateAdministrator) GetSignBytes() []byte { + return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(msg)) +} + +func (msg MsgUpdateAdministrator) GetSigners() []sdk.AccAddress { + return []sdk.AccAddress{msg.Sender} +} diff --git a/x/wasm/internal/types/msg_test.go b/x/wasm/internal/types/msg_test.go index a2e0a7be..9f87553f 100644 --- a/x/wasm/internal/types/msg_test.go +++ b/x/wasm/internal/types/msg_test.go @@ -209,7 +209,76 @@ func TestInstantiateContractValidation(t *testing.T) { } }) } +} +func TestMsgUpdateAdministrator(t *testing.T) { + badAddress, err := sdk.AccAddressFromHex("012345") + require.NoError(t, err) + // proper address size + goodAddress := sdk.AccAddress(make([]byte, 20)) + otherGoodAddress := sdk.AccAddress(bytes.Repeat([]byte{0x1}, 20)) + anotherGoodAddress := sdk.AccAddress(bytes.Repeat([]byte{0x2}, 20)) + + specs := map[string]struct { + src MsgUpdateAdministrator + expErr bool + }{ + "all good": { + src: MsgUpdateAdministrator{ + Sender: goodAddress, + NewAdmin: otherGoodAddress, + Contract: anotherGoodAddress, + }, + }, + "new admin optional": { + src: MsgUpdateAdministrator{ + Sender: goodAddress, + Contract: anotherGoodAddress, + }, + }, + "bad sender": { + src: MsgUpdateAdministrator{ + Sender: badAddress, + NewAdmin: otherGoodAddress, + Contract: anotherGoodAddress, + }, + expErr: true, + }, + "bad new admin": { + src: MsgUpdateAdministrator{ + Sender: goodAddress, + NewAdmin: badAddress, + Contract: anotherGoodAddress, + }, + expErr: true, + }, + "bad contract addr": { + src: MsgUpdateAdministrator{ + Sender: goodAddress, + NewAdmin: otherGoodAddress, + Contract: badAddress, + }, + expErr: true, + }, + "new admin same as old admin": { + src: MsgUpdateAdministrator{ + Sender: goodAddress, + NewAdmin: goodAddress, + Contract: anotherGoodAddress, + }, + expErr: true, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + err := spec.src.ValidateBasic() + if spec.expErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } } func TestMsgMigrateContract(t *testing.T) {