Update TxBuilder to use SignatureV2 (#6443)

* Update TxBuilder to use SignatureV2

* Fix unit tests

* Lint

* lint

* Cleanup

* Address review feedback

* Add Sign tests

* Add more cases for sign tests

* Fix lint

* Fix gofmt

* Deprecate StdTx

* Undeprecate StdTx, but leave a comment to use the protobuf Tx

* Address review comment

Co-authored-by: anilCSE <anil@vitwit.com>

CHANGELOG.md

@@ -123,6 +123,7 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa
 * (modules) [\#6326](https://github.com/cosmos/cosmos-sdk/pull/6326) `AppModuleBasic.GetQueryCmd` now takes a single `CLIContext` parameter.
 * (modules) [\#6336](https://github.com/cosmos/cosmos-sdk/pull/6336) `AppModuleBasic.RegisterQueryService` method was added to support gRPC queries, and `QuerierRoute` and `NewQuerierHandler` were deprecated.
 * (modules) [\#6311](https://github.com/cosmos/cosmos-sdk/issues/6311) Remove `alias.go` usage
+* (x/auth) [\#6443](https://github.com/cosmos/cosmos-sdk/issues/6443) Move `FeeTx` and `TxWithMemo` interfaces from `x/auth/ante` to `types`.
 
   Migration guide:
 

client/flags/flags.go

@@ -64,6 +64,7 @@ const (
 	FlagKeyringBackend   = "keyring-backend"
 	FlagPage             = "page"
 	FlagLimit            = "limit"
+	FlagSignMode         = "sign-mode"
 )
 
 // LineBreak can be included in a command list to provide a blank line
@@ -116,6 +117,7 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
 		c.Flags().Bool(FlagOffline, false, "Offline mode (does not allow any online functionality")
 		c.Flags().BoolP(FlagSkipConfirmation, "y", false, "Skip tx broadcasting prompt confirmation")
 		c.Flags().String(FlagKeyringBackend, DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test)")
+		c.Flags().String(FlagSignMode, "", "Choose sign mode (direct|amino-json), this is an advanced feature")
 
 		// --gas can accept integers and "simulate"
 		c.Flags().Var(&GasFlagVar, "gas", fmt.Sprintf(

client/tx/factory.go

@@ -9,6 +9,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/tx/signing"
 )
 
 // Factory defines a client transaction factory that facilitates generating and
@@ -21,13 +22,19 @@ type Factory struct {
 	sequence           uint64
 	gas                uint64
 	gasAdjustment      float64
-	simulateAndExecute bool
 	chainID            string
 	memo               string
 	fees               sdk.Coins
 	gasPrices          sdk.DecCoins
+	signMode           signing.SignMode
+	simulateAndExecute bool
 }
 
+const (
+	signModeDirect    = "direct"
+	signModeAminoJSON = "amino-json"
+)
+
 func NewFactoryFromCLI(input io.Reader) Factory {
 	kb, err := keyring.New(
 		sdk.KeyringServiceName(),
@@ -39,6 +46,15 @@ func NewFactoryFromCLI(input io.Reader) Factory {
 		panic(err)
 	}
 
+	signModeStr := viper.GetString(flags.FlagSignMode)
+	signMode := signing.SignMode_SIGN_MODE_UNSPECIFIED
+	switch signModeStr {
+	case signModeDirect:
+		signMode = signing.SignMode_SIGN_MODE_DIRECT
+	case signModeAminoJSON:
+		signMode = signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON
+	}
+
 	f := Factory{
 		keybase:            kb,
 		accountNumber:      viper.GetUint64(flags.FlagAccountNumber),
@@ -48,6 +64,7 @@ func NewFactoryFromCLI(input io.Reader) Factory {
 		simulateAndExecute: flags.GasFlagVar.Simulate,
 		chainID:            viper.GetString(flags.FlagChainID),
 		memo:               viper.GetString(flags.FlagMemo),
+		signMode:           signMode,
 	}
 
 	f = f.WithFees(viper.GetString(flags.FlagFees))

client/tx/tx.go

@@ -13,10 +13,11 @@ import (
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/client/input"
-	clientkeys "github.com/cosmos/cosmos-sdk/client/keys"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/types/rest"
+	"github.com/cosmos/cosmos-sdk/types/tx/signing"
+	authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
 )
 
 // GenerateOrBroadcastTx will either generate and print and unsigned transaction
@@ -108,7 +109,12 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error {
 		}
 	}
 
-	txBytes, err := Sign(txf, clientCtx.GetFromName(), clientkeys.DefaultKeyPass, tx)
+	err = Sign(txf, clientCtx.GetFromName(), tx)
+	if err != nil {
+		return err
+	}
+
+	txBytes, err := clientCtx.TxGenerator.MarshalTx(tx.GetTx())
 	if err != nil {
 		return err
 	}
@@ -209,24 +215,14 @@ func BuildUnsignedTx(txf Factory, msgs ...sdk.Msg) (client.TxBuilder, error) {
 		}
 	}
 
-	clientFee := txf.txGenerator.NewFee()
-	clientFee.SetAmount(fees)
-	clientFee.SetGas(txf.gas)
-
-	tx := txf.txGenerator.NewTx()
-	tx.SetMemo(txf.memo)
-
-	if err := tx.SetFee(clientFee); err != nil {
-		return nil, err
-	}
-
-	if err := tx.SetSignatures(); err != nil {
-		return nil, err
-	}
+	tx := txf.txGenerator.NewTxBuilder()
 
 	if err := tx.SetMsgs(msgs...); err != nil {
 		return nil, err
 	}
+	tx.SetMemo(txf.memo)
+	tx.SetFeeAmount(fees)
+	tx.SetGasLimit(txf.gas)
 
 	return tx, nil
 }
@@ -242,7 +238,7 @@ func BuildSimTx(txf Factory, msgs ...sdk.Msg) ([]byte, error) {
 
 	// Create an empty signature literal as the ante handler will populate with a
 	// sentinel pubkey.
-	sig := txf.txGenerator.NewSignature()
+	sig := signing.SignatureV2{}
 
 	if err := tx.SetSignatures(sig); err != nil {
 		return nil, err
@@ -312,33 +308,60 @@ func PrepareFactory(clientCtx client.Context, txf Factory) (Factory, error) {
 //
 // Note, It is assumed the Factory has the necessary fields set that are required
 // by the CanonicalSignBytes call.
-func Sign(txf Factory, name, passphrase string, tx client.TxBuilder) ([]byte, error) {
+func Sign(txf Factory, name string, tx client.TxBuilder) error {
 	if txf.keybase == nil {
-		return nil, errors.New("keybase must be set prior to signing a transaction")
+		return errors.New("keybase must be set prior to signing a transaction")
+	}
+
+	signMode := txf.signMode
+	if signMode == signing.SignMode_SIGN_MODE_UNSPECIFIED {
+		// use the SignModeHandler's default mode if unspecified
+		signMode = txf.txGenerator.SignModeHandler().DefaultMode()
 	}
 
-	signBytes, err := tx.CanonicalSignBytes(txf.chainID, txf.accountNumber, txf.sequence)
+	key, err := txf.keybase.Key(name)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	sigBytes, pubkey, err := txf.keybase.Sign(name, signBytes)
+	pubKey := key.GetPubKey()
+	sigData := &signing.SingleSignatureData{
+		SignMode:  signMode,
+		Signature: nil,
+	}
+	sig := signing.SignatureV2{
+		PubKey: pubKey,
+		Data:   sigData,
+	}
+	err = tx.SetSignatures(sig)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	sig := txf.txGenerator.NewSignature()
-	sig.SetSignature(sigBytes)
+	signBytes, err := txf.txGenerator.SignModeHandler().GetSignBytes(
+		signMode,
+		authsigning.SignerData{
+			ChainID:         txf.chainID,
+			AccountNumber:   txf.accountNumber,
+			AccountSequence: txf.sequence,
+		}, tx.GetTx(),
+	)
+	if err != nil {
+		return err
+	}
 
-	if err := sig.SetPubKey(pubkey); err != nil {
-		return nil, err
+	sigBytes, _, err := txf.keybase.Sign(name, signBytes)
+	if err != nil {
+		return err
 	}
 
-	if err := tx.SetSignatures(sig); err != nil {
-		return nil, err
+	sigData.Signature = sigBytes
+	sig = signing.SignatureV2{
+		PubKey: pubKey,
+		Data:   sigData,
 	}
 
-	return txf.txGenerator.MarshalTx(tx.GetTx())
+	return tx.SetSignatures(sig)
 }
 
 // GasEstimateResponse defines a response definition for tx gas estimation.

client/tx/tx_test.go

@@ -4,6 +4,12 @@ import (
 	"errors"
 	"testing"
 
+	"github.com/cosmos/cosmos-sdk/crypto/hd"
+	"github.com/cosmos/cosmos-sdk/crypto/keyring"
+	"github.com/cosmos/cosmos-sdk/tests"
+
+	"github.com/cosmos/cosmos-sdk/x/auth/ante"
+
 	"github.com/stretchr/testify/require"
 
 	"github.com/cosmos/cosmos-sdk/client"
@@ -105,5 +111,56 @@ func TestBuildUnsignedTx(t *testing.T) {
 	tx, err := tx.BuildUnsignedTx(txf, msg)
 	require.NoError(t, err)
 	require.NotNil(t, tx)
-	require.Equal(t, []sdk.Signature{}, tx.GetSignatures())
+	require.Empty(t, tx.GetTx().(ante.SigVerifiableTx).GetSignatures())
+}
+
+func TestSign(t *testing.T) {
+	dir, clean := tests.NewTestCaseDir(t)
+	t.Cleanup(clean)
+
+	path := hd.CreateHDPath(118, 0, 0).String()
+	kr, err := keyring.New(t.Name(), "test", dir, nil)
+	require.NoError(t, err)
+
+	var from = "test_sign"
+
+	_, seed, err := kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1)
+	require.NoError(t, err)
+	require.NoError(t, kr.Delete(from))
+
+	info, err := kr.NewAccount(from, seed, "", path, hd.Secp256k1)
+	require.NoError(t, err)
+
+	txf := tx.Factory{}.
+		WithTxGenerator(NewTestTxGenerator()).
+		WithAccountNumber(50).
+		WithSequence(23).
+		WithFees("50stake").
+		WithMemo("memo").
+		WithChainID("test-chain")
+
+	msg := banktypes.NewMsgSend(info.GetAddress(), sdk.AccAddress("to"), nil)
+	txn, err := tx.BuildUnsignedTx(txf, msg)
+	require.NoError(t, err)
+
+	t.Log("should failed if txf without keyring")
+	err = tx.Sign(txf, from, txn)
+	require.Error(t, err)
+
+	txf = tx.Factory{}.
+		WithKeybase(kr).
+		WithTxGenerator(NewTestTxGenerator()).
+		WithAccountNumber(50).
+		WithSequence(23).
+		WithFees("50stake").
+		WithMemo("memo").
+		WithChainID("test-chain")
+
+	t.Log("should succeed if txf with keyring")
+	err = tx.Sign(txf, from, txn)
+	require.NoError(t, err)
+
+	t.Log("should fail for non existing key")
+	err = tx.Sign(txf, "non_existing_key", txn)
+	require.Error(t, err)
 }

client/tx_generator.go

@@ -1,51 +1,32 @@
 package client
 
 import (
-	"github.com/tendermint/tendermint/crypto"
-
-	"github.com/cosmos/cosmos-sdk/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
+	"github.com/cosmos/cosmos-sdk/x/auth/signing"
 )
 
 type (
 	// TxGenerator defines an interface a client can utilize to generate an
 	// application-defined concrete transaction type. The type returned must
 	// implement TxBuilder.
 	TxGenerator interface {
-		NewTx() TxBuilder
-		NewFee() Fee
-		NewSignature() Signature
-		MarshalTx(tx types.Tx) ([]byte, error)
-	}
-
-	Fee interface {
-		types.Fee
-		SetGas(uint64)
-		SetAmount(types.Coins)
-	}
-
-	Signature interface {
-		types.Signature
-		SetPubKey(crypto.PubKey) error
-		SetSignature([]byte)
+		NewTxBuilder() TxBuilder
+		SignModeHandler() signing.SignModeHandler
+		MarshalTx(tx sdk.Tx) ([]byte, error)
 	}
 
 	// TxBuilder defines an interface which an application-defined concrete transaction
 	// type must implement. Namely, it must be able to set messages, generate
 	// signatures, and provide canonical bytes to sign over. The transaction must
 	// also know how to encode itself.
 	TxBuilder interface {
-		GetTx() types.Tx
-
-		SetMsgs(...types.Msg) error
-		GetSignatures() []types.Signature
-		SetSignatures(...Signature) error
-		GetFee() types.Fee
-		SetFee(Fee) error
-		GetMemo() string
-		SetMemo(string)
+		GetTx() sdk.Tx
 
-		// CanonicalSignBytes returns the canonical sign bytes to sign over, given a
-		// chain ID, along with an account and sequence number.
-		CanonicalSignBytes(cid string, num, seq uint64) ([]byte, error)
+		SetMsgs(msgs ...sdk.Msg) error
+		SetSignatures(signatures ...signingtypes.SignatureV2) error
+		SetMemo(memo string)
+		SetFeeAmount(amount sdk.Coins)
+		SetGasLimit(limit uint64)
 	}
 )

types/tx_msg.go

@@ -57,6 +57,20 @@ type (
 		// require access to any other information.
 		ValidateBasic() error
 	}
+
+	// FeeTx defines the interface to be implemented by Tx to use the FeeDecorators
+	FeeTx interface {
+		Tx
+		GetGas() uint64
+		GetFee() Coins
+		FeePayer() AccAddress
+	}
+
+	// Tx must have GetMemo() method to use ValidateMemoDecorator
+	TxWithMemo interface {
+		Tx
+		GetMemo() string
+	}
 )
 
 // TxDecoder unmarshals transaction bytes