Repository navigation
feat(storage): OrbPcpStore on walletkit-db - #2
danielle-tfh wants to merge 8 commits into
Conversation
40e7a69 to
bbd857d
Compare
3cae9c2 to
5c5c675
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c5c675b0c
ℹ️ 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".
|
cursor review |
|
|
||
| ## How storage is protected | ||
|
|
||
| OrbKit's PCP vault is a sqlite3mc-encrypted `SQLite` file. Page-level encryption uses a 32-byte working key (`K_intermediate`); that key is itself sealed under the device's hardware keystore key (`K_device`) using ChaCha20-Poly1305 AEAD with a per-consumer associated-data string, and persisted as a CBOR envelope. |
There was a problem hiding this comment.
why is OrbKit's vault an sqlite db? taking a step back, the PCPs should live as associated data in walletkit's credential vault, I'm not sure introducing another sqlite layer makes sense
There was a problem hiding this comment.
fair q! main reason is PCPs exist before theres a credential to attach them to (downloaded, enrollment pending, no PoH cred yet), plus per-tier shards and the status machine.
What ya think?
There was a problem hiding this comment.
oops missed this. well one way to look at it is that each PCP is an Orb credential. I think nesting another sqlite DB into the credential vault will probably bring some headaches
There was a problem hiding this comment.
yup @paolodamico, i was mixing up reusing walletkit-db with sharing the credential vault. my clanker checked worldcoin/walletkit#633 and pending PCPs no longer need an issued credential.
lets keep the lookup as SQL rows in that same vault. walletkit handles persistence, orbkit keeps the enrollment rules, and package + lookup writes commit together. PCPs are excluded from that PR's credential export, so device transfer still needs explicit support.
@Dzejkop can you take over the lookup API and migration from here? ive updated this PR with the handoff.
put_package is strict so a re-download cannot reset a row past Downloaded. walletkit-db bumped to 0.20.4, whose Keystore::seal takes slices.
5c5c675 to
70e4545
Compare
oxide only ever writes tier 0 in production (tier-1 extraction is test-only), so the composite key and per-tier API were unused. Removes the mixed-tier status case entirely.
Matches what oxide does on a repeat download today; the only caller would swallow AlreadyExists anyway.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2d114a3. Configure here.
Drop unused secrecy/serde deps and the gitignore drive-by. open takes the vault directory and owns its file names and lock, so paths.rs goes away. Storage submodules are private behind the re-exports.
| use crate::storage::types::{CreationSource, PackageStatus, PcpRecord, SignupId}; | ||
|
|
||
| // On-disk format: renaming any of these orphans every existing vault. | ||
| const VAULT_FILENAME: &str = "orb_pcp.sqlite"; |
There was a problem hiding this comment.
should the orb pcp be another sqlite db? walletkit has a vault db where the idea is that arbitrary associated data blobs (e.g. PCPs) can be added. the associated data blobs can be anything, but I don't think it makes a lot of sense for it to be another sqlite db
| ingest.encrypted_bytes, | ||
| ingest.now_seconds, | ||
| )?; | ||
| let stored_cid = conn.query_row( |
There was a problem hiding this comment.
should we be storing a bunch of PCPs instead of just the latest?
| impl PackageStatus { | ||
| /// Stable string used as the SQL `TEXT` value. | ||
| #[must_use] | ||
| pub const fn as_str(self) -> &'static str { |
There was a problem hiding this comment.
nit. you can use strum to avoid the extra code

PCP storage groundwork; the shared-vault integration below is still pending.
Notes
OrbPcpStorecurrently owns a separate encrypted SQLite file on walletkit-db. It stores one row per signup, enrollment state and pending download acknowledgements.put_packagemerges only the acknowledgement flag and returns the original content id. It now checks the existing row before storing bytes, so different retry payloads cannot leave unused blobs or rewind enrollment.mark_ackreports missing signups;unacked_signupssupports acknowledgement retries. Walletkit remains responsible for selecting the latest credential.PcpVaultfrom feat(storage): store PCPs in a dedicated vault table walletkit#633 for sealed package bytes and a separateorb_pcp_lookupSQL table in the same vault for signup, status, acknowledgement, version, source, signup reason and timestamps. Walletkit owns persistence; OrbKit owns enrollment rules.pcp_recordsbut have incompatible schemas, so this needs an explicit migration.cargo test -p orb-kit --locked --offline(10 passed),cargo clippy -p orb-kit --all-targets --locked --offline -- -D warnings, andcargo fmt --all -- --check.Note
Medium Risk
Adds encrypted local storage and a lifecycle state machine for PCPs; on-disk schema and filenames are a long-lived contract, though behavior is covered by unit and integration tests.
Overview
Introduces the first orb-kit slice of on-device Personal Custody Package (PCP) storage, built on
walletkit-dbinstead of oxide. The publicstoragemodule exposesOrbPcpStore, which opens a dedicated encrypted vault (fixed filenames, envelope AD, lock) and manages onepcp_recordsrow per signup with encrypted package blobs inblob_objects.put_packageinserts new downloads atDownloaded; repeat downloads only merge the download-ack flag and return the existing content id so status and blobs are not reset.update_statusenforces aPackageStatustransition graph;get,mark_ack, andunacked_signupssupport reads and cold-start ack retry. The DDL is guarded by a frozen schema hash test, and repoAGENTS.mddocuments byte-stable format andwalletkit-dbboundary rules.Cargo.tomladdswalletkit-db0.20.4 andthiserror; integration tests use in-memory keystore/blob stubs.Reviewed by Cursor Bugbot for commit 687fbe9. Bugbot is set up for automated code reviews on this repo. Configure here.