feat(keys): VerifyingKeys can be transcoded to/from SPKI DER/PEM #273
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
WalkthroughThis change refactors the way verifying keys are serialized and deserialized in the codebase. It removes ad-hoc DER encoding/decoding methods and introduces standardized Subject Public Key Info (SPKI) DER and PEM methods for Ed25519, Secp256k1, and Secp256r1 key types. The test suite is updated to use these new SPKI-based methods, including serialization to and from PEM files, and explicit error handling is added for unsupported algorithms. The verification keys JSON file is updated with new key values. The Dockerfile updates the Rust base image version. No changes are made to the signatures of exported or public entities outside of the verifying key serialization/deserialization API. Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant VerifyingKey
participant FileSystem
Test->>VerifyingKey: to_spki_der()
VerifyingKey->>VerifyingKey: to_spki_der_doc()
VerifyingKey-->>Test: DER bytes
Test->>VerifyingKey: from_spki_der(DER bytes)
VerifyingKey->>VerifyingKey: from_spki(SubjectPublicKeyInfoRef)
VerifyingKey-->>Test: VerifyingKey instance
Test->>VerifyingKey: to_spki_pem_file(filename)
VerifyingKey->>FileSystem: Write PEM file
Test->>VerifyingKey: from_spki_pem_file(filename)
VerifyingKey->>FileSystem: Read PEM file
VerifyingKey->>VerifyingKey: from_spki(SubjectPublicKeyInfoRef)
VerifyingKey-->>Test: VerifyingKey instance
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
jns-ps
force-pushed
the
spki-transcoding
branch
from
d1d7865 to
ba3b813
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/keys/src/tests.rs (1)
54-71: Add a verifying-key test for SPKI PEM files.A previous reviewer suggested adding a test that reads verifying keys from SPKI PEM files, similar to the SigningKey tests. This would ensure consistent coverage of the new PEM-based functionality.
🧹 Nitpick comments (4)
crates/keys/src/verifying_keys.rs (4)
169-181: Consider returning typed errors for unimplemented algorithms.For EIP-191 and Cosmos ADR-36, you currently bail with a generic message. Introducing a dedicated error variant (for example, a custom enum) can help downstream callers distinguish between actual decoding errors and “not implemented yet” scenarios.
187-191: Preserve underlying error details in the error message.You’re discarding the original I/O error details with
map_err(|_| anyhow!("Creating PKCS8 PEM file failed")). Retaining the original error message will make debugging issues easier:- .map_err(|_| anyhow!("Creating PKCS8 PEM file failed")) + .map_err(|err| anyhow!("Creating PKCS8 PEM file failed: {}", err))
193-215: Use a consistent approach to handle unsupported algorithms.Just like in
to_spki_der_doc, you bail out for EIP-191 and Cosmos ADR-36. Consider returning a typed error to unify your approach. This yields more clarity when distinguishing between a corrupted DER and an unimplemented algorithm.
222-227: Improve PEM label error message clarity.Similar to writing the PEM file, you discard the underlying error with
map_err(|_| anyhow!("Incorrect PEM label")). Preserving the original error details will help diagnose label mismatch cases:-map_err(|_| anyhow!("Incorrect PEM label")) +map_err(|err| anyhow!("Incorrect PEM label: {}", err))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/keys/src/tests.rs(1 hunks)crates/keys/src/verifying_keys.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: unused dependencies
- GitHub Check: integration-test
- GitHub Check: unit-test
- GitHub Check: build-and-push-image
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
crates/keys/src/verifying_keys.rs (1)
217-220: SPKI DER parsing logic looks good.This straightforward wrapper on
from_spkiis clear and efficient. Great job!
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores