diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index aac1e6542..c100f12bc 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,6 +3,7 @@ \*\* Special thanks to external contributors on this release: +@princesinha19 Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermint). @@ -17,6 +18,8 @@ program](https://hackerone.com/tendermint). ### FEATURES: +- [rpc] [\#3333] Add `order_by` to `/tx_search` endpoint, allowing to change default ordering from asc to desc (more in the future) (@princesinha19) + ### IMPROVEMENTS: ### BUG FIXES: diff --git a/lite2/proxy/routes.go b/lite2/proxy/routes.go index 110cfe88f..f7d5cd25b 100644 --- a/lite2/proxy/routes.go +++ b/lite2/proxy/routes.go @@ -26,7 +26,7 @@ func RPCRoutes(c *lrpc.Client) map[string]*rpcserver.RPCFunc { "block_results": rpcserver.NewRPCFunc(makeBlockResultsFunc(c), "height"), "commit": rpcserver.NewRPCFunc(makeCommitFunc(c), "height"), "tx": rpcserver.NewRPCFunc(makeTxFunc(c), "hash,prove"), - "tx_search": rpcserver.NewRPCFunc(makeTxSearchFunc(c), "query,prove,page,per_page"), + "tx_search": rpcserver.NewRPCFunc(makeTxSearchFunc(c), "query,prove,page,per_page,order_by"), "validators": rpcserver.NewRPCFunc(makeValidatorsFunc(c), "height,page,per_page"), "dump_consensus_state": rpcserver.NewRPCFunc(makeDumpConsensusStateFunc(c), ""), "consensus_state": rpcserver.NewRPCFunc(makeConsensusStateFunc(c), ""), @@ -122,11 +122,12 @@ func makeTxFunc(c *lrpc.Client) rpcTxFunc { } type rpcTxSearchFunc func(ctx *rpctypes.Context, query string, prove bool, - page, perPage int) (*ctypes.ResultTxSearch, error) + page, perPage int, orderBy string) (*ctypes.ResultTxSearch, error) func makeTxSearchFunc(c *lrpc.Client) rpcTxSearchFunc { - return func(ctx *rpctypes.Context, query string, prove bool, page, perPage int) (*ctypes.ResultTxSearch, error) { - return c.TxSearch(query, prove, page, perPage) + return func(ctx *rpctypes.Context, query string, prove bool, page, perPage int, orderBy string) ( + *ctypes.ResultTxSearch, error) { + return c.TxSearch(query, prove, page, perPage, orderBy) } } diff --git a/lite2/rpc/client.go b/lite2/rpc/client.go index 458e871ad..9778878ac 100644 --- a/lite2/rpc/client.go +++ b/lite2/rpc/client.go @@ -295,8 +295,9 @@ func (c *Client) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { return res, res.Proof.Validate(h.DataHash) } -func (c *Client) TxSearch(query string, prove bool, page, perPage int) (*ctypes.ResultTxSearch, error) { - return c.next.TxSearch(query, prove, page, perPage) +func (c *Client) TxSearch(query string, prove bool, page, perPage int, orderBy string) ( + *ctypes.ResultTxSearch, error) { + return c.next.TxSearch(query, prove, page, perPage, orderBy) } func (c *Client) Validators(height *int64, page, perPage int) (*ctypes.ResultValidators, error) { diff --git a/rpc/client/httpclient.go b/rpc/client/httpclient.go index 29175fe19..021df82e6 100644 --- a/rpc/client/httpclient.go +++ b/rpc/client/httpclient.go @@ -348,13 +348,15 @@ func (c *baseRPCClient) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { return result, nil } -func (c *baseRPCClient) TxSearch(query string, prove bool, page, perPage int) (*ctypes.ResultTxSearch, error) { +func (c *baseRPCClient) TxSearch(query string, prove bool, page, perPage int, orderBy string) ( + *ctypes.ResultTxSearch, error) { result := new(ctypes.ResultTxSearch) params := map[string]interface{}{ "query": query, "prove": prove, "page": page, "per_page": perPage, + "order_by": orderBy, } _, err := c.caller.Call("tx_search", params, result) if err != nil { diff --git a/rpc/client/interface.go b/rpc/client/interface.go index 72e899551..408d803c8 100644 --- a/rpc/client/interface.go +++ b/rpc/client/interface.go @@ -69,7 +69,7 @@ type SignClient interface { Commit(height *int64) (*ctypes.ResultCommit, error) Validators(height *int64, page, perPage int) (*ctypes.ResultValidators, error) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) - TxSearch(query string, prove bool, page, perPage int) (*ctypes.ResultTxSearch, error) + TxSearch(query string, prove bool, page, perPage int, orderBy string) (*ctypes.ResultTxSearch, error) } // HistoryClient provides access to data from genesis to now in large chunks. diff --git a/rpc/client/localclient.go b/rpc/client/localclient.go index c1d0809a6..e6b0eb937 100644 --- a/rpc/client/localclient.go +++ b/rpc/client/localclient.go @@ -160,8 +160,9 @@ func (c *Local) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { return core.Tx(c.ctx, hash, prove) } -func (c *Local) TxSearch(query string, prove bool, page, perPage int) (*ctypes.ResultTxSearch, error) { - return core.TxSearch(c.ctx, query, prove, page, perPage) +func (c *Local) TxSearch(query string, prove bool, page, perPage int, orderBy string) ( + *ctypes.ResultTxSearch, error) { + return core.TxSearch(c.ctx, query, prove, page, perPage, orderBy) } func (c *Local) BroadcastEvidence(ev types.Evidence) (*ctypes.ResultBroadcastEvidence, error) { diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index 52f0961b9..55c7755ac 100644 --- a/rpc/client/rpc_test.go +++ b/rpc/client/rpc_test.go @@ -418,7 +418,7 @@ func TestTxSearch(t *testing.T) { c := getHTTPClient() _, _, tx := MakeTxKV() bres, err := c.BroadcastTxCommit(tx) - require.Nil(t, err, "%+v", err) + require.Nil(t, err) txHeight := bres.Height txHash := bres.Hash @@ -430,8 +430,8 @@ func TestTxSearch(t *testing.T) { // now we query for the tx. // since there's only one tx, we know index=0. - result, err := c.TxSearch(fmt.Sprintf("tx.hash='%v'", txHash), true, 1, 30) - require.Nil(t, err, "%+v", err) + result, err := c.TxSearch(fmt.Sprintf("tx.hash='%v'", txHash), true, 1, 30, "asc") + require.Nil(t, err) require.Len(t, result.Txs, 1) ptx := result.Txs[0] @@ -448,33 +448,53 @@ func TestTxSearch(t *testing.T) { } // query by height - result, err = c.TxSearch(fmt.Sprintf("tx.height=%d", txHeight), true, 1, 30) - require.Nil(t, err, "%+v", err) + result, err = c.TxSearch(fmt.Sprintf("tx.height=%d", txHeight), true, 1, 30, "asc") + require.Nil(t, err) require.Len(t, result.Txs, 1) // query for non existing tx - result, err = c.TxSearch(fmt.Sprintf("tx.hash='%X'", anotherTxHash), false, 1, 30) - require.Nil(t, err, "%+v", err) + result, err = c.TxSearch(fmt.Sprintf("tx.hash='%X'", anotherTxHash), false, 1, 30, "asc") + require.Nil(t, err) require.Len(t, result.Txs, 0) // query using a compositeKey (see kvstore application) - result, err = c.TxSearch("app.creator='Cosmoshi Netowoko'", false, 1, 30) - require.Nil(t, err, "%+v", err) + result, err = c.TxSearch("app.creator='Cosmoshi Netowoko'", false, 1, 30, "asc") + require.Nil(t, err) if len(result.Txs) == 0 { t.Fatal("expected a lot of transactions") } // query using a compositeKey (see kvstore application) and height - result, err = c.TxSearch("app.creator='Cosmoshi Netowoko' AND tx.height<10000", true, 1, 30) - require.Nil(t, err, "%+v", err) + result, err = c.TxSearch("app.creator='Cosmoshi Netowoko' AND tx.height<10000", true, 1, 30, "asc") + require.Nil(t, err) if len(result.Txs) == 0 { t.Fatal("expected a lot of transactions") } // query a non existing tx with page 1 and txsPerPage 1 - result, err = c.TxSearch("app.creator='Cosmoshi Neetowoko'", true, 1, 1) - require.Nil(t, err, "%+v", err) + result, err = c.TxSearch("app.creator='Cosmoshi Neetowoko'", true, 1, 1, "asc") + require.Nil(t, err) require.Len(t, result.Txs, 0) + + // broadcast another transaction to make sure we have at least two. + _, _, tx2 := MakeTxKV() + _, err = c.BroadcastTxCommit(tx2) + require.Nil(t, err) + + // chech sorting + result, err = c.TxSearch(fmt.Sprintf("tx.height >= 1"), false, 1, 30, "asc") + require.Nil(t, err) + for k := 0; k < len(result.Txs)-1; k++ { + require.LessOrEqual(t, result.Txs[k].Height, result.Txs[k+1].Height) + require.LessOrEqual(t, result.Txs[k].Index, result.Txs[k+1].Index) + } + + result, err = c.TxSearch(fmt.Sprintf("tx.height >= 1"), false, 1, 30, "desc") + require.Nil(t, err) + for k := 0; k < len(result.Txs)-1; k++ { + require.GreaterOrEqual(t, result.Txs[k].Height, result.Txs[k+1].Height) + require.GreaterOrEqual(t, result.Txs[k].Index, result.Txs[k+1].Index) + } } } diff --git a/rpc/core/routes.go b/rpc/core/routes.go index 0e959d6d4..42a57456a 100644 --- a/rpc/core/routes.go +++ b/rpc/core/routes.go @@ -23,7 +23,7 @@ var Routes = map[string]*rpc.RPCFunc{ "block_results": rpc.NewRPCFunc(BlockResults, "height"), "commit": rpc.NewRPCFunc(Commit, "height"), "tx": rpc.NewRPCFunc(Tx, "hash,prove"), - "tx_search": rpc.NewRPCFunc(TxSearch, "query,prove,page,per_page"), + "tx_search": rpc.NewRPCFunc(TxSearch, "query,prove,page,per_page,order_by"), "validators": rpc.NewRPCFunc(Validators, "height,page,per_page"), "dump_consensus_state": rpc.NewRPCFunc(DumpConsensusState, ""), "consensus_state": rpc.NewRPCFunc(ConsensusState, ""), diff --git a/rpc/core/tx.go b/rpc/core/tx.go index f3cd54e4f..cd6306060 100644 --- a/rpc/core/tx.go +++ b/rpc/core/tx.go @@ -2,9 +2,11 @@ package core import ( "fmt" + "sort" + + "github.com/pkg/errors" tmmath "github.com/tendermint/tendermint/libs/math" - tmquery "github.com/tendermint/tendermint/libs/pubsub/query" ctypes "github.com/tendermint/tendermint/rpc/core/types" rpctypes "github.com/tendermint/tendermint/rpc/lib/types" @@ -53,10 +55,11 @@ func Tx(ctx *rpctypes.Context, hash []byte, prove bool) (*ctypes.ResultTx, error // TxSearch allows you to query for multiple transactions results. It returns a // list of transactions (maximum ?per_page entries) and the total count. // More: https://docs.tendermint.com/master/rpc/#/Info/tx_search -func TxSearch(ctx *rpctypes.Context, query string, prove bool, page, perPage int) (*ctypes.ResultTxSearch, error) { +func TxSearch(ctx *rpctypes.Context, query string, prove bool, page, perPage int, orderBy string) ( + *ctypes.ResultTxSearch, error) { // if index is disabled, return error if _, ok := txIndexer.(*null.TxIndex); ok { - return nil, fmt.Errorf("transaction indexing is disabled") + return nil, errors.New("transaction indexing is disabled") } q, err := tmquery.New(query) @@ -100,5 +103,26 @@ func TxSearch(ctx *rpctypes.Context, query string, prove bool, page, perPage int } } + if len(apiResults) > 1 { + switch orderBy { + case "desc": + sort.Slice(apiResults, func(i, j int) bool { + if apiResults[i].Height == apiResults[j].Height { + return apiResults[i].Index > apiResults[j].Index + } + return apiResults[i].Height > apiResults[j].Height + }) + case "asc", "": + sort.Slice(apiResults, func(i, j int) bool { + if apiResults[i].Height == apiResults[j].Height { + return apiResults[i].Index < apiResults[j].Index + } + return apiResults[i].Height < apiResults[j].Height + }) + default: + return nil, errors.New("expected order_by to be either `asc` or `desc` or empty") + } + } + return &ctypes.ResultTxSearch{Txs: apiResults, TotalCount: totalCount}, nil } diff --git a/rpc/swagger/swagger.yaml b/rpc/swagger/swagger.yaml index f56380450..aa4d837ba 100644 --- a/rpc/swagger/swagger.yaml +++ b/rpc/swagger/swagger.yaml @@ -859,6 +859,14 @@ paths: type: number default: 30 example: 30 + - in: query + name: order_by + description: Order in which transactions are sorted ("asc" or "desc"), by height & index. If empty, default sorting will be still applied. + required: false + schema: + type: string + default: "asc" + example: "asc" tags: - Info description: | diff --git a/state/txindex/kv/kv.go b/state/txindex/kv/kv.go index ce894f25b..5fcd5ab73 100644 --- a/state/txindex/kv/kv.go +++ b/state/txindex/kv/kv.go @@ -4,7 +4,6 @@ import ( "bytes" "encoding/hex" "fmt" - "sort" "strconv" "strings" "time" @@ -160,12 +159,14 @@ func (txi *TxIndex) indexEvents(result *types.TxResult, hash []byte, store dbm.S } } -// Search performs a search using the given query. It breaks the query into -// conditions (like "tx.height > 5"). For each condition, it queries the DB -// index. One special use cases here: (1) if "tx.hash" is found, it returns tx -// result for it (2) for range queries it is better for the client to provide -// both lower and upper bounds, so we are not performing a full scan. Results -// from querying indexes are then intersected and returned to the caller. +// Search performs a search using the given query. +// +// It breaks the query into conditions (like "tx.height > 5"). For each +// condition, it queries the DB index. One special use cases here: (1) if +// "tx.hash" is found, it returns tx result for it (2) for range queries it is +// better for the client to provide both lower and upper bounds, so we are not +// performing a full scan. Results from querying indexes are then intersected +// and returned to the caller, in no particular order. func (txi *TxIndex) Search(q *query.Query) ([]*types.TxResult, error) { var hashesInitialized bool filteredHashes := make(map[string][]byte) @@ -250,14 +251,6 @@ func (txi *TxIndex) Search(q *query.Query) ([]*types.TxResult, error) { results = append(results, res) } - // sort by height & index by default - sort.Slice(results, func(i, j int) bool { - if results[i].Height == results[j].Height { - return results[i].Index < results[j].Index - } - return results[i].Height < results[j].Height - }) - return results, nil } diff --git a/state/txindex/kv/kv_test.go b/state/txindex/kv/kv_test.go index e6c077917..efb767f21 100644 --- a/state/txindex/kv/kv_test.go +++ b/state/txindex/kv/kv_test.go @@ -272,29 +272,6 @@ func TestTxSearchMultipleTxs(t *testing.T) { assert.NoError(t, err) require.Len(t, results, 3) - assert.Equal(t, []*types.TxResult{txResult3, txResult2, txResult}, results) -} - -func TestIndexAllTags(t *testing.T) { - indexer := NewTxIndex(db.NewMemDB(), IndexAllEvents()) - - txResult := txResultWithEvents([]abci.Event{ - {Type: "account", Attributes: []kv.Pair{{Key: []byte("owner"), Value: []byte("Ivan")}}}, - {Type: "account", Attributes: []kv.Pair{{Key: []byte("number"), Value: []byte("1")}}}, - }) - - err := indexer.Index(txResult) - require.NoError(t, err) - - results, err := indexer.Search(query.MustParse("account.number >= 1")) - assert.NoError(t, err) - assert.Len(t, results, 1) - assert.Equal(t, []*types.TxResult{txResult}, results) - - results, err = indexer.Search(query.MustParse("account.owner = 'Ivan'")) - assert.NoError(t, err) - assert.Len(t, results, 1) - assert.Equal(t, []*types.TxResult{txResult}, results) } func txResultWithEvents(events []abci.Event) *types.TxResult {