Skip to content

crypto: fix TestToECDSAErrors for nocgo#15745

Merged
fjl merged 2 commits into
ethereum:masterfrom
dindinw:master
Jan 2, 2018
Merged

crypto: fix TestToECDSAErrors for nocgo#15745
fjl merged 2 commits into
ethereum:masterfrom
dindinw:master

Conversation

@dindinw
Copy link
Copy Markdown
Contributor

@dindinw dindinw commented Dec 24, 2017

see #15744

@GitCop
Copy link
Copy Markdown

GitCop commented Dec 24, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 332931066a41f79ffd3801410365baee08a42afd
  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@dindinw dindinw changed the title fix TestToECDSAErrors for nocgo crypto: fix TestToECDSAErrors for nocgo Dec 24, 2017
Comment thread crypto/crypto.go Outdated
}
// The priv.D must not be zero or negative.
zero := new(big.Int).SetInt64(0)
if priv.D.Cmp(zero) <= 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A better way to check this is to use priv.D.Sign() <= 0.

@fjl
Copy link
Copy Markdown
Contributor

fjl commented Dec 28, 2017

Thank you so much for looking into this.

@dindinw
Copy link
Copy Markdown
Contributor Author

dindinw commented Dec 31, 2017

@fjl changed to use priv.D.Sign() as your suggestion, please review again. Thanks

Copy link
Copy Markdown
Contributor Author

@dindinw dindinw left a comment

Choose a reason for hiding this comment

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

use priv.D.Sign() <= 0

@fjl fjl merged commit 6cd6b92 into ethereum:master Jan 2, 2018
@fjl fjl assigned frozeman and unassigned frozeman Jan 2, 2018
@fjl fjl added this to the 1.8.0 milestone Jan 2, 2018
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.

4 participants