feat(x/tx): legacy amino json sign mode handler

scripts/protocgen-pulsar.sh

@@ -17,3 +17,6 @@ echo "Generating API module"
 
 echo "Generate Pulsar Test Data"
 (cd testutil/testdata; buf generate --template buf.gen.pulsar.yaml)
+
+echo "Generate x/tx"
+(cd x/tx; make codegen)

tests/go.mod

@@ -195,6 +195,7 @@ replace (
 	cosmossdk.io/x/evidence => ../x/evidence
 	cosmossdk.io/x/feegrant => ../x/feegrant
 	cosmossdk.io/x/nft => ../x/nft
+	cosmossdk.io/x/tx => ../x/tx
 	cosmossdk.io/x/upgrade => ../x/upgrade
 )
 

tests/go.sum

@@ -204,8 +204,6 @@ cosmossdk.io/log v1.0.0 h1:NGKZ/A5rd4PduDfoscgABklX557PWjQINbosZy/m3Jk=
 cosmossdk.io/log v1.0.0/go.mod h1:CwX9BLiBruZb7lzLlRr3R231d/fVPUXk8gAdV4LQap0=
 cosmossdk.io/math v1.0.0 h1:ro9w7eKx23om2tZz/VM2Pf+z2WAbGX1yDQQOJ6iGeJw=
 cosmossdk.io/math v1.0.0/go.mod h1:Ygz4wBHrgc7g0N+8+MrnTfS9LLn9aaTGa9hKopuym5k=
-cosmossdk.io/x/tx v0.5.0 h1:01wPSoiYDHlfudV0fn867SBXI3uI/8tpatBgVVSnFzI=
-cosmossdk.io/x/tx v0.5.0/go.mod h1:kDcwrN6QbCj+9NXVHL8qipMzA9YlMr9NpZa08SHCdLA=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
 filippo.io/edwards25519 v1.0.0 h1:0wAIcmJUqRdI8IJ/3eGi5/HwXZWPujYXXlkrQogz0Ek=
 filippo.io/edwards25519 v1.0.0/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns=

tests/integration/aminojson/aminojson_test.go

@@ -1,6 +1,7 @@
 package aminojson
 
 import (
+	"context"
 	"fmt"
 	"reflect"
 	"testing"
@@ -38,13 +39,15 @@ import (
 	paramsapi "cosmossdk.io/api/cosmos/params/v1beta1"
 	slashingapi "cosmossdk.io/api/cosmos/slashing/v1beta1"
 	stakingapi "cosmossdk.io/api/cosmos/staking/v1beta1"
+	txv1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1"
 	upgradeapi "cosmossdk.io/api/cosmos/upgrade/v1beta1"
 	vestingapi "cosmossdk.io/api/cosmos/vesting/v1beta1"
 	"cosmossdk.io/x/evidence"
 	evidencetypes "cosmossdk.io/x/evidence/types"
 	feegranttypes "cosmossdk.io/x/feegrant"
 	feegrantmodule "cosmossdk.io/x/feegrant/module"
 	"cosmossdk.io/x/tx/signing/aminojson"
+	signing_testutil "cosmossdk.io/x/tx/signing/testutil"
 	"cosmossdk.io/x/upgrade"
 	upgradetypes "cosmossdk.io/x/upgrade/types"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
@@ -57,7 +60,10 @@ import (
 	"github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/bech32"
 	"github.com/cosmos/cosmos-sdk/types/module/testutil"
+	signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
 	"github.com/cosmos/cosmos-sdk/x/auth"
+	"github.com/cosmos/cosmos-sdk/x/auth/signing"
+	"github.com/cosmos/cosmos-sdk/x/auth/tx"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/cosmos/cosmos-sdk/x/auth/vesting"
 	vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
@@ -367,6 +373,50 @@ func TestAminoJSON_Equivalence(t *testing.T) {
 				aminoJSON, err := aj.Marshal(msg)
 				require.NoError(t, err)
 				require.Equal(t, string(legacyAminoJSON), string(aminoJSON))
+
+				// test amino json signer handler equivalence
+				gogoMsg, ok := gogo.(types.Msg)
+				if !ok {
+					// not signable
+					return
+				}
+
+				handlerOptions := signing_testutil.HandlerArgumentOptions{
+					ChainId:       "test-chain",
+					Memo:          "sometestmemo",
+					Msg:           tt.pulsar,
+					AccNum:        1,
+					AccSeq:        2,
+					SignerAddress: "signerAddress",
+					Fee: &txv1beta1.Fee{
+						Amount: []*v1beta1.Coin{{Denom: "uatom", Amount: "1000"}},
+					},
+				}
+
+				signerData, txData, err := signing_testutil.MakeHandlerArguments(handlerOptions)
+				require.NoError(t, err)
+
+				handler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{})
+				signBz, err := handler.GetSignBytes(context.Background(), signerData, txData)
+				require.NoError(t, err)
+
+				legacyHandler := tx.NewSignModeLegacyAminoJSONHandler()
+				txBuilder := encCfg.TxConfig.NewTxBuilder()
+				require.NoError(t, txBuilder.SetMsgs([]types.Msg{gogoMsg}...))
+				txBuilder.SetMemo(handlerOptions.Memo)
+				txBuilder.SetFeeAmount(types.Coins{types.NewInt64Coin("uatom", 1000)})
+				theTx := txBuilder.GetTx()
+
+				legacySigningData := signing.SignerData{
+					ChainID:       handlerOptions.ChainId,
+					Address:       handlerOptions.SignerAddress,
+					AccountNumber: handlerOptions.AccNum,
+					Sequence:      handlerOptions.AccSeq,
+				}
+				legacySignBz, err := legacyHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON,
+					legacySigningData, theTx)
+				require.NoError(t, err)
+				require.Equal(t, string(legacySignBz), string(signBz))
 			})
 		})
 	}
@@ -624,6 +674,50 @@ func TestAminoJSON_LegacyParity(t *testing.T) {
 				return
 			}
 			require.Equal(t, string(gogoBytes), string(newGogoBytes))
+
+			// test amino json signer handler equivalence
+			msg, ok := tc.gogo.(types.Msg)
+			if !ok {
+				// not signable
+				return
+			}
+
+			handlerOptions := signing_testutil.HandlerArgumentOptions{
+				ChainId:       "test-chain",
+				Memo:          "sometestmemo",
+				Msg:           tc.pulsar,
+				AccNum:        1,
+				AccSeq:        2,
+				SignerAddress: "signerAddress",
+				Fee: &txv1beta1.Fee{
+					Amount: []*v1beta1.Coin{{Denom: "uatom", Amount: "1000"}},
+				},
+			}
+
+			signerData, txData, err := signing_testutil.MakeHandlerArguments(handlerOptions)
+			require.NoError(t, err)
+
+			handler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{})
+			signBz, err := handler.GetSignBytes(context.Background(), signerData, txData)
+			require.NoError(t, err)
+
+			legacyHandler := tx.NewSignModeLegacyAminoJSONHandler()
+			txBuilder := encCfg.TxConfig.NewTxBuilder()
+			require.NoError(t, txBuilder.SetMsgs([]types.Msg{msg}...))
+			txBuilder.SetMemo(handlerOptions.Memo)
+			txBuilder.SetFeeAmount(types.Coins{types.NewInt64Coin("uatom", 1000)})
+			theTx := txBuilder.GetTx()
+
+			legacySigningData := signing.SignerData{
+				ChainID:       handlerOptions.ChainId,
+				Address:       handlerOptions.SignerAddress,
+				AccountNumber: handlerOptions.AccNum,
+				Sequence:      handlerOptions.AccSeq,
+			}
+			legacySignBz, err := legacyHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON,
+				legacySigningData, theTx)
+			require.NoError(t, err)
+			require.Equal(t, string(legacySignBz), string(signBz))
 		})
 	}
 }
@@ -687,14 +781,14 @@ func TestDecimalMutation(t *testing.T) {
 	require.NoError(t, err)
 	rateBz, _ = encCfg.Amino.MarshalJSON(rates)
 
-	// these assertions show behavior prior to the merge of https://github.com/cosmos/cosmos-sdk/pull/15506
-	// and should be updated to reflect the new behavior once a release of math is made and updated in ./tests/go.mod
-	// require.NotEqual(t, `{"rate":"0","max_rate":"0","max_change_rate":"0"}`, string(rateBz))
-	// require.Equal(t,
-	// 	`{"rate":"0.000000000000000000","max_rate":"0.000000000000000000","max_change_rate":"0.000000000000000000"}`,
-	// 	string(rateBz))
+	// prior to the merge of https://github.com/cosmos/cosmos-sdk/pull/15506
+	// gogoproto.Marshal would mutate Decimal fields changing JSON output as shown in the assertions below
+	//require.NotEqual(t, `{"rate":"0","max_rate":"0","max_change_rate":"0"}`, string(rateBz))
+	//require.Equal(t,
+	//	`{"rate":"0.000000000000000000","max_rate":"0.000000000000000000","max_change_rate":"0.000000000000000000"}`,
+	//	string(rateBz))
 
-	// new behavior
+	// This is no longer the case, new behavior:
 	require.Equal(t, `{"rate":"0","max_rate":"0","max_change_rate":"0"}`, string(rateBz))
 }
 

x/auth/tx/legacy_amino_json.go

@@ -20,6 +20,12 @@ var _ signing.SignModeHandler = signModeLegacyAminoJSONHandler{}
 // SignModeHandler.
 type signModeLegacyAminoJSONHandler struct{}
 
+// NewSignModeLegacyAminoJSONHandler returns a new signModeLegacyAminoJSONHandler.
+// Note: The public constructor is only used for testing.
+func NewSignModeLegacyAminoJSONHandler() signing.SignModeHandler {
+	return signModeLegacyAminoJSONHandler{}
+}
+
 func (s signModeLegacyAminoJSONHandler) DefaultMode() signingtypes.SignMode {
 	return signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON
 }

x/tx/CHANGELOG.md

@@ -48,3 +48,4 @@ require a `signing.ProtoFileResolver` interface instead of `protodesc.Resolver`.
 
 * [#15302](https://github.com/cosmos/cosmos-sdk/pull/15302) Add support for a custom registry (e.g. gogo's MergedRegistry) to be plugged into SIGN_MODE_TEXTUAL.
 * [#15557](https://github.com/cosmos/cosmos-sdk/pull/15557) Implement unknown field filtering.
+* [#15515](https://github.com/cosmos/cosmos-sdk/pull/15515) Implement SIGN_MODE_LEGACY_AMINO_JSON handler.

x/tx/Makefile

@@ -1,2 +1,3 @@
 codegen:
 	@(cd internal/testpb; buf generate)
+	@(cd signing/aminojson/internal; make codegen)

x/tx/internal/testpb/unknonwnproto.proto → x/tx/internal/testpb/unknownproto.proto

@@ -1,7 +1,6 @@
 syntax = "proto3";
 package testpb;
 
-import "gogoproto/gogo.proto";
 import "google/protobuf/any.proto";
 import "cosmos/tx/v1beta1/tx.proto";
 
@@ -103,7 +102,7 @@ message TestVersion1 {
   TestVersion1          a = 2;
   TestVersion1          b = 3; // [(gogoproto.nullable) = false] generates invalid recursive structs;
   repeated TestVersion1 c = 4;
-  repeated TestVersion1 d = 5 [(gogoproto.nullable) = false];
+  repeated TestVersion1 d = 5;
   oneof                 sum {
     int32        e = 6;
     TestVersion1 f = 7;
@@ -112,7 +111,7 @@ message TestVersion1 {
   repeated TestVersion1 h = 9; // [(gogoproto.castrepeated) = "TestVersion1"];
   // google.protobuf.Timestamp i = 10;
   // google.protobuf.Timestamp j = 11; // [(gogoproto.stdtime) = true];
-  Customer1 k = 12 [(gogoproto.embed) = true];
+  Customer1 k = 12;
 }
 message TestVersion2 {
   int64                 x = 1;
@@ -128,7 +127,7 @@ message TestVersion2 {
   repeated TestVersion1 h = 9; // [(gogoproto.castrepeated) = "TestVersion1"];
   // google.protobuf.Timestamp i = 10;
   // google.protobuf.Timestamp j = 11; // [(gogoproto.stdtime) = true];
-  Customer1 k         = 12 [(gogoproto.embed) = true];
+  Customer1 k         = 12;
   uint64    new_field = 25;
 }
 message TestVersion3 {
@@ -145,7 +144,7 @@ message TestVersion3 {
   repeated TestVersion1 h = 9; //[(gogoproto.castrepeated) = "TestVersion1"];
   // google.protobuf.Timestamp i = 10;
   // google.protobuf.Timestamp j = 11; // [(gogoproto.stdtime) = true];
-  Customer1 k                  = 12 [(gogoproto.embed) = true];
+  Customer1 k                  = 12;
   string    non_critical_field = 1031;
 }
 
@@ -162,7 +161,7 @@ message TestVersion3LoneOneOfValue {
   repeated TestVersion1 h = 9; //[(gogoproto.castrepeated) = "TestVersion1"];
   // google.protobuf.Timestamp i = 10;
   // google.protobuf.Timestamp j = 11; // [(gogoproto.stdtime) = true];
-  Customer1 k                  = 12 [(gogoproto.embed) = true];
+  Customer1 k                  = 12;
   string    non_critical_field = 1031;
 }
 
@@ -179,7 +178,7 @@ message TestVersion3LoneNesting {
   repeated TestVersion1 h = 9; //[(gogoproto.castrepeated) = "TestVersion1"];
   // google.protobuf.Timestamp i = 10;
   // google.protobuf.Timestamp j = 11; // [(gogoproto.stdtime) = true];
-  Customer1 k                  = 12 [(gogoproto.embed) = true];
+  Customer1 k                  = 12;
   string    non_critical_field = 1031;
 
   message Inner1 {
@@ -220,7 +219,7 @@ message TestVersion4LoneNesting {
   repeated TestVersion1 h = 9; //[(gogoproto.castrepeated) = "TestVersion1"];
   // google.protobuf.Timestamp i = 10;
   // google.protobuf.Timestamp j = 11; // [(gogoproto.stdtime) = true];
-  Customer1 k                  = 12 [(gogoproto.embed) = true];
+  Customer1 k                  = 12;
   string    non_critical_field = 1031;
 
   message Inner1 {
@@ -271,7 +270,7 @@ message TestVersionFD1WithExtraAny {
 }
 
 message AnyWithExtra {
-  google.protobuf.Any a = 1 [(gogoproto.embed) = true];
+  google.protobuf.Any a = 1;
   int64               b = 3;
   int64               c = 4;
 }