Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Disable ED25519 Account Keys #3763

Merged
merged 2 commits into from
Mar 1, 2019
Merged

Disable ED25519 Account Keys #3763

merged 2 commits into from
Mar 1, 2019

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Feb 28, 2019 •
edited
Loading

Disable the use of ED25519 account key pairs as these result in completely untested code execution paths.

closes: #3751

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

@alexanderbez alexanderbez added WIP T: State Machine Breaking State machine breaking changes (impacts consensus). labels Feb 28, 2019
@alexanderbez alexanderbez marked this pull request as ready for review February 28, 2019 19:09
@codecov
Copy link

codecov bot commented Feb 28, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@10bd98e). Click here to learn what that means.
The diff coverage is 70%.

@@            Coverage Diff             @@
##             develop    #3763   +/-   ##
==========================================
  Coverage           ?   61.35%           
==========================================
  Files              ?      189           
  Lines              ?    14098           
  Branches           ?        0           
==========================================
  Hits               ?     8650           
  Misses             ?     4893           
  Partials           ?      555

@jackzampolin
Copy link
Member

Way better errors here.

alessio
alessio approved these changes Mar 1, 2019
View reviewed changes
Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Just a small comment, looks good otherwise

@jackzampolin jackzampolin merged commit 5d8b949 into develop Mar 1, 2019
@jackzampolin jackzampolin deleted the bez/3751-disable-ed25519-acc-keys branch March 1, 2019 17:12
cwgoes
cwgoes reviewed Mar 2, 2019
View reviewed changes
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.

LGTM, thanks @alexanderbez

@assafmo
Copy link
Contributor

assafmo commented May 7, 2020

Is this permanent?

@snyk-bot snyk-bot mentioned this pull request Sep 5, 2021
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@cwgoes cwgoes cwgoes left review comments

@alessio alessio alessio approved these changes

@jackzampolin jackzampolin jackzampolin approved these changes

@ebuchman ebuchman Awaiting requested review from ebuchman

@rigelrozanski rigelrozanski Awaiting requested review from rigelrozanski

@mossid mossid Awaiting requested review from mossid

Assignees
No one assigned
Labels
T: State Machine Breaking State machine breaking changes (impacts consensus).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Disable ed25519 account keys
5 participants
@alexanderbez @jackzampolin @assafmo @alessio @cwgoes