[Feature] Add KeyStore interface for persistent Proving and Verifying Key storage #1138
Conversation
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
Member
Author
|
@copilot please add the desired unit tests specified in this PR in a file in a ./sdk/tests/keystore.test.ts file. |
|
@iamalwaysuncomfortable I've opened a new pull request, #1139, to work on those changes. Once the pull request is ready, I'll request review from you. |
iamalwaysuncomfortable
requested review from
Pauan,
Roee-87,
alexpitsikoulis and
esren0x
There was a problem hiding this comment.
Pull request overview
This PR adds a KeyStore interface to the Aleo SDK for persistent proving and verifying key storage, addressing the challenge of key management across different JavaScript runtime environments (browser, Node.js, etc.). The changes introduce a flexible abstraction layer with a default filesystem implementation for Node.js users.
Key Changes:
- Introduced the
KeyStoreinterface with methods for storing/retrieving proving and verifying keys - Added
LocalFileKeyStoreclass for Node.js filesystem-based key persistence - Added
keyStore()method to theFunctionKeyProviderinterface for convenient KeyStore access - Refactored key-related modules into a new directory structure (
sdk/src/keys/) - Fixed a bug where
checkFee()was incorrectly called for private fee transactions
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
sdk/src/keys/keystore/keystore.ts |
Defines the KeyStore interface with methods for key storage and retrieval |
sdk/src/keys/keystore/file.ts |
Implements LocalFileKeyStore for filesystem-based key persistence in Node.js |
sdk/src/keys/keystore/memory.ts |
Provides utility to adapt existing Map-based cache to KeyStore interface |
sdk/src/keys/provider/function-key-provider.ts |
Adds keyStore() method to FunctionKeyProvider interface and refactors imports |
sdk/src/keys/provider/offline-key-provider.ts |
Implements keyStore() method for OfflineKeyProvider |
sdk/src/models/keyPair.ts |
Defines FunctionKeyPair and CachedKeyPair types in a dedicated model file |
sdk/src/program-manager.ts |
Fixes bug where checkFee() was called even with privateFee transactions |
sdk/src/browser.ts |
Exports new KeyStore interface and related utilities for browser usage |
sdk/src/node.ts |
Exports LocalFileKeyStore for Node.js usage |
docs/api_reference/*.md |
Updates documentation links to reflect new file structure |
.github/workflows/sdk.yml |
Restructures workflow triggers to run on pull requests and branch pushes |
Comments suppressed due to low confidence (3)
sdk/src/keys/provider/offline-key-provider.ts:25
- [nitpick] Missing semicolon at the end of the import statement. This is inconsistent with other import statements in the same file (lines 4 and 14 have semicolons).
sdk/src/keys/provider/function-key-provider.ts:27 - [nitpick] Inconsistent use of
.jsfile extensions in imports. Lines 10 and 24 omit the.jsextension from the import paths, while line 26 includes it. For consistency with the rest of the file (line 26) and typical ES Module conventions, all relative imports should include the.jsextension.
sdk/src/keys/provider/function-key-provider.ts:176 - The new
keyStore()method introduced in theFunctionKeyProviderinterface lacks test coverage. Since the existing test filesdk/tests/key-provider.test.tshas comprehensive tests for otherAleoKeyProviderandOfflineKeyProvidermethods, tests should be added to verify thatkeyStore()returns the expectedKeyStoreimplementation for both providers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sdk/src/keys/keystore/keystore.ts
Outdated
Comment on lines
12
to
91
| getKeys(locator:string): Promise<FunctionKeyPair | null> | ||
|
|
||
| /** | ||
| * Returns the proving and verifying key as raw bytes for a given key locator from key storage. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| * | ||
| * @returns {Promise<CachedKeyPair | null>} Returns the proving and verifying keys for the given locator as raw bytes if they exist or null if they do not. */ | ||
| getKeysRaw(locator:string): Promise<CachedKeyPair | null> | ||
|
|
||
| /** | ||
| * Returns the `ProvingKey` for a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired `ProvingKey`. | ||
| * | ||
| * @returns {Promise<CachedKeyPair | null>} Returns the `ProvingKey` for the given locator if it exists or null if they do not. | ||
| */ | ||
| getProvingKey(locator:string): Promise<ProvingKey | null> | ||
|
|
||
| /** | ||
| * Returns the raw bytes of a `ProvingKey` for a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired `ProvingKey`. | ||
| * | ||
| * @returns {Promise<Uint8Array | null>} Returns the raw bytes of a `ProvingKey` for the given locator if it exists or null if it does not exist. | ||
| */ | ||
| getProvingKeyRaw(locator:string): Promise<Uint8Array | null> | ||
|
|
||
| /** | ||
| * Returns the `VerifyingKey` for a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired `VerifyingKey`. | ||
| * | ||
| * @returns {Promise<VerifyingKey | null>} Returns the `VerifyingKey` for the given locator if it exists or null if it does not exist. | ||
| */ | ||
| getVerifyingKey(locator:string): Promise<VerifyingKey | null> | ||
|
|
||
| /** | ||
| * Returns the raw bytes of a `VerifyingKey` for a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired `VerifyingKey`. | ||
| * | ||
| * @returns {Promise<Uint8Array | null>} Returns the raw bytes of a `VerifyingKey` for the given locator if it exists or null if it does not exist. | ||
| */ | ||
| getVerifyingKeyRaw(locator:string): Promise<Uint8Array | null> | ||
|
|
||
| /** | ||
| * Stores proving and verifying keys in key storage. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| * @param {FunctionKeyPair} keys The proving and verifying keys. | ||
| */ | ||
| setKeys(locator:string, keys: FunctionKeyPair): Promise<void> | ||
|
|
||
| /** | ||
| * Stores the raw proving and verifying key bytes in key storage. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| * @param {CachedKeyPair} keys The raw proving and verifying key bytes. | ||
| */ | ||
| setKeysRaw(locator:string, keys: CachedKeyPair): Promise<void> | ||
|
|
||
| /** | ||
| * Determines if a given keypair exists or not. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| */ | ||
| has(locator:string): Promise<boolean> | ||
|
|
||
| /** | ||
| * Deletes a keypair corresponding to a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| */ | ||
| delete(locator:string): Promise<void> | ||
|
|
||
| /** | ||
| * Clears all keys in the keystore. | ||
| */ | ||
| clear(): Promise<void> |
There was a problem hiding this comment.
[nitpick] Missing semicolons at the end of interface method declarations. While optional in TypeScript, they should be consistent with the rest of the interface (other methods like delete() and clear() also lack semicolons, suggesting this may be intentional, but it's best practice to be consistent across the entire interface).
sdk/src/keys/keystore/keystore.ts
Outdated
Comment on lines
12
to
91
| getKeys(locator:string): Promise<FunctionKeyPair | null> | ||
|
|
||
| /** | ||
| * Returns the proving and verifying key as raw bytes for a given key locator from key storage. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| * | ||
| * @returns {Promise<CachedKeyPair | null>} Returns the proving and verifying keys for the given locator as raw bytes if they exist or null if they do not. */ | ||
| getKeysRaw(locator:string): Promise<CachedKeyPair | null> | ||
|
|
||
| /** | ||
| * Returns the `ProvingKey` for a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired `ProvingKey`. | ||
| * | ||
| * @returns {Promise<CachedKeyPair | null>} Returns the `ProvingKey` for the given locator if it exists or null if they do not. | ||
| */ | ||
| getProvingKey(locator:string): Promise<ProvingKey | null> | ||
|
|
||
| /** | ||
| * Returns the raw bytes of a `ProvingKey` for a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired `ProvingKey`. | ||
| * | ||
| * @returns {Promise<Uint8Array | null>} Returns the raw bytes of a `ProvingKey` for the given locator if it exists or null if it does not exist. | ||
| */ | ||
| getProvingKeyRaw(locator:string): Promise<Uint8Array | null> | ||
|
|
||
| /** | ||
| * Returns the `VerifyingKey` for a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired `VerifyingKey`. | ||
| * | ||
| * @returns {Promise<VerifyingKey | null>} Returns the `VerifyingKey` for the given locator if it exists or null if it does not exist. | ||
| */ | ||
| getVerifyingKey(locator:string): Promise<VerifyingKey | null> | ||
|
|
||
| /** | ||
| * Returns the raw bytes of a `VerifyingKey` for a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired `VerifyingKey`. | ||
| * | ||
| * @returns {Promise<Uint8Array | null>} Returns the raw bytes of a `VerifyingKey` for the given locator if it exists or null if it does not exist. | ||
| */ | ||
| getVerifyingKeyRaw(locator:string): Promise<Uint8Array | null> | ||
|
|
||
| /** | ||
| * Stores proving and verifying keys in key storage. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| * @param {FunctionKeyPair} keys The proving and verifying keys. | ||
| */ | ||
| setKeys(locator:string, keys: FunctionKeyPair): Promise<void> | ||
|
|
||
| /** | ||
| * Stores the raw proving and verifying key bytes in key storage. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| * @param {CachedKeyPair} keys The raw proving and verifying key bytes. | ||
| */ | ||
| setKeysRaw(locator:string, keys: CachedKeyPair): Promise<void> | ||
|
|
||
| /** | ||
| * Determines if a given keypair exists or not. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| */ | ||
| has(locator:string): Promise<boolean> | ||
|
|
||
| /** | ||
| * Deletes a keypair corresponding to a given locator. | ||
| * | ||
| * @param {string} locator The unique locator for the desired keypair. | ||
| */ | ||
| delete(locator:string): Promise<void> | ||
|
|
||
| /** | ||
| * Clears all keys in the keystore. | ||
| */ | ||
| clear(): Promise<void> |
There was a problem hiding this comment.
[nitpick] Missing space after colon in all method parameter type declarations. TypeScript/JavaScript style guides typically recommend locator: string rather than locator:string for better readability.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Mike Turner <mike@provable.com>
Signed-off-by: Mike Turner <mike@provable.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Aleo functions require a proving key and verifying key per function. On execution, if the keys do not currently exist, they are synthesized and stored in the local
Processobject. However This synthesis process takes a long time and execution time can be cut down drastically by inserting pre-synthesized keys. Normally on executors with filesystems, these keys are stored locally on disk automatically bySnarkVM. Howeverwasmhas no direct access to a filesystem, thus proving and verifying keys must be passed intowasmexternally by JS.Because JS can operate in multiple contexts (such as the
browser,node.json a machine with a filesystem, or perhaps an enclave environment) key storage will necessarily need to be different depending on the operational context.Currently there no convenient abstractions in the SDK for managing proving and verifying keys and it is not clear to many users how to do proper proving and verifying key management. This PR adds a
KeyStoreabstraction that provides methods for storing and retrieving proving and verifying keys and provides a default implementation called theFileSystemKeyProviderfor NodeJS sdk users that have access to filesystems.This PR also adds the
KeyStoreobject to theFunctionKeyProviderinterface for convenient access.Test Plan
FileSystemKeyProviderto ensure theKeyStoreinterface operates as desired.OfflineKeyProviderandAleoKeyProviderto ensure these interfaces work as desired.