platform icon indicating copy to clipboard operation
platform copied to clipboard

feat(sdk): add validation/tests for registerName publicKeyId parameter

Open thephez opened this issue 2 months ago • 3 comments

Issue being fixed or feature implemented

Developers using the DPNS registration API were encountering a cryptic error message "Invalid public key security level MASTER" when the identity key parameter was not passed. This error only appeared after attempting to sign the state transition, making it difficult to understand what went wrong and how to fix it.

What was done?

Added comprehensive early validation and helpful error messages for DPNS name registration:

  • WASM SDK validation (packages/wasm-sdk/src/dpns.rs): Validate key purpose is AUTHENTICATION, security level is CRITICAL or HIGH (not MASTER), and private key matches the specified public key before attempting cryptographic operations
  • JS SDK validation (packages/js-evo-sdk/src/dpns/facade.ts): Add input validation for publicKeyId parameter with clear error messages and comprehensive JSDoc documentation
  • Test coverage (packages/js-evo-sdk/tests/unit/facades/dpns.spec.mjs): Add 6 test cases covering validation of omitted, undefined, null, negative, non-number, and valid publicKeyId values

Error messages now explain which requirement failed, list all suitable keys available in the identity, and provide specific guidance (e.g., "Use key 1 (CRITICAL) or 2 (HIGH), NOT 0 (MASTER)").

How Has This Been Tested?

  • Unit tests added for all validation scenarios (6 new test cases, all passing)

Breaking Changes

None. This is fully backwards compatible - it only adds validation and improves error messages.

Checklist:

  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [x] I have added or updated relevant unit/integration/functional/e2e tests
  • [ ] I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • [x] I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • [ ] I have assigned this pull request to a milestone

Summary by CodeRabbit

Release Notes

  • New Features

    • Added DPNS name registration functionality with comprehensive validation to ensure proper key configuration and security requirements are met.
  • Tests

    • Added test coverage for DPNS registration validation scenarios, including edge cases and error handling.

thephez avatar Oct 28 '25 18:10 thephez

@coderabbitai review

thephez avatar Oct 28 '25 18:10 thephez

Walkthrough

These changes introduce a new registerName public method to the DPNS facade with input validation at the TypeScript layer. The WASM layer adds corresponding key validation to ensure the public key has AUTHENTICATION purpose, appropriate security levels (CRITICAL or HIGH), and matching private key credentials before proceeding with DPNS registration.

Changes

Cohort / File(s) Summary
JavaScript/TypeScript DPNS facade
packages/js-evo-sdk/src/dpns/facade.ts, packages/js-evo-sdk/tests/unit/facades/dpns.spec.mjs
Adds registerName method to DpnsFacade with publicKeyId validation (null/undefined checks, non-negative number requirement). Companion test suite validates edge cases (missing, undefined, null, negative, non-numeric values) and positive registration path.
Rust/WASM validation layer
packages/wasm-sdk/src/dpns.rs
Introduces DPNS key validation logic: retrieves identity public key, enforces AUTHENTICATION purpose, validates security level (CRITICAL or HIGH), and verifies private key matches via signer. Returns descriptive errors with guidance for invalid keys.

Sequence Diagram

sequenceDiagram
    participant User as User Code
    participant JsFacade as JS Facade
    participant Wasm as WASM SDK
    participant KeyValidator as Key Validator
    
    User->>JsFacade: registerName(label, identityId, publicKeyId, ...)
    activate JsFacade
    JsFacade->>JsFacade: Validate publicKeyId (not null/undefined, non-negative)
    alt publicKeyId invalid
        JsFacade-->>User: Throw validation error
    else publicKeyId valid
        JsFacade->>Wasm: w.dpnsRegisterName(label, identityId, publicKeyId, privateKeyWif, ...)
        activate Wasm
        Wasm->>KeyValidator: Get and validate identity public key
        activate KeyValidator
        KeyValidator->>KeyValidator: Check purpose = AUTHENTICATION
        KeyValidator->>KeyValidator: Check security level = CRITICAL/HIGH
        KeyValidator->>KeyValidator: Verify private key matches
        alt Key validation fails
            KeyValidator-->>Wasm: Invalid key error
            Wasm-->>User: Throw validation error
        else Key validation succeeds
            KeyValidator-->>Wasm: Valid key
            Wasm->>Wasm: Register DPNS name
            Wasm-->>JsFacade: Success
            deactivate Wasm
        end
        deactivate KeyValidator
        JsFacade-->>User: Registration result
    end
    deactivate JsFacade

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • packages/js-evo-sdk/src/dpns/facade.ts — Review new public API method, ensure validation logic is comprehensive and error messages are appropriate
  • packages/wasm-sdk/src/dpns.rs — Verify key validation logic (purpose and security level checks) and private key matching via signer; confirm error handling with meaningful guidance
  • Test coverage — Confirm all validation edge cases are tested and positive path execution is verified

Poem

🐰 A new gateway opens wide,
With keys validated side by side,
AUTHENTICATION strong and true,
CRITICAL or HIGH—we'll see you through!
DPNS names now safer rise,
Beneath the rabbit's watchful eyes. 🔐

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title Check ✅ Passed The pull request title "feat(sdk): add validation/tests for registerName publicKeyId parameter" clearly and accurately describes the primary change in the changeset. The title correctly identifies the feature scope (SDK), the action (adding validation and tests), and the specific aspect affected (registerName's publicKeyId parameter). While the changeset includes additional WASM SDK validation logic for key purpose, security level, and private key matching, the title's focus on the publicKeyId parameter represents the main user-facing API change in the JS SDK facade and corresponds to the test coverage added. The title is concise, specific enough for teammates to understand the primary change, and does not use vague terminology.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch feat/dpns-key-validation

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.

coderabbitai[bot] avatar Oct 28 '25 18:10 coderabbitai[bot]

✅ 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[bot] avatar Oct 28 '25 18:10 coderabbitai[bot]