pkc%feat: adjust APIs in preparation for downstream integration, derive Copy on non-secret operational types, extend CodeQL to enforce trait parity

maint/codeql/rust/lib/imports.qll

@@ -53,17 +53,20 @@ predicate isPublicMod(Module m) {
   not exists(m.getVisibility().getPath())
 }
 
+/** Holds if `u` sits directly inside the crate-root module `name`. */
+private predicate isInRootModule(Use u, string name) {
+  exists(Module m |
+    m.getName().getText() = name and
+    u.getParentNode() = m.getItemList() and
+    isRootModule(m)
+  )
+}
+
 /**
  * Holds if `u` lives inside a crate-root `__private` module
  * (intentional crate-level re-exports for macro support).
  */
-predicate isMacroReexport(Use u) {
-  exists(Module priv |
-    priv.getName().getText() = "__private" and
-    u.getParentNode() = priv.getItemList() and
-    isRootModule(priv)
-  )
-}
+predicate isMacroReexport(Use u) { isInRootModule(u, "__private") }
 
 /** Holds if `u` is an allowlisted re-export from a foreign crate. */
 private predicate isAllowlistedReexport(Use u) {
@@ -74,6 +77,29 @@ private predicate isAllowlistedReexport(Use u) {
     u.getUseTree().getPath().getSegment().getIdentifier().getText() = "Numeric"
   )
   or
+  fileOf(u).getAbsolutePath().matches("%pkgs/pkc/%") and
+  // Crate emits types relying on types or traits defined by a dependency, part of public API
+  isInRootModule(u, "__deps") and
+  (
+    usePrefix(u) = "blst"
+    or
+    usePrefix(u) = "dash_num"
+    or
+    usePrefix(u) = "dash_types"
+    or
+    usePrefix(u) = "ff"
+    or
+    usePrefix(u) = "group"
+    or
+    usePrefix(u) = "rand_core"
+    or
+    usePrefix(u) = "secp256k1"
+    or
+    usePrefix(u) = "subtle"
+    or
+    usePrefix(u) = "zeroize"
+  )
+  or
   fileOf(u).getAbsolutePath().matches("%pkgs/script/%") and
   (
     // Workaround for the orphan rule, not part of public API

maint/codeql/rust/lib/pkc.qll

@@ -9,9 +9,11 @@
 /**
  * Holds if `name` belongs to `arm` alone for `role`, exempting the other arms
  * from offering it. Rows live in `pkc.model.yml`.
- *
- * The rows name what an arm lacks rather than what the arms share, so a
- * method added to one arm and forgotten in another is reported with no list
- * to maintain.
  */
 extensible predicate armOnly(string arm, string role, string name);
+
+/**
+ * Holds if `arm` cannot carry `trait` at `role`, though the other arms do.
+ * Rows live in `pkc.model.yml`.
+ */
+extensible predicate armLacksTrait(string arm, string role, string trait);

maint/codeql/rust/lib/traits.qll

@@ -102,6 +102,32 @@ predicate implementsTrait(TypeItem t, string traitName) {
   exists(Impl i | i.getSelf() = t and implTraitName(i) = traitName)
 }
 
+/**
+ * Holds if `i`'s trait reference carries a type or const argument, naming
+ * `From<Foo>` rather than `Copy`. A lifetime does not count, since
+ * `Deserialize<'de>` is still a property of the type.
+ */
+private predicate hasTraitTypeArg(Impl i) {
+  exists(GenericArg a |
+    a = implTraitPath(i).getSegment().getGenericArgList().getAGenericArg() and
+    not a instanceof LifetimeArg
+  )
+}
+
+/**
+ * Holds if `t` implements `traitName` and the trait takes no type argument,
+ * through a derive, a hand-written impl, or a macro. Parameterised traits are
+ * excluded.
+ */
+predicate implementsPlainTrait(TypeItem t, string traitName) {
+  exists(Impl i |
+    i.getSelf() = t and
+    fileOf(i).fromSource() and
+    not hasTraitTypeArg(i) and
+    traitName = implTraitName(i)
+  )
+}
+
 /**
  * Holds if `t` has a derived impl for `traitName` under `crate`
  * (i.e. the trait path is `::<crate>::<traitName>`).

maint/codeql/rust/pkc.model.yml

@@ -64,3 +64,7 @@ extensions:
       # Recoverable signatures
       - ["Ecdsa", "SecretKey", "sign_recoverable"]        # Sign
       - ["Ecdsa", "PublicKey", "recover"]                 # Get public key from a signature
+  - addsTo:
+      pack: base-sdk/codeql-rust
+      extensible: armLacksTrait
+    data: []

maint/codeql/rust/pkc.ql

@@ -5,7 +5,7 @@
  *
  * @id base-sdk/pkc-rules
  * @name Rules for dash-pkc
- * @description The arms must offer the same operations under the same names.
+ * @description The arms must offer the same operations and carry the same traits.
  * @kind problem
  * @precision high
  * @problem.severity warning
@@ -77,6 +77,32 @@ predicate shapeGap(TypeItem lacks, string role, string name, string arm) {
   )
 }
 
-from TypeItem t, string role, string name, string arm
-where shapeGap(t, role, name, arm)
-select t, fmt("{0} offers {1}, {2} does not", arm + role, fmt("{0}()", name), t.getName().getText())
+/**
+ * Holds if `lacks` is missing `trait`, which `arm` carries for the same role
+ * inclusive of derives gated by `cfg_attr`.
+ */
+predicate traitGap(TypeItem lacks, string role, string trait, string arm) {
+  exists(TypeItem offers, string lacking |
+    armRole(offers, arm, role) and
+    armRole(lacks, lacking, role) and
+    lacking != arm and
+    implementsPlainTrait(offers, trait) and
+    not implementsPlainTrait(lacks, trait) and
+    not hasDerive(lacks, trait) and
+    not armLacksTrait(lacking, role, trait)
+  )
+}
+
+from TypeItem t, string message
+where
+  exists(string role, string name, string arm |
+    shapeGap(t, role, name, arm) and
+    message =
+      fmt("{0} offers {1}, {2} does not", arm + role, fmt("{0}()", name), t.getName().getText())
+  )
+  or
+  exists(string role, string trait, string arm |
+    traitGap(t, role, trait, arm) and
+    message = fmt("{0} implements {1}, {2} does not", arm + role, trait, t.getName().getText())
+  )
+select t, message

pkgs/num/src/lib.rs

@@ -33,4 +33,5 @@ pub use crate::arith256::Arith256;
 pub use crate::compact::{CompactTarget, DecodedTarget};
 pub use crate::hash::{Hash160, Hash256, HashBlob, ParseHexError};
 
+// TODO(kwvg): move to mod __deps with crate-level export
 pub use dash_types::Numeric;

pkgs/pkc/bench/ecdsa.rs

@@ -30,7 +30,7 @@ fn verify(bencher: divan::Bencher) {
   let pk = sk.public_key();
   bencher
     .counter(divan::counter::ItemsCount::new(1u32))
-    .bench(|| pk.verify(&msg, &sig));
+    .bench(|| pk.verify(&msg, sig));
 }
 
 #[divan::bench]

pkgs/pkc/src/bls/ies_ops.rs

@@ -141,7 +141,7 @@ impl<S: BlsScheme> BlsIesBlob<S> {
 impl<S: BlsScheme> Clone for BlsIesBlob<S> {
   fn clone(&self) -> Self {
     Self {
-      ephemeral_pk: self.ephemeral_pk.clone(),
+      ephemeral_pk: self.ephemeral_pk,
       iv_seed: self.iv_seed,
       data: self.data.clone(),
     }
@@ -241,7 +241,7 @@ impl<S: BlsScheme> BlsIesMulti<S> {
   /// recipient index.
   pub fn to_blob(&self, index: usize) -> Option<BlsIesBlob<S>> {
     Some(BlsIesBlob::new(
-      self.ephemeral_pk.clone(),
+      self.ephemeral_pk,
       self.iv_seed,
       self.blobs.get(index)?.clone(),
     ))
@@ -261,7 +261,7 @@ impl<S: BlsScheme> BlsIesMulti<S> {
 impl<S: BlsScheme> Clone for BlsIesMulti<S> {
   fn clone(&self) -> Self {
     Self {
-      ephemeral_pk: self.ephemeral_pk.clone(),
+      ephemeral_pk: self.ephemeral_pk,
       iv_seed: self.iv_seed,
       blobs: self.blobs.clone(),
     }
@@ -486,7 +486,7 @@ mod tests {
     /// The multi-recipient message the vectors record.
     fn message(&self) -> BlsIesMulti<S> {
       BlsIesMulti::new(
-        self.eph_pk.clone(),
+        self.eph_pk,
         self.iv_seed,
         self.recipients.iter().map(|r| vec_from_hex(&r.ciphertext)).collect(),
       )
@@ -710,9 +710,9 @@ mod tests {
     let kat = load_kat::<BlsScIetf>("ietf");
     let misaligned = vec![0u8; 17];
 
-    let blob = BlsIesBlob::new(kat.eph_pk.clone(), kat.iv_seed, misaligned.clone());
+    let blob = BlsIesBlob::new(kat.eph_pk, kat.iv_seed, misaligned.clone());
     assert_eq!(blob.data(), misaligned);
-    assert!(BlsIesMulti::new(kat.eph_pk.clone(), kat.iv_seed, vec![misaligned.clone()]).is_ok());
+    assert!(BlsIesMulti::new(kat.eph_pk, kat.iv_seed, vec![misaligned.clone()]).is_ok());
 
     let bag = BlsIesBlobBytes::new(BlsPkBytes::from(&kat.eph_pk), kat.iv_seed, misaligned.clone());
     assert_eq!(BlsIesBlob::<BlsScIetf>::try_from(&bag).unwrap().data(), misaligned);

pkgs/pkc/src/bls/public_ops.rs

@@ -140,10 +140,12 @@ impl BlsPublicKey<BlsScIetf> {
 
 impl<S: BlsScheme> Clone for BlsPublicKey<S> {
   fn clone(&self) -> Self {
-    Self(self.0.clone())
+    *self
   }
 }
 
+impl<S: BlsScheme> Copy for BlsPublicKey<S> {}
+
 impl<S: BlsScheme> Debug for BlsPublicKey<S> {
   fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
     qtypestr(f, type_name::<Self>())?;

pkgs/pkc/src/bls/scheme_ops.rs

@@ -49,9 +49,9 @@ pub trait BlsScheme: BlsSchemeId + sealed::Sealed + Sized {
   /// Inner secret key representation.
   type InnerSk: Clone + Send + Sync;
   /// Inner public key representation.
-  type InnerPk: Clone + Debug + PartialEq + Eq + Send + Sync;
+  type InnerPk: Copy + Debug + PartialEq + Eq + Send + Sync;
   /// Inner signature representation.
-  type InnerSig: Clone + Debug + PartialEq + Eq + Send + Sync;
+  type InnerSig: Copy + Debug + PartialEq + Eq + Send + Sync;
   /// Message type accepted by signing and verification.
   type Msg: ?Sized;
 

pkgs/pkc/src/bls/share_ops.rs

@@ -100,7 +100,7 @@ impl<S: BlsScheme> Clone for BlsSigShare<S> {
   fn clone(&self) -> Self {
     Self {
       id: self.id,
-      sig: self.sig.clone(),
+      sig: self.sig,
     }
   }
 }
@@ -157,7 +157,7 @@ impl<S: BlsScheme> Clone for BlsPkShare<S> {
   fn clone(&self) -> Self {
     Self {
       id: self.id,
-      pk: self.pk.clone(),
+      pk: self.pk,
     }
   }
 }

pkgs/pkc/src/bls/sig_basic.rs

@@ -74,10 +74,12 @@ impl<S: BlsScheme> BlsSignature<S> {
 
 impl<S: BlsScheme> Clone for BlsSignature<S> {
   fn clone(&self) -> Self {
-    Self(self.0.clone())
+    *self
   }
 }
 
+impl<S: BlsScheme> Copy for BlsSignature<S> {}
+
 impl<S: BlsScheme> Debug for BlsSignature<S> {
   fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
     qtypestr(f, core::any::type_name::<Self>())?;

pkgs/pkc/src/ecdsa/public_ops.rs

@@ -44,7 +44,7 @@ pub(super) enum PkForm {
 }
 
 /// A secp256k1 public key.
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
 #[cfg_attr(feature = "codec", derive(TypeId))]
 #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))]
 #[cfg_attr(feature = "serde", serde(into = "EcdsaPkBytes", try_from = "EcdsaPkBytes"))]
@@ -267,6 +267,14 @@ type_cvrt!(TryFrom<EcdsaPkBytes> for EcdsaPublicKey, EcdsaError, |bytes| {
   Self::from_bytes(bytes.as_bytes())
 });
 
+type_cvrt!(From<EcdsaPublicKey> for PublicKey, |pk| {
+  pk.inner
+});
+
+type_cvrt!(From<PublicKey> for EcdsaPublicKey, |inner| {
+  Self::from_inner(*inner, Compression::Compressed)
+});
+
 #[cfg(test)]
 #[expect(clippy::unwrap_used, reason = "test code")]
 mod tests {
@@ -292,6 +300,22 @@ mod tests {
     pk: String,
   }
 
+  #[rstest]
+  fn backend_roundtrip_keeps_point_and_defaults_to_compressed(alice_pk: EcdsaPublicKey) {
+    let inner = secp256k1::PublicKey::from(&alice_pk);
+    assert_eq!(inner.serialize(), alice_pk.to_compressed());
+
+    let mut lifted = EcdsaPublicKey::from(inner);
+    assert!(lifted.is_compressed());
+    assert_eq!(lifted, alice_pk);
+    lifted.decompress();
+    assert_eq!(
+      secp256k1::PublicKey::from(&lifted),
+      inner,
+      "the form does not touch the point"
+    );
+  }
+
   #[rstest]
   fn compressed_roundtrip(alice_pk: EcdsaPublicKey) {
     let bytes = alice_pk.to_compressed();
@@ -413,6 +437,6 @@ mod tests {
   fn verify_rejects_wrong_message(alice_pk: EcdsaPublicKey, alice_sig: EcdsaSignature) {
     let mut bad = MSG;
     bad[0] ^= 0xff;
-    assert!(alice_pk.verify(&bad, &alice_sig).is_err());
+    assert!(alice_pk.verify(&bad, alice_sig).is_err());
   }
 }