Add Scalar newtype and use it in tweaking APIs
#445
Conversation
This adds `Scalar` newtype to better represent values accepted by tweaking functions. This type is always 32-bytes and guarantees being within curve order.
|
Very interesting. In future this also may give us a hook to provide direct rust implementations of some functionality (though we probably don't want to use it by default, for perf reasons) and/or support for reduced-size groups for exhaustive testing. |
|
Sneaking this in before #444 |
|
|
||
|
|
||
| /// Error returned when the value of scalar is invalid - larger than the curve order. | ||
| // Intentionally doesn't implement `Copy` to improve forward compatibility. |
There was a problem hiding this comment.
Why does implementing Copy hurt forward compatibility @Kixunil?
There was a problem hiding this comment.
I had the idea of a feature gate adding boxed input. (boxed to decrease cost of moving - this is common for error types)
|
For comparison: I did something similar last year |
|
|
||
| /// Tries to deserialize from big endian bytes | ||
| /// | ||
| /// **Security warning:** this function is not constant time! |
There was a problem hiding this comment.
If there is interest here, you can also add my implementation that is constant time for non-zero values.
BlockstreamResearch/rust-secp256k1-zkp@53d1947
I think it makes sense to use the _var suffix naming from upstream to signify variable time.
There was a problem hiding this comment.
Cool trick! I would like to make it constant time and in pure Rust in the future because it'll also be needed for private key checking without the C library. Looking at the C code, there doesn't seem to be anything C-specific that would make it constant time, so I think it should be doable.
There was a problem hiding this comment.
Should we rename things to _var before cutting a release?
There was a problem hiding this comment.
I would rather use @sanket1729 trick to not cause churn if we don't want non-constant-time functions without the suffix.
(Also secp256k1 can do the exact check we need but I guess the function is private.)
…t in tweaking APIs
5a0332463d1ae6b982258999b1ddf7ef0ef23a1f Add `Scalar` newtype and use it in tweaking APIs (Martin Habovstiak)
Pull request description:
This adds `Scalar` newtype to better represent values accepted by
tweaking functions. This type is always 32-bytes and guarantees being
within curve order.
This is an initial iteration. Things I'm not 100% sure of, would appreciate help:
* Is it actually required for scalar to be within curve order?
* Are non-constant-time operations actually an issue?
Alterntive tweaking API designs:
* Accept `Into<Scalar>` generic argument, so people can pass `SecretKey`, may slightly worsen performance
* `unsafe`-ly implement `AsRef<Scalar> for SecretKey` (casting raw pointers) and accept that
Possible extension: API to convert from slices so that the user doesn't need two steps (convert to array first).
ACKs for top commit:
apoelstra:
ACK 5a0332463d1ae6b982258999b1ddf7ef0ef23a1f
Tree-SHA512: cf639c91f0c6f6e0178d8a6d4125fd041708a59661fbd2f1924881001011e4cfe2b998148ae652995cdc9a3791c6644fcd5f004e059d76d9182a5e409b791446
…t in tweaking APIs
5a0332463d1ae6b982258999b1ddf7ef0ef23a1f Add `Scalar` newtype and use it in tweaking APIs (Martin Habovstiak)
Pull request description:
This adds `Scalar` newtype to better represent values accepted by
tweaking functions. This type is always 32-bytes and guarantees being
within curve order.
This is an initial iteration. Things I'm not 100% sure of, would appreciate help:
* Is it actually required for scalar to be within curve order?
* Are non-constant-time operations actually an issue?
Alterntive tweaking API designs:
* Accept `Into<Scalar>` generic argument, so people can pass `SecretKey`, may slightly worsen performance
* `unsafe`-ly implement `AsRef<Scalar> for SecretKey` (casting raw pointers) and accept that
Possible extension: API to convert from slices so that the user doesn't need two steps (convert to array first).
ACKs for top commit:
apoelstra:
ACK 5a0332463d1ae6b982258999b1ddf7ef0ef23a1f
Tree-SHA512: cf639c91f0c6f6e0178d8a6d4125fd041708a59661fbd2f1924881001011e4cfe2b998148ae652995cdc9a3791c6644fcd5f004e059d76d9182a5e409b791446
4 participants
This adds
Scalarnewtype to better represent values accepted bytweaking functions. This type is always 32-bytes and guarantees being
within curve order.
This is an initial iteration. Things I'm not 100% sure of, would appreciate help:
Alterntive tweaking API designs:
Into<Scalar>generic argument, so people can passSecretKey, may slightly worsen performanceunsafe-ly implementAsRef<Scalar> for SecretKey(casting raw pointers) and accept thatPossible extension: API to convert from slices so that the user doesn't need two steps (convert to array first).