Skip to content

Commit

Permalink
secp256k1: Return normalized val from DecompressY.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davecgh committed Sep 12, 2024
1 parent e28f4e4 commit 7333f80
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 12 deletions.
11 changes: 8 additions & 3 deletions dcrec/secp256k1/curve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
}
4 changes: 0 additions & 4 deletions dcrec/secp256k1/curve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down
7 changes: 4 additions & 3 deletions dcrec/secp256k1/ecdsa/signature.go
Original file line number Diff line number Diff line change
@@ -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.

Expand Down Expand Up @@ -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.
//
Expand All @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions dcrec/secp256k1/pubkey.go
Original file line number Diff line number Diff line change
@@ -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.

Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 7333f80

Please sign in to comment.