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

Remove ripemd160 entirely #2308

Merged
merged 2 commits into from
Sep 12, 2018
Merged

Remove ripemd160 entirely #2308

merged 2 commits into from
Sep 12, 2018

Conversation

ValarDragon
Copy link
Contributor

@ValarDragon ValarDragon commented Sep 11, 2018

We already made this decision awhile ago, and have had tendermint switched
for awhile. I was surprised to find ripemd still used within the storeinfo.
This actually leads me to think the new "byter" API change in the tendermint
PR, for RFC compliance is better, as it avoids things like this from ever happening.

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

  • 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)

We already made this decision awhile ago, and have had tendermint switched
for awhile. I was surprised to find ripemd still used within the storeinfo.
This actually leads me to think the new "byter" API change in the tendermint
PR RFC compliance is better, as it avoids things like this from ever happening.
@ValarDragon ValarDragon added ready-for-review T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). labels Sep 11, 2018
@codecov
Copy link

codecov bot commented Sep 11, 2018

Codecov Report

Merging #2308 into develop will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           develop    #2308   +/-   ##
========================================
  Coverage    63.94%   63.94%           
========================================
  Files          140      140           
  Lines         8689     8689           
========================================
  Hits          5556     5556           
  Misses        2757     2757           
  Partials       376      376

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

utACK

Agreed on all counts.

@cwgoes cwgoes merged commit 358b487 into develop Sep 12, 2018
@cwgoes cwgoes deleted the dev/remove_ripemd160 branch September 12, 2018 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants