Add SecretPoint. Improve SecretScalar by maurges · Pull Request #92 · LFDT-Lockness/generic-ec

maurges · 2026-06-17T14:20:46Z

SecretPoint is like SecretScalar but for points
Add Zeroize instance to scalar and point byte representations
SecretPoint and SecretScalar provide methods to get secret bytes

Signed-off-by: maurges <git@morj.men>

For some reason cargo build --no-default-features didn't fully check every definition? Signed-off-by: maurges <git@morj.men>

Signed-off-by: maurges <git@morj.men>

survived · 2026-06-23T07:30:30Z

+    pub(super) fn inner<T>(x: Secret<T>) -> T
+    where
+        T: zeroize::Zeroize + Clone,
+    {
+        (**x).clone()


The function signature suggest that it takes ownership of x and retrieves the wrapped value, where as in reality it just clones the wrapped value.

I think it should accept x: &Secret<T>. It's not only clearer, but also more efficient, as we don't need to increment/decrement Arc's reference counter

Another name could be a better fit. E.g. clone_inner?

When I was implementing this, I was thinking about the non-Arc case: you take the secret value, take it out of the Zeroizing, and obtain a non-secret. But then I realized that you can't into_inner the Zeroizing struct, and can only clone from within, but I was commited to the function idea. Now I need to reflect on what I want from it.

survived · 2026-06-23T07:35:22Z

+    }
+
+    /// Convert the representation into not-secret, dropping all guarantees
+    pub fn into_not_secret(self) -> EncodedPoint<E> {


I like the naming. Probably, SecretScalar::as_ref() should be renamed into as_not_secret() to be clearer.

That would break the API, but we could deprecate as_ref and provide a new as_not_secret as the suggested replacement.

Ah I just realized that we can't do that, since we rely a lot on that &SecretScalar can be passed to a function that accepts x: impl AsRef<Scalar<E>>...

I can add the method with this name at least

Though its implementation will be as stupid as for the encoded one, with clone, because as I see I can't convert out of Zeroizing.

Unless we want to use unsafe transmute, which will work because Zeroizing is guaranteed to have the same representation

Though its implementation will be as stupid as for the encoded one, with clone, because as I see I can't convert out of Zeroizing.

Why? You can reference inner data (via Deref on Zeroizing), you don't need to clone it

I think this method would be a great addition. It would replicate existing AsRef implementation. We might consider in the future removing AsRef to force users using explicit method with "dangerous" name.

survived · 2026-06-23T07:41:22Z

+
+    /// Obtain the reference to the encoded bytes. This bypasses all the secrecy
+    /// guarantees, so be careful when handling them
+    pub fn as_bytes(&self) -> &[u8] {


in the spirit of into_not_secret method, I'm inclined to propose to rename this one to as_not_secret_bytes, though that becomes quite long (but maybe it's justified?)

Maybe slight shortening to as_nonsecret_bytes? I was initially debating the method name between into_not_secret and into_nonsecret

as_nonsecret_bytes sounds good to me!

Signed-off-by: maurges <git@morj.men>

survived · 2026-06-26T09:31:56Z

@@ -0,0 +1,55 @@
+//! Internals for both Secret structs, like `SecretScalar` and `SecretEncodedPoint`. They have a different representation based on the presence of `alloc`: either a reference-counted nonmoving one, or a moving value


(there are more than 2 secret structs)

Suggested change

//! Internals for both Secret structs, like `SecretScalar` and `SecretEncodedPoint`. They have a different representation based on the presence of `alloc`: either a reference-counted nonmoving one, or a moving value

//! Internals for Secret structs, like `SecretScalar` and `SecretEncodedPoint`. They have a different representation based on the presence of `alloc`: either a reference-counted nonmoving one, or a moving value

survived · 2026-06-26T09:34:11Z

+    }
+
+    /// Convert the representation into not-secret, dropping all guarantees
+    pub fn into_not_secret(self) -> EncodedPoint<E> {


Though its implementation will be as stupid as for the encoded one, with clone, because as I see I can't convert out of Zeroizing.

Why? You can reference inner data (via Deref on Zeroizing), you don't need to clone it

survived · 2026-06-26T09:34:38Z

+        value_on_heap
+    }
+
+    pub fn inner<T>(x: Secret<T>) -> T


clone_inner feels like better name for it

maurges added 6 commits June 17, 2026 11:27

Add Zeroize impl for encoded point and scalar

4baa567

Signed-off-by: maurges <git@morj.men>

Add SecretPoint. Add to_bytes method for SecretScalar

3059d17

Signed-off-by: maurges <git@morj.men>

Simplify the definition of SecretScalar

328e62c

Signed-off-by: maurges <git@morj.men>

Add non-zero SecretPoint and other missing impls

414a661

Signed-off-by: maurges <git@morj.men>

Remove unnecessary docsrs cfg

19909f3

Signed-off-by: maurges <git@morj.men>

Fix readme and no-std build

d2b2917

For some reason cargo build --no-default-features didn't fully check every definition? Signed-off-by: maurges <git@morj.men>

maurges marked this pull request as ready for review June 17, 2026 14:33

maurges requested a review from survived June 17, 2026 14:34

survived reviewed Jun 17, 2026

View reviewed changes

Comment thread generic-ec/src/non_zero/mod.rs

Comment thread generic-ec/src/secret_scalar/mod.rs Outdated

Comment thread generic-ec/src/secret_scalar/mod.rs Outdated

Comment thread generic-ec/src/non_zero/mod.rs

Add EncodedSecret*. More symmetry in secret point and scalar

108a855

Signed-off-by: maurges <git@morj.men>

survived reviewed Jun 23, 2026

View reviewed changes

maurges added 2 commits June 23, 2026 17:46

Remove nonsecret-leaking trait impls

bdeb16d

Signed-off-by: maurges <git@morj.men>

Unify the definitions of all secret structs

33495a7

Signed-off-by: maurges <git@morj.men>

survived reviewed Jun 26, 2026

View reviewed changes

		@@ -0,0 +1,55 @@
		//! Internals for both Secret structs, like `SecretScalar` and `SecretEncodedPoint`. They have a different representation based on the presence of `alloc`: either a reference-counted nonmoving one, or a moving value

	//! Internals for both Secret structs, like `SecretScalar` and `SecretEncodedPoint`. They have a different representation based on the presence of `alloc`: either a reference-counted nonmoving one, or a moving value
	//! Internals for Secret structs, like `SecretScalar` and `SecretEncodedPoint`. They have a different representation based on the presence of `alloc`: either a reference-counted nonmoving one, or a moving value

Uh oh!

Conversation

maurges commented Jun 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants