Browse Source

refactor: build the basepoints only on the call that loads them

`cmz_group_init` built a `CMZBasepoints` and handed it to `load_bp`, which
then dropped it on the floor unless the map was empty -- and `load_bp`
cloned it when it was not. Callers invoke this on every request, so take
a closure and construct only on the call that actually populates the map.

By itself that is a shape fix, not a measured win: `wnaf_is_constant_time`
is not a default feature and nothing enables it, so `CMZBasepoints` is
`{A_, B_}` and construction plus clone costs about 8ns. It matters under
that feature, where construction is two WnafBase tables.

The cost callers actually pay is the argument, not the call:
`cmz_group_init(G::hash_from_bytes::<Sha512>(...))` evaluates the
hash-to-curve eagerly, at roughly 4.7us per request, and nothing on the
callee side can reach it. So `cmz_group_init_with` takes the generator as
a closure too, and the hash happens once, on the call that initializes.
Both sides of a round trip call this, so it is roughly 9us a round trip
for a one-word change at each call site. `cmz_group_init` is unchanged.
Michele Orrù 4 weeks ago
parent
commit
db5a2b8552
1 changed files with 46 additions and 18 deletions
  1. 46 18
      src/lib.rs

+ 46 - 18
src/lib.rs

@@ -238,20 +238,27 @@ lazy_static! {
     static ref basepoints_map: StaticTypeMap<Box<dyn CMZbp>> = StaticTypeMap::new();
 }
 
-/// For a given group type `G`, if `bp` is `Some(b)`, then load the
-/// mapping from `G` to `b` into the `basepoints_map`.  (If a mapping
-/// from `G` already exists, the old one will be kept and the new one
-/// ignored.)  Whether `bp` is `Some(b)` or `None`, this function
-/// returns the (possibly new) target of the `basepoints_map`, as a
-/// `&'static CMZBasepoints<G>`.
-fn load_bp<G: Group>(bp: Option<CMZBasepoints<G>>) -> &'static CMZBasepoints<G> {
-    match bp {
-        Some(b) => basepoints_map.call_once::<G, _>(|| Box::new(b.clone())),
-        None => basepoints_map.call_once::<G, _>(|| panic!("basepoints uninitialized")),
-    }
-    .as_any()
-    .downcast_ref::<CMZBasepoints<G>>()
-    .unwrap()
+/// For a given group type `G`, return the target of the `basepoints_map`
+/// as a `&'static CMZBasepoints<G>`, calling `init` to build it if no
+/// mapping exists yet.  (If a mapping from `G` already exists, the old
+/// one is kept and `init` is never called.)
+///
+/// `init` is a closure rather than a value so that the basepoints (and,
+/// under `wnaf_is_constant_time`, their WnafBase tables) are built only
+/// on the call that actually populates the map.  Callers invoke this on
+/// every request.
+fn load_bp_with<G: Group>(init: impl FnOnce() -> CMZBasepoints<G>) -> &'static CMZBasepoints<G> {
+    basepoints_map
+        .call_once::<G, _>(|| Box::new(init()))
+        .as_any()
+        .downcast_ref::<CMZBasepoints<G>>()
+        .unwrap()
+}
+
+/// The already-loaded `CMZBasepoints<G>`; panics if `cmz_group_init` has
+/// not been called for `G`.
+fn load_bp<G: Group>() -> &'static CMZBasepoints<G> {
+    load_bp_with::<G>(|| panic!("basepoints uninitialized"))
 }
 
 /// Initialize the required second generator for a `PrimeGroup`.
@@ -277,18 +284,39 @@ fn load_bp<G: Group>(bp: Option<CMZBasepoints<G>>) -> &'static CMZBasepoints<G>
 /// given group will need to use the same `A`.  You need to call this
 /// before doing any operations with a credential.
 pub fn cmz_group_init<G: PrimeGroup>(generator_A: G) {
-    let bp = CMZBasepoints::<G>::init(generator_A);
-    load_bp(Some(bp));
+    load_bp_with(|| CMZBasepoints::<G>::init(generator_A));
+}
+
+/// [`cmz_group_init`], deferring construction of `generator_A` to the call
+/// that actually initializes the group.
+///
+/// The basepoints are memoized, so calling `cmz_group_init` once per request
+/// is harmless -- except that its argument is evaluated first, every time.
+/// Callers typically derive `A` by hashing to the curve, which costs far more
+/// than the initialization it feeds:
+///
+/// ```no_run
+/// # use cmz::{cmz_group_init, cmz_group_init_with};
+/// # use curve25519_dalek::ristretto::RistrettoPoint as G;
+/// # use sha2::Sha512;
+/// // ~4.7us of hash-to-curve on every call, thrown away after the first:
+/// cmz_group_init(G::hash_from_bytes::<Sha512>(b"CMZ Generator A"));
+///
+/// // hashed once, on the call that initializes:
+/// cmz_group_init_with(|| G::hash_from_bytes::<Sha512>(b"CMZ Generator A"));
+/// ```
+pub fn cmz_group_init_with<G: PrimeGroup>(generator_A: impl FnOnce() -> G) {
+    load_bp_with(|| CMZBasepoints::<G>::init(generator_A()));
 }
 
 /// Get the loaded CMZBasepoints for the given group
 pub fn cmz_basepoints<G: PrimeGroup>() -> &'static CMZBasepoints<G> {
-    load_bp(None)
+    load_bp()
 }
 
 /// Compute a public key from a private key
 pub fn cmz_privkey_to_pubkey<G: PrimeGroup>(privkey: &CMZPrivkey<G>) -> CMZPubkey<G> {
-    let bp = load_bp::<G>(None);
+    let bp = load_bp::<G>();
     let X0: Option<G> = if privkey.muCMZ {
         Some(bp.mulB(&privkey.x0))
     } else {