secp256k1: Reduce compat shim inputs mod prime.#3620
Merged
davecgh merged 4 commits intodecred:masterfrom Feb 24, 2026
Merged
Conversation
matheusd
approved these changes
Feb 20, 2026
Comment on lines
+43
to
+46
| x1: "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc30", | ||
| y1: "4218f20ae6c646b363db68605822fb14264ca8d2587fdd6fbc750d587e76a7ee", | ||
| x2: "1", | ||
| y2: "4218f20ae6c646b363db68605822fb14264ca8d2587fdd6fbc750d587e76a7ee", |
Member
There was a problem hiding this comment.
Verified manually
P = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F
print(hex(P+1))
# == x1
Comment on lines
+49
to
+52
| x1: "1", | ||
| y1: "014218f20ae6c646b363db68605822fb14264ca8d2587fdd6fbc750d577e76a41d", | ||
| x2: "1", | ||
| y2: "4218f20ae6c646b363db68605822fb14264ca8d2587fdd6fbc750d587e76a7ee", |
Member
There was a problem hiding this comment.
Verified manually
P = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F
print(hex(P+0x4218f20ae6c646b363db68605822fb14264ca8d2587fdd6fbc750d587e76a7ee))
# == x1
davecgh
force-pushed
the
secp256k1_reduce_compat_shim
branch
from
d573927 to
8030f3d
Compare
Member
Author
|
Since the the standard lib |
jrick
approved these changes
Feb 20, 2026
This converts the crypto/elliptic compat shim tests to use table-driven tests and adds a few additional tests for it.
This adds some tests for using unreduced inputs in some of the crypto/elliptic compat shim methods that currently fail. An upcoming commit will reduce the inputs modulo the field prime which will cause the tests to pass.
It was reported that fuzz testing discovered that some inputs to the crypto/elliptic compat shim point addition method that are not reduced do not produce expected points. It is highly debatable whether that behavior should be considered a bug because it is realistically an improper use of the stdlib curve interface to call it with unreduced points as there are no guarantees by the interface contract that it will reduce them, and its documentation even explicitly states "The behavior of Add, Double, and ScalarMult when the input is not a point on the curve is undefined." Further, none of the methods in the lib, even when using the compatibility shim, nor any real crypto code for that matter, will ever produce points with unreduced coordinates. So, in practice, no real code would ever encounter the observed behavior. Nevertheless, it is still a good idea to reduce them anyway so that callers that failed to do so will still get the same results they would if they had properly reduced the inputs themselves. With that in mind, this modifies the method that converts big integers in all of the crypto/elliptic compat shim methods to reduce the inputs modulo the field prime prior to conversion to ensure the result is always in the expected range and thus all equivalence classes result in strict equality.
The standard lib elliptic.Curve interface has been deprecated since August of 2023, so this deprecates all methods that implement it. While here, it also adds some additional notes for behavior the interface requires which might be surprising for consumers.
davecgh
force-pushed
the
secp256k1_reduce_compat_shim
branch
from
8030f3d to
76c0dc4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It was reported that fuzz testing discovered that some inputs to the crypto/elliptic compat shim point addition method that are not reduced do not produce expected points.
It is highly debatable whether that behavior should be considered a bug because it is realistically an improper use of the stdlib curve interface to call it with unreduced points as there are no stated guarantees by the interface contract that it will reduce them, and its documentation even explicitly states:
Further, none of the methods in the lib, even when using the compatibility shim, nor any real crypto code for that matter, will ever produce points with unreduced coordinates. So, in practice, no real code would ever encounter the observed behavior.
Nevertheless, it is still a good idea to reduce them anyway so that callers that failed to do so will still get the same results they would if they had properly reduced the inputs themselves.
With that in mind, this modifies the method that converts big integers in all of the crypto/elliptic compat shim methods to reduce the inputs modulo the field prime prior to conversion to ensure the result is always in the expected range and thus all equivalence classes result in strict equality.
It also adds additional tests to ensure the new behavior works as intended via separate commits. The second commit contains tests that fail under the current behavior but pass with the new behavior.
Since the the standard lib
elliptic.Curveinterface has been deprecated since August of 2023, this deprecates all methods that implement it.Finally, while here, it also adds some additional notes for behavior the interface requires which might be surprising for consumers.
See each commit for more details.