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

Search transaction usability #5638

Closed
feri42 opened this issue Jul 17, 2019 · 15 comments · Fixed by #5648
Closed

Search transaction usability #5638

feri42 opened this issue Jul 17, 2019 · 15 comments · Fixed by #5648

Comments

@feri42
Copy link

feri42 commented Jul 17, 2019

Issue:
Using the GET /txs gaia-lite API call:

  1. It is currently not possible to search for all transactions for a given address.
  2. It is not possible to resume a search at some given point without fetching transactions that have already been fetched.
  3. It is not possible to only fetch the latest transactions.

Description:

  1. A common usage pattern for transaction searches is querying for all transactions for a given address. As it stands now we must make two requests to the server once for the recipient once for the sender. This effectively doubles the request load on the server. It also makes it more complicated on the client side where the two sets of transactions must be merged.
  2. A common usage pattern is also to have a cursor to continue the search where we left off last time. For example if we fetch transactions up to block N, then next time we would like to start fetching at block N + 1. As it stands now we must use the existing paging implementation where we use the page as a cursor. The downside is that we must fetch the last page over and over again.
  3. Yet another pattern is to only fetch a few most recent transactions. As it stands now we must always start with the oldest transactions because there is no way to guess on what page the most recent transactions can be found.

Proposed solution

  1. Add an optional address query parameter that searches for recipient OR sender.
  2. Add optional start block and/or start time/end time parameters
  3. Add a search direction parameter, or failing that reverse the return order to be from latest to oldest, which is much more useful.
@alexanderbez
Copy link
Contributor

  1. It is currently not possible to search for all transactions for a given address.

Search for all txs that were sent by some sender? If so, this is possible. /txs?mesage.sender={addr}.

  1. It is not possible to resume a search at some given point without fetching transactions that have already been fetched.

Pagination should allow you to do this.

  1. It is not possible to only fetch the latest transactions.

Define latest transactions?

@feri42
Copy link
Author

feri42 commented Jul 18, 2019

Search for all txs that were sent by some sender?

No, search for messages sent by some address OR recevied to the same address. Think of an address as an account.

Pagination should allow you to do this.

Only in a very sloppy fashion. Say I finished on page 5 at the last request, now I need to re-fetch page 5 every time to see if something was added, wasting bandwidth. Or I would need to ask for page=&limit=1, and then make one more request to see if more than one was added. Again wasting bandwidth and creating unnecessary load on the server.

Define latest transactions?

Think "most recent N transactions". Even most recent page would make sense.

@alexanderbez
Copy link
Contributor

alexanderbez commented Jul 18, 2019

No, search for messages sent by some address OR recevied to the same address. Think of an address as an account.

Yes, you can do this with events. To search for all transactions that contain any message which has a sender A: /txs?message.sender={A}. To search for all transactions where there is a MsgSend message which has a recipient B: /txs?transfer.recipient={B}. In addition, you can combine these with the AND operand.

Each module has it's own xx_events.md file under the spec/ directory. You can see all events there.

Only in a very sloppy fashion. Say I finished on page 5 at the last request, now I need to re-fetch page 5 every time to see if something was added, wasting bandwidth. Or I would need to ask for page=&limit=1, and then make one more request to see if more than one was added. Again wasting bandwidth and creating unnecessary load on the server.

Sure. But in light of this "issue", it would be immediately clear to me that if this is your use case, then you're better off creating a subscription opposed to querying directly. Through a subscription, you'll get these txs in real time! That being said, do you have suggestions on alternatives?

Think "most recent N transactions". Even most recent page would make sense.

I see. I think we can amend the pagination logic to support such a query. In other words, support both pagination params and count|num or whatever we want to call it to get the last n txs.

@feri42
Copy link
Author

feri42 commented Jul 18, 2019

Yes, you can do this with events

This sounds great. Can I do something like: message.sender={A} OR transfer.recipient={A}? That is what I am looking for. Is there any documentation on the syntax of using the AND operand?

you're better off creating a subscription

I would be more than happy to have a subscription model available, but it does not seem like the Gaia-lite API supports subsriptions: https://cosmos.network/rpc/#/. I see that the Tendermint API supports subscription models, but there I have to worry about decoding (javascript).

support both pagination params and count|num or whatever we want to call it to get the last n txs

Pagination is fine. What is missing is the starting point of pagination (e.g. block number or date).

@alexanderbez
Copy link
Contributor

This sounds great. Can I do something like: message.sender={A} OR transfer.recipient={A}? That is what I am looking for. Is there any documentation on the syntax of using the AND operand?

Unfortunately, I do not think Tendermint currently supports the OR operand. So replicate this, it would be two distinct queries or subscriptions.

When you query for txs through Gaia REST (/txs?...) the ANDis automatically done for you. e.g./txs?message.sender=...&transfer.recipient=...&...`

I would be more than happy to have a subscription model available, but it does not seem like the Gaia-lite API supports subsriptions: https://cosmos.network/rpc/#/. I see that the Tendermint API supports subscription models, but there I have to worry about decoding (javascript).

This is what I mean. Subscription through Tendermint RPC. Having it in gaia would serve no benefit really. There are javascript libraries out there that allow you to decode.

@feri42
Copy link
Author

feri42 commented Jul 19, 2019

Unfortunately, I do not think Tendermint currently supports the OR operand. So replicate this, it would be two distinct queries or subscriptions.

That is what I thought. But doing two queries is not too bad. It is much worse that there is no starting point for querying.

There are javascript libraries out there that allow you to decode.

I am only aware of js-amino, which does not really work well. Do you know of any other?

@alexanderbez
Copy link
Contributor

@jordansexton @marbar3778 do you know of other JS libraries that help with amino decoding?

@webmaster128
Copy link
Member

webmaster128 commented Feb 12, 2020

What is missing is the starting point of pagination (e.g. block number or date).

In raw Tendermint you can do both tx.height=123 and tx.height>123. The first one is (undocumentedly) supported by the REST server (try GET /txs?tx.height=123). Unfortunately the second one results in a parse error like

TxSearch: RPC error -32603 - Internal error:
parse error near digit (line 1 symbol 15 - line 1 symbol 16):
"0"

It should not be super hard to pass > through the REST server just like =, given that Tendermint has support for both.

@alexanderbez alexanderbez transferred this issue from cosmos/gaia Feb 12, 2020
@alexanderbez
Copy link
Contributor

@webmaster128, correct, the REST server only allows event queries where the values must be an exact match. The REST client was never intended to match the Tendermint RPC query semantics.

@gagbo
Copy link
Contributor

gagbo commented Feb 14, 2020

Since we're already handling TxHeightKey in a special manner when handling /txs keys, would it be fine if we added some "virtual" keys types.TxMinHeightKey and/or types.TxMaxHeightKey to translate properly into Tendermint requests ? That would help a lot regarding incremental updates

@alexanderbez
Copy link
Contributor

Can you elaborate a bit more please?

@gagbo
Copy link
Contributor

gagbo commented Feb 14, 2020

That's the kind of change I mean : gagbo@b4ba3ea

Shown inline for easy check (I might force push that commit)

commit b4ba3ea71c39be5bb17d389c2c87fc4f83ff0283
Author: Gerry Agbobada
Date:   Fri Feb 14 14:34:03 2020 +0100

    Add min-height and max-height filters to TxSearch

diff --git a/types/rest/rest.go b/types/rest/rest.go
index a58a56252..0a2f6486a 100644
--- a/types/rest/rest.go
+++ b/types/rest/rest.go
@@ -22,6 +22,8 @@ import (
 const (
 	DefaultPage  = 1
 	DefaultLimit = 30 // should be consistent with tendermint/tendermint/rpc/core/pipe.go:19
+	TxMinHeightKey = "tx.minheight" // Inclusive minimum height filter
+	TxMaxHeightKey = "tx.maxheight" // Inclusive maximum height filter
 )
 
 // ResponseWithHeight defines a response object type that wraps an original
@@ -337,6 +339,10 @@ func ParseHTTPArgsWithLimit(r *http.Request, defaultLimit int) (tags []string, p
 		var tag string
 		if key == types.TxHeightKey {
 			tag = fmt.Sprintf("%s=%s", key, value)
+		} else if key == TxMinHeightKey {
+			tag = fmt.Sprintf("%s>=%s", types.TxHeightKey, value)
+		} else if key == TxMaxHeightKey {
+			tag = fmt.Sprintf("%s<=%s", types.TxHeightKey, value)
 		} else {
 			tag = fmt.Sprintf("%s='%s'", key, value)
 		}

EDIT : I didn't test it (still running a synchronization to test the changes) so maybe it's not going to work like I expect it to.

@alexanderbez
Copy link
Contributor

alexanderbez commented Feb 14, 2020

Yeah, I think this should work! Would you like to open a PR?

@gagbo
Copy link
Contributor

gagbo commented Feb 14, 2020

Will do if you like for sure !

@gagbo
Copy link
Contributor

gagbo commented Feb 17, 2020

For the record, it did reduce the load on my machine
Without filtering :

curl -X GET "http://localhost:1317/txs?transfer.recipient=[redacted address]&page=1&limit=1" | python -m json.tool
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4178    0  4178    0     0   1157      0 --:--:--  0:00:03 --:--:--  1157
{
    "total_count": "97004",
    "count": "1",
    "page_number": "1",
    "page_total": "97004",
    "limit": "1",
    "txs": [
...
# gaiacli rest-server process
I[2020-02-15|16:26:27.599] Served RPC HTTP response                     module=rest-server method=GET url="/txs?transfer.recipient=[redacted address]&page=1&limit=1" status=200 duration=3607 remoteAddr=127.0.0.1:44292

With height filtering :

curl -X GET "http://localhost:1317/txs?transfer.recipient=[redacted address]&tx.minheight=800000&page=1&limit=1" | python -m json.tool
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4129    0  4129    0     0  11533      0 --:--:-- --:--:-- --:--:-- 11501
{
    "total_count": "730",
    "count": "1",
    "page_number": "1",
    "page_total": "730",
    "limit": "1",
    "txs": [
...
# gaiacli rest-server process
I[2020-02-15|16:27:05.663] Served RPC HTTP response                     module=rest-server method=GET url="/txs?transfer.recipient=[redacted address]&tx.minheight=800000&page=1&limit=1" status=200 duration=357 remoteAddr=127.0.0.1:44298

From ~3000ms to ~350ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants