refactor!: rm GetSignBytes (#16062)

CHANGELOG.md

@@ -245,6 +245,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
     * Removed: keeper `GetConstantFee`, `SetConstantFee` 
 * (x/mint) [#16329](https://github.com/cosmos/cosmos-sdk/pull/16329) Use collections for state management:
   * Removed: keeper `GetParams`, `SetParams`, `GetMinter`, `SetMinter`.
+* (x/*all*) [#16052](https://github.com/cosmos/cosmos-sdk/pull/16062) `GetSignBytes` implementations on messages and global legacy amino codec definitions have been removed from all modules.
+* (sims) [#16052](https://github.com/cosmos/cosmos-sdk/pull/16062) `GetOrGenerate` no longer requires a codec argument is now 4-arity instead of 5-arity.
 
 ### Client Breaking Changes
 

UPGRADING.md

@@ -53,7 +53,11 @@ ClevelDB, BoltDB and BadgerDB are not supported anymore. To migrate from a unsup
 
 ### Protobuf
 
-The SDK is in the process of removing all `gogoproto` annotations.
+With the deprecation of the amino JSON codec defined in [cosmos/gogoproto](https://github.com/cosmos/gogoproto) in favor of the protoreflect powered x/tx/aminojson codec, module developers are encouraged verify that their messages have the correct protobuf annotations to deterministically produce identical output from both codecs.
+
+For core SDK types equivalence is asserted by generative testing of [SignableTypes](https://github.com/cosmos/cosmos-sdk/blob/76f0d101530ed78befc95506ab473c771d0d8a8c/tests/integration/rapidgen/rapidgen.go#L106) in [TestAminoJSON_Equivalence](https://github.com/cosmos/cosmos-sdk/blob/76f0d101530ed78befc95506ab473c771d0d8a8c/tests/integration/aminojson/aminojson_test.go#L90).
+
+TODO: summarize proto annotation requirements.
 
 #### Stringer
 
@@ -181,6 +185,8 @@ The return type of the interface method `TxConfig.SignModeHandler()` has been ch
 The `sdk.Msg` interface has been updated to not require the implementation of the `ValidateBasic` method.
 It is now recommended to validate message directly in the message server. When the validation is performed in the message server, the `ValidateBasic` method on a message is no longer required and can be removed.
 
+Messages no longer need to implement the `LegacyMsg` interface and implementations of `GetSignBytes` can be deleted.  Because of this change, global legacy Amino codec definitions and their registration in `init()` can safely be removed as well.  
+
 #### `x/auth`
 
 For ante handler construction via `ante.NewAnteHandler`, the field `ante.HandlerOptions.SignModeHandler` has been updated to `x/tx/signing/HandlerMap` from `x/auth/signing/SignModeHandler`.  Callers typically fetch this value from `client.TxConfig.SignModeHandler()` (which is also changed) so this change should be transparent to most users.

codec/proto_codec.go

@@ -2,6 +2,7 @@ package codec
 
 import (
 	"encoding/binary"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"strings"
@@ -17,6 +18,7 @@ import (
 	"google.golang.org/protobuf/types/known/anypb"
 
 	"cosmossdk.io/x/tx/signing/aminojson"
+
 	"github.com/cosmos/cosmos-sdk/codec/types"
 )
 
@@ -196,7 +198,23 @@ func (pc *ProtoCodec) MarshalAminoJSON(msg gogoproto.Message) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	return jsonBytes, nil
+	// TODO: remove this sort once https://github.com/cosmos/cosmos-sdk/issues/2350#issuecomment-1542715157 lands
+	// the encoder should be rendering in lexical order
+	return sortJSON(jsonBytes)
+}
+
+// sortJSON sorts the JSON keys of the given JSON encoded byte slice.
+func sortJSON(toSortJSON []byte) ([]byte, error) {
+	var c interface{}
+	err := json.Unmarshal(toSortJSON, &c)
+	if err != nil {
+		return nil, err
+	}
+	js, err := json.Marshal(c)
+	if err != nil {
+		return nil, err
+	}
+	return js, nil
 }
 
 // UnmarshalJSON implements JSONCodec.UnmarshalJSON method,

core/coins/format.go

@@ -67,7 +67,7 @@ func formatCoin(coin *basev1beta1.Coin, metadata *bankv1beta1.Metadata) (string,
 	return vr + " " + dispDenom, err
 }
 
-// formatCoins formats Coins into a value-rendered string, which uses
+// FormatCoins formats Coins into a value-rendered string, which uses
 // `formatCoin` separated by ", " (a comma and a space), and sorted
 // alphabetically by value-rendered denoms. It expects an array of metadata
 // (optionally nil), where each metadata at index `i` MUST match the coin denom

go.mod

@@ -159,6 +159,8 @@ require (
 
 // Below are the short-lived replace of the Cosmos SDK
 replace (
+	// TODO remove after cosmossdk.io/api release
+	cosmossdk.io/api => ./api
 	// TODO: remove me after collections 0.2. is released.
 	cosmossdk.io/collections => ./collections
 	cosmossdk.io/core => ./core

go.sum

@@ -35,8 +35,6 @@ cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohl
 cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs=
 cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0=
 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo=
-cosmossdk.io/api v0.4.1 h1:0ikaYM6GyxTYYcfBiyR8YnLCfhNnhKpEFnaSepCTmqg=
-cosmossdk.io/api v0.4.1/go.mod h1:jR7k5ok90LxW2lFUXvd8Vpo/dr4PpiyVegxdm7b1ZdE=
 cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw=
 cosmossdk.io/depinject v1.0.0-alpha.3/go.mod h1:eRbcdQ7MRpIPEM5YUJh8k97nxHpYbc3sMUnEtt8HPWU=
 cosmossdk.io/errors v1.0.0-beta.7.0.20230524212735-6cabb6aa5741 h1:BCRz06fvddw7cKGiEGDiSox3qMsjQ97f92K+PDZDHdc=

proto/cosmos/gov/v1/tx.proto

@@ -55,7 +55,12 @@ message MsgSubmitProposal {
   repeated google.protobuf.Any messages = 1;
 
   // initial_deposit is the deposit value that must be paid at proposal submission.
-  repeated cosmos.base.v1beta1.Coin initial_deposit = 2 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true];
+  repeated cosmos.base.v1beta1.Coin initial_deposit = 2 [
+    (gogoproto.nullable) = false,
+    (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins",
+    (amino.dont_omitempty) = true,
+    (amino.encoding) = "legacy_coins"
+  ];
 
   // proposer is the account address of the proposer.
   string proposer = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];

proto/cosmos/tx/v1beta1/tx.proto

@@ -1,6 +1,7 @@
 syntax = "proto3";
 package cosmos.tx.v1beta1;
 
+import "amino/amino.proto";
 import "gogoproto/gogo.proto";
 import "cosmos/crypto/multisig/v1beta1/multisig.proto";
 import "cosmos/base/v1beta1/coin.proto";
@@ -205,8 +206,12 @@ message ModeInfo {
 // which must be above some miminum to be accepted into the mempool.
 message Fee {
   // amount is the amount of coins to be paid as a fee
-  repeated cosmos.base.v1beta1.Coin amount = 1
-      [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
+  repeated cosmos.base.v1beta1.Coin amount = 1 [
+    (gogoproto.nullable) = false,
+    (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins",
+    (amino.dont_omitempty) = true,
+    (amino.encoding) = "legacy_coins"
+  ];
 
   // gas_limit is the maximum gas that can be used in transaction processing
   // before an out of gas error occurs
@@ -228,8 +233,12 @@ message Fee {
 // Since: cosmos-sdk 0.46
 message Tip {
   // amount is the amount of the tip
-  repeated cosmos.base.v1beta1.Coin amount = 1
-      [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
+  repeated cosmos.base.v1beta1.Coin amount = 1 [
+    (gogoproto.nullable) = false,
+    (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins",
+    (amino.dont_omitempty) = true,
+    (amino.encoding) = "legacy_coins"
+  ];
   // tipper is the address of the account paying for the tip
   string tipper = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 }

tests/integration/aminojson/aminojson_test.go

@@ -7,6 +7,8 @@ import (
 	"testing"
 	"time"
 
+	gov_v1_api "cosmossdk.io/api/cosmos/gov/v1"
+	msgv1 "cosmossdk.io/api/cosmos/msg/v1"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/anypb"
@@ -36,6 +38,7 @@ import (
 	"cosmossdk.io/x/tx/signing/aminojson"
 	signing_testutil "cosmossdk.io/x/tx/signing/testutil"
 	"cosmossdk.io/x/upgrade"
+
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	ed25519types "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
@@ -64,6 +67,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/distribution"
 	disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
 	"github.com/cosmos/cosmos-sdk/x/gov"
+	gov_v1_types "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
 	gov_v1beta1_types "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
 	groupmodule "github.com/cosmos/cosmos-sdk/x/group/module"
 	"github.com/cosmos/cosmos-sdk/x/mint"
@@ -94,10 +98,12 @@ func TestAminoJSON_Equivalence(t *testing.T) {
 		distribution.AppModuleBasic{}, evidence.AppModuleBasic{}, feegrantmodule.AppModuleBasic{},
 		gov.AppModuleBasic{}, groupmodule.AppModuleBasic{}, mint.AppModuleBasic{}, params.AppModuleBasic{},
 		slashing.AppModuleBasic{}, staking.AppModuleBasic{}, upgrade.AppModuleBasic{}, vesting.AppModuleBasic{})
+	legacytx.RegressionTestingAminoCodec = encCfg.Amino
 	aj := aminojson.NewEncoder(aminojson.EncoderOptions{})
 
 	for _, tt := range rapidgen.DefaultGeneratedTypes {
-		name := string(tt.Pulsar.ProtoReflect().Descriptor().FullName())
+		desc := tt.Pulsar.ProtoReflect().Descriptor()
+		name := string(desc.FullName())
 		t.Run(name, func(t *testing.T) {
 			gen := rapidproto.MessageGenerator(tt.Pulsar, tt.Opts)
 			fmt.Printf("testing %s\n", tt.Pulsar.ProtoReflect().Descriptor().FullName())
@@ -132,8 +138,7 @@ func TestAminoJSON_Equivalence(t *testing.T) {
 				require.Equal(t, string(legacyAminoJSON), string(aminoJSON))
 
 				// test amino json signer handler equivalence
-				gogoMsg, ok := gogo.(legacytx.LegacyMsg)
-				if !ok {
+				if !proto.HasExtension(desc.Options(), msgv1.E_Signer) {
 					// not signable
 					return
 				}
@@ -163,7 +168,7 @@ func TestAminoJSON_Equivalence(t *testing.T) {
 
 				legacyHandler := tx.NewSignModeLegacyAminoJSONHandler()
 				txBuilder := encCfg.TxConfig.NewTxBuilder()
-				require.NoError(t, txBuilder.SetMsgs([]types.Msg{gogoMsg}...))
+				require.NoError(t, txBuilder.SetMsgs([]types.Msg{tt.Gogo}...))
 				txBuilder.SetMemo(handlerOptions.Memo)
 				txBuilder.SetFeeAmount(types.Coins{types.NewInt64Coin("uatom", 1000)})
 				txBuilder.SetTip(&txtypes.Tip{
@@ -201,7 +206,8 @@ func newAny(t *testing.T, msg proto.Message) *anypb.Any {
 func TestAminoJSON_LegacyParity(t *testing.T) {
 	encCfg := testutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, authzmodule.AppModuleBasic{},
 		bank.AppModuleBasic{}, distribution.AppModuleBasic{}, slashing.AppModuleBasic{}, staking.AppModuleBasic{},
-		vesting.AppModuleBasic{})
+		vesting.AppModuleBasic{}, gov.AppModuleBasic{})
+	legacytx.RegressionTestingAminoCodec = encCfg.Amino
 
 	aj := aminojson.NewEncoder(aminojson.EncoderOptions{})
 	addr1 := types.AccAddress("addr1")
@@ -332,6 +338,10 @@ func TestAminoJSON_LegacyParity(t *testing.T) {
 			gogo:   &banktypes.MsgMultiSend{},
 			pulsar: &bankapi.MsgMultiSend{},
 		},
+		"gov/v1_msg_submit_proposal": {
+			gogo:   &gov_v1_types.MsgSubmitProposal{},
+			pulsar: &gov_v1_api.MsgSubmitProposal{},
+		},
 		"slashing/params/empty_dec": {
 			gogo:   &slashingtypes.Params{DowntimeJailDuration: 1e9 + 7},
 			pulsar: &slashingapi.Params{DowntimeJailDuration: &durationpb.Duration{Seconds: 1, Nanos: 7}},

tests/integration/tx/decode_test.go

@@ -3,6 +3,7 @@ package tx
 import (
 	"testing"
 
+	msgv1 "cosmossdk.io/api/cosmos/msg/v1"
 	"github.com/cosmos/cosmos-proto/rapidproto"
 	gogoproto "github.com/cosmos/gogoproto/proto"
 	"github.com/stretchr/testify/require"
@@ -14,6 +15,7 @@ import (
 	"cosmossdk.io/x/tx/decode"
 	txsigning "cosmossdk.io/x/tx/signing"
 	"cosmossdk.io/x/upgrade"
+
 	"github.com/cosmos/cosmos-sdk/codec/legacy"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/tests/integration/rapidgen"
@@ -44,6 +46,7 @@ func TestDecode(t *testing.T) {
 		distribution.AppModuleBasic{}, evidence.AppModuleBasic{}, feegrantmodule.AppModuleBasic{},
 		gov.AppModuleBasic{}, groupmodule.AppModuleBasic{}, mint.AppModuleBasic{}, params.AppModuleBasic{},
 		slashing.AppModuleBasic{}, staking.AppModuleBasic{}, upgrade.AppModuleBasic{}, vesting.AppModuleBasic{})
+	legacytx.RegressionTestingAminoCodec = encCfg.Amino
 
 	fee := sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(100)))
 	gas := uint64(200)
@@ -74,7 +77,8 @@ func TestDecode(t *testing.T) {
 	require.NoError(t, err)
 
 	for _, tt := range rapidgen.SignableTypes {
-		name := string(tt.Pulsar.ProtoReflect().Descriptor().FullName())
+		desc := tt.Pulsar.ProtoReflect().Descriptor()
+		name := string(desc.FullName())
 		t.Run(name, func(t *testing.T) {
 			gen := rapidproto.MessageGenerator(tt.Pulsar, tt.Opts)
 			rapid.Check(t, func(t *rapid.T) {
@@ -102,10 +106,9 @@ func TestDecode(t *testing.T) {
 					Sequence: accSeq,
 				}
 
-				gogoMsg, ok := gogo.(legacytx.LegacyMsg)
-				require.True(t, ok)
+				require.True(t, proto.HasExtension(desc.Options(), msgv1.E_Signer))
 
-				err = txBuilder.SetMsgs(gogoMsg)
+				err = txBuilder.SetMsgs(tt.Gogo)
 				require.NoError(t, err)
 				txBuilder.SetFeeAmount(fee)
 				txBuilder.SetGasLimit(gas)
@@ -129,7 +132,7 @@ func TestDecode(t *testing.T) {
 
 				require.Equal(t, authInfoBytes, decodedTx.TxRaw.AuthInfoBytes)
 
-				anyGogoMsg, err := codectypes.NewAnyWithValue(gogoMsg)
+				anyGogoMsg, err := codectypes.NewAnyWithValue(tt.Gogo)
 				require.NoError(t, err)
 
 				txBody := &txtypes.TxBody{
@@ -145,6 +148,8 @@ func TestDecode(t *testing.T) {
 			})
 		})
 	}
+
+	legacytx.RegressionTestingAminoCodec = nil
 }
 
 type dummyAddressCodec struct{}

testutil/sims/state_helpers.go

@@ -204,11 +204,11 @@ func AppStateRandomizedFn(
 		initialStake       math.Int
 	)
 	appParams.GetOrGenerate(
-		cdc, StakePerAccount, &initialStake, r,
+		StakePerAccount, &initialStake, r,
 		func(r *rand.Rand) { initialStake = math.NewInt(r.Int63n(1e12)) },
 	)
 	appParams.GetOrGenerate(
-		cdc, InitiallyBondedValidators, &numInitiallyBonded, r,
+		InitiallyBondedValidators, &numInitiallyBonded, r,
 		func(r *rand.Rand) { numInitiallyBonded = int64(r.Intn(300)) },
 	)
 

testutil/testdata/tx.go

@@ -1,7 +1,6 @@
 package testdata
 
 import (
-	"encoding/json"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -78,14 +77,6 @@ func NewTestMsg(addrs ...sdk.AccAddress) *TestMsg {
 
 var _ sdk.Msg = (*TestMsg)(nil)
 
-func (msg *TestMsg) GetSignBytes() []byte {
-	bz, err := json.Marshal(msg.Signers)
-	if err != nil {
-		panic(err)
-	}
-	return sdk.MustSortJSON(bz)
-}
-
 func (msg *TestMsg) GetSigners() []sdk.AccAddress {
 	signers := make([]sdk.AccAddress, 0, len(msg.Signers))
 	for _, addr := range msg.Signers {

types/result.go

@@ -15,8 +15,6 @@ import (
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 )
 
-var cdc = codec.NewLegacyAmino()
-
 func (gi GasInfo) String() string {
 	bz, _ := codec.MarshalYAML(codec.NewProtoCodec(nil), &gi)
 	return string(bz)
@@ -50,6 +48,7 @@ func NewABCIMessageLog(i uint32, log string, events Events) ABCIMessageLog {
 // String implements the fmt.Stringer interface for the ABCIMessageLogs type.
 func (logs ABCIMessageLogs) String() (str string) {
 	if logs != nil {
+		cdc := codec.NewLegacyAmino()
 		raw, err := cdc.MarshalJSON(logs)
 		if err == nil {
 			str = string(raw)

types/simulation/types.go

@@ -9,7 +9,6 @@ import (
 	"github.com/cosmos/gogoproto/proto"
 
 	"github.com/cosmos/cosmos-sdk/baseapp"
-	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/kv"
 )
@@ -158,7 +157,7 @@ type AppParams map[string]json.RawMessage
 // object. If it exists, it'll be decoded and returned. Otherwise, the provided
 // ParamSimulator is used to generate a random value or default value (eg: in the
 // case of operation weights where Rand is not used).
-func (sp AppParams) GetOrGenerate(_ codec.JSONCodec, key string, ptr interface{}, r *rand.Rand, ps ParamSimulator) {
+func (sp AppParams) GetOrGenerate(key string, ptr interface{}, r *rand.Rand, ps ParamSimulator) {
 	if v, ok := sp[key]; ok && v != nil {
 		err := json.Unmarshal(v, ptr)
 		if err != nil {