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

R4R: Fixed merkle proof error #3236

Closed
wants to merge 3 commits into from
Closed

R4R: Fixed merkle proof error #3236

wants to merge 3 commits into from

Conversation

ironman0x7b2
Copy link
Contributor

@ironman0x7b2 ironman0x7b2 commented Jan 5, 2019

  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added entries in PENDING.md with issue #

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@ironman0x7b2 ironman0x7b2 changed the title Fixed merkle proof error R4R: Fixed merkle proof error Jan 5, 2019
@codecov
Copy link

codecov bot commented Jan 5, 2019

Codecov Report

Merging #3236 into develop will decrease coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #3236      +/-   ##
===========================================
- Coverage    54.88%   54.87%   -0.01%     
===========================================
  Files          133      133              
  Lines         9555     9556       +1     
===========================================
  Hits          5244     5244              
- Misses        3989     3990       +1     
  Partials       322      322

@codecov
Copy link

codecov bot commented Jan 5, 2019

Codecov Report

Merging #3236 into develop will increase coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #3236      +/-   ##
===========================================
+ Coverage    55.35%   55.37%   +0.02%     
===========================================
  Files          134      134              
  Lines         9594     9594              
===========================================
+ Hits          5311     5313       +2     
+ Misses        3951     3949       -2     
  Partials       332      332

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a small remark on the changes, but they LGTM 👍

Also, this warrants a pending log entry. Any chance we could test this behavior too?

client/context/query.go Show resolved Hide resolved
@jackzampolin
Copy link
Member

This looks like a duplicate PR of #3191

@abelliumnt
Copy link

This PR has the same issue as #3191. In this PR, the lcd test doesn't run in distrust mode, so all tests passed.

@cwgoes
Copy link
Contributor

cwgoes commented Jan 29, 2019

Closing in favor of #3191.

@cwgoes cwgoes closed this Jan 29, 2019
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.

5 participants