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

secp256k1: Add fixed-precision group order type. #2060

Merged
merged 1 commit into from
Feb 19, 2020

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Feb 7, 2020

This requires #2059.

This introduces a specialized type for handling arithmetic modulo the group order which is significantly faster than using big integers from the standard library

The implementation is also constant time, whereas big integers from the standard library are not, which helps protect against side channel attacks in certain scenarios.

Comprehensive tests with 100% coverage and a full suite of benchmarks are included.

Due to the fact the code relies on a lot of low-level bit manipulations to achieve the speed and constant time characteristics, it is not the easiest code to review for correctness, so significant effort has been put into attempting to clearly explain the details via comments to help with review.

For now, this merely introduces the type without modifying any of the code to make use of it, and it also does not yet include the ability to perform inversions which will be required in order to switch over to it. That capability will be added in a future commit.

The following benchmarks show a comparison between the new specialized type and generic big ints for various operations:

benchmark                  old ns/op   new ns/op     delta
------------------------------------------------------------
BenchmarkModN              267         18.9          -92.92%
BenchmarkZero              7.28        0.64          -91.25%
BenchmarkIsZero            0.32        0.32          +0.00%
BenchmarkEquals            9.14        5.58          -38.95%
BenchmarkAddModN           305         23.8          -92.20%
BenchmarkMulModN           457         237           -48.14%
BenchmarkSquareModN        459         238           -48.15%
BenchmarkNegateModN        295         9.82          -96.67%
BenchmarkIsOverHalfOrder   9.25        8.55          -7.57%

benchmark                  old allocs   new allocs   delta
------------------------------------------------------------
BenchmarkModN              2            0            -100.00%
BenchmarkZero              0            0            +0.00%
BenchmarkIsZero            0            0            +0.00%
BenchmarkEquals            0            0            +0.00%
BenchmarkAddModN           2            0            -100.00%
BenchmarkMulModN           2            0            -100.00%
BenchmarkSquareModN        2            0            -100.00%
BenchmarkNegateModN        2            0            -100.00%
BenchmarkIsOverHalfOrder   0            0            +0.00%

@davecgh davecgh added this to the 1.6.0 milestone Feb 7, 2020
@davecgh davecgh force-pushed the secp256k1_modnscalar branch from 0099740 to 5de2f48 Compare February 7, 2020 07:02
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
@davecgh
Copy link
Member Author

davecgh commented Feb 8, 2020

Test coverage:

$ go test -coverprofile=cov.out && go tool cover -func=cov.out | grep modnscalar | sed -e "s#github.com/decred/dcrd/dcrec/secp256k1/v3/##"
modnscalar.go:142:              String                  100.0%
modnscalar.go:152:              Set                     100.0%
modnscalar.go:160:              Zero                    100.0%
modnscalar.go:172:              IsZero                  100.0%
modnscalar.go:184:              SetInt                  100.0%
modnscalar.go:191:              constantTimeEq          100.0%
modnscalar.go:196:              constantTimeNotEq       100.0%
modnscalar.go:201:              constantTimeLess        100.0%
modnscalar.go:206:              constantTimeLessOrEq    100.0%
modnscalar.go:211:              constantTimeGreater     100.0%
modnscalar.go:216:              constantTimeGreaterOrEq 100.0%
modnscalar.go:221:              constantTimeMin         100.0%
modnscalar.go:227:              overflows               100.0%
modnscalar.go:258:              reduce256               100.0%
modnscalar.go:297:              SetBytes                100.0%
modnscalar.go:328:              SetByteSlice            100.0%
modnscalar.go:340:              PutBytes                100.0%
modnscalar.go:382:              Bytes                   100.0%
modnscalar.go:389:              IsOdd                   100.0%
modnscalar.go:395:              Equals                  100.0%
modnscalar.go:410:              Add2                    100.0%
modnscalar.go:439:              Add                     100.0%
modnscalar.go:450:              Add                     100.0%
modnscalar.go:460:              Rsh32                   100.0%
modnscalar.go:468:              reduce385               100.0%
modnscalar.go:597:              reduce512               100.0%
modnscalar.go:761:              Mul2                    100.0%
modnscalar.go:922:              Mul                     100.0%
modnscalar.go:931:              SquareVal               100.0%
modnscalar.go:948:              Square                  100.0%
modnscalar.go:957:              NegateVal               100.0%
modnscalar.go:997:              Negate                  100.0%
modnscalar.go:1075:             IsOverHalfOrder         100.0%

Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

This was a doozy to review indeed :P

Only two small nits.

dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Show resolved Hide resolved
@davecgh davecgh force-pushed the secp256k1_modnscalar branch from 97b0625 to 9d41b55 Compare February 13, 2020 20:21
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
@davecgh davecgh force-pushed the secp256k1_modnscalar branch 2 times, most recently from 5646278 to 2baa8f2 Compare February 13, 2020 20:23
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

Woot! 🎉

dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
dcrec/secp256k1/modnscalar.go Outdated Show resolved Hide resolved
@davecgh davecgh force-pushed the secp256k1_modnscalar branch from 2baa8f2 to 406e4d3 Compare February 15, 2020 23:44
Copy link
Member

@dnldd dnldd left a comment

Choose a reason for hiding this comment

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

Wheew, looks good.

@davecgh davecgh force-pushed the secp256k1_modnscalar branch from 406e4d3 to 98b4c94 Compare February 19, 2020 05:13
This introduces a specialized type for handling arithmetic modulo the
group order which is significantly faster than using big integers from
the standard library

The implementation is also constant time, whereas big integers from the
standard library are not, which helps protect against side channel
attacks in certain scenarios.

Comprehensive tests with 100% coverage and a full suite of benchmarks
are included.

Due to the fact the code relies on a lot of low-level bit manipulations
to achieve the speed and constant time characteristics, it is not the
easiest code to review for correctness, so significant effort has been
put into attempting to clearly explain the details via comments to help
with review.

For now, this merely introduces the type without modifying any of the
code to make use of it, and it also does not yet include the ability to
perform inversions which will be required in order to switch over to it.
That capability will be added in a future commit.

The following benchmarks show a comparison between the new specialized
type and generic big ints for various operations:

benchmark                  old ns/op   new ns/op     delta
------------------------------------------------------------
BenchmarkModN              267         18.9          -92.92%
BenchmarkZero              7.28        0.64          -91.25%
BenchmarkIsZero            0.32        0.32          +0.00%
BenchmarkEquals            9.14        5.58          -38.95%
BenchmarkAddModN           305         23.8          -92.20%
BenchmarkMulModN           457         237           -48.14%
BenchmarkSquareModN        459         238           -48.15%
BenchmarkNegateModN        295         9.82          -96.67%
BenchmarkIsOverHalfOrder   9.25        8.55          -7.57%

benchmark                  old allocs   new allocs   delta
------------------------------------------------------------
BenchmarkModN              2            0            -100.00%
BenchmarkZero              0            0            +0.00%
BenchmarkIsZero            0            0            +0.00%
BenchmarkEquals            0            0            +0.00%
BenchmarkAddModN           2            0            -100.00%
BenchmarkMulModN           2            0            -100.00%
BenchmarkSquareModN        2            0            -100.00%
BenchmarkNegateModN        2            0            -100.00%
BenchmarkIsOverHalfOrder   0            0            +0.00%
@davecgh davecgh force-pushed the secp256k1_modnscalar branch from 98b4c94 to 6889976 Compare February 19, 2020 05:15
@davecgh davecgh merged commit 6889976 into decred:master Feb 19, 2020
@davecgh davecgh deleted the secp256k1_modnscalar branch February 19, 2020 05:17
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