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

Add transaction time in gaia lite #3238

Closed
4 tasks
faboweb opened this issue Jan 5, 2019 · 9 comments · Fixed by #4007
Closed
4 tasks

Add transaction time in gaia lite #3238

faboweb opened this issue Jan 5, 2019 · 9 comments · Fixed by #4007
Assignees
Labels
S:proposed T: Dev UX UX for SDK developers (i.e. how to call our code)
Milestone

Comments

@faboweb
Copy link
Contributor

faboweb commented Jan 5, 2019

Summary

Transactions queried via /txs don't hold the time. They only hold the block height and then the client can query the block meta information to enrich the transaction. It would be nicer to push this enrichment into gaia lite as this is a procedure probably most of the clients will do.

Problem Definition

Proposal


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

What you're asking for is to include metadata in each tx via this endpoint? Do we need to perform any additional queries for this data? If so, is it worth the overhead? If not, great!

@faboweb
Copy link
Contributor Author

faboweb commented Jan 6, 2019

If we have the meta data for each block locally we don't need to query anything else I think.

@dogemos
Copy link
Member

dogemos commented Feb 21, 2019

We (and likely a lot of third party wallet developers) will absolutely love this!

@alexanderbez
Copy link
Contributor

Looking through Tendermint RPC, looks like this will require an additional query. The thing is, this endpoint queries a series of paginated txs by tags, correct? Thus, the included txs can have varying block times? If so, then we'd have to perform a block query for each unique block height.

@fedekunze fedekunze added the T: Dev UX UX for SDK developers (i.e. how to call our code) label Mar 15, 2019
@yangyanqing
Copy link
Contributor

Looking through Tendermint RPC, looks like this will require an additional query. The thing is, this endpoint queries a series of paginated txs by tags, correct? Thus, the included txs can have varying block times? If so, then we'd have to perform a block query for each unique block height.

I think time should be left to the wallet (query by height and cache it) or browser (record time for each tx when scan block) to handle, the blockchain should be as simple as possible.

@faboweb
Copy link
Contributor Author

faboweb commented Mar 29, 2019

the blockchain should be as simple as possible

This is not about adding it to chain but letting the REST interface handle the caching and tx enriching to prevent the caching to be duplicated by every wallet.

@alexanderbez
Copy link
Contributor

@faboweb there is absolutely zero caching in the REST client right now. imho, this will a very unclean implementation and I second the suggestion by @yangyanqing.

@faboweb
Copy link
Contributor Author

faboweb commented Mar 29, 2019

I disagree. And maybe I was miss understood when I said caching. The REST client should in comparison to RPC provide well formatted and enriched data. The REST client has access to the whole storage of the node which makes it a lot more suited to perform enriching tasks than a client which needs to do several calls to stitch the information.

@alexanderbez
Copy link
Contributor

I get that. Thing is the REST client will be doing the exact same work you're client would have to do here -- make a block query for every unique tx height. If paginated results contain n txs, the worst case total # of requests will be O(n) + 1.

I don't mind doing this @faboweb just I want us to come to an agreement that the REST client will no longer be just a simple proxy between the app and the RPC, but rather data aggregation as well 👍

@alexanderbez alexanderbez self-assigned this Mar 29, 2019
@alexanderbez alexanderbez added this to the v0.34.0 milestone Mar 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:proposed T: Dev UX UX for SDK developers (i.e. how to call our code)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants