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

refactor(baseapp,types)!: msg responses #17348

Merged
merged 10 commits into from
Aug 18, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/genutil) [#17098](https://github.com/cosmos/cosmos-sdk/pull/17098) `GenAppStateFromConfig`, AddGenesisAccountCmd and `GenTxCmd` takes an addresscodec to decode addresses
* (x/distribution) [#17098](https://github.com/cosmos/cosmos-sdk/pull/17098) `NewMsgDepositValidatorRewardsPool`, `NewMsgFundCommunityPool`, `NewMsgWithdrawValidatorCommission` and `NewMsgWithdrawDelegatorReward` takes a string instead of `sdk.ValAddress` or `sdk.AccAddress`
* (client) [#17215](https://github.com/cosmos/cosmos-sdk/pull/17215) `server.StartCmd`,`server.ExportCmd`,`server.NewRollbackCmd`,`pruning.Cmd`,`genutilcli.InitCmd`,`genutilcli.GenTxCmd`,`genutilcli.CollectGenTxsCmd`,`genutilcli.AddGenesisAccountCmd`, do not take a home directory anymore. It is inferred from the root command.
* (types) [#17348](https://github.com/cosmos/cosmos-sdk/pull/17348) Remove the `WrapServiceResult` function.
Copy link
Member

@julienrbrt julienrbrt Aug 16, 2023

Choose a reason for hiding this comment

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

Can we explain the behavior change as well, if someone is calling a msg the msg server router directly and expects the .Data (even if it is deprecated) field to be filled that won't be there.

* (x/staking) [#17157](https://github.com/cosmos/cosmos-sdk/pull/17157) `GetValidatorsByPowerIndexKey` and `ValidateBasic` for historical info takes a validator address codec in order to be able to decode/encode addresses.
* `GetOperator()` now returns the address as it is represented in state, by default this is an encoded address
* `GetConsAddr() ([]byte, error)` returns `[]byte` instead of sdk.ConsAddres.
Expand Down
8 changes: 3 additions & 5 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, res
// Result is returned. The caller must not commit state if an error is returned.
func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, msgsV2 []protov2.Message, mode execMode) (*sdk.Result, error) {
events := sdk.EmptyEvents()
var msgResponses []*codectypes.Any
msgResponses := make([]*codectypes.Any, 0, len(msgs))

// NOTE: GasWanted is determined by the AnteHandler and GasUsed by the GasMeter.
for i, msg := range msgs {
Expand Down Expand Up @@ -997,17 +997,15 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, msgsV2 []protov2.Me

// Each individual sdk.Result that went through the MsgServiceRouter
// (which should represent 99% of the Msgs now, since everyone should
// be using protobuf Msgs) has exactly one Msg response, set inside
// `WrapServiceResult`. We take that Msg response, and aggregate it
// into an array.
// be using protobuf Msgs) has exactly one Msg response.
// We take that Msg response, and aggregate it into an array.
if len(msgResult.MsgResponses) > 0 {
msgResponse := msgResult.MsgResponses[0]
if msgResponse == nil {
return nil, sdkerrors.ErrLogic.Wrapf("got nil Msg response at index %d for msg %s", i, sdk.MsgTypeURL(msg))
}
msgResponses = append(msgResponses, msgResponse)
}

}

data, err := makeABCIData(msgResponses)
Expand Down
16 changes: 15 additions & 1 deletion baseapp/msg_service_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

abci "github.com/cometbft/cometbft/abci/types"
gogogrpc "github.com/cosmos/gogoproto/grpc"
"github.com/cosmos/gogoproto/proto"
"google.golang.org/grpc"
Expand Down Expand Up @@ -157,7 +158,20 @@ func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler inter
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "Expecting proto.Message, got %T", resMsg)
}

return sdk.WrapServiceResult(ctx, resMsg, err)
anyResp, err := codectypes.NewAnyWithValue(resMsg)
if err != nil {
return nil, err
}

var events []abci.Event
if evtMgr := ctx.EventManager(); evtMgr != nil {
events = evtMgr.ABCIEvents()
}

return &sdk.Result{
Events: events,
MsgResponses: []*codectypes.Any{anyResp},
}, nil
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/building-modules/03-msg-services.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ ctx.EventManager().EmitEvent(

These events are relayed back to the underlying consensus engine and can be used by service providers to implement services around the application. Click [here](../core/08-events.md) to learn more about events.

The invoked `msgServer` method returns a `proto.Message` response and an `error`. These return values are then wrapped into an `*sdk.Result` or an `error` using `sdk.WrapServiceResult(ctx context.Context, res proto.Message, err error)`:
The invoked `msgServer` method returns a `proto.Message` response and an `error`. These return values are then wrapped into an `*sdk.Result` or an `error`:

```go reference
https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/baseapp/msg_service_router.go#L160
Copy link
Member

Choose a reason for hiding this comment

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

The snippet here has to change in a follow-up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't understand what you mean? Do you need to revert the changes here?

Expand Down
35 changes: 0 additions & 35 deletions types/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ import (
"encoding/json"
"strings"

abci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
coretypes "github.com/cometbft/cometbft/rpc/core/types"
"github.com/cosmos/gogoproto/proto"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
Expand Down Expand Up @@ -195,39 +193,6 @@ func (r TxResponse) GetTx() HasMsgs {
return nil
}

// WrapServiceResult wraps a result from a protobuf RPC service method call (res proto.Message, err error)
// in a Result object or error. This method takes care of marshaling the res param to
// protobuf and attaching any events on the ctx.EventManager() to the Result.
func WrapServiceResult(ctx Context, res proto.Message, err error) (*Result, error) {
zakir-code marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}

any, err := codectypes.NewAnyWithValue(res)
if err != nil {
return nil, err
}

var data []byte
if res != nil {
data, err = proto.Marshal(res)
if err != nil {
return nil, err
}
}

var events []abci.Event
if evtMgr := ctx.EventManager(); evtMgr != nil {
events = evtMgr.ABCIEvents()
}

return &Result{
Data: data,
Events: events,
MsgResponses: []*codectypes.Any{any},
}, nil
}

// calculate total pages in an overflow safe manner
func calcTotalPages(totalCount, limit int64) int64 {
totalPages := int64(0)
Expand Down
34 changes: 0 additions & 34 deletions types/result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package types_test

import (
"encoding/hex"
"fmt"
"strings"
"testing"
"time"
Expand All @@ -11,12 +10,9 @@ import (
cmtt "github.com/cometbft/cometbft/proto/tendermint/types"
coretypes "github.com/cometbft/cometbft/rpc/core/types"
cmt "github.com/cometbft/cometbft/types"
"github.com/golang/protobuf/proto" //nolint:staticcheck // grpc-gateway uses deprecated golang/protobuf
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
)

Expand Down Expand Up @@ -178,33 +174,3 @@ func (s *resultTestSuite) TestResponseResultBlock() {

s.Require().Equal(want, sdk.NewResponseResultBlock(resultBlock, timestampStr))
}

func TestWrapServiceResult(t *testing.T) {
ctx := sdk.Context{}

res, err := sdk.WrapServiceResult(ctx, nil, fmt.Errorf("test"))
require.Nil(t, res)
require.NotNil(t, err)

res, err = sdk.WrapServiceResult(ctx, &testdata.Dog{}, nil)
require.NotNil(t, res)
require.Nil(t, err)
require.Empty(t, res.Events)

ctx = ctx.WithEventManager(sdk.NewEventManager())
ctx.EventManager().EmitEvent(sdk.NewEvent("test"))
res, err = sdk.WrapServiceResult(ctx, &testdata.Dog{}, nil)
require.NotNil(t, res)
require.Nil(t, err)
require.Len(t, res.Events, 1)

spot := testdata.Dog{Name: "spot"}
res, err = sdk.WrapServiceResult(ctx, &spot, nil)
require.NotNil(t, res)
require.Nil(t, err)
require.Len(t, res.Events, 1)
var spot2 testdata.Dog
err = proto.Unmarshal(res.Data, &spot2)
require.NoError(t, err)
require.Equal(t, spot, spot2)
}