Skip to content

k256::FieldElement: PrimeField + FromUniformBytes<64>#1703

Open
kayabaNerve wants to merge 2 commits intoRustCrypto:masterfrom
kayabaNerve:k256-fieldelement-fromuniformbytes-64
Open

k256::FieldElement: PrimeField + FromUniformBytes<64>#1703
kayabaNerve wants to merge 2 commits intoRustCrypto:masterfrom
kayabaNerve:k256-fieldelement-fromuniformbytes-64

Conversation

@kayabaNerve
Copy link
Copy Markdown
Contributor

I don't love this implementation. crypto_bigint could generate a faster
modular reduction, and I'm sure the tailored arithmetic would also be faster if
this was expressed as a linear combination of hi * 2**256 + lo. This is just
the most direct possible way to implement this.

As for why perform a hit and run for this specific function, I maintain an
implementation of secq256k1 where the Scalar, FieldElement types are
secp256k1's FieldElement, Scalar types (respectively). This means I need
all functions for FieldElement as one would want (or as I would want) from
Scalar. I wanted to make this PR now as obviously, many RustCrypto crates
have recently moved out of rc, so I'm double checking if anything slipped
through the cracks on my end for features I want to ensure are included.
Historically, I implemented this trait for the Scalar types with
#1379. While that included
feedback on deferring to Reduce, this type does not have any outstanding
implementations of Reduce.


Same rationale as the prior PR, yet the implementation is effectively
copy/pasted from Scalar and should be without reservation.


s/prior PR/prior commit/

The above is my commit messages, which I find sufficiently descriptive for the PR as a whole.

This satisfies the type contracts I currently require in my secq256k1 library, allowing me to replace my bespoke type (which largely just defers to crypto-bigint, which is amazing for things like this). While I did verify this satisfies the type contract after backporting to the 0.13 releases, I realized the 0.13 FieldElement isn't safe for public consumption as it isn't normalized, despite implementing a variety of the traits from ff which don't allow representing non-normalized values. I'm reminded of the inactive PR on curve25519-dalek which showed one can do this with the type system, but it becomes quite complicated (dalek-cryptography/curve25519-dalek#816). For my own current use case, I just want to be able to, as a consumer, use the FieldElement.

Shotgunning calls to normalize appears to work, but isn't something I'm actually fine moving to. I'll make an issue to track this other blocker properly, as I believe this issue is still present in the current HEAD.

I don't _love_ this implementation. `crypto_bigint` could generate a faster
modular reduction, and I'm sure the tailored arithmetic would also be faster if
this was expressed as a linear combination of `hi * 2**256 + lo`. This is just
the most direct possible way to implement this.

As for why perform a hit and run for this specific function, I maintain an
implementation of secq256k1 where the `Scalar`, `FieldElement` types are
secp256k1's `FieldElement`, `Scalar` types (respectively). This means I need
all functions for `FieldElement` as one would want (or as I would want) from
`Scalar`. I wanted to make this PR now as obviously, many RustCrypto crates
have recently moved out of `rc`, so I'm double checking if anything slipped
through the cracks on my end for features I want to ensure are included.
Historically, I implemented this trait for the `Scalar` types with
RustCrypto#1379. While that included
feedback on deferring to `Reduce`, this type does not have any outstanding
implementations of `Reduce`.
Same rationale as the prior PR, yet the implementation is effectively
copy/pasted from `Scalar` and should be without reservation.
@kayabaNerve
Copy link
Copy Markdown
Contributor Author

Instead of an issue for my remaining issues on trying to make this usable, I added this comment to the relevant issue: #531 (comment)

@tarcieri
Copy link
Copy Markdown
Member

tarcieri commented Apr 8, 2026

Aah, I was going to say why not use wide* then I saw it's only implemented for scalars /cc @fjarri

@kayabaNerve
Copy link
Copy Markdown
Contributor Author

Yeah, unfortunately, it seems like k256 and curve25519-dalek have neglected field element code (k256 exposing one which panics if used via the ff API, dalek not exposing one at all). I think overall, given the gap between:

  • What people want from field elements as for point addition
  • What people want from field elements in general

There likely has to be an overarching design decision on how to move forward and the future of expose-field features for both libs (where I group the two as they both matter to me, both have this discussion open, and are both effectively under RustCrypto's maintainership as of right now). My advocacy would be to publicly expose normalized types without footguns nor caveats, but if we want to maintain the performance of point addition, that likely means the published type has to be distinct from the type used internally. That was sketched here for dalek: dalek-cryptography/curve25519-dalek#787 but pushed back on (reasonably, as having two types is poor) here: dalek-cryptography/curve25519-dalek#787 (comment)

That just doesn't change this is a continued "want" on my end, with many possible options, and no agreed path forward :/

@tarcieri
Copy link
Copy Markdown
Member

tarcieri commented Apr 8, 2026

#531 and introducing a LazyFieldElement type similar to the p521::LooseFieldElement type seems like the way forward there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants