fix: AminoJSON missing fields (Ledger signing not working) (cosmos#16032

)

x/auth/tx/legacy_amino_json_test.go

@@ -1,8 +1,11 @@
 package tx
 
 import (
+	"context"
 	"testing"
 
+	txsigning "cosmossdk.io/x/tx/signing"
+	"cosmossdk.io/x/tx/signing/aminojson"
 	"github.com/stretchr/testify/require"
 
 	cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
@@ -12,6 +15,7 @@ import (
 	signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
 	"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
 	"github.com/cosmos/cosmos-sdk/x/auth/signing"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
 var (
@@ -20,7 +24,7 @@ var (
 
 	coins   = sdk.Coins{sdk.NewInt64Coin("foocoin", 10)}
 	gas     = uint64(10000)
-	msg     = testdata.NewTestMsg(addr1, addr2)
+	msg     = banktypes.NewMsgSend(addr1, addr2, coins)
 	memo    = "foo"
 	timeout = uint64(10)
 )
@@ -138,6 +142,87 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestLegacyAminoJSONHandler_AllGetSignBytesComparison(t *testing.T) {
+	var (
+		chainID = "test-chain"
+		accNum  uint64
+		seqNum  uint64 = 7
+		tip            = &tx.Tip{Tipper: addr1.String(), Amount: coins}
+	)
+
+	modeHandler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{})
+	mode, _ := signing.APISignModeToInternal(modeHandler.Mode())
+
+	testcases := []struct {
+		name           string
+		signer         string
+		malleate       func(*wrapper)
+		expectedSignBz []byte
+	}{
+		{
+			"signer which is also fee payer (no tips)", addr1.String(),
+			func(w *wrapper) {},
+			legacytx.StdSignBytes(chainID, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas}, []sdk.Msg{msg}, memo, nil),
+		},
+		{
+			"signer which is also fee payer (with tips)", addr2.String(),
+			func(w *wrapper) { w.SetTip(tip) },
+			legacytx.StdSignBytes(chainID, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas}, []sdk.Msg{msg}, memo, tip),
+		},
+		{
+			"explicit fee payer", addr1.String(),
+			func(w *wrapper) { w.SetFeePayer(addr2) },
+			legacytx.StdSignBytes(chainID, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas, Payer: addr2.String()}, []sdk.Msg{msg}, memo, nil),
+		},
+		{
+			"explicit fee granter", addr1.String(),
+			func(w *wrapper) { w.SetFeeGranter(addr2) },
+			legacytx.StdSignBytes(chainID, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas, Granter: addr2.String()}, []sdk.Msg{msg}, memo, nil),
+		},
+		{
+			"explicit fee payer and fee granter", addr1.String(),
+			func(w *wrapper) {
+				w.SetFeePayer(addr2)
+				w.SetFeeGranter(addr2)
+			},
+			legacytx.StdSignBytes(chainID, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas, Payer: addr2.String(), Granter: addr2.String()}, []sdk.Msg{msg}, memo, nil),
+		},
+		{
+			"signer which is also tipper", addr1.String(),
+			func(w *wrapper) { w.SetTip(tip) },
+			legacytx.StdSignBytes(chainID, accNum, seqNum, timeout, legacytx.StdFee{}, []sdk.Msg{msg}, memo, tip),
+		},
+	}
+
+	handler := signModeLegacyAminoJSONHandler{}
+	for _, tc := range testcases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			bldr := newBuilder(nil)
+			buildTx(t, bldr)
+			tx := bldr.GetTx()
+			tc.malleate(bldr)
+
+			signingData := signing.SignerData{
+				Address:       tc.signer,
+				ChainID:       chainID,
+				AccountNumber: accNum,
+				Sequence:      seqNum,
+				PubKey:        pubkey1,
+			}
+			signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx)
+			require.NoError(t, err)
+
+			// compare with new signing
+			newSignBz, err := signing.GetSignBytesAdapter(context.Background(), txsigning.NewHandlerMap(modeHandler), mode, signingData, tx)
+			require.NoError(t, err)
+
+			require.Equal(t, string(tc.expectedSignBz), string(signBz))
+			require.Equal(t, string(tc.expectedSignBz), string(newSignBz))
+		})
+	}
+}
+
 func TestLegacyAminoJSONHandler_DefaultMode(t *testing.T) {
 	handler := signModeLegacyAminoJSONHandler{}
 	require.Equal(t, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, handler.DefaultMode())

x/tx/signing/aminojson/internal/aminojsonpb/aminojson.proto

@@ -18,13 +18,14 @@ message AminoSignFee {
 
 // AminoSignDoc is a message container used to generate the SIGN_MODE_LEGACY_AMINO_JSON sign bytes with proto3 API.
 // Note: This is only used for generated JSON in signing, see x/tx/signing/aminojson.go for more details.
+// We allow omitempty based on what is already defined in the legacy StdSignDoc.
 message AminoSignDoc {
-  uint64 account_number = 1;
-  uint64 sequence = 2;
+  uint64 account_number = 1 [(amino.dont_omitempty) = true];
+  uint64 sequence = 2 [(amino.dont_omitempty) = true];
   uint64 timeout_height = 3;
-  string chain_id = 4;
-  string memo = 5;
-  AminoSignFee fee = 6;
-  repeated google.protobuf.Any msgs = 7;
+  string chain_id = 4 [(amino.dont_omitempty) = true];
+  string memo = 5 [(amino.dont_omitempty) = true];
+  AminoSignFee fee = 6 [(amino.dont_omitempty) = true];
+  repeated google.protobuf.Any msgs = 7 [(amino.dont_omitempty) = true];
   cosmos.tx.v1beta1.Tip tip = 8;
 }