From cfcea693f61e75ef016914ef8858db8323c0577a Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Fri, 6 Sep 2024 21:13:20 -0500 Subject: [PATCH] secp256k1: Return normalized val from DecompressY. The result of the DecompressY function, as the documentation describes, currently returns a value that either has a maximum magnitude of 1 or 2 which also implies that it is not necessarily normalized either. This means the caller is currently responsible for normalization. While there is no logic issue with that approach, it does mean that callers realistically have to unconditionally normalize the result for almost all realistic use cases even though it might not actually need it. Those extra normalizations can result in a minor average comparative performance loss when amortized across millions of point decompressions. Further, putting the responsibility on the caller makes it easier make a mistake. To improve both of those cases, this updates the DecompressY function to normalize the result in all cases when the result is a valid point on the secp256k1 curve (aka the function returns true) before returning it and updates the callers accordingly. This change also means the result will now always have a max magnitude of 1. --- dcrec/secp256k1/curve.go | 11 ++++++++--- dcrec/secp256k1/curve_test.go | 4 ---- dcrec/secp256k1/ecdsa/signature.go | 7 ++++--- dcrec/secp256k1/pubkey.go | 3 +-- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/dcrec/secp256k1/curve.go b/dcrec/secp256k1/curve.go index b0df22c07e..6d6d669f19 100644 --- a/dcrec/secp256k1/curve.go +++ b/dcrec/secp256k1/curve.go @@ -1285,8 +1285,13 @@ func isOnCurve(fx, fy *FieldVal) bool { // based on the desired oddness and returns whether or not it was successful // since not all X coordinates are valid. // -// The magnitude of the provided X coordinate field val must be a max of 8 for a -// correct result. The resulting Y field val will have a max magnitude of 2. +// The magnitude of the provided X coordinate field value must be a max of 8 for +// a correct result. The resulting Y field value will have a magnitude of 1. +// +// Preconditions: +// - The input field value MUST have a max magnitude of 8 +// Output Normalized: Yes if the func returns true, no otherwise +// Output Max Magnitude: 1 func DecompressY(x *FieldVal, odd bool, resultY *FieldVal) bool { // The curve equation for secp256k1 is: y^2 = x^3 + 7. Thus // y = +-sqrt(x^3 + 7). @@ -1299,7 +1304,7 @@ func DecompressY(x *FieldVal, odd bool, resultY *FieldVal) bool { return false } if resultY.Normalize().IsOdd() != odd { - resultY.Negate(1) + resultY.Negate(1).Normalize() } return true } diff --git a/dcrec/secp256k1/curve_test.go b/dcrec/secp256k1/curve_test.go index 9f5e11dd79..8868674226 100644 --- a/dcrec/secp256k1/curve_test.go +++ b/dcrec/secp256k1/curve_test.go @@ -1194,7 +1194,6 @@ func TestDecompressY(t *testing.T) { } // Ensure the decompressed odd Y coordinate is the expected value. - oddY.Normalize() wantOddY := new(FieldVal).SetHex(test.wantOddY) if !wantOddY.Equals(&oddY) { t.Errorf("%s: mismatched odd y\ngot: %v, want: %v", test.name, @@ -1203,7 +1202,6 @@ func TestDecompressY(t *testing.T) { } // Ensure the decompressed even Y coordinate is the expected value. - evenY.Normalize() wantEvenY := new(FieldVal).SetHex(test.wantEvenY) if !wantEvenY.Equals(&evenY) { t.Errorf("%s: mismatched even y\ngot: %v, want: %v", test.name, @@ -1264,8 +1262,6 @@ func TestDecompressYRandom(t *testing.T) { // Ensure that the resulting y coordinates match their respective // expected oddness. - oddY.Normalize() - evenY.Normalize() if !oddY.IsOdd() { t.Fatalf("requested odd y is even for x = %v", x) } diff --git a/dcrec/secp256k1/ecdsa/signature.go b/dcrec/secp256k1/ecdsa/signature.go index dfee489e72..84d61c956e 100644 --- a/dcrec/secp256k1/ecdsa/signature.go +++ b/dcrec/secp256k1/ecdsa/signature.go @@ -1,5 +1,5 @@ // Copyright (c) 2013-2014 The btcsuite developers -// Copyright (c) 2015-2022 The Decred developers +// Copyright (c) 2015-2024 The Decred developers // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. @@ -931,6 +931,7 @@ func RecoverCompact(signature, hash []byte) (*secp256k1.PublicKey, bool, error) // r = r + N (mod P) fieldR.Add(&orderAsFieldVal) } + fieldR.Normalize() // Step 4. // @@ -952,8 +953,8 @@ func RecoverCompact(signature, hash []byte) (*secp256k1.PublicKey, bool, error) // // X = (r, y) var X secp256k1.JacobianPoint - X.X.Set(fieldR.Normalize()) - X.Y.Set(y.Normalize()) + X.X.Set(&fieldR) + X.Y.Set(&y) X.Z.SetInt(1) // Step 6. diff --git a/dcrec/secp256k1/pubkey.go b/dcrec/secp256k1/pubkey.go index 54c54be5f1..2f8815bedf 100644 --- a/dcrec/secp256k1/pubkey.go +++ b/dcrec/secp256k1/pubkey.go @@ -1,5 +1,5 @@ // Copyright (c) 2013-2014 The btcsuite developers -// Copyright (c) 2015-2022 The Decred developers +// Copyright (c) 2015-2024 The Decred developers // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. @@ -177,7 +177,6 @@ func ParsePubKey(serialized []byte) (key *PublicKey, err error) { "the secp256k1 curve", x) return nil, makeError(ErrPubKeyNotOnCurve, str) } - y.Normalize() default: str := fmt.Sprintf("malformed public key: invalid length: %d",