Skip to content

feat: add decryption key to identity creation - #18

Open
thepastaclaw wants to merge 6 commits into
dashpay:mainfrom
thepastaclaw:tracker-946-auto-decryption-key
Open

feat: add decryption key to identity creation#18
thepastaclaw wants to merge 6 commits into
dashpay:mainfrom
thepastaclaw:tracker-946-auto-decryption-key

Conversation

@thepastaclaw

@thepastaclaw thepastaclaw commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Create identities with a default DECRYPTION key alongside the existing
ENCRYPTION key.

This also tightens the create/manage UI state handling so ENCRYPTION and
DECRYPTION keys cannot retain invalid security levels when edited.

Changes

  • add a default DECRYPTION key to both default identity key-generation paths
  • extend KeyPurpose typing and UI purpose options to include DECRYPTION
  • constrain ENCRYPTION/DECRYPTION to MEDIUM security level in create/manage
    state transitions
  • add tests covering default key layout, registration wiring, and
    security-level coercion

Validation

  • npx vitest run
    • 3 test files passed
    • 18 tests passed
  • npm run build
    • passed
  • code-review PastaPastaPasta/dash-bridge origin/main \ tracker-946-auto-decryption-key ...
    • final verdict: ship

Summary by CodeRabbit

  • New Features

    • Added DECRYPTION key purpose and expanded default identity keys to six entries (with separate ENCRYPTION and DECRYPTION).
  • Updates

    • DECRYPTION is now available in the key configuration UI.
    • ENCRYPTION and DECRYPTION keys are automatically constrained to MEDIUM security level.
  • Tests

    • Added coverage for six-key generation and security-level coercion (including deprecated default key generation).
    • Made SML diff snapshot tests run only when the local fixture is available.

@coderabbitai

coderabbitai Bot commented Apr 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7492a703-1cda-45d3-b7ff-4c8e78a20b69

📥 Commits

Reviewing files that changed from the base of the PR and between f5dd474 and 0102b84.

📒 Files selected for processing (5)
  • src/api/sml.test.ts
  • src/types.ts
  • src/ui/components.ts
  • src/ui/state.test.ts
  • src/ui/state.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/types.ts
  • src/ui/state.ts
  • src/api/sml.test.ts
  • src/ui/state.test.ts

📝 Walkthrough

Walkthrough

Adds a DECRYPTION key purpose, expands default identity keys from five to six (adding a Decryption key), constrains ENCRYPTION/DECRYPTION to MEDIUM security in UI and state updates, and adds tests for generation, state transitions, and security-level coercion.

Changes

DECRYPTION key purpose rollout

Layer / File(s) Summary
Type & UI constants
src/types.ts, src/ui/components.ts
KeyPurpose union now includes 'DECRYPTION'. KEY_PURPOSES adds DECRYPTION, and getAllowedSecurityLevels returns only ['MEDIUM'] for both ENCRYPTION and DECRYPTION.
Key generation implementation & tests
src/crypto/keys.ts, src/crypto/keys.test.ts
generateDefaultIdentityKeys and generateDefaultIdentityKeysHD now append a sixth Decryption key (id/keyIndex 5, purpose DECRYPTION, securityLevel MEDIUM). Tests added validating the 6-key layout for both HD and deprecated non-HD APIs.
State enforcement & tests
src/ui/state.ts, src/ui/state.test.ts
updateIdentityKey and updateManageNewKey coerce effective securityLevel to MEDIUM when purpose is ENCRYPTION or DECRYPTION. Tests verify setMode("create") yields six identity keys and that security-level coercion occurs in both update paths.
Conditional test fixture
src/api/sml.test.ts
decodeSmlDiff test suite runs only when a local fixture exists; imports existsSync and loads fixture bytes in beforeAll.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • dashpay/dash-bridge#5: Modifies supported key purposes in src/ui/components.ts, related to key purpose handling.

Poem

🐰 I nibble at code till morning light,
Six keys now guard the secret night.
DECRYPTION hops into the fold,
MEDIUM shields, steadfast and bold.
A rabbit cheers—secure and bright!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and directly describes the main change: adding a DECRYPTION key to the identity creation process, which aligns perfectly with the core objectives.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 8, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw
thepastaclaw force-pushed the tracker-946-auto-decryption-key branch 2 times, most recently from ff2576e to d651256 Compare April 8, 2026 17:41
@thepastaclaw
thepastaclaw marked this pull request as ready for review April 8, 2026 17:43
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@thepastaclaw
thepastaclaw force-pushed the tracker-946-auto-decryption-key branch from d651256 to 505289d Compare May 27, 2026 22:50
@thepastaclaw

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/main to resolve the merge conflict that appeared after the base moved.

Conflict

  • src/ui/state.test.ts — add/add conflict: both branches added a new test file. Resolved by combining both test suites; all 6 pre-existing setError tests are preserved alongside the new DECRYPTION coercion tests.
  • Other touched files (state.ts, components.ts, keys.ts, types.ts) auto-merged cleanly.

Validation

  • npm run test — all 15 tests pass across 3 files (proof builder, keys, state).
  • npm run build — fails on a pre-existing missing import in upstream/main itself (src/utils/islock-debug.js is referenced by src/api/dapi.ts and src/platform/identity.ts but never committed). Verified independently against a fresh upstream/main checkout; not introduced by this rebase.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown

@thepastaclaw Thanks for the detailed rebase notes! The pre-existing src/utils/islock-debug.js missing-import issue in upstream/main is clearly out of scope here. Triggering a review of the rebased branch now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/ui/components.ts (1)

74-86: ⚡ Quick win

Update function documentation to include ENCRYPTION/DECRYPTION restriction.

The comment currently only mentions the TRANSFER purpose restriction, but the function now also restricts ENCRYPTION and DECRYPTION purposes to MEDIUM security level. Update the JSDoc to document all purpose-specific restrictions.

📝 Proposed documentation update
 /**
  * Get allowed security levels for a given purpose
- * TRANSFER purpose only allows CRITICAL security level
+ * - TRANSFER purpose only allows CRITICAL security level
+ * - ENCRYPTION and DECRYPTION purposes only allow MEDIUM security level
  */
 function getAllowedSecurityLevels(purpose: KeyPurpose, includeMaster = true): SecurityLevel[] {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ui/components.ts` around lines 74 - 86, The JSDoc above
getAllowedSecurityLevels is out of date: it only mentions the TRANSFER
restriction but the implementation also restricts ENCRYPTION and DECRYPTION to
MEDIUM. Update the comment for getAllowedSecurityLevels to list all
purpose-specific rules (TRANSFER => CRITICAL only; ENCRYPTION and DECRYPTION =>
MEDIUM only) and mention the includeMaster behavior and default return of
SECURITY_LEVELS (and filtering out 'MASTER' when includeMaster is false) so
callers of KeyPurpose and SecurityLevel see the accurate contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/ui/components.ts`:
- Around line 74-86: The JSDoc above getAllowedSecurityLevels is out of date: it
only mentions the TRANSFER restriction but the implementation also restricts
ENCRYPTION and DECRYPTION to MEDIUM. Update the comment for
getAllowedSecurityLevels to list all purpose-specific rules (TRANSFER =>
CRITICAL only; ENCRYPTION and DECRYPTION => MEDIUM only) and mention the
includeMaster behavior and default return of SECURITY_LEVELS (and filtering out
'MASTER' when includeMaster is false) so callers of KeyPurpose and SecurityLevel
see the accurate contract.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 91168e4f-c047-4125-b5ef-887ad5de5432

📥 Commits

Reviewing files that changed from the base of the PR and between d651256 and 505289d.

📒 Files selected for processing (6)
  • src/crypto/keys.test.ts
  • src/crypto/keys.ts
  • src/types.ts
  • src/ui/components.ts
  • src/ui/state.test.ts
  • src/ui/state.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/ui/state.ts
  • src/types.ts
  • src/crypto/keys.ts
  • src/crypto/keys.test.ts
  • src/ui/state.test.ts

@thepastaclaw

Copy link
Copy Markdown
Contributor Author

Addressed CodeRabbit's JSDoc nit in 5c857c1 by documenting the TRANSFER, ENCRYPTION, and DECRYPTION security-level restrictions plus the includeMaster behavior.

Validation:

  • git diff --check
  • npx vitest run — 15 tests passed

Note: the pre-commit npm run build check still fails on the pre-existing src/utils/islock-debug.js missing import from upstream/main, same as noted after the rebase; this docs-only change does not affect that.

@thepastaclaw

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

src/api/dapi.ts and src/platform/identity.ts import describeIslock and
diffIslockInputsAgainstTx from ../utils/islock-debug.js, but the module
was never committed, breaking tsc --noEmit in CI.

Add a best-effort ISLock parser that handles both DIP-22 (v1) and
DIP-24 (v2) layouts and returns a log-friendly structured summary.
@thepastaclaw

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

thepastaclaw and others added 2 commits May 28, 2026 19:43
Take upstream's richer islock-debug.ts (uses dashcore-lib InstantLock
parser, exposes requestId/hashId) over the fork's hand-rolled binary
parser; the merged utils/index.ts already exports the upstream API
(IslockDebugInfo) referenced across the codebase.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CBOR snapshot at scripts/fixtures/sml-paloma-snapshot.bin is a
developer-only devnet capture that is not checked into the repo, so the
top-level readFileSync threw ENOENT in CI before any test could run.
Guard the describe with existsSync and load the bytes lazily in
beforeAll so the suite is skipped cleanly when the fixture is missing
and still runs locally when developers have it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thepastaclaw

thepastaclaw commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

✅ Review complete (commit 0102b84)

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

PR adds a DECRYPTION identity key alongside ENCRYPTION across types, both default key generators (legacy and HD), the UI purpose list, the create/manage security-level coercion paths, and tests. Registration wiring is automatic because registerIdentity already forwards purpose.toLowerCase() and the SDK's KeyPurpose enum already includes DECRYPTION. The unrelated sml.test.ts fixture-skip change is small and benign.

@thepastaclaw

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant