Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update SigVerifiableTx.GetPubKeys to return an error #8828

Merged
merged 3 commits into from
Mar 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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