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.)
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).