add credential storage docs from notion#335
Open
lukejmann wants to merge 5 commits into
Open
Conversation
paolodamico
force-pushed
the
credential-storage-documentation
branch
from
32e2538 to
c96c89b
Compare
Contributor
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 837711ee4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+7
to
+9
| //! - `0x01 || …` — Merkle inclusion proof; value is the proof bytes. | ||
| //! - `0x02 || oprf_seed` — session seed; value is the `session_id_r_seed`. | ||
| //! - `0x03 || nullifier` — replay guard; value is a presence marker. |
Contributor
There was a problem hiding this comment.
Could we move
https://github.com/worldcoin/walletkit/blob/main/walletkit-core/src/storage/cache/util.rs#L38-L40
pub(super) const CACHE_KEY_PREFIX_MERKLE: u8 = 0x01;
pub(super) const CACHE_KEY_PREFIX_SESSION: u8 = 0x02;
pub(super) const CACHE_KEY_PREFIX_REPLAY_NULLIFIER: u8 = 0x03;below this comment?
Dzejkop
reviewed
Jul 3, 2026
| //! key/value/TTL structure. Entries are distinguished by a 1-byte key prefix | ||
| //! followed by type-specific key material (see `cache/util.rs`): | ||
| //! | ||
| //! - `0x01 || …` — Merkle inclusion proof; value is the proof bytes. |
Contributor
There was a problem hiding this comment.
Suggested change
| //! - `0x01 || …` — Merkle inclusion proof; value is the proof bytes. | |
| //! - `0x01` — Merkle inclusion proof; at most one entry; value is the proof bytes. |
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+3
to
+5
| //! All cacheable data lives in a single `cache_entries` table with a unified | ||
| //! key/value/TTL structure. Entries are distinguished by a 1-byte key prefix | ||
| //! followed by type-specific key material (see `cache/util.rs`): |
Contributor
There was a problem hiding this comment.
Suggested change
| //! All cacheable data lives in a single `cache_entries` table with a unified | |
| //! key/value/TTL structure. Entries are distinguished by a 1-byte key prefix | |
| //! followed by type-specific key material (see `cache/util.rs`): | |
| //! A table for storing cachable data. Each row has a key (`key_bytes`), | |
| //! value (`value_bytes`) and TTL columns (see `ensure_entries_schema` for details). | |
| //! | |
| //! The keys adhere to the following schema: |
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+35
to
+37
| /// | ||
| /// Bounded rather than indefinite so the cache doesn't grow into a permanent | ||
| /// record. |
Contributor
There was a problem hiding this comment.
Suggested change
| /// | |
| /// Bounded rather than indefinite so the cache doesn't grow into a permanent | |
| /// record. |
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+7
to
+15
| //! | ||
| //! Each `replay_guard_set` insert is atomic and idempotent. Detection is *not* | ||
| //! atomic end-to-end, though: the proof flow calls `is_nullifier_replay` (a read), | ||
| //! generates the proof, then calls `replay_guard_set` (the insert) as separate | ||
| //! steps, holding no lock across them. Two concurrent requests for the same | ||
| //! nullifier can therefore both pass the check before either records its guard and | ||
| //! both disclose. This guard defends against sequential reuse, not a concurrent | ||
| //! race; callers that need strict single-use must serialize the check-and-set | ||
| //! themselves. |
Contributor
There was a problem hiding this comment.
Suggested change
| //! | |
| //! Each `replay_guard_set` insert is atomic and idempotent. Detection is *not* | |
| //! atomic end-to-end, though: the proof flow calls `is_nullifier_replay` (a read), | |
| //! generates the proof, then calls `replay_guard_set` (the insert) as separate | |
| //! steps, holding no lock across them. Two concurrent requests for the same | |
| //! nullifier can therefore both pass the check before either records its guard and | |
| //! both disclose. This guard defends against sequential reuse, not a concurrent | |
| //! race; callers that need strict single-use must serialize the check-and-set | |
| //! themselves. |
that's just how db operations work - I think this is detailing too much
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+56
to
+61
| /// | ||
| /// `path` is a bare, storage-relative filename (e.g. `account_keys.bin`), not an | ||
| /// absolute path — the implementation resolves it against whatever root it chooses. | ||
| /// That root is independent of [`StoragePaths`], so the key envelope need not live | ||
| /// under `<root>/worldid/`; a host that backs up or deletes an account must persist | ||
| /// and remove these blobs separately from the `worldid/` files. |
Contributor
There was a problem hiding this comment.
Should we even need to specify all this? Feels like it's all up to the foreign implementor
Suggested change
| /// | |
| /// `path` is a bare, storage-relative filename (e.g. `account_keys.bin`), not an | |
| /// absolute path — the implementation resolves it against whatever root it chooses. | |
| /// That root is independent of [`StoragePaths`], so the key envelope need not live | |
| /// under `<root>/worldid/`; a host that backs up or deletes an account must persist | |
| /// and remove these blobs separately from the `worldid/` files. |
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+20
to
+23
| //! | ||
| //! Filenames are fixed and generic by design: none of them encode a relying-party | ||
| //! ID, action name, issuer name, or the account's leaf index, so directory listings | ||
| //! and backup archives don't leak which accounts or actions exist on a device. |
Contributor
There was a problem hiding this comment.
Suggested change
| //! | |
| //! Filenames are fixed and generic by design: none of them encode a relying-party | |
| //! ID, action name, issuer name, or the account's leaf index, so directory listings | |
| //! and backup archives don't leak which accounts or actions exist on a device. |
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+5
to
+8
| //! The storage layer handles structured storage of all credentials and their | ||
| //! associated data (only storage, the semantics of the associated data is the | ||
| //! Issuer's responsibility). In addition the storage layer handles encryption | ||
| //! and clean up after expiration. |
Contributor
There was a problem hiding this comment.
Suggested change
| //! The storage layer handles structured storage of all credentials and their | |
| //! associated data (only storage, the semantics of the associated data is the | |
| //! Issuer's responsibility). In addition the storage layer handles encryption | |
| //! and clean up after expiration. | |
| //! The storage layer handles structured storage of all credentials and their | |
| //! associated data. In addition the storage layer handles encryption | |
| //! and clean up after expiration. |
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+45
to
+49
| //! | ||
| //! - No filesystem paths encode `leaf_index`, RP identifiers, issuer names, or action | ||
| //! name. See [`crate::storage::StoragePaths`]. | ||
| //! - The vault (authoritative) holds the `leaf_index`, credentials, and blobs; the | ||
| //! cache DB holds only regenerable, TTL-bounded entries. |
Contributor
There was a problem hiding this comment.
Suggested change
| //! | |
| //! - No filesystem paths encode `leaf_index`, RP identifiers, issuer names, or action | |
| //! name. See [`crate::storage::StoragePaths`]. | |
| //! - The vault (authoritative) holds the `leaf_index`, credentials, and blobs; the | |
| //! cache DB holds only regenerable, TTL-bounded entries. |
Dzejkop
reviewed
Jul 3, 2026
Comment on lines
+42
to
+45
| /// | ||
| /// Replay guard entries keyed by a nullifier are intentionally short-lived | ||
| /// (bounded by TTL) to avoid accumulating a long-lived "interaction history" | ||
| /// on device. |
Contributor
There was a problem hiding this comment.
Suggested change
| /// | |
| /// Replay guard entries keyed by a nullifier are intentionally short-lived | |
| /// (bounded by TTL) to avoid accumulating a long-lived "interaction history" | |
| /// on device. |
I'd remove this - this feels like storage semantics of each nullifier, not enforced by the Nullifier type alias
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Low Risk
Comment and documentation changes only; no runtime or storage behavior is modified.
Overview
Adds module-level Rust docs across the credential storage layer, largely ported from internal Notion material, so hosts and reviewers can understand layout and behavior without digging through implementation.
The
storagecrate root (mod.rs) now documents the overall Credential Store model: vault vs cache DB roles,CredentialStoreas the UniFFI facade, key usage viawalletkit-db, on-disk layout underworldid/, and privacy notes (generic filenames, no RP/action leakage).paths,traits, andkeysdocs clarify integration boundaries: what lives underStoragePathsvs the hostAtomicBlobStoreforaccount_keys.bin, per-platform keystore/blob expectations, and delegation of theK_device→K_intermediatehierarchy towalletkit-db.Cache docs in
schemadescribe the unifiedcache_entrieskey-prefix scheme;nullifiersexpands replay-guard semantics (NBF grace, TTL retention) and explicitly documents that replay detection is not atomic across concurrent proof flows (sequential reuse only unless callers serialize check-and-set).typesadds a short note that nullifier replay entries are TTL-bounded to avoid a permanent on-device interaction history.Reviewed by Cursor Bugbot for commit 4f0858c. Bugbot is set up for automated code reviews on this repo. Configure here.