DIP: Dash Platform Application Key Exchange and State Transition Signing#181
DIP: Dash Platform Application Key Exchange and State Transition Signing#181PastaPastaPasta wants to merge 2 commits intodashpay:masterfrom
Conversation
…ansition Signing Defines the dash-key: and dash-st: URI protocols for wallet-based QR code login to Dash Platform applications. Covers ECDH key exchange, AES-256-GCM encryption, BIP32+HKDF key derivation, the loginKeyResponse contract schema, and first-time key registration flow.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new DIP describing two URI-based protocols for wallet–app interaction: dash-key (Key Exchange Protocol) and dash-st (State Transition Signing Protocol), specifying URI formats, payload encodings, cryptographic key derivation/encryption flows, data contract schemas, validation rules, and security/UX considerations. Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
dip-pasta-yappr.md (2)
262-265: Consider clarifying concurrent request handling.The keyIndex logic states that if no document exists,
keyIndexis0. If a wallet processes multiple key exchange requests for the same identity and contract concurrently (before the first document is created), both could usekeyIndex0, potentially creating a race condition. Consider adding guidance on how wallets should handle this scenario (e.g., serialize requests per identity+contract or retry on conflict).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dip-pasta-yappr.md` around lines 262 - 265, Clarify concurrent-request behavior for keyIndex/loginKeyResponse: explain that when no loginKeyResponse exists wallets must handle races by serializing requests per identity+contract or using a create-if-not-exists atomic operation and retry-on-conflict; reference the keyIndex and loginKeyResponse documents and state a recommended strategy (e.g., queue/serialize per identity+contract, use optimistic concurrency/version checks on loginKeyResponse, or retry on conflict) so two concurrent requests cannot both assume keyIndex = 0 and create duplicate keys.
391-401: Consider exponential backoff for revision conflict retries.The document lifecycle specifies retrying up to 3 times with a fixed 500ms delay on revision conflicts. Consider recommending exponential backoff (e.g., 500ms, 1000ms, 2000ms) to reduce contention under high load while maintaining the same maximum retry count.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dip-pasta-yappr.md` around lines 391 - 401, The retry logic for handling revision conflicts when updating a loginKeyResponse document should use exponential backoff instead of a fixed 500ms delay: update the retry loop that currently retries up to 3 times with a fixed 500ms pause to instead wait 500ms, then 1000ms, then 2000ms between attempts (keeping the same max retry count), and continue to re-query the loginKeyResponse document on each retry and preserve the existing update/create flow and revision bump behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dip-pasta-yappr.md`:
- Line 36: The Table of Contents link fragment "#supported-encodings" is broken
because the actual header is "## Encoding"; update either the TOC entry or the
heading so they match—e.g., change the TOC item "* [Supported
Encodings](`#supported-encodings`)" to "* [Encoding](`#encoding`)" or rename the
header "## Encoding" to "## Supported Encodings" so the fragment and header text
are consistent.
---
Nitpick comments:
In `@dip-pasta-yappr.md`:
- Around line 262-265: Clarify concurrent-request behavior for
keyIndex/loginKeyResponse: explain that when no loginKeyResponse exists wallets
must handle races by serializing requests per identity+contract or using a
create-if-not-exists atomic operation and retry-on-conflict; reference the
keyIndex and loginKeyResponse documents and state a recommended strategy (e.g.,
queue/serialize per identity+contract, use optimistic concurrency/version checks
on loginKeyResponse, or retry on conflict) so two concurrent requests cannot
both assume keyIndex = 0 and create duplicate keys.
- Around line 391-401: The retry logic for handling revision conflicts when
updating a loginKeyResponse document should use exponential backoff instead of a
fixed 500ms delay: update the retry loop that currently retries up to 3 times
with a fixed 500ms pause to instead wait 500ms, then 1000ms, then 2000ms between
attempts (keeping the same max retry count), and continue to re-query the
loginKeyResponse document on each retry and preserve the existing update/create
flow and revision bump behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6edfd7c4-83bf-45c9-b579-c785373b1631
📒 Files selected for processing (1)
dip-pasta-yappr.md
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9482c77c3f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dip-pasta-yappr.md
Outdated
| | ---------- | ----- | ----------- | | ||
| | 9' | Feature purpose (per [DIP-0009](https://github.com/dashpay/dips/blob/master/dip-0009.md)) | | | ||
| | coin_type' | `5` (mainnet) or `1` (testnet/devnet/regtest) | Per [BIP-0044](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki) | | ||
| | 21' | Key exchange feature index | | |
dip-pasta-yappr.md
Outdated
| | --------- | -------- | ----------- | | ||
| | `n` | Yes | Network identifier (`m`, `t`, `d`, or `r`) | | ||
| | `v` | Yes | Protocol version; must be `1` | | ||
| | `id` | No | Base58-encoded identity identifier hint (32 bytes); wallet should pre-select this identity | |
There was a problem hiding this comment.
Extraneous word?
| | `id` | No | Base58-encoded identity identifier hint (32 bytes); wallet should pre-select this identity | | |
| | `id` | No | Base58-encoded identity identifier (32 bytes); wallet should pre-select this identity | |
- Fix broken ToC link: rename heading to 'Supported Encodings' to match ToC entry - Make Binary Payload Layout a subsection of URI Format and reorder above Network Identifiers - Simplify BIP32 path table by removing empty Description column - Add hyperlink to Network Identifiers from dash-st component table - Remove extraneous 'hint' from id parameter description - Remove unnecessary 'Read Only' rows from key registration tables
2 participants
Summary
dash-key:URI protocol for wallet-based QR code login to Dash Platform applications using ECDH key exchange and AES-256-GCM encryptiondash-st:URI protocol for requesting wallet signature and broadcast of unsigned state transitionsm/9'/coin_type'/21'/account'), HKDF key derivation, ECDH shared secret, and per-contract/per-identity key isolationloginKeyResponsePlatform contract schema, indices, and polling mechanismIdentityUpdateTransitionwith ECDSA_HASH160 keysTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit