Clean Any interface (#8167)

* Clean Any interface + removed Any.Pack method. This method is not needed, because we have specialized functions for createing new Any values. It could be used inappropriately with existing Any objects. * removed deprecated PackAny function * fix linter issue * update nil handling * NewAnyWithValue returns error on nil instead of panic * NewMsgCreateValidator: handle nil pubkey * Remove AsAny and tx builder workarounds * fix linter issue * Add AsAny methods to TxBuilder and StdTxConfig, but keep intoAny interface private * remove tx.PubKeyToAny * cosmetic updates * fix method interface * move ProtoTxProvider to x/auth/tx
cosmos · Dec 18, 2020 · b18a033 · b18a033
1 parent f602e9e
commit b18a033
Show file tree

Hide file tree

Showing 21 changed files with 96 additions and 145 deletions.
diff --git a/client/tx/tx.go b/client/tx/tx.go
@@ -12,7 +12,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/client/input"
-	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -21,6 +20,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/types/tx"
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
 	authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
+	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
 )
 
 // GenerateOrBroadcastTxCLI will either generate and print and unsigned transaction
@@ -263,22 +263,15 @@ func BuildSimTx(txf Factory, msgs ...sdk.Msg) ([]byte, error) {
 		},
 		Sequence: txf.Sequence(),
 	}
-
 	if err := txb.SetSignatures(sig); err != nil {
 		return nil, err
 	}
 
-	any, ok := txb.(codectypes.IntoAny)
-	if !ok {
-		return nil, fmt.Errorf("cannot simulate tx that cannot be wrapped into any")
-	}
-	cached := any.AsAny().GetCachedValue()
-	protoTx, ok := cached.(*tx.Tx)
+	protoProvider, ok := txb.(authtx.ProtoTxProvider)
 	if !ok {
 		return nil, fmt.Errorf("cannot simulate amino tx")
 	}
-
-	simReq := tx.SimulateRequest{Tx: protoTx}
+	simReq := tx.SimulateRequest{Tx: protoProvider.GetProtoTx()}
 
 	return simReq.Marshal()
 }

diff --git a/codec/types/any.go b/codec/types/any.go
@@ -58,15 +58,11 @@ type Any struct {
 // returns an error if that value couldn't be packed. This also caches
 // the packed value so that it can be retrieved from GetCachedValue without
 // unmarshaling
-func NewAnyWithValue(value proto.Message) (*Any, error) {
-	any := &Any{}
-
-	err := any.Pack(value)
-	if err != nil {
-		return nil, err
+func NewAnyWithValue(v proto.Message) (*Any, error) {
+	if v == nil {
+		return nil, sdkerrors.Wrap(sdkerrors.ErrPackAny, "Expecting non nil value to create a new Any")
 	}
-
-	return any, nil
+	return NewAnyWithCustomTypeURL(v, "/"+proto.MessageName(v))
 }
 
 // NewAnyWithCustomTypeURL same as NewAnyWithValue, but sets a custom type url, instead
@@ -82,22 +78,6 @@ func NewAnyWithCustomTypeURL(v proto.Message, typeURL string) (*Any, error) {
 	}, err
 }
 
-// Pack packs the value x in the Any or returns an error. This also caches
-// the packed value so that it can be retrieved from GetCachedValue without
-// unmarshaling
-func (any *Any) Pack(x proto.Message) error {
-	any.TypeUrl = "/" + proto.MessageName(x)
-	bz, err := proto.Marshal(x)
-	if err != nil {
-		return err
-	}
-
-	any.Value = bz
-	any.cachedValue = x
-
-	return nil
-}
-
 // UnsafePackAny packs the value x in the Any and instead of returning the error
 // in the case of a packing failure, keeps the cached value. This should only
 // be used in situations where compatibility is needed with amino. Amino-only
@@ -113,21 +93,20 @@ func UnsafePackAny(x interface{}) *Any {
 	return &Any{cachedValue: x}
 }
 
-// PackAny is a checked and safe version of UnsafePackAny. It assures that
-// `x` implements the proto.Message interface and uses it to serialize `x`.
-// [DEPRECATED]: should be moved away: https://github.com/cosmos/cosmos-sdk/issues/7479
-func PackAny(x interface{}) (*Any, error) {
-	if x == nil {
-		return nil, nil
-	}
-	if intoany, ok := x.(IntoAny); ok {
-		return intoany.AsAny(), nil
-	}
-	protoMsg, ok := x.(proto.Message)
-	if !ok {
-		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "Expecting %T to implement proto.Message", x)
+// pack packs the value x in the Any or returns an error. This also caches
+// the packed value so that it can be retrieved from GetCachedValue without
+// unmarshaling
+func (any *Any) pack(x proto.Message) error {
+	any.TypeUrl = "/" + proto.MessageName(x)
+	bz, err := proto.Marshal(x)
+	if err != nil {
+		return err
 	}
-	return NewAnyWithValue(protoMsg)
+
+	any.Value = bz
+	any.cachedValue = x
+
+	return nil
 }
 
 // GetCachedValue returns the cached value from the Any if present
@@ -139,8 +118,3 @@ func (any *Any) GetCachedValue() interface{} {
 func (any *Any) ClearCachedValue() {
 	any.cachedValue = nil
 }
-
-// IntoAny represents a type that can be wrapped into an Any.
-type IntoAny interface {
-	AsAny() *Any
-}
diff --git a/codec/types/compat.go b/codec/types/compat.go
@@ -89,8 +89,7 @@ func (a AminoUnpacker) UnpackAny(any *Any, iface interface{}) error {
 		return err
 	}
 	if m, ok := val.(proto.Message); ok {
-		err := any.Pack(m)
-		if err != nil {
+		if err = any.pack(m); err != nil {
 			return err
 		}
 	} else {
@@ -148,8 +147,7 @@ func (a AminoJSONUnpacker) UnpackAny(any *Any, iface interface{}) error {
 		return err
 	}
 	if m, ok := val.(proto.Message); ok {
-		err := any.Pack(m)
-		if err != nil {
+		if err = any.pack(m); err != nil {
 			return err
 		}
 	} else {

diff --git a/codec/types/types_test.go b/codec/types/types_test.go
@@ -22,24 +22,19 @@ func TestPackUnpack(t *testing.T) {
 	registry := testdata.NewTestInterfaceRegistry()
 
 	spot := &testdata.Dog{Name: "Spot"}
-	any := types.Any{}
-	err := any.Pack(spot)
-	require.NoError(t, err)
-
-	require.Equal(t, spot, any.GetCachedValue())
-
-	// without cache
-	any.ClearCachedValue()
 	var animal testdata.Animal
-	err = registry.UnpackAny(&any, &animal)
-	require.NoError(t, err)
-	require.Equal(t, spot, animal)
 
 	// with cache
-	err = any.Pack(spot)
+	any, err := types.NewAnyWithValue(spot)
+	require.NoError(t, err)
 	require.Equal(t, spot, any.GetCachedValue())
+	err = registry.UnpackAny(any, &animal)
 	require.NoError(t, err)
-	err = registry.UnpackAny(&any, &animal)
+	require.Equal(t, spot, animal)
+
+	// without cache
+	any.ClearCachedValue()
+	err = registry.UnpackAny(any, &animal)
 	require.NoError(t, err)
 	require.Equal(t, spot, animal)
 }

diff --git a/simapp/test_helpers.go b/simapp/test_helpers.go
@@ -95,7 +95,7 @@ func SetupWithGenesisValSet(t *testing.T, valSet *tmtypes.ValidatorSet, genAccs
 	for _, val := range valSet.Validators {
 		pk, err := cryptocodec.FromTmPubKeyInterface(val.PubKey)
 		require.NoError(t, err)
-		pkAny, err := codectypes.PackAny(pk)
+		pkAny, err := codectypes.NewAnyWithValue(pk)
 		require.NoError(t, err)
 		validator := stakingtypes.Validator{
 			OperatorAddress:   sdk.ValAddress(val.Address).String(),

diff --git a/x/auth/client/query.go b/x/auth/client/query.go
@@ -89,7 +89,7 @@ func QueryTx(clientCtx client.Context, hashHexStr string) (*sdk.TxResponse, erro
 		return nil, err
 	}
 
-	out, err := formatTxResult(clientCtx.TxConfig, resTx, resBlocks[resTx.Height])
+	out, err := mkTxResult(clientCtx.TxConfig, resTx, resBlocks[resTx.Height])
 	if err != nil {
 		return out, err
 	}
@@ -102,7 +102,7 @@ func formatTxResults(txConfig client.TxConfig, resTxs []*ctypes.ResultTx, resBlo
 	var err error
 	out := make([]*sdk.TxResponse, len(resTxs))
 	for i := range resTxs {
-		out[i], err = formatTxResult(txConfig, resTxs[i], resBlocks[resTxs[i].Height])
+		out[i], err = mkTxResult(txConfig, resTxs[i], resBlocks[resTxs[i].Height])
 		if err != nil {
 			return nil, err
 		}
@@ -133,27 +133,21 @@ func getBlocksForTxResults(clientCtx client.Context, resTxs []*ctypes.ResultTx)
 	return resBlocks, nil
 }
 
-func formatTxResult(txConfig client.TxConfig, resTx *ctypes.ResultTx, resBlock *ctypes.ResultBlock) (*sdk.TxResponse, error) {
-	anyTx, err := parseTx(txConfig, resTx.Tx)
+func mkTxResult(txConfig client.TxConfig, resTx *ctypes.ResultTx, resBlock *ctypes.ResultBlock) (*sdk.TxResponse, error) {
+	txb, err := txConfig.TxDecoder()(resTx.Tx)
 	if err != nil {
 		return nil, err
 	}
-
-	return sdk.NewResponseResultTx(resTx, anyTx.AsAny(), resBlock.Block.Time.Format(time.RFC3339)), nil
-}
-
-func parseTx(txConfig client.TxConfig, txBytes []byte) (codectypes.IntoAny, error) {
-	var tx sdk.Tx
-
-	tx, err := txConfig.TxDecoder()(txBytes)
-	if err != nil {
-		return nil, err
-	}
-
-	anyTx, ok := tx.(codectypes.IntoAny)
+	p, ok := txb.(intoAny)
 	if !ok {
-		return nil, fmt.Errorf("tx cannot be packed into Any")
+		return nil, fmt.Errorf("expecting a type implementing intoAny, got: %T", txb)
 	}
+	any := p.AsAny()
+	return sdk.NewResponseResultTx(resTx, any, resBlock.Block.Time.Format(time.RFC3339)), nil
+}
 
-	return anyTx, nil
+// Deprecated: this interface is used only internally for scenario we are
+// deprecating (StdTxConfig support)
+type intoAny interface {
+	AsAny() *codectypes.Any
 }
diff --git a/x/auth/legacy/legacytx/stdtx.go b/x/auth/legacy/legacytx/stdtx.go
@@ -16,10 +16,9 @@ import (
 
 // Interface implementation checks
 var (
-	_ sdk.Tx             = (*StdTx)(nil)
-	_ codectypes.IntoAny = (*StdTx)(nil)
-	_ sdk.TxWithMemo     = (*StdTx)(nil)
-	_ sdk.FeeTx          = (*StdTx)(nil)
+	_ sdk.Tx         = (*StdTx)(nil)
+	_ sdk.TxWithMemo = (*StdTx)(nil)
+	_ sdk.FeeTx      = (*StdTx)(nil)
 )
 
 // StdFee includes the amount of coins paid in fees and the maximum
@@ -172,7 +171,9 @@ func (tx StdTx) ValidateBasic() error {
 	return nil
 }
 
-// AsAny implements IntoAny.AsAny.
+// Deprecated: AsAny implements intoAny. It doesn't work for protobuf serialization,
+// so it can't be saved into protobuf configured storage. We are using it only for API
+// compatibility.
 func (tx *StdTx) AsAny() *codectypes.Any {
 	return codectypes.UnsafePackAny(tx)
 }

diff --git a/x/auth/legacy/v040/migrate.go b/x/auth/legacy/v040/migrate.go
@@ -4,7 +4,6 @@ import (
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	v039auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v039"
-	"github.com/cosmos/cosmos-sdk/x/auth/tx"
 	v040auth "github.com/cosmos/cosmos-sdk/x/auth/types"
 	v040vesting "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
 )
@@ -16,7 +15,7 @@ func convertBaseAccount(old *v039auth.BaseAccount) *v040auth.BaseAccount {
 	// just leave it nil.
 	if old.PubKey != nil {
 		var err error
-		any, err = tx.PubKeyToAny(old.PubKey)
+		any, err = codectypes.NewAnyWithValue(old.PubKey)
 		if err != nil {
 			panic(err)
 		}

diff --git a/x/auth/tx/builder.go b/x/auth/tx/builder.go
@@ -34,7 +34,7 @@ var (
 	_ client.TxBuilder           = &wrapper{}
 	_ ante.HasExtensionOptionsTx = &wrapper{}
 	_ ExtensionOptionsTxBuilder  = &wrapper{}
-	_ codectypes.IntoAny         = &wrapper{}
+	_ ProtoTxProvider            = &wrapper{}
 )
 
 // ExtensionOptionsTxBuilder defines a TxBuilder that can also set extensions.
@@ -284,7 +284,7 @@ func (w *wrapper) SetSignatures(signatures ...signing.SignatureV2) error {
 	for i, sig := range signatures {
 		var modeInfo *tx.ModeInfo
 		modeInfo, rawSigs[i] = SignatureDataToModeInfoAndSig(sig.Data)
-		any, err := PubKeyToAny(sig.PubKey)
+		any, err := codectypes.NewAnyWithValue(sig.PubKey)
 		if err != nil {
 			return err
 		}
@@ -315,10 +315,13 @@ func (w *wrapper) GetTx() authsigning.Tx {
 	return w
 }
 
-// GetProtoTx returns the tx as a proto.Message.
+func (w *wrapper) GetProtoTx() *tx.Tx {
+	return w.tx
+}
+
+// Deprecated: AsAny extracts proto Tx and wraps it into Any.
+// NOTE: You should probably use `GetProtoTx` if you want to serialize the transaction.
 func (w *wrapper) AsAny() *codectypes.Any {
-	// We're sure here that w.tx is a proto.Message, so this will call
-	// codectypes.NewAnyWithValue under the hood.
 	return codectypes.UnsafePackAny(w.tx)
 }
 
@@ -347,7 +350,7 @@ func (w *wrapper) SetNonCriticalExtensionOptions(extOpts ...*codectypes.Any) {
 	w.bodyBz = nil
 }
 
-// PubKeyToAny converts a cryptotypes.PubKey to a proto Any.
-func PubKeyToAny(key cryptotypes.PubKey) (*codectypes.Any, error) {
-	return codectypes.NewAnyWithValue(key)
+// ProtoTxProvider is a type which can provide a proto transaction.
+type ProtoTxProvider interface {
+	GetProtoTx() *tx.Tx
 }
diff --git a/x/auth/tx/builder_test.go b/x/auth/tx/builder_test.go
@@ -24,8 +24,7 @@ func TestTxBuilder(t *testing.T) {
 	memo := "sometestmemo"
 	msgs := []sdk.Msg{testdata.NewTestMsg(addr)}
 	accSeq := uint64(2) // Arbitrary account sequence
-
-	any, err := PubKeyToAny(pubkey)
+	any, err := codectypes.NewAnyWithValue(pubkey)
 	require.NoError(t, err)
 
 	var signerInfo []*txtypes.SignerInfo

diff --git a/x/auth/tx/direct_test.go b/x/auth/tx/direct_test.go
@@ -27,8 +27,7 @@ func TestDirectModeHandler(t *testing.T) {
 	memo := "sometestmemo"
 	msgs := []sdk.Msg{testdata.NewTestMsg(addr)}
 	accSeq := uint64(2) // Arbitrary account sequence
-
-	any, err := PubKeyToAny(pubkey)
+	any, err := codectypes.NewAnyWithValue(pubkey)
 	require.NoError(t, err)
 
 	var signerInfo []*txtypes.SignerInfo

diff --git a/x/auth/tx/service_test.go b/x/auth/tx/service_test.go
@@ -10,7 +10,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	clienttx "github.com/cosmos/cosmos-sdk/client/tx"
-	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/testutil/network"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -20,6 +19,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/types/tx"
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
 	authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
+	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
 	bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/testutil"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 )
@@ -456,20 +456,10 @@ func (s IntegrationTestSuite) mkTxBuilder() client.TxBuilder {
 
 // txBuilderToProtoTx converts a txBuilder into a proto tx.Tx.
 func txBuilderToProtoTx(txBuilder client.TxBuilder) (*tx.Tx, error) { // nolint
-	intoAnyTx, ok := txBuilder.(codectypes.IntoAny)
+	protoProvider, ok := txBuilder.(authtx.ProtoTxProvider)
 	if !ok {
-		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", (codectypes.IntoAny)(nil), intoAnyTx)
+		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected proto tx builder, got %T", txBuilder)
 	}
 
-	any := intoAnyTx.AsAny().GetCachedValue()
-	if any == nil {
-		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "any's cached value is empty")
-	}
-
-	protoTx, ok := any.(*tx.Tx)
-	if !ok {
-		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", (codectypes.IntoAny)(nil), intoAnyTx)
-	}
-
-	return protoTx, nil
+	return protoProvider.GetProtoTx(), nil
 }