Skip to content

Commit

Permalink
Merge pull request cosmos#534 from CosmWasm/response_428
Browse files Browse the repository at this point in the history 
Cleanup keeper result types
  • Loading branch information
@alpe
alpe authored Jun 14, 2021
2 parents 3b82807 + 44e7669 commit f8e39bf
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 67 deletions.
8 changes: 4 additions & 4 deletions x/wasm/keeper/contract_keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ var _ types.ContractOpsKeeper = PermissionedKeeper{}
type decoratedKeeper interface {
create(ctx sdk.Context, creator sdk.AccAddress, wasmCode []byte, source string, builder string, instantiateAccess *types.AccessConfig, authZ AuthorizationPolicy) (codeID uint64, err error)
instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.AccAddress, initMsg []byte, label string, deposit sdk.Coins, authZ AuthorizationPolicy) (sdk.AccAddress, []byte, error)
migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newCodeID uint64, msg []byte, authZ AuthorizationPolicy) (*sdk.Result, error)
migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newCodeID uint64, msg []byte, authZ AuthorizationPolicy) ([]byte, error)
setContractAdmin(ctx sdk.Context, contractAddress, caller, newAdmin sdk.AccAddress, authZ AuthorizationPolicy) error
pinCode(ctx sdk.Context, codeID uint64) error
unpinCode(ctx sdk.Context, codeID uint64) error
execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, msg []byte, coins sdk.Coins) (*sdk.Result, error)
execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, msg []byte, coins sdk.Coins) ([]byte, error)
setContractInfoExtension(ctx sdk.Context, contract sdk.AccAddress, extra types.ContractInfoExtension) error
}

Expand Down Expand Up @@ -44,11 +44,11 @@ func (p PermissionedKeeper) Instantiate(ctx sdk.Context, codeID uint64, creator,
return p.nested.instantiate(ctx, codeID, creator, admin, initMsg, label, deposit, p.authZPolicy)
}

func (p PermissionedKeeper) Execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, msg []byte, coins sdk.Coins) (*sdk.Result, error) {
func (p PermissionedKeeper) Execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, msg []byte, coins sdk.Coins) ([]byte, error) {
return p.nested.execute(ctx, contractAddress, caller, msg, coins)
}

func (p PermissionedKeeper) Migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newCodeID uint64, msg []byte) (*sdk.Result, error) {
func (p PermissionedKeeper) Migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newCodeID uint64, msg []byte) ([]byte, error) {
return p.nested.migrate(ctx, contractAddress, caller, newCodeID, msg, p.authZPolicy)
}

Expand Down
27 changes: 8 additions & 19 deletions x/wasm/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func (k Keeper) instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.A
}

// Execute executes the contract instance
func (k Keeper) execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, msg []byte, coins sdk.Coins) (*sdk.Result, error) {
func (k Keeper) execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, msg []byte, coins sdk.Coins) ([]byte, error) {
defer telemetry.MeasureSince(time.Now(), "wasm", "contract", "execute")
contractInfo, codeInfo, prefixStore, err := k.contractInstance(ctx, contractAddress)
if err != nil {
Expand Down Expand Up @@ -361,13 +361,10 @@ func (k Keeper) execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller
if err != nil {
return nil, sdkerrors.Wrap(err, "dispatch")
}

return &sdk.Result{
Data: data,
}, nil
return data, nil
}

func (k Keeper) migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newCodeID uint64, msg []byte, authZ AuthorizationPolicy) (*sdk.Result, error) {
func (k Keeper) migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newCodeID uint64, msg []byte, authZ AuthorizationPolicy) ([]byte, error) {
defer telemetry.MeasureSince(time.Now(), "wasm", "contract", "migrate")
if !k.IsPinnedCode(ctx, newCodeID) {
ctx.GasMeter().ConsumeGas(k.instanceCost, "Loading CosmWasm module: migrate")
Expand Down Expand Up @@ -433,16 +430,13 @@ func (k Keeper) migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller
if err != nil {
return nil, sdkerrors.Wrap(err, "dispatch")
}

return &sdk.Result{
Data: data,
}, nil
return data, nil
}

// Sudo allows priviledged access to a contract. This can never be called by governance or external tx, but only by
// another native Go module directly. Thus, the keeper doesn't place any access controls on it, that is the
// responsibility or the app developer (who passes the wasm.Keeper in app.go)
func (k Keeper) Sudo(ctx sdk.Context, contractAddress sdk.AccAddress, msg []byte) (*sdk.Result, error) {
func (k Keeper) Sudo(ctx sdk.Context, contractAddress sdk.AccAddress, msg []byte) ([]byte, error) {
defer telemetry.MeasureSince(time.Now(), "wasm", "contract", "sudo")
contractInfo, codeInfo, prefixStore, err := k.contractInstance(ctx, contractAddress)
if err != nil {
Expand Down Expand Up @@ -473,15 +467,12 @@ func (k Keeper) Sudo(ctx sdk.Context, contractAddress sdk.AccAddress, msg []byte
if err != nil {
return nil, sdkerrors.Wrap(err, "dispatch")
}

return &sdk.Result{
Data: data,
}, nil
return data, nil
}

// reply is only called from keeper internal functions (dispatchSubmessages) after processing the submessage
// it
func (k Keeper) reply(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
func (k Keeper) reply(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
contractInfo, codeInfo, prefixStore, err := k.contractInstance(ctx, contractAddress)
if err != nil {
return nil, err
Expand Down Expand Up @@ -515,9 +506,7 @@ func (k Keeper) reply(ctx sdk.Context, contractAddress sdk.AccAddress, reply was
if err != nil {
return nil, sdkerrors.Wrap(err, "dispatch")
}
return &sdk.Result{
Data: data,
}, nil
return data, nil
}

// addToContractCodeSecondaryIndex adds element to the index for contracts-by-codeid queries
Expand Down
8 changes: 3 additions & 5 deletions x/wasm/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1009,10 +1009,9 @@ func TestMigrateWithDispatchedMessage(t *testing.T) {

migMsgBz := BurnerExampleInitMsg{Payout: myPayoutAddr}.GetBytes(t)
ctx = ctx.WithEventManager(sdk.NewEventManager()).WithBlockHeight(ctx.BlockHeight() + 1)
res, err := keeper.Migrate(ctx, contractAddr, fred, burnerContractID, migMsgBz)
data, err := keeper.Migrate(ctx, contractAddr, fred, burnerContractID, migMsgBz)
require.NoError(t, err)
assert.Equal(t, "burnt 1 keys", string(res.Data))
assert.Equal(t, "", res.Log)
assert.Equal(t, "burnt 1 keys", string(data))
type dict map[string]interface{}
expEvents := []dict{
{
Expand Down Expand Up @@ -1184,9 +1183,8 @@ func TestSudo(t *testing.T) {
sudoMsg, err := json.Marshal(msg)
require.NoError(t, err)

res, err := keepers.WasmKeeper.Sudo(ctx, addr, sudoMsg)
_, err = keepers.WasmKeeper.Sudo(ctx, addr, sudoMsg)
require.NoError(t, err)
require.NotNil(t, res)

// ensure community now exists and got paid
comAcct = accKeeper.GetAccount(ctx, community)
Expand Down
8 changes: 4 additions & 4 deletions x/wasm/keeper/msg_dispatcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Messenger interface {

// replyer is a subset of keeper that can handle replies to submessages
type replyer interface {
reply(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error)
reply(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error)
}

// MessageDispatcher coordinates message sending and submessage reply/ state commits
Expand Down Expand Up @@ -140,12 +140,12 @@ func (d MessageDispatcher) DispatchSubmessages(ctx sdk.Context, contractAddr sdk

// we can ignore any result returned as there is nothing to do with the data
// and the events are already in the ctx.EventManager()
rData, err := d.keeper.reply(ctx, contractAddr, reply)
rspData, err := d.keeper.reply(ctx, contractAddr, reply)
switch {
case err != nil:
return nil, sdkerrors.Wrap(err, "reply")
case rData.Data != nil:
rsp = rData.Data
case len(rspData) != 0:
rsp = rspData
}
}
return rsp, nil
Expand Down
50 changes: 34 additions & 16 deletions x/wasm/keeper/msg_dispatcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ func TestDispatchSubmessages(t *testing.T) {
ReplyOn: wasmvmtypes.ReplySuccess,
}},
replyer: &mockReplyer{
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
return &sdk.Result{Data: []byte("myReplyData")}, nil
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
return []byte("myReplyData"), nil
},
},
msgHandler: &wasmtesting.MockMessageHandler{
Expand All @@ -67,8 +67,8 @@ func TestDispatchSubmessages(t *testing.T) {
ReplyOn: wasmvmtypes.ReplyError,
}},
replyer: &mockReplyer{
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
return &sdk.Result{Data: []byte("myReplyData")}, nil
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
return []byte("myReplyData"), nil
},
},
msgHandler: &wasmtesting.MockMessageHandler{
Expand All @@ -84,8 +84,8 @@ func TestDispatchSubmessages(t *testing.T) {
ReplyOn: wasmvmtypes.ReplySuccess,
}},
replyer: &mockReplyer{
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
return &sdk.Result{Data: []byte("myReplyData")}, nil
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
return []byte("myReplyData"), nil
},
},
msgHandler: &wasmtesting.MockMessageHandler{
Expand All @@ -106,7 +106,7 @@ func TestDispatchSubmessages(t *testing.T) {
ReplyOn: wasmvmtypes.ReplySuccess,
}},
replyer: &mockReplyer{
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
return nil, errors.New("reply failed")
},
},
Expand All @@ -124,8 +124,8 @@ func TestDispatchSubmessages(t *testing.T) {
ReplyOn: wasmvmtypes.ReplyError,
}},
replyer: &mockReplyer{
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
return &sdk.Result{Data: []byte("myReplyData")}, nil
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
return []byte("myReplyData"), nil
},
},
msgHandler: &wasmtesting.MockMessageHandler{
Expand Down Expand Up @@ -154,8 +154,8 @@ func TestDispatchSubmessages(t *testing.T) {
"multiple msg - last reply": {
msgs: []wasmvmtypes.SubMsg{{ID: 1, ReplyOn: wasmvmtypes.ReplyError}, {ID: 2, ReplyOn: wasmvmtypes.ReplyError}},
replyer: &mockReplyer{
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
return &sdk.Result{Data: []byte(fmt.Sprintf("myReplyData:%d", reply.ID))}, nil
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
return []byte(fmt.Sprintf("myReplyData:%d", reply.ID)), nil
},
},
msgHandler: &wasmtesting.MockMessageHandler{
Expand All @@ -169,11 +169,29 @@ func TestDispatchSubmessages(t *testing.T) {
"multiple msg - last reply with non nil": {
msgs: []wasmvmtypes.SubMsg{{ID: 1, ReplyOn: wasmvmtypes.ReplyError}, {ID: 2, ReplyOn: wasmvmtypes.ReplyError}},
replyer: &mockReplyer{
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
if reply.ID == 2 {
return &sdk.Result{}, nil
return nil, nil
}
return &sdk.Result{Data: []byte("myReplyData:1")}, nil
return []byte("myReplyData:1"), nil
},
},
msgHandler: &wasmtesting.MockMessageHandler{
DispatchMsgFn: func(ctx sdk.Context, contractAddr sdk.AccAddress, contractIBCPortID string, msg wasmvmtypes.CosmosMsg) (events []sdk.Event, data [][]byte, err error) {
return nil, nil, errors.New("my error")
},
},
expData: []byte("myReplyData:1"),
expCommits: []bool{false, false},
},
"multiple msg - last reply can be empty to overwrite": {
msgs: []wasmvmtypes.SubMsg{{ID: 1, ReplyOn: wasmvmtypes.ReplyError}, {ID: 2, ReplyOn: wasmvmtypes.ReplyError}},
replyer: &mockReplyer{
replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
if reply.ID == 2 {
return []byte{}, nil
}
return []byte("myReplyData:1"), nil
},
},
msgHandler: &wasmtesting.MockMessageHandler{
Expand Down Expand Up @@ -225,10 +243,10 @@ func TestDispatchSubmessages(t *testing.T) {
}

type mockReplyer struct {
replyFn func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error)
replyFn func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error)
}

func (m mockReplyer) reply(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) (*sdk.Result, error) {
func (m mockReplyer) reply(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
if m.replyFn == nil {
panic("not expected to be called")
}
Expand Down
9 changes: 4 additions & 5 deletions x/wasm/keeper/msg_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (m msgServer) ExecuteContract(goCtx context.Context, msg *types.MsgExecuteC
return nil, sdkerrors.Wrap(err, "contract")
}

res, err := m.keeper.Execute(ctx, contractAddr, senderAddr, msg.Msg, msg.Funds)
data, err := m.keeper.Execute(ctx, contractAddr, senderAddr, msg.Msg, msg.Funds)
if err != nil {
return nil, err
}
Expand All @@ -98,9 +98,8 @@ func (m msgServer) ExecuteContract(goCtx context.Context, msg *types.MsgExecuteC
))

return &types.MsgExecuteContractResponse{
Data: res.Data,
Data: data,
}, nil

}

func (m msgServer) MigrateContract(goCtx context.Context, msg *types.MsgMigrateContract) (*types.MsgMigrateContractResponse, error) {
Expand All @@ -114,7 +113,7 @@ func (m msgServer) MigrateContract(goCtx context.Context, msg *types.MsgMigrateC
return nil, sdkerrors.Wrap(err, "contract")
}

res, err := m.keeper.Migrate(ctx, contractAddr, senderAddr, msg.CodeID, msg.MigrateMsg)
data, err := m.keeper.Migrate(ctx, contractAddr, senderAddr, msg.CodeID, msg.MigrateMsg)
if err != nil {
return nil, err
}
Expand All @@ -127,7 +126,7 @@ func (m msgServer) MigrateContract(goCtx context.Context, msg *types.MsgMigrateC
))

return &types.MsgMigrateContractResponse{
Data: res.Data,
Data: data,
}, nil
}

Expand Down
10 changes: 1 addition & 9 deletions x/wasm/keeper/proposal_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func handleMigrateProposal(ctx sdk.Context, k types.ContractOpsKeeper, p types.M
if err != nil {
return sdkerrors.Wrap(err, "run as address")
}
res, err := k.Migrate(ctx, contractAddr, runAsAddr, p.CodeID, p.MigrateMsg)
_, err = k.Migrate(ctx, contractAddr, runAsAddr, p.CodeID, p.MigrateMsg)
if err != nil {
return err
}
Expand All @@ -123,14 +123,6 @@ func handleMigrateProposal(ctx sdk.Context, k types.ContractOpsKeeper, p types.M
sdk.NewAttribute(types.AttributeKeyContract, p.Contract),
)
ctx.EventManager().EmitEvent(ourEvent)

for _, e := range res.Events {
attr := make([]sdk.Attribute, len(e.Attributes))
for i, a := range e.Attributes {
attr[i] = sdk.NewAttribute(string(a.Key), string(a.Value))
}
ctx.EventManager().EmitEvent(sdk.NewEvent(e.Type, attr...))
}
return nil
}

Expand Down
8 changes: 5 additions & 3 deletions x/wasm/keeper/test_common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,13 +406,15 @@ func handleExecute(ctx sdk.Context, k types.ContractOpsKeeper, msg *types.MsgExe
if err != nil {
return nil, sdkerrors.Wrap(err, "admin")
}
res, err := k.Execute(ctx, contractAddr, senderAddr, msg.Msg, msg.Funds)
data, err := k.Execute(ctx, contractAddr, senderAddr, msg.Msg, msg.Funds)
if err != nil {
return nil, err
}

res.Events = ctx.EventManager().Events().ToABCIEvents()
return res, nil
return &sdk.Result{
Data: data,
Events: ctx.EventManager().Events().ToABCIEvents(),
}, nil
}

func RandomAccountAddress(_ TestingT) sdk.AccAddress {
Expand Down
4 changes: 2 additions & 2 deletions x/wasm/types/exported_keepers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ type ContractOpsKeeper interface {
Instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.AccAddress, initMsg []byte, label string, deposit sdk.Coins) (sdk.AccAddress, []byte, error)

// Execute executes the contract instance
Execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, msg []byte, coins sdk.Coins) (*sdk.Result, error)
Execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, msg []byte, coins sdk.Coins) ([]byte, error)

// Migrate allows to upgrade a contract to a new code with data migration.
Migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newCodeID uint64, msg []byte) (*sdk.Result, error)
Migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newCodeID uint64, msg []byte) ([]byte, error)

// UpdateContractAdmin sets the admin value on the ContractInfo. It must be a valid address (use ClearContractAdmin to remove it)
UpdateContractAdmin(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, newAdmin sdk.AccAddress) error
Expand Down

0 comments on commit f8e39bf

Please sign in to comment.