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(simulations): marshal OperationMsg.Msg as protoBytes #16155

Merged
merged 14 commits into from
May 15, 2023
Merged
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/gov) [](https://github.com/cosmos/cosmos-sdk/pull/16127) Use collections for deposit management:
- The following methods are removed from the gov keeper: `GetDeposit`, `GetAllDeposits`, `IterateAllDeposits`.
- The following functions are removed from the gov types: `DepositKey`, `DepositsKey`.
* (sims) [#16155](https://github.com/cosmos/cosmos-sdk/pull/16155)
* `simulation.NewOperationMsg` now marshals the operation msg as proto bytes instead of legacy amino JSON bytes.
* Following this, the field `OperationMsg.Msg` is now of type `[]byte` instead of `json.RawMessage`.

### Client Breaking Changes

Expand Down
24 changes: 11 additions & 13 deletions types/simulation/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import (
"math/rand"
"time"

"github.com/cosmos/gogoproto/proto"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/kv"
)
Expand Down Expand Up @@ -70,11 +71,11 @@ type Operation func(r *rand.Rand, app *baseapp.BaseApp,

// OperationMsg - structure for operation output
type OperationMsg struct {
Route string `json:"route" yaml:"route"` // msg route (i.e module name)
Name string `json:"name" yaml:"name"` // operation name (msg Type or "no-operation")
Comment string `json:"comment" yaml:"comment"` // additional comment
OK bool `json:"ok" yaml:"ok"` // success
Msg json.RawMessage `json:"msg" yaml:"msg"` // JSON encoded msg
Route string `json:"route" yaml:"route"` // msg route (i.e module name)
Name string `json:"name" yaml:"name"` // operation name (msg Type or "no-operation")
Comment string `json:"comment" yaml:"comment"` // additional comment
OK bool `json:"ok" yaml:"ok"` // success
Msg []byte `json:"msg" yaml:"msg"` // protobuf encoded msg
}

// NewOperationMsgBasic creates a new operation message from raw input.
Expand All @@ -89,21 +90,18 @@ func NewOperationMsgBasic(moduleName, msgType, comment string, ok bool, msg []by
}

// NewOperationMsg - create a new operation message from sdk.Msg
func NewOperationMsg(msg sdk.Msg, ok bool, comment string, cdc *codec.ProtoCodec) OperationMsg {
func NewOperationMsg(msg sdk.Msg, ok bool, comment string, _ *codec.ProtoCodec) OperationMsg {
Copy link
Member

@julienrbrt julienrbrt May 15, 2023

Choose a reason for hiding this comment

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

I think we should add a changelog entry for chains using this.

Copy link
Member

@julienrbrt julienrbrt May 15, 2023

Choose a reason for hiding this comment

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

Given that this is anyway a behavior change, why not making it API breaking and remove this argument?

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes sense to me.

msgType := sdk.MsgTypeURL(msg)
moduleName := sdk.GetModuleNameFromTypeURL(msgType)
if moduleName == "" {
moduleName = msgType
}
if cdc == nil {
cdc = codec.NewProtoCodec(types.NewInterfaceRegistry())
}
jsonBytes, err := cdc.MarshalAminoJSON(msg)
protoBz, err := proto.Marshal(msg)
if err != nil {
panic(fmt.Errorf("failed to marshal aminon JSON: %w", err))
panic(fmt.Errorf("failed to marshal proto message: %w", err))
}

return NewOperationMsgBasic(moduleName, msgType, comment, ok, jsonBytes)
return NewOperationMsgBasic(moduleName, msgType, comment, ok, protoBz)
}

// NoOpMsg - create a no-operation message
Expand Down
13 changes: 7 additions & 6 deletions x/authz/simulation/operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"cosmossdk.io/log"
abci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/cosmos/gogoproto/proto"
"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -138,7 +139,8 @@ func (suite *SimTestSuite) TestSimulateGrant() {
suite.Require().NoError(err)

var msg authz.MsgGrant
suite.legacyAmino.UnmarshalJSON(operationMsg.Msg, &msg)
err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().True(operationMsg.OK)
suite.Require().Equal(granter.Address.String(), msg.Granter)
suite.Require().Equal(grantee.Address.String(), msg.Grantee)
Expand Down Expand Up @@ -176,8 +178,8 @@ func (suite *SimTestSuite) TestSimulateRevoke() {
suite.Require().NoError(err)

var msg authz.MsgRevoke
suite.legacyAmino.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().True(operationMsg.OK)
suite.Require().Equal(granter.Address.String(), msg.Granter)
suite.Require().Equal(grantee.Address.String(), msg.Grantee)
Expand Down Expand Up @@ -211,9 +213,8 @@ func (suite *SimTestSuite) TestSimulateExec() {
suite.Require().NoError(err)

var msg authz.MsgExec

suite.legacyAmino.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().True(operationMsg.OK)
suite.Require().Equal(grantee.Address.String(), msg.Grantee)
suite.Require().Len(futureOperations, 0)
Expand Down
17 changes: 9 additions & 8 deletions x/bank/simulation/operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"cosmossdk.io/log"
abci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/cosmos/gogoproto/proto"
"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -114,8 +115,8 @@ func (suite *SimTestSuite) TestSimulateMsgSend() {
suite.Require().NoError(err)

var msg types.MsgSend
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().True(operationMsg.OK)
suite.Require().Equal("65337742stake", msg.Amount.String())
suite.Require().Equal("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r", msg.FromAddress)
Expand All @@ -142,8 +143,8 @@ func (suite *SimTestSuite) TestSimulateMsgMultiSend() {
require.NoError(err)

var msg types.MsgMultiSend
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
require.True(operationMsg.OK)
require.Len(msg.Inputs, 1)
require.Equal("cosmos1tnh2q55v8wyygtt9srz5safamzdengsnqeycj3", msg.Inputs[0].Address)
Expand Down Expand Up @@ -178,8 +179,8 @@ func (suite *SimTestSuite) TestSimulateModuleAccountMsgSend() {
suite.Require().Error(err)

var msg types.MsgSend
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().False(operationMsg.OK)
suite.Require().Equal(operationMsg.Comment, "invalid transfers")
suite.Require().Equal(sdk.MsgTypeURL(&types.MsgSend{}), sdk.MsgTypeURL(&msg))
Expand All @@ -206,8 +207,8 @@ func (suite *SimTestSuite) TestSimulateMsgMultiSendToModuleAccount() {
suite.Require().Error(err)

var msg types.MsgMultiSend
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().False(operationMsg.OK) // sending tokens to a module account should fail
suite.Require().Equal(operationMsg.Comment, "invalid transfers")
suite.Require().Equal(sdk.MsgTypeURL(&types.MsgMultiSend{}), sdk.MsgTypeURL(&msg))
Expand Down
17 changes: 9 additions & 8 deletions x/distribution/simulation/operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"cosmossdk.io/math"
abci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/cosmos/gogoproto/proto"
"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -81,8 +82,8 @@ func (suite *SimTestSuite) TestSimulateMsgSetWithdrawAddress() {
suite.Require().NoError(err)

var msg types.MsgSetWithdrawAddress
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().True(operationMsg.OK)
suite.Require().Equal("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r", msg.DelegatorAddress)
suite.Require().Equal("cosmos1p8wcgrjr4pjju90xg6u9cgq55dxwq8j7u4x9a0", msg.WithdrawAddress)
Expand Down Expand Up @@ -121,8 +122,8 @@ func (suite *SimTestSuite) TestSimulateMsgWithdrawDelegatorReward() {
suite.Require().NoError(err)

var msg types.MsgWithdrawDelegatorReward
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().True(operationMsg.OK)
suite.Require().Equal("cosmosvaloper1l4s054098kk9hmr5753c6k3m2kw65h686d3mhr", msg.ValidatorAddress)
suite.Require().Equal("cosmos1d6u7zhjwmsucs678d7qn95uqajd4ucl9jcjt26", msg.DelegatorAddress)
Expand Down Expand Up @@ -181,8 +182,8 @@ func (suite *SimTestSuite) testSimulateMsgWithdrawValidatorCommission(tokenName
suite.Require().NoError(err)

var msg types.MsgWithdrawValidatorCommission
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().True(operationMsg.OK)
suite.Require().Equal("cosmosvaloper1tnh2q55v8wyygtt9srz5safamzdengsn9dsd7z", msg.ValidatorAddress)
suite.Require().Equal(sdk.MsgTypeURL(&types.MsgWithdrawValidatorCommission{}), sdk.MsgTypeURL(&msg))
Expand All @@ -207,8 +208,8 @@ func (suite *SimTestSuite) TestSimulateMsgFundCommunityPool() {
suite.Require().NoError(err)

var msg types.MsgFundCommunityPool
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

err = proto.Unmarshal(operationMsg.Msg, &msg)
suite.Require().NoError(err)
suite.Require().True(operationMsg.OK)
suite.Require().Equal("4896096stake", msg.Amount.String())
suite.Require().Equal("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r", msg.Depositor)
Expand Down
Loading