update SigVerifiableTx.GetPubKeys to return an error (#8828)

closes: #8129

Author: robert-zaremba

CHANGELOG.md

@@ -41,12 +41,14 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#8559](https://github.com/cosmos/cosmos-sdk/pull/8559) Added Protobuf compatible secp256r1 ECDSA signatures.
 * [\#8786](https://github.com/cosmos/cosmos-sdk/pull/8786) Enabled secp256r1 in x/auth.
 
+
 ### Client Breaking Changes
 
 * [\#8363](https://github.com/cosmos/cosmos-sdk/pull/8363) Addresses no longer have a fixed 20-byte length. From the SDK modules' point of view, any 1-255 bytes-long byte array is a valid address.
 * [\#8346](https://github.com/cosmos/cosmos-sdk/pull/8346) All CLI `tx` commands generate ServiceMsgs by default. Graceful Amino support has been added to ServiceMsgs to support signing legacy Msgs.
 * (crypto/ed25519) [\#8690] Adopt zip1215 ed2559 verification rules.
 
+
 ### API Breaking Changes
 
 * (keyring) [#\8662](https://github.com/cosmos/cosmos-sdk/pull/8662) `NewMnemonic` now receives an additional `passphrase` argument to secure the key generated by the bip39 mnemonic.
@@ -58,6 +60,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#8629](https://github.com/cosmos/cosmos-sdk/pull/8629) Deprecated `SetFullFundraiserPath` from `Config` in favor of `SetPurpose` and `SetCoinType`.
 * (x/upgrade) [\#8673](https://github.com/cosmos/cosmos-sdk/pull/8673) Remove IBC logic from x/upgrade. Deprecates IBC fields in an Upgrade Plan. IBC upgrade logic moved to 02-client and an IBC UpgradeProposal is added.
 * (x/bank) [\#8517](https://github.com/cosmos/cosmos-sdk/pull/8517) `SupplyI` interface and `Supply` are removed and uses `sdk.Coins` for supply tracking
+* (x/auth) [\#8129](https://github.com/cosmos/cosmos-sdk/pull/8828) Updated `SigVerifiableTx.GetPubKeys` method signature to return error.
+
 
 ### State Machine Breaking
 

x/auth/ante/sigverify.go

@@ -59,7 +59,10 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
 		return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid tx type")
 	}
 
-	pubkeys := sigTx.GetPubKeys()
+	pubkeys, err := sigTx.GetPubKeys()
+	if err != nil {
+		return ctx, err
+	}
 	signers := sigTx.GetSigners()
 
 	for i, pk := range pubkeys {
@@ -339,7 +342,10 @@ func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim
 	}
 
 	params := vscd.ak.GetParams(ctx)
-	pubKeys := sigTx.GetPubKeys()
+	pubKeys, err := sigTx.GetPubKeys()
+	if err != nil {
+		return ctx, err
+	}
 
 	sigCount := 0
 	for _, pk := range pubKeys {

x/auth/legacy/legacytx/stdtx.go

@@ -246,14 +246,14 @@ func (tx StdTx) GetSignaturesV2() ([]signing.SignatureV2, error) {
 
 // GetPubkeys returns the pubkeys of signers if the pubkey is included in the signature
 // If pubkey is not included in the signature, then nil is in the slice instead
-func (tx StdTx) GetPubKeys() []cryptotypes.PubKey {
+func (tx StdTx) GetPubKeys() ([]cryptotypes.PubKey, error) {
 	pks := make([]cryptotypes.PubKey, len(tx.Signatures))
 
 	for i, stdSig := range tx.Signatures {
 		pks[i] = stdSig.GetPubKey()
 	}
 
-	return pks
+	return pks, nil
 }
 
 // GetGas returns the Gas in StdFee

x/auth/signing/sig_verifiable_tx.go

@@ -11,7 +11,7 @@ import (
 type SigVerifiableTx interface {
 	types.Tx
 	GetSigners() []types.AccAddress
-	GetPubKeys() []cryptotypes.PubKey // If signer already has pubkey in context, this list will have nil in its place
+	GetPubKeys() ([]cryptotypes.PubKey, error) // If signer already has pubkey in context, this list will have nil in its place
 	GetSignaturesV2() ([]signing.SignatureV2, error)
 }
 

x/auth/testutil/suite.go

@@ -258,7 +258,9 @@ func (s *TxConfigTestSuite) TestTxEncodeDecode() {
 	tx3Sigs, err := tx3.GetSignaturesV2()
 	s.Require().NoError(err)
 	s.Require().Equal([]signingtypes.SignatureV2{sig}, tx3Sigs)
-	s.Require().Equal([]cryptotypes.PubKey{pubkey}, tx3.GetPubKeys())
+	pks, err := tx3.GetPubKeys()
+	s.Require().NoError(err)
+	s.Require().Equal([]cryptotypes.PubKey{pubkey}, pks)
 
 	log("JSON encode transaction")
 	jsonTxBytes, err := s.TxConfig.TxJSONEncoder()(tx)
@@ -277,7 +279,9 @@ func (s *TxConfigTestSuite) TestTxEncodeDecode() {
 	tx3Sigs, err = tx3.GetSignaturesV2()
 	s.Require().NoError(err)
 	s.Require().Equal([]signingtypes.SignatureV2{sig}, tx3Sigs)
-	s.Require().Equal([]cryptotypes.PubKey{pubkey}, tx3.GetPubKeys())
+	pks, err = tx3.GetPubKeys()
+	s.Require().NoError(err)
+	s.Require().Equal([]cryptotypes.PubKey{pubkey}, pks)
 }
 
 func (s *TxConfigTestSuite) TestWrapTxBuilder() {

x/auth/tx/builder.go

@@ -7,6 +7,7 @@ import (
 	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/ante"
@@ -100,7 +101,7 @@ func (w *wrapper) GetSigners() []sdk.AccAddress {
 	return w.tx.GetSigners()
 }
 
-func (w *wrapper) GetPubKeys() []cryptotypes.PubKey {
+func (w *wrapper) GetPubKeys() ([]cryptotypes.PubKey, error) {
 	signerInfos := w.tx.AuthInfo.SignerInfos
 	pks := make([]cryptotypes.PubKey, len(signerInfos))
 
@@ -111,13 +112,16 @@ func (w *wrapper) GetPubKeys() []cryptotypes.PubKey {
 			continue
 		}
 
-		pk, ok := si.PublicKey.GetCachedValue().(cryptotypes.PubKey)
+		pkAny := si.PublicKey.GetCachedValue()
+		pk, ok := pkAny.(cryptotypes.PubKey)
 		if ok {
 			pks[i] = pk
+		} else {
+			return nil, sdkerrors.Wrapf(sdkerrors.ErrLogic, "Expecting PubKey, got: %T", pkAny)
 		}
 	}
 
-	return pks
+	return pks, nil
 }
 
 func (w *wrapper) GetGas() uint64 {
@@ -165,7 +169,10 @@ func (w *wrapper) GetTimeoutHeight() uint64 {
 func (w *wrapper) GetSignaturesV2() ([]signing.SignatureV2, error) {
 	signerInfos := w.tx.AuthInfo.SignerInfos
 	sigs := w.tx.Signatures
-	pubKeys := w.GetPubKeys()
+	pubKeys, err := w.GetPubKeys()
+	if err != nil {
+		return nil, err
+	}
 	n := len(signerInfos)
 	res := make([]signing.SignatureV2, n)
 

x/auth/tx/builder_test.go

@@ -89,7 +89,9 @@ func TestTxBuilder(t *testing.T) {
 	txBuilder.SetMemo(memo)
 	require.Equal(t, bodyBytes, txBuilder.getBodyBytes())
 	require.Equal(t, len(msgs), len(txBuilder.GetMsgs()))
-	require.Equal(t, 0, len(txBuilder.GetPubKeys()))
+	pks, err := txBuilder.GetPubKeys()
+	require.NoError(t, err)
+	require.Empty(t, pks)
 
 	t.Log("verify that updated AuthInfo  results in the correct getAuthInfoBytes and GetPubKeys")
 	require.NotEqual(t, authInfoBytes, txBuilder.getAuthInfoBytes())
@@ -104,8 +106,10 @@ func TestTxBuilder(t *testing.T) {
 	require.Equal(t, authInfoBytes, txBuilder.getAuthInfoBytes())
 
 	require.Equal(t, len(msgs), len(txBuilder.GetMsgs()))
-	require.Equal(t, 1, len(txBuilder.GetPubKeys()))
-	require.Equal(t, legacy.Cdc.MustMarshalBinaryBare(pubkey), legacy.Cdc.MustMarshalBinaryBare(txBuilder.GetPubKeys()[0]))
+	pks, err = txBuilder.GetPubKeys()
+	require.NoError(t, err)
+	require.Equal(t, 1, len(pks))
+	require.True(t, pubkey.Equals(pks[0]))
 
 	any, err = codectypes.NewAnyWithValue(testdata.NewTestMsg())
 	require.NoError(t, err)