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

types/tx: add missing fields to GetTxResponse #7867

Closed
wants to merge 2 commits into from

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Nov 9, 2020

adds tx Height, Index and Proof to the response fields

@fedekunze fedekunze requested a review from amaury1093 November 9, 2020 17:26
@fedekunze fedekunze marked this pull request as ready for review November 9, 2020 17:26
@fedekunze fedekunze requested review from anilcse and blushi November 9, 2020 17:28
@amaury1093
Copy link
Contributor

Hey Fede, have a look at #7842 (comment). I was proposing to have a struct to represent this (similar to abci's TxResponse).

This struct can be re-used in TxsByEvent.

@codecov
Copy link

codecov bot commented Nov 9, 2020

Codecov Report

Merging #7867 (51c16d3) into master (55772ae) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #7867   +/-   ##
=======================================
  Coverage   54.19%   54.20%           
=======================================
  Files         610      610           
  Lines       38811    38815    +4     
=======================================
+ Hits        21035    21039    +4     
  Misses      15621    15621           
  Partials     2155     2155           

@fedekunze
Copy link
Collaborator Author

@amaurymartiny makes sense. Should I close this PR then?

@amaury1093
Copy link
Contributor

yeah, i'd say so.

Would love your ideas on: #7842 (comment). i.e. what fields to put in that struct?

@fedekunze fedekunze closed this Nov 9, 2020
@fedekunze fedekunze deleted the fedekunze/get-tx branch November 9, 2020 22:19
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 this pull request may close these issues.

2 participants