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

feat: remove prove field in the GetTxsEventRequest of tx proto. #584

Merged
merged 2 commits into from
Jul 8, 2022

Conversation

zemyblue
Copy link
Member

@zemyblue zemyblue commented Jul 6, 2022

Description

The prove field is added int #291. But in this PR, there is not the reason why this function is necessary, etc. I think it is to select whether to output the evidence that the corresponding tx is included in the block in the result according to the value of prove when We request GetTxsEvent.

However, there is not the prove field in the cosmos-sdk, and evidence of including block is to be added to all results by default. And TODO is written that it needs to be fixed.

The modified some codes in the #291 was reverted in the #495 which update to cosmos-sdk v0.45.1. We cannot select the result of GetTxsEvent's prove for now, but it is not occurred any problem.

I think it would be better to revert prove selection function for compability with cosmos-sdk.

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

@zemyblue zemyblue marked this pull request as draft July 6, 2022 13:17
@zemyblue zemyblue self-assigned this Jul 6, 2022
@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #584 (68a1f28) into main (3743fd6) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #584      +/-   ##
==========================================
- Coverage   57.68%   57.68%   -0.01%     
==========================================
  Files         793      793              
  Lines       87013    87013              
==========================================
- Hits        50193    50190       -3     
- Misses      33662    33664       +2     
- Partials     3158     3159       +1     
Impacted Files Coverage Δ
x/wasm/keeper/keeper.go 84.85% <0.00%> (-0.37%) ⬇️

@zemyblue zemyblue requested review from 0Tech and dudong2 July 7, 2022 06:18
@zemyblue zemyblue marked this pull request as ready for review July 7, 2022 06:18
Signed-off-by: zemyblue <[email protected]>
@zemyblue zemyblue merged commit 5fabafa into Finschia:main Jul 8, 2022
@zemyblue zemyblue deleted the remove_prove_in_tx branch October 19, 2022 02:51
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.

3 participants