refactor(client): Remove usage of GetSignBytes from AuxTxBuilder (#16025

)
cosmos · May 4, 2023 · 117865a · 117865a
1 parent 34018f7
commit 117865a
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 25 deletions.
diff --git a/client/tx/aux_builder.go b/client/tx/aux_builder.go
@@ -1,15 +1,21 @@
 package tx
 
 import (
+	"context"
+
 	"github.com/cosmos/gogoproto/proto"
+	"google.golang.org/protobuf/types/known/anypb"
 
+	basev1beta1 "cosmossdk.io/api/cosmos/base/v1beta1"
+	txv1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1"
+	txsigning "cosmossdk.io/x/tx/signing"
+	"cosmossdk.io/x/tx/signing/aminojson"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/types/tx"
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
-	"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
 )
 
 // AuxTxBuilder is a client-side builder for creating an AuxSignerData.
@@ -19,7 +25,7 @@ type AuxTxBuilder struct {
 	// - b.msgs is used for constructing the AMINO sign bz,
 	// - b.body is used for constructing the DIRECT_AUX sign bz.
 	msgs          []sdk.Msg
-	body          *tx.TxBody
+	body          *txv1beta1.TxBody
 	auxSignerData *tx.AuxSignerData
 }
 
@@ -54,13 +60,16 @@ func (b *AuxTxBuilder) SetTimeoutHeight(height uint64) {
 
 // SetMsgs sets an array of Msgs in the tx.
 func (b *AuxTxBuilder) SetMsgs(msgs ...sdk.Msg) error {
-	anys := make([]*codectypes.Any, len(msgs))
+	anys := make([]*anypb.Any, len(msgs))
 	for i, msg := range msgs {
-		var err error
-		anys[i], err = codectypes.NewAnyWithValue(msg)
+		legacyAny, err := codectypes.NewAnyWithValue(msg)
 		if err != nil {
 			return err
 		}
+		anys[i] = &anypb.Any{
+			TypeUrl: legacyAny.TypeUrl,
+			Value:   legacyAny.Value,
+		}
 	}
 
 	b.checkEmptyFields()
@@ -95,14 +104,13 @@ func (b *AuxTxBuilder) SetSequence(accSeq uint64) {
 
 // SetPubKey sets the aux signer's pubkey in the AuxSignerData.
 func (b *AuxTxBuilder) SetPubKey(pk cryptotypes.PubKey) error {
-	anyAnimal, err := codectypes.NewAnyWithValue(pk)
+	anyPk, err := codectypes.NewAnyWithValue(pk)
 	if err != nil {
 		return err
 	}
 
 	b.checkEmptyFields()
-
-	b.auxSignerData.SignDoc.PublicKey = anyAnimal
+	b.auxSignerData.SignDoc.PublicKey = anyPk
 
 	return nil
 }
@@ -113,18 +121,17 @@ func (b *AuxTxBuilder) SetSignMode(mode signing.SignMode) error {
 	switch mode {
 	case signing.SignMode_SIGN_MODE_DIRECT_AUX, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON:
 	default:
-		return sdkerrors.ErrInvalidRequest.Wrapf("AuxTxBuilder can only sign with %s or %s", signing.SignMode_SIGN_MODE_DIRECT_AUX, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
+		return sdkerrors.ErrInvalidRequest.Wrapf("AuxTxBuilder can only sign with %s or %s",
+			signing.SignMode_SIGN_MODE_DIRECT_AUX, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
 	}
 
 	b.auxSignerData.Mode = mode
-
 	return nil
 }
 
 // SetTip sets an optional tip in the AuxSignerData.
 func (b *AuxTxBuilder) SetTip(tip *tx.Tip) {
 	b.checkEmptyFields()
-
 	b.auxSignerData.SignDoc.Tip = tip
 }
 
@@ -139,15 +146,29 @@ func (b *AuxTxBuilder) SetSignature(sig []byte) {
 func (b *AuxTxBuilder) SetExtensionOptions(extOpts ...*codectypes.Any) {
 	b.checkEmptyFields()
 
-	b.body.ExtensionOptions = extOpts
+	anyExtOpts := make([]*anypb.Any, len(extOpts))
+	for i, extOpt := range extOpts {
+		anyExtOpts[i] = &anypb.Any{
+			TypeUrl: extOpt.TypeUrl,
+			Value:   extOpt.Value,
+		}
+	}
+	b.body.ExtensionOptions = anyExtOpts
 	b.auxSignerData.SignDoc.BodyBytes = nil
 }
 
 // SetSignature sets the aux signer's signature.
 func (b *AuxTxBuilder) SetNonCriticalExtensionOptions(extOpts ...*codectypes.Any) {
 	b.checkEmptyFields()
 
-	b.body.NonCriticalExtensionOptions = extOpts
+	anyNonCritExtOpts := make([]*anypb.Any, len(extOpts))
+	for i, extOpt := range extOpts {
+		anyNonCritExtOpts[i] = &anypb.Any{
+			TypeUrl: extOpt.TypeUrl,
+			Value:   extOpt.Value,
+		}
+	}
+	b.body.NonCriticalExtensionOptions = anyNonCritExtOpts
 	b.auxSignerData.SignDoc.BodyBytes = nil
 }
 
@@ -174,8 +195,7 @@ func (b *AuxTxBuilder) GetSignBytes() ([]byte, error) {
 	}
 
 	sd.BodyBytes = bodyBz
-
-	if err := b.auxSignerData.SignDoc.ValidateBasic(); err != nil {
+	if err = sd.ValidateBasic(); err != nil {
 		return nil, err
 	}
 
@@ -190,16 +210,54 @@ func (b *AuxTxBuilder) GetSignBytes() ([]byte, error) {
 		}
 	case signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON:
 		{
-			signBz = legacytx.StdSignBytes(
-				b.auxSignerData.SignDoc.ChainId, b.auxSignerData.SignDoc.AccountNumber,
-				b.auxSignerData.SignDoc.Sequence, b.body.TimeoutHeight,
-				// Aux signer never signs over fee.
-				// For LEGACY_AMINO_JSON, we use the convention to sign
-				// over empty fees.
-				// ref: https://github.com/cosmos/cosmos-sdk/pull/10348
-				legacytx.StdFee{},
-				b.msgs, b.body.Memo, b.auxSignerData.SignDoc.Tip,
+			handler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{
+				FileResolver: proto.HybridResolver,
+			})
+			legacyTip := b.auxSignerData.SignDoc.Tip
+			tip := &txv1beta1.Tip{
+				Amount: make([]*basev1beta1.Coin, len(legacyTip.Amount)),
+				Tipper: legacyTip.Tipper,
+			}
+			auxBody := &txv1beta1.TxBody{
+				Messages:      body.Messages,
+				Memo:          body.Memo,
+				TimeoutHeight: body.TimeoutHeight,
+				// AuxTxBuilder has no concern with extension options, so we set them to nil.
+				// This preserves pre-PR#16025 behavior where extension options were ignored, this code path:
+				// https://github.com/cosmos/cosmos-sdk/blob/ac3c209326a26b46f65a6cc6f5b5ebf6beb79b38/client/tx/aux_builder.go#L193
+				// https://github.com/cosmos/cosmos-sdk/blob/ac3c209326a26b46f65a6cc6f5b5ebf6beb79b38/x/auth/migrations/legacytx/stdsign.go#L49
+				ExtensionOptions:            nil,
+				NonCriticalExtensionOptions: nil,
+			}
+			for i, coin := range legacyTip.Amount {
+				tip.Amount[i] = &basev1beta1.Coin{
+					Denom:  coin.Denom,
+					Amount: coin.Amount.String(),
+				}
+			}
+			signBz, err = handler.GetSignBytes(
+				context.Background(),
+				txsigning.SignerData{
+					Address:       b.auxSignerData.Address,
+					ChainID:       b.auxSignerData.SignDoc.ChainId,
+					AccountNumber: b.auxSignerData.SignDoc.AccountNumber,
+					Sequence:      b.auxSignerData.SignDoc.Sequence,
+					PubKey:        nil,
+				},
+				txsigning.TxData{
+					Body: auxBody,
+					AuthInfo: &txv1beta1.AuthInfo{
+						SignerInfos: nil,
+						// Aux signer never signs over fee.
+						// For LEGACY_AMINO_JSON, we use the convention to sign
+						// over empty fees.
+						// ref: https://github.com/cosmos/cosmos-sdk/pull/10348
+						Fee: &txv1beta1.Fee{},
+						Tip: tip,
+					},
+				},
 			)
+			return signBz, err
 		}
 	default:
 		return nil, sdkerrors.ErrInvalidRequest.Wrapf("got unknown sign mode %s", b.auxSignerData.Mode)
@@ -219,7 +277,7 @@ func (b *AuxTxBuilder) GetAuxSignerData() (tx.AuxSignerData, error) {
 
 func (b *AuxTxBuilder) checkEmptyFields() {
 	if b.body == nil {
-		b.body = &tx.TxBody{}
+		b.body = &txv1beta1.TxBody{}
 	}
 
 	if b.auxSignerData == nil {

diff --git a/x/auth/tx/aux_test.go b/x/auth/tx/aux_test.go
@@ -9,6 +9,7 @@ import (
 	clienttx "github.com/cosmos/cosmos-sdk/client/tx"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
+	_ "github.com/cosmos/cosmos-sdk/testutil/testdata/testpb"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
 	txtypes "github.com/cosmos/cosmos-sdk/types/tx"