rpc: add sort_order option to tx_search (#4342)

I have added order_by which can be "asc" or "desc" (should be in string format) in the tx_search RPC method.

Fixes: #3333

Author: @princesinha19

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:

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)
 	}
 }
 

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) {

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 {

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.

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) {

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)
+		}
 	}
 }
 

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, ""),

rpc/core/tx.go

@@ -2,9 +2,11 @@ package core
 
 import (
 	"fmt"
+	"sort"
 
-	tmmath "github.com/tendermint/tendermint/libs/math"
+	"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
 }

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: |

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
 }
 

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 {