Skip to content

Commit

Permalink
update SigVerifiableTx.GetPubKeys to return an error (#8828)
Browse files Browse the repository at this point in the history
closes: #8129
  • Loading branch information
robert-zaremba authored Mar 10, 2021
1 parent 24ed401 commit 8f7cdaf
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 14 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down
10 changes: 8 additions & 2 deletions x/auth/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions x/auth/legacy/legacytx/stdtx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion x/auth/signing/sig_verifiable_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
8 changes: 6 additions & 2 deletions x/auth/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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() {
Expand Down
15 changes: 11 additions & 4 deletions x/auth/tx/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))

Expand All @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down
10 changes: 7 additions & 3 deletions x/auth/tx/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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)
Expand Down

0 comments on commit 8f7cdaf

Please sign in to comment.