From 7b22e19b56f3e00779c9bda1a05dba9848d7b2e3 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 10 Mar 2021 00:39:23 +0100 Subject: [PATCH 1/3] update SigVerifiableTx.GetPubKeys to return an error --- types/errors/errors.go | 6 ++++++ x/auth/ante/sigverify.go | 10 ++++++++-- x/auth/legacy/legacytx/stdtx.go | 4 ++-- x/auth/signing/sig_verifiable_tx.go | 2 +- x/auth/testutil/suite.go | 8 ++++++-- x/auth/tx/builder.go | 17 +++++++++++++---- x/auth/tx/builder_test.go | 10 +++++++--- 7 files changed, 43 insertions(+), 14 deletions(-) diff --git a/types/errors/errors.go b/types/errors/errors.go index 1dcc63e6566..50d4d0f9e23 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -257,6 +257,12 @@ func (e *Error) Is(err error) bool { } } +// Wrap extends this error with an additional information. +// It's a handy function to call Wrap with sdk errors. +func (e Error) Wrap(desc string) error { + return Wrap(e, desc) +} + func isNilErr(err error) bool { // Reflect usage is necessary to correctly compare with // a nil implementation of an error. diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 19bbdcc3a2a..daa59d0b52d 100644 --- a/x/auth/ante/sigverify.go +++ b/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 { diff --git a/x/auth/legacy/legacytx/stdtx.go b/x/auth/legacy/legacytx/stdtx.go index 2b8a6c0e054..c1a982c85ef 100644 --- a/x/auth/legacy/legacytx/stdtx.go +++ b/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 diff --git a/x/auth/signing/sig_verifiable_tx.go b/x/auth/signing/sig_verifiable_tx.go index 8381ad491aa..2d8aeb49db9 100644 --- a/x/auth/signing/sig_verifiable_tx.go +++ b/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) } diff --git a/x/auth/testutil/suite.go b/x/auth/testutil/suite.go index 15f6334b40a..253ce3409c5 100644 --- a/x/auth/testutil/suite.go +++ b/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() { diff --git a/x/auth/tx/builder.go b/x/auth/tx/builder.go index 06e347dfc7f..22d78344ed5 100644 --- a/x/auth/tx/builder.go +++ b/x/auth/tx/builder.go @@ -1,12 +1,15 @@ package tx import ( + "fmt" + "github.com/gogo/protobuf/proto" "github.com/cosmos/cosmos-sdk/client" 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 +103,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 +114,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.ErrLogic.Wrap(fmt.Sprintf("Expecting PubKey, got: %T", pkAny)) } } - return pks + return pks, nil } func (w *wrapper) GetGas() uint64 { @@ -165,7 +171,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) diff --git a/x/auth/tx/builder_test.go b/x/auth/tx/builder_test.go index ddec560851b..6948299df51 100644 --- a/x/auth/tx/builder_test.go +++ b/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) From e234c7680aa6c233d1a4c9436e0fbdc1c52e84a2 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 10 Mar 2021 00:44:19 +0100 Subject: [PATCH 2/3] Changelog update --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 972be3a8077..af60dafd1a4 100644 --- a/CHANGELOG.md +++ b/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 From 7126c06e51a52f2bb7188104e299a742b5e4e6cc Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 10 Mar 2021 00:46:26 +0100 Subject: [PATCH 3/3] Move sdkerrors.Error.Wrap --- types/errors/errors.go | 6 ------ x/auth/tx/builder.go | 4 +--- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/types/errors/errors.go b/types/errors/errors.go index 50d4d0f9e23..1dcc63e6566 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -257,12 +257,6 @@ func (e *Error) Is(err error) bool { } } -// Wrap extends this error with an additional information. -// It's a handy function to call Wrap with sdk errors. -func (e Error) Wrap(desc string) error { - return Wrap(e, desc) -} - func isNilErr(err error) bool { // Reflect usage is necessary to correctly compare with // a nil implementation of an error. diff --git a/x/auth/tx/builder.go b/x/auth/tx/builder.go index 22d78344ed5..25295b46505 100644 --- a/x/auth/tx/builder.go +++ b/x/auth/tx/builder.go @@ -1,8 +1,6 @@ package tx import ( - "fmt" - "github.com/gogo/protobuf/proto" "github.com/cosmos/cosmos-sdk/client" @@ -119,7 +117,7 @@ func (w *wrapper) GetPubKeys() ([]cryptotypes.PubKey, error) { if ok { pks[i] = pk } else { - return nil, sdkerrors.ErrLogic.Wrap(fmt.Sprintf("Expecting PubKey, got: %T", pkAny)) + return nil, sdkerrors.Wrapf(sdkerrors.ErrLogic, "Expecting PubKey, got: %T", pkAny) } }