feat: add pubky keys type #276

SHAcollision · 2025-11-27T09:01:28Z

Introduced shared PublicKey/Keypair wrappers in pubky-common::crypto::keys so every crate displays identifiers as pubky<z32> by default while still exposing .z32() for raw hostnames and registry usage.
For now we go with pubky... as default prefix for the public key identifier. This will be easy to change if the product team decides otherwise.
Replaced remaining direct pkarr key usages across the workspace (SDK, JS bindings, homeserver, testnet) with the shared wrappers to guarantee consistent formatting and serde behavior. The pkarr-republisher keeps using pkarr keys.
Removed the re-exported pkarr keys, instead it now re-export pubky keys from pubky-sdk, keeping documentation and examples aligned with the new display defaults.

ok300 · 2025-11-28T09:35:41Z

Nice, I was about to look into a similar PR, glad this is already in place.

Can you please add a way to get from a Keypair to a PubkyId?

For example:

impl Keypair {
    fn pubky_id(&self) -> Result<PubkyId, String> {
        PubkyId::try_from(self.public_key().to_z32().as_str())
    }
}

which allows

// Before (verbose)
let pubky_id = PubkyId::try_from(&keypair.public_key().to_z32())?;

// After (concise)
let pubky_id = keypair.pubky_id()?;

SHAcollision · 2025-11-28T11:12:51Z

Nice, I was about to look into a similar PR, glad this is already in place.

Can you please add a way to get from a Keypair to a PubkyId?

For example:
impl Keypair {
    fn pubky_id(&self) -> Result<PubkyId, String> {
        PubkyId::try_from(self.public_key().to_z32().as_str())
    }
}
which allows
// Before (verbose)
let pubky_id = PubkyId::try_from(&keypair.public_key().to_z32())?;

// After (concise)
let pubky_id = keypair.pubky_id()?;

This would introduce a dependency on pubky-app-specs here, right? Can we do it without this dependency? I don't think we want that under any circumstance. Instead we likely want a method in pubky-app-specs that can From a PubkyId from a pubky::PublicKey ?

SHAcollision · 2025-12-03T15:28:34Z

@SeverinAlexB Marking as ready for review here. No rush:

There is a small conflict with the changes introduced in feat(sdk): added auth signup flow #268 . When this conflict is solved, tests fail: most likely we need to use the new stringification methods correctly on the new AuthFlow but needs some research.
Warning AI slop: there are large parts of this PR that have been fixed by AI agents (e2e tests, etc). I haven't dived into all changes line by line. A more conscious work to verify the whole PR is yet to be done, unsafe to merge without it!
Feel free to push commits here and make changes as you like, It's good to me if you want to take ownership of it and merge it when it's OK according to your view. That's much preferable to having it stale and possibly even block v0.6.0 release until January. Feel free to delegate to someone else as well!

SeverinAlexB

I think this is good. No idea if this will lead to accidental breaking changes in the homeserver.

Does the new KeyPair/PublicKey have the same lib export path as the old one? Maybe it is better to export it in a different path so one needs to explicitly upgrade to the new KeyPair/PublicKey so the dev can check each case.

pubky-common/src/crypto/keys.rs

SHAcollision · 2025-12-15T07:18:50Z

Does the new KeyPair/PublicKey have the same lib export path as the old one? Maybe it is better to export it in a different path so one needs to explicitly upgrade to the new KeyPair/PublicKey so the dev can check each case.

yeha, good question. The wrapper keep the same public export locations (pubky_common::crypto::Keypair/PublicKey, re‑exported via pubky::Keypair/PublicKey) , downstream code continues to use the existing paths. The change is behavioral (prefixed display) rather than a rename. Consumers will now be using pubky:: keys instead of pkarr:: keys so indeed they are different types on the same path, yet I believe it's correct to fully delete the previous pkarr keys export and replace tat path with the new type since pkarr:: keys are of no use at all because no function on SDK / Homeserver accepts them anymore.

Overall, 0.6.0 vs previous 0.5.4 upgrade (big breaking change) leads consumers to fully re-do their use of the crate, and this is probably one of the least impactful changes.

SHAcollision · 2026-01-02T09:38:29Z

Merged main and fixes a myriad of conflicts and problems that rose from working in paralell with the new signup/signin authflow:

Pubky key wrappers wired through auth flow + deep link signup (hostnames use .z32() now) in auth_flow.rs and signup.rs.
Session export/import now compiles on native with a non‑WASM import guard in core.rs; native prepare_request added in native.rs.
JS bindings aligned to wrapper API in keys.rs and pkarr cache seeding uses wrapper conversion in http.rs.
Deterministic SQL ordering for list operations. I'm getting different ordering from deep list test local vs github runner. I losen the test to not expect some exact ordering. Needs more research.
Metrics test now uses z32 host strings in metrics.rs; macro doctest example ignored in lib.rs.
Tightened prefix handling in keys.rs to avoid misparsing raw keys that start with pubky.
Switched homeserver URL helpers back to .z32() for raw pkarr-style URLs.

SeverinAlexB

Few comments but I think most of it is good.

Did you ever close the prod or staging database and tested it with the data there to find issues?

SeverinAlexB · 2026-01-09T08:47:07Z

pubky-common/src/crypto/keys.rs

+#[derive(Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[serde(transparent)]
+pub struct PublicKey(pkarr::PublicKey);


Claude Review which I agree with

SeverinAlexB · 2026-01-09T08:49:29Z

pubky-homeserver/src/client_server/layers/pubky_host.rs

+                if is_prefixed_pubky(s) {
+                    continue;
+                }


Do I see this correctly that the new pubky prefixed keys are not supported in pubky_host?

SeverinAlexB · 2026-01-09T08:52:40Z

pubky-common/src/crypto/keys.rs

+impl Deref for PublicKey {
+    type Target = pkarr::PublicKey;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}


Claude Review. Interesting food for thought. Not sure what's best here.

SeverinAlexB · 2026-01-09T08:59:05Z

pubky-common/src/crypto/keys.rs

+/// Returns true if the value is in `pubky<z32>` form.
+pub fn is_prefixed_pubky(value: &str) -> bool {
+    matches!(value.strip_prefix("pubky"), Some(stripped) if stripped.len() == 52)
+}


NIT: Shouldn't this be a static method that belongs to the PublicKey like PublicKey::is_pubky_prefixed(key: &str)?

SeverinAlexB · 2026-01-09T09:07:18Z

pubky-common/src/crypto/keys.rs

+    /// Export the secret key bytes.
+    #[must_use]
+    pub fn secret_key(&self) -> [u8; 32] {
+        let mut out = [0u8; 32];
+        out.copy_from_slice(self.0.secret_key().as_ref());
+        out
+    }


In theory, this is the secret aka seed and not the secret_key aka private key. I know pkarr is already naming it this way but should we fix the naming here at least?

There are obviously other methods that would need to be renamed.

SeverinAlexB · 2026-01-09T09:09:51Z

pubky-common/src/crypto/keys.rs

+fn parse_public_key(value: &str) -> Result<pkarr::PublicKey, ParseError> {
+    let raw = if is_prefixed_pubky(value) {
+        value.strip_prefix("pubky").unwrap_or(value)
+    } else {
+        value
+    };
+    pkarr::PublicKey::try_from(raw.to_string())
+}


NIT: Similar to the is_prefixed_pubky, shouldn't this be a static method on PublicKey?

SeverinAlexB · 2026-01-09T09:15:49Z

pubky-common/src/crypto/keys.rs

+    #[must_use]
+    pub const fn as_inner(&self) -> &pkarr::PublicKey {
+        &self.0
+    }
+
+    /// Extract the inner [`pkarr::PublicKey`].
+    #[must_use]
+    pub fn into_inner(self) -> pkarr::PublicKey {
+        self.0
+    }
+
+    /// Return the raw z-base32 representation without the `pubky` prefix.
+    #[must_use]
+    pub fn z32(&self) -> String {
+        self.0.to_string()
+    }


Nothing you need to change, just a comment:

I find the introduction of the #[must_use] tag really weird. For me, this feels like it clutters the code unnecessarily. Feels like a rust language downgrade.

SHAcollision added 3 commits November 27, 2025 09:55

feat: add pubky keys type

dfcd5af

fix clippy

b7a93e9

fixes

acb6c77

SHAcollision force-pushed the feat/pubky-keys branch from 242ba73 to acb6c77 Compare November 27, 2025 11:18

fix e2e

2a51a79

SHAcollision force-pushed the feat/pubky-keys branch from 4f9d2e7 to 2a51a79 Compare November 28, 2025 11:16

fix wasm tests

b563ea4

ok300 mentioned this pull request Nov 29, 2025

PubkyId: derive From<PublicKey>, From<Keypair> pubky/pubky-app-specs#80

Merged

SHAcollision force-pushed the feat/pubky-keys branch from 5c0a77e to b563ea4 Compare December 2, 2025 12:25

SHAcollision marked this pull request as ready for review December 3, 2025 15:28

SeverinAlexB reviewed Dec 11, 2025

View reviewed changes

pubky-common/src/crypto/keys.rs Show resolved Hide resolved

pubky-common/src/crypto/keys.rs Show resolved Hide resolved

pubky-common/src/crypto/keys.rs Show resolved Hide resolved

SHAcollision added 4 commits December 15, 2025 09:05

add secret export method to keypair wrapper

f6b5970

Merge main into feat/pubky-keys

9a87205

fmt

15fd7ff

fix clippy

cc55c1e

SHAcollision force-pushed the feat/pubky-keys branch from 706a598 to cc55c1e Compare January 2, 2026 08:43

tigten prefix handling

c8766c7

SHAcollision added 6 commits January 2, 2026 13:14

tighten wasm public key conversion

11eb9ac

ensure no double encoding

4ae962c

fix docs and enfore non-prefixed keys on transport

668ee4d

fix docs, remove collate C

f09c910

more strict where z32 only is expected with useful messages

bc12a84

centralized is_prefixed_pubky check on pubky-common

dbdc988

SHAcollision force-pushed the feat/pubky-keys branch from f9fe9f7 to dbdc988 Compare January 3, 2026 14:08

use z32 on query params

e66af2e

SHAcollision added 2 commits January 4, 2026 04:48

fix wasm clippy

a9eaff4

fix e2e

5b31093

SHAcollision force-pushed the feat/pubky-keys branch 2 times, most recently from 1510562 to cecdc58 Compare January 8, 2026 07:55

SHAcollision added 2 commits January 8, 2026 09:04

Merge branch 'main' into feat/pubky-keys

a88ddc9

fix admin tests for pubky keys

5dac082

SHAcollision force-pushed the feat/pubky-keys branch from cecdc58 to 5dac082 Compare January 8, 2026 08:26

SHAcollision added 2 commits January 8, 2026 09:57

aling expected e2e order/cursor to collate C expectations

ac61098

revert e2e storage utils

f7cdd45

SeverinAlexB requested changes Jan 9, 2026

View reviewed changes

feat: add pubky keys type #276

Are you sure you want to change the base?

feat: add pubky keys type #276

Uh oh!

Conversation

SHAcollision commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ok300 commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SHAcollision commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SHAcollision commented Dec 3, 2025

Uh oh!

SeverinAlexB left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SHAcollision commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SHAcollision commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SeverinAlexB left a comment

Choose a reason for hiding this comment

Uh oh!

SeverinAlexB Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

SeverinAlexB Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

SeverinAlexB Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

SeverinAlexB Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

SeverinAlexB Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

SeverinAlexB Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

SeverinAlexB Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SHAcollision commented Nov 27, 2025 •

edited

Loading

ok300 commented Nov 28, 2025 •

edited

Loading

SHAcollision commented Nov 28, 2025 •

edited

Loading

SHAcollision commented Dec 15, 2025 •

edited

Loading

SHAcollision commented Jan 2, 2026 •

edited

Loading