Skip to content

Conversation

thephez
Copy link
Collaborator

@thephez thephez commented Sep 3, 2025

Issue being fixed or feature implemented

Remove unused parameters from state transitions.

What was done?

  • Removed unused _key_id parameters from 5 document state transition functions (document_replace, document_delete,
    document_transfer, document_purchase, document_set_price)
  • Removed keyId parameter from tokenDirectPurchase state transition
  • Updated JavaScript bindings in index.html to remove hardcoded 0 values for key_id
  • Updated test fixtures to remove the unused keyId field
  • Regenerated documentation to reflect the API changes

How Has This Been Tested?

Locally via playwright tests

Breaking Changes

This is technically a breaking change for the WASM SDK API but it only removes parameters that were never functional. The SDK continues to work correctly by automatically determining the appropriate key for signing operations.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • 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
  • 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

  • Refactor

    • Simplified SDK APIs by removing the explicit Key ID parameter from document actions (create/replace/delete/transfer/set price/purchase) and token direct purchase; signing is now derived automatically.
    • Clarified naming by renaming the “identityId” parameter to “frozenIdentityId” for token frozen-destroy operations.
  • Documentation

    • Updated API reference and examples to reflect the removed Key ID parameter and the new “frozenIdentityId” name.
  • Tests

    • Adjusted fixtures to match the updated input shapes.
  • Chores

    • Regenerated documentation metadata.

thephez and others added 2 commits September 3, 2025 16:33
…ctions

Remove unused _key_id and key_id parameters from document state transition functions:
- document_replace, document_delete, document_transfer (removed _key_id: u32)
- document_purchase, document_set_price (removed key_id: u32)

These parameters were never used as the functions automatically find the correct
authentication key using find_authentication_key() based on the private key.

Also updated index.html JavaScript calls to remove the hardcoded 0 values that
were being passed for these unused parameters.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove keyId field from api-definitions.json
- Remove keyId from test-data.js fixture
- Regenerate docs without keyId parameter
- Fix frozenIdentityId parameter name in AI_REFERENCE.md

The keyId parameter was never used as the SDK automatically finds
the correct authentication key based on the provided private key.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@thephez thephez added this to the v2.1 milestone Sep 3, 2025
Copy link
Contributor

coderabbitai bot commented Sep 3, 2025

Walkthrough

Removes the keyId parameter from multiple WASM SDK document and token APIs and docs, updates API definitions and examples accordingly, and renames tokenDestroyFrozen’s identityId parameter to frozenIdentityId. The Rust bindings now derive the signing key from the provided private key. Test fixtures and generated docs metadata were updated.

Changes

Cohort / File(s) Summary
Docs: Token and API reference
packages/wasm-sdk/AI_REFERENCE.md, packages/wasm-sdk/docs.html
Removed keyId from tokenDirectPurchase docs; renamed tokenDestroyFrozen parameter from identityId to frozenIdentityId; pruned corresponding docs block in HTML.
API schema
packages/wasm-sdk/api-definitions.json
Deleted keyId input from tokenDirectPurchase transition schema; no new inputs added.
WASM SDK bindings (Rust)
packages/wasm-sdk/src/state_transitions/documents/mod.rs
Removed key_id parameter from document_create/replace/delete/transfer/purchase/set_price; derive signing key via find_authentication_key from private_key_wif.
Web demo invocations
packages/wasm-sdk/index.html
Updated document* calls to drop trailing key_id argument across five operations.
Test fixtures
packages/wasm-sdk/test/ui-automation/fixtures/test-data.js
Removed keyId field from tokenDirectPurchase test data entry.
Docs manifest
packages/wasm-sdk/docs_manifest.json
Updated generated_at timestamp only.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant UI as App/UI
  participant SDK as Wasm SDK (documents)
  participant Key as find_authentication_key
  participant Sig as Signer
  participant Plat as Platform Node

  UI->>SDK: document_replace(contractId, type, id, identityId, data, rev, privateKey)
  SDK->>Key: derive matching_key from privateKey
  Key-->>SDK: matching_key
  SDK->>Sig: sign transition with matching_key
  Sig-->>SDK: signed transition
  SDK->>Plat: submit signed transition
  Plat-->>SDK: result/ack
  SDK-->>UI: response
Loading
sequenceDiagram
  autonumber
  participant UI as App/UI
  participant SDK as Wasm SDK (token)
  participant Plat as Platform Node

  UI->>SDK: tokenDirectPurchase(contractId, tokenPosition, amount, totalAgreedPrice, identityId, privateKey)
  note right of SDK: keyId parameter removed
  SDK->>Plat: submit transition (signed via derived key)
  Plat-->>SDK: result/ack
  SDK-->>UI: response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • QuantumExplorer
  • pauldelucia

Poem

I nudge the keys, but drop the ID—
a tidy hop through fields of API.
From private whiskers, signatures grow,
while frozen tokens learn new names to know.
Thump-thump! The docs and tests align—
less clutter now, more clean design.
ʕ•ᴥ•ʔ… or was it (\\/) all the time? 🥕

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch wasm-sdk-feat-cleanup-unused-key-params

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/wasm-sdk/index.html (1)

3025-3060: Drop explicit null key_id args in UI calls
packages/wasm-sdk/index.html L3490 and L3504: remove the null positional argument so the SDK auto-selects the key.
No other hardcoded key_id or 0/null positional usages remain in UI or tests.

🧹 Nitpick comments (4)
packages/wasm-sdk/index.html (4)

3338-3345: Removed unused key_id from documentSetPrice — good; minor input guard

BigInt(values.price || 0) is fine. Consider trimming to avoid " 123" throwing if users paste with spaces.

-            BigInt(values.price || 0), // price in credits, 0 to remove price
+            BigInt(String(values.price ?? '').trim() || 0), // price in credits, 0 to remove

3470-3478: Removed unused key_id from documentPurchase — good; add basic validation

Add a quick check so empty price doesn’t throw at runtime.

-          result = await sdk.documentPurchase(
+          if (!values.price || String(values.price).trim() === '') {
+            throw new Error('Price is required for document purchase');
+          }
+          result = await sdk.documentPurchase(
             values.contractId,
             values.documentType,
             values.documentId,
             identityId,
-            BigInt(values.price),
+            BigInt(String(values.price).trim()),
             privateKey
           );

141-144: Clarify “:keyId” suffix scope in label

Label says “optionally with :keyId”. Since key_id was removed from most transitions and auto-selection is default, consider clarifying this is only needed for DPNS voting/registration flows that still require explicit key selection in the UI.

-            <label for="privateKey">Private Key (WIF or hex, optionally with :keyId)</label>
+            <label for="privateKey">Private Key (WIF or hex; append :keyId only for DPNS vote/registration)</label>

3286-3317: Doc replace path relies on loadedDocumentRevision — guard against stale value

You already check for null; also guard that it’s numeric before BigInt to avoid NaN-to-BigInt errors if the loader changes.

-            BigInt(loadedDocumentRevision),
+            BigInt(String(loadedDocumentRevision)),
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a33e99e and 1cd1c4c.

📒 Files selected for processing (7)
  • packages/wasm-sdk/AI_REFERENCE.md (1 hunks)
  • packages/wasm-sdk/api-definitions.json (0 hunks)
  • packages/wasm-sdk/docs.html (0 hunks)
  • packages/wasm-sdk/docs_manifest.json (1 hunks)
  • packages/wasm-sdk/index.html (5 hunks)
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs (0 hunks)
  • packages/wasm-sdk/test/ui-automation/fixtures/test-data.js (0 hunks)
💤 Files with no reviewable changes (4)
  • packages/wasm-sdk/test/ui-automation/fixtures/test-data.js
  • packages/wasm-sdk/docs.html
  • packages/wasm-sdk/api-definitions.json
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs
🧰 Additional context used
📓 Path-based instructions (2)
packages/wasm-sdk/index.html

📄 CodeRabbit inference engine (CLAUDE.md)

Test the WASM SDK using the web interface at 'index.html' in 'packages/wasm-sdk'

Files:

  • packages/wasm-sdk/index.html
packages/wasm-sdk/**/index.html

📄 CodeRabbit inference engine (packages/wasm-sdk/CLAUDE.md)

packages/wasm-sdk/**/index.html: When adding new queries or state transitions, update the definitions in index.html.
The WASM SDK now fully supports where and orderBy clauses for document queries; use the specified JSON array formats and supported operators.

Files:

  • packages/wasm-sdk/index.html
🧠 Learnings (11)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.
📚 Learning: 2025-09-03T14:42:29.907Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/docs.html:1970-1971
Timestamp: 2025-09-03T14:42:29.907Z
Learning: In packages/wasm-sdk/, the docs.html file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in docs.html, as manual changes to docs.html would be overwritten during regeneration.

Applied to files:

  • packages/wasm-sdk/docs_manifest.json
  • packages/wasm-sdk/index.html
📚 Learning: 2025-09-03T14:41:16.119Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/AI_REFERENCE.md:766-766
Timestamp: 2025-09-03T14:41:16.119Z
Learning: In packages/wasm-sdk/, the AI_REFERENCE.md file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in AI_REFERENCE.md, as manual changes to AI_REFERENCE.md would be overwritten during regeneration.

Applied to files:

  • packages/wasm-sdk/docs_manifest.json
  • packages/wasm-sdk/AI_REFERENCE.md
  • packages/wasm-sdk/index.html
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : For WASM builds, fix 'time not implemented on this platform' errors by using js_sys::Date::now().

Applied to files:

  • packages/wasm-sdk/docs_manifest.json
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/**/index.html : When adding new queries or state transitions, update the definitions in index.html.

Applied to files:

  • packages/wasm-sdk/docs_manifest.json
📚 Learning: 2025-07-23T08:31:05.082Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:05.082Z
Learning: Applies to packages/wasm-sdk/generate_docs.py : Keep documentation for the WASM SDK in sync by running 'python3 generate_docs.py' in 'packages/wasm-sdk'

Applied to files:

  • packages/wasm-sdk/docs_manifest.json
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/**/index.html : The WASM SDK now fully supports where and orderBy clauses for document queries; use the specified JSON array formats and supported operators.

Applied to files:

  • packages/wasm-sdk/docs_manifest.json
  • packages/wasm-sdk/index.html
📚 Learning: 2025-07-28T20:00:08.502Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.

Applied to files:

  • packages/wasm-sdk/docs_manifest.json
  • packages/wasm-sdk/AI_REFERENCE.md
  • packages/wasm-sdk/index.html
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : When implementing WASM SDK functionality, always refer to AI_REFERENCE.md first for accurate method signatures and examples.

Applied to files:

  • packages/wasm-sdk/AI_REFERENCE.md
📚 Learning: 2025-08-28T14:06:02.805Z
Learnt from: thephez
PR: dashpay/platform#2739
File: packages/wasm-sdk/test/ui-automation/fixtures/test-data.js:711-723
Timestamp: 2025-08-28T14:06:02.805Z
Learning: The tokenDestroyFrozen operation destroys the entire identity balance for that token and does not require an amount parameter.

Applied to files:

  • packages/wasm-sdk/AI_REFERENCE.md
📚 Learning: 2025-07-28T20:00:24.323Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.

Applied to files:

  • packages/wasm-sdk/index.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build JS packages / Build JS
  • GitHub Check: Rust crates security audit
  • GitHub Check: build-wasm-sdk
🔇 Additional comments (7)
packages/wasm-sdk/docs_manifest.json (1)

2-2: Docs manifest timestamp updated — OK

The generated_at update reflects regenerated docs. No other structural changes.

packages/wasm-sdk/AI_REFERENCE.md (1)

1177-1177: Rename to frozenIdentityId is consistently applied; no lingering identityId under tokenDestroyFrozen.

packages/wasm-sdk/index.html (5)

3311-3317: Removed unused key_id from documentDelete — good

Signature now: documentDelete(contractId, documentType, documentId, identityId, privateKey).


3324-3331: Removed unused key_id from documentTransfer — good

Signature now: documentTransfer(contractId, documentType, documentId, identityId, recipientId, privateKey).


2991-3011: Token Direct Purchase call no longer passes keyId — consistent

tokenDirectPurchase now omits keyId and passes totalAgreedPrice nullable. Matches the doc change.


3191-3198: Arg order for tokenDirectPurchase verified
The JS invocation’s parameters—(contractId, tokenPosition, amount, identityId, totalAgreedPrice | null, privateKey)—match the WASM export signature (data_contract_id, token_position, amount, identity_id, total_agreed_price, private_key_wif). No changes required.


3291-3300: Approve documentReplace signature
The JSX call’s seven parameters (contractId, documentType, documentId, ownerId, documentData JSON string, BigInt revision, privateKey) exactly match the #[wasm_bindgen(js_name = documentReplace)] pub async fn document_replace(&self, data_contract_id: String, document_type: String, document_id: String, owner_id: String, document_data: String, revision: u64, private_key_wif: String) binding in state_transitions/documents/mod.rs.

@QuantumExplorer QuantumExplorer merged commit 5679dcd into v2.1-dev Sep 4, 2025
19 of 20 checks passed
@QuantumExplorer QuantumExplorer deleted the wasm-sdk-feat-cleanup-unused-key-params branch September 4, 2025 17:04
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.

2 participants