From d1fad97d87d2b4c3a98f304575f395607a5eea4f Mon Sep 17 00:00:00 2001 From: Tyler <48813565+technicallyty@users.noreply.github.com> Date: Thu, 7 Jul 2022 09:42:24 -0700 Subject: [PATCH 1/4] fix(x/auth): allow multiple = signs in `GetTxsEvent` (#12474) (cherry picked from commit 18da0e9c15e0210fdd289e3f1f0f5fefe3f6b72a) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 23 ++++++++++++ x/auth/tx/service.go | 12 +++++-- x/auth/tx/service_test.go | 75 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc4eb910e908..e1bcfe093cd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,6 +102,7 @@ replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v8.0 ### Bug Fixes +<<<<<<< HEAD * (types) [#13265](https://github.com/cosmos/cosmos-sdk/pull/13265) Correctly coalesce coins even with repeated denominations & simplify logic. * (x/auth) [#13200](https://github.com/cosmos/cosmos-sdk/pull/13200) Fix wrong sequences in `sign-batch`. * (export) [#13029](https://github.com/cosmos/cosmos-sdk/pull/13029) Fix exporting the blockParams regression. @@ -109,6 +110,28 @@ replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v8.0 * (store) [#13336](https://github.com/cosmos/cosmos-sdk/pull/13336) Call streaming listeners for deliver tx event, it was removed accidentally, backport #13334. * (grpc) [#13417](https://github.com/cosmos/cosmos-sdk/pull/13417) fix grpc query panic that could crash the node (backport #13352). * (grpc) [#13418](https://github.com/cosmos/cosmos-sdk/pull/13418) Add close for grpc only mode. +======= +* [#12416](https://github.com/cosmos/cosmos-sdk/pull/12416) Prevent zero gas transactions in the `DeductFeeDecorator` AnteHandler decorator. +* (x/mint) [#12384](https://github.com/cosmos/cosmos-sdk/pull/12384) Ensure `GoalBonded` must be positive when performing `x/mint` parameter validation. +* (x/auth) [#12261](https://github.com/cosmos/cosmos-sdk/pull/12261) Deprecate pagination in GetTxsEventRequest/Response in favor of page and limit to align with tendermint `SignClient.TxSearch` +* (vesting) [#12190](https://github.com/cosmos/cosmos-sdk/pull/12190) Replace https://github.com/cosmos/cosmos-sdk/pull/12190 to use `NewBaseAccountWithAddress` in all vesting account message handlers. +* (linting) [#12135](https://github.com/cosmos/cosmos-sdk/pull/12135) Fix variable naming issues per enabled linters. Run gofumpt to ensure easy reviews of ongoing linting work. +* (linting) [#12132](https://github.com/cosmos/cosmos-sdk/pull/12132) Change sdk.Int to math.Int, run `gofumpt -w -l .`, and `golangci-lint run ./... --fix` +* (cli) [#12127](https://github.com/cosmos/cosmos-sdk/pull/12127) Fix the CLI not always taking into account `--fee-payer` and `--fee-granter` flags. +* (migrations) [#12028](https://github.com/cosmos/cosmos-sdk/pull/12028) Fix v0.45->v0.46 in-place store migrations. +* (baseapp) [#12089](https://github.com/cosmos/cosmos-sdk/pull/12089) Include antehandler and runMsgs events in SimulateTx. +* (cli) [#12095](https://github.com/cosmos/cosmos-sdk/pull/12095) Fix running a tx with --dry-run returns an error +* (x/auth) [#12108](https://github.com/cosmos/cosmos-sdk/pull/12108) Fix GetBlockWithTxs error when querying block with 0 tx +* (genutil) [#12140](https://github.com/cosmos/cosmos-sdk/pull/12140) Fix staking's genesis JSON migrate in the `simd migrate v0.46` CLI command. +* (types) [#12154](https://github.com/cosmos/cosmos-sdk/pull/12154) Add `baseAccountGetter` to avoid invalid account error when create vesting account. +* (x/authz) [#12184](https://github.com/cosmos/cosmos-sdk/pull/12184) Fix MsgExec not verifying the validity of nested messages. +* (x/crisis) [#12208](https://github.com/cosmos/cosmos-sdk/pull/12208) Fix progress index of crisis invariant assertion logs. +* (types) [#12229](https://github.com/cosmos/cosmos-sdk/pull/12229) Increase sdk.Dec maxApproxRootIterations to 300 +* (x/staking) [#12303](https://github.com/cosmos/cosmos-sdk/pull/12303) Use bytes instead of string comparison in delete validator queue +* (testutil/sims) [#12374](https://github.com/cosmos/cosmos-sdk/pull/12374) fix the non-determinstic behavior in simulations caused by `GenSignedMockTx` and check +empty coins slice before it is used to create `banktype.MsgSend`. +* (x/auth/tx) [#12474](https://github.com/cosmos/cosmos-sdk/pull/12474) Remove condition in GetTxsEvent that disallowed multiple equal signs, which would break event queries with base64 strings (i.e. query by signature). +>>>>>>> 18da0e9c1 (fix(x/auth): allow multiple = signs in `GetTxsEvent` (#12474)) ## [v0.46.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.1) - 2022-08-24 diff --git a/x/auth/tx/service.go b/x/auth/tx/service.go index 00910aab0c04..8c8e0a2347f9 100644 --- a/x/auth/tx/service.go +++ b/x/auth/tx/service.go @@ -3,6 +3,7 @@ package tx import ( "context" "fmt" + "regexp" "strings" "github.com/cosmos/cosmos-sdk/client/grpc/tmservice" @@ -40,13 +41,18 @@ func NewTxServer(clientCtx client.Context, simulate baseAppSimulateFn, interface } } -var _ txtypes.ServiceServer = txServer{} +var ( + _ txtypes.ServiceServer = txServer{} + + // EventRegex checks that an event string is formatted with {alphabetic}.{alphabetic}={value} + EventRegex = regexp.MustCompile(`^[a-zA-Z]+\.[a-zA-Z]+=\S+$`) +) const ( eventFormat = "{eventType}.{eventAttribute}={value}" ) -// TxsByEvents implements the ServiceServer.TxsByEvents RPC method. +// GetTxsEvent implements the ServiceServer.TxsByEvents RPC method. func (s txServer) GetTxsEvent(ctx context.Context, req *txtypes.GetTxsEventRequest) (*txtypes.GetTxsEventResponse, error) { if req == nil { return nil, status.Error(codes.InvalidArgument, "request cannot be nil") @@ -70,7 +76,7 @@ func (s txServer) GetTxsEvent(ctx context.Context, req *txtypes.GetTxsEventReque } for _, event := range req.Events { - if !strings.Contains(event, "=") || strings.Count(event, "=") > 1 { + if !EventRegex.Match([]byte(event)) { return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("invalid event; event %s should be of the format: %s", event, eventFormat)) } } diff --git a/x/auth/tx/service_test.go b/x/auth/tx/service_test.go index ca1a0186e333..997491c80671 100644 --- a/x/auth/tx/service_test.go +++ b/x/auth/tx/service_test.go @@ -4,10 +4,12 @@ package tx_test import ( "context" + "encoding/base64" "fmt" "strings" "testing" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/cosmos/cosmos-sdk/client" @@ -28,6 +30,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/tx/signing" authclient "github.com/cosmos/cosmos-sdk/x/auth/client" authtest "github.com/cosmos/cosmos-sdk/x/auth/client/testutil" + 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" ) @@ -111,6 +114,78 @@ func (s *IntegrationTestSuite) TearDownSuite() { s.network.Cleanup() } +func (s *IntegrationTestSuite) TestQueryBySig() { + // broadcast tx + txb := s.mkTxBuilder() + txbz, err := s.cfg.TxConfig.TxEncoder()(txb.GetTx()) + s.Require().NoError(err) + _, err = s.queryClient.BroadcastTx(context.Background(), &tx.BroadcastTxRequest{TxBytes: txbz, Mode: tx.BroadcastMode_BROADCAST_MODE_BLOCK}) + s.Require().NoError(err) + + // get the signature out of the builder + sigs, err := txb.GetTx().GetSignaturesV2() + s.Require().NoError(err) + s.Require().Len(sigs, 1) + sig, ok := sigs[0].Data.(*signing.SingleSignatureData) + s.Require().True(ok) + + // encode, format, query + b64Sig := base64.StdEncoding.EncodeToString(sig.Signature) + sigFormatted := fmt.Sprintf("%s.%s='%s'", sdk.EventTypeTx, sdk.AttributeKeySignature, b64Sig) + res, err := s.queryClient.GetTxsEvent(context.Background(), &tx.GetTxsEventRequest{ + Events: []string{sigFormatted}, + OrderBy: 0, + Page: 0, + Limit: 10, + }) + s.Require().NoError(err) + s.Require().Len(res.Txs, 1) + s.Require().Len(res.Txs[0].Signatures, 1) + s.Require().Equal(res.Txs[0].Signatures[0], sig.Signature) + + // bad format should error + _, err = s.queryClient.GetTxsEvent(context.Background(), &tx.GetTxsEventRequest{Events: []string{"tx.foo.bar='baz'"}}) + s.Require().ErrorContains(err, "invalid event;") +} + +func TestEventRegex(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + event string + match bool + }{ + { + name: "valid: with quotes", + event: "tx.message='something'", + match: true, + }, + { + name: "valid: no quotes", + event: "tx.message=something", + match: true, + }, + { + name: "invalid: too many separators", + event: "tx.message.foo='bar'", + match: false, + }, + { + name: "valid: symbols ok", + event: "tx.signature='foobar/baz123=='", + match: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + match := authtx.EventRegex.Match([]byte(tc.event)) + require.Equal(t, tc.match, match) + }) + } +} + func (s IntegrationTestSuite) TestSimulateTx_GRPC() { val := s.network.Validators[0] txBuilder := s.mkTxBuilder() From d7beb35f3a5a5a9261ee0f01860f3812179d5666 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 20 Oct 2022 14:06:02 +0200 Subject: [PATCH 2/4] fix changelog --- CHANGELOG.md | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1bcfe093cd3..864799df8ab5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v8.0 * For applying the patch please refer to the [RELEASE NOTES](./RELEASE_NOTES.md) * (store) [#13459](https://github.com/cosmos/cosmos-sdk/pull/13459) Don't let state listener observe the uncommitted writes. * [#12548](https://github.com/cosmos/cosmos-sdk/pull/12548) Prevent signing from wrong key while using multisig. +* (x/auth/tx) [#12474](https://github.com/cosmos/cosmos-sdk/pull/12474) Remove condition in GetTxsEvent that disallowed multiple equal signs, which would break event queries with base64 strings (i.e. query by signature). ### API Breaking Changes @@ -102,7 +103,6 @@ replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v8.0 ### Bug Fixes -<<<<<<< HEAD * (types) [#13265](https://github.com/cosmos/cosmos-sdk/pull/13265) Correctly coalesce coins even with repeated denominations & simplify logic. * (x/auth) [#13200](https://github.com/cosmos/cosmos-sdk/pull/13200) Fix wrong sequences in `sign-batch`. * (export) [#13029](https://github.com/cosmos/cosmos-sdk/pull/13029) Fix exporting the blockParams regression. @@ -110,28 +110,6 @@ replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v8.0 * (store) [#13336](https://github.com/cosmos/cosmos-sdk/pull/13336) Call streaming listeners for deliver tx event, it was removed accidentally, backport #13334. * (grpc) [#13417](https://github.com/cosmos/cosmos-sdk/pull/13417) fix grpc query panic that could crash the node (backport #13352). * (grpc) [#13418](https://github.com/cosmos/cosmos-sdk/pull/13418) Add close for grpc only mode. -======= -* [#12416](https://github.com/cosmos/cosmos-sdk/pull/12416) Prevent zero gas transactions in the `DeductFeeDecorator` AnteHandler decorator. -* (x/mint) [#12384](https://github.com/cosmos/cosmos-sdk/pull/12384) Ensure `GoalBonded` must be positive when performing `x/mint` parameter validation. -* (x/auth) [#12261](https://github.com/cosmos/cosmos-sdk/pull/12261) Deprecate pagination in GetTxsEventRequest/Response in favor of page and limit to align with tendermint `SignClient.TxSearch` -* (vesting) [#12190](https://github.com/cosmos/cosmos-sdk/pull/12190) Replace https://github.com/cosmos/cosmos-sdk/pull/12190 to use `NewBaseAccountWithAddress` in all vesting account message handlers. -* (linting) [#12135](https://github.com/cosmos/cosmos-sdk/pull/12135) Fix variable naming issues per enabled linters. Run gofumpt to ensure easy reviews of ongoing linting work. -* (linting) [#12132](https://github.com/cosmos/cosmos-sdk/pull/12132) Change sdk.Int to math.Int, run `gofumpt -w -l .`, and `golangci-lint run ./... --fix` -* (cli) [#12127](https://github.com/cosmos/cosmos-sdk/pull/12127) Fix the CLI not always taking into account `--fee-payer` and `--fee-granter` flags. -* (migrations) [#12028](https://github.com/cosmos/cosmos-sdk/pull/12028) Fix v0.45->v0.46 in-place store migrations. -* (baseapp) [#12089](https://github.com/cosmos/cosmos-sdk/pull/12089) Include antehandler and runMsgs events in SimulateTx. -* (cli) [#12095](https://github.com/cosmos/cosmos-sdk/pull/12095) Fix running a tx with --dry-run returns an error -* (x/auth) [#12108](https://github.com/cosmos/cosmos-sdk/pull/12108) Fix GetBlockWithTxs error when querying block with 0 tx -* (genutil) [#12140](https://github.com/cosmos/cosmos-sdk/pull/12140) Fix staking's genesis JSON migrate in the `simd migrate v0.46` CLI command. -* (types) [#12154](https://github.com/cosmos/cosmos-sdk/pull/12154) Add `baseAccountGetter` to avoid invalid account error when create vesting account. -* (x/authz) [#12184](https://github.com/cosmos/cosmos-sdk/pull/12184) Fix MsgExec not verifying the validity of nested messages. -* (x/crisis) [#12208](https://github.com/cosmos/cosmos-sdk/pull/12208) Fix progress index of crisis invariant assertion logs. -* (types) [#12229](https://github.com/cosmos/cosmos-sdk/pull/12229) Increase sdk.Dec maxApproxRootIterations to 300 -* (x/staking) [#12303](https://github.com/cosmos/cosmos-sdk/pull/12303) Use bytes instead of string comparison in delete validator queue -* (testutil/sims) [#12374](https://github.com/cosmos/cosmos-sdk/pull/12374) fix the non-determinstic behavior in simulations caused by `GenSignedMockTx` and check -empty coins slice before it is used to create `banktype.MsgSend`. -* (x/auth/tx) [#12474](https://github.com/cosmos/cosmos-sdk/pull/12474) Remove condition in GetTxsEvent that disallowed multiple equal signs, which would break event queries with base64 strings (i.e. query by signature). ->>>>>>> 18da0e9c1 (fix(x/auth): allow multiple = signs in `GetTxsEvent` (#12474)) ## [v0.46.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.1) - 2022-08-24 From 56b77b86ced9996af99c2907344baf8fcaca601a Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 20 Oct 2022 15:11:28 +0200 Subject: [PATCH 3/4] changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 864799df8ab5..4154a0c0eec1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +## Bug Fixes + +* (x/auth/tx) [#12474](https://github.com/cosmos/cosmos-sdk/pull/12474) Remove condition in GetTxsEvent that disallowed multiple equal signs, which would break event queries with base64 strings (i.e. query by signature). + ## [v0.46.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.3) - 2022-10-16 ATTENTION: @@ -69,7 +73,6 @@ replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v8.0 * For applying the patch please refer to the [RELEASE NOTES](./RELEASE_NOTES.md) * (store) [#13459](https://github.com/cosmos/cosmos-sdk/pull/13459) Don't let state listener observe the uncommitted writes. * [#12548](https://github.com/cosmos/cosmos-sdk/pull/12548) Prevent signing from wrong key while using multisig. -* (x/auth/tx) [#12474](https://github.com/cosmos/cosmos-sdk/pull/12474) Remove condition in GetTxsEvent that disallowed multiple equal signs, which would break event queries with base64 strings (i.e. query by signature). ### API Breaking Changes From da9a5a5488cca7e404876f3ec0e8e572ce750951 Mon Sep 17 00:00:00 2001 From: tyler <48813565+technicallyty@users.noreply.github.com> Date: Thu, 20 Oct 2022 10:13:03 -0700 Subject: [PATCH 4/4] fix: flakey test --- x/auth/tx/service_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x/auth/tx/service_test.go b/x/auth/tx/service_test.go index 997491c80671..f5cccb5ae8db 100644 --- a/x/auth/tx/service_test.go +++ b/x/auth/tx/service_test.go @@ -119,8 +119,11 @@ func (s *IntegrationTestSuite) TestQueryBySig() { txb := s.mkTxBuilder() txbz, err := s.cfg.TxConfig.TxEncoder()(txb.GetTx()) s.Require().NoError(err) - _, err = s.queryClient.BroadcastTx(context.Background(), &tx.BroadcastTxRequest{TxBytes: txbz, Mode: tx.BroadcastMode_BROADCAST_MODE_BLOCK}) + resp, err := s.queryClient.BroadcastTx(context.Background(), &tx.BroadcastTxRequest{TxBytes: txbz, Mode: tx.BroadcastMode_BROADCAST_MODE_SYNC}) s.Require().NoError(err) + s.Require().NotEmpty(resp.TxResponse.TxHash) + + s.Require().NoError(s.network.WaitForNextBlock()) // get the signature out of the builder sigs, err := txb.GetTx().GetSignaturesV2()