docs(dip-9): add asset lock subfeatures and document key types#177
Open
QuantumExplorer wants to merge 6 commits intomasterfrom
Open
docs(dip-9): add asset lock subfeatures and document key types#177QuantumExplorer wants to merge 6 commits intomasterfrom
QuantumExplorer wants to merge 6 commits intomasterfrom
Conversation
…ey types Add subfeatures 4' (Asset Lock Address Topup Funding) and 5' (Asset Lock Shielded Address Topup Funding) to the Identity Keys feature assignments. Document ECDSA (0') and BLS (1') key types within the authentication subfeature. Fix typo in invitation funding link anchor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughUpdated two specification documents: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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)
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 |
Clarify that subfeatures 1'-3' are asset lock funding paths by renaming them to Identity Registration/Topup/Invitation Asset Lock Funding, consistent with the naming of subfeatures 4' and 5'. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![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 (1)
dip-0009/assignments.md (1)
9-9: Nit:bls12381is a non-standard form of the curve name.Every canonical reference (IETF, Zcash, Ethereum) spells it
BLS12-381(hyphenated). Compare with the ECDSA curve identifier in the same line:secp256k1is a well-established, hyphen-free identifier, whereasBLS12-381carries the hyphen as part of its standard name. Usingbls12381may cause confusion for readers cross-referencing specs.✏️ Suggested fix
- `1'` - BLS (bls12381) + `1'` - BLS (BLS12-381)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dip-0009/assignments.md` at line 9, Update the non-standard curve name "bls12381" to the canonical "BLS12-381" in the Identity Keys table entry where Key types are listed (specifically the line showing `1' - BLS (bls12381)`), and ensure the change is reflected wherever that identifier appears in the same section (e.g., under Identity Authentication Keys and any examples referencing BLS key type) so readers see the hyphenated standard form.
🤖 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-0009/assignments.md`:
- Line 9: The two new sub-path entries `4'/*` and `5'/*` lack specification
links — update the entries for `4'/*` - Asset Lock Address Topup Funding and
`5'/*` - Asset Lock Shielded Address Topup Funding to include the proper DIP
reference (e.g., add the DIP-0013 anchor links like the other entries) if those
sections exist in DIP-13, otherwise append a short explicit note such as `(spec
pending)` to each entry so readers know the spec is intentionally missing;
ensure you mirror the existing link/format style used by `0'`–`3'` for
consistency.
---
Nitpick comments:
In `@dip-0009/assignments.md`:
- Line 9: Update the non-standard curve name "bls12381" to the canonical
"BLS12-381" in the Identity Keys table entry where Key types are listed
(specifically the line showing `1' - BLS (bls12381)`), and ensure the change is
reflected wherever that identifier appears in the same section (e.g., under
Identity Authentication Keys and any examples referencing BLS key type) so
readers see the hyphenated standard form.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@dip-0009/assignments.md`:
- Line 9: Update the `4'/*` and `5'/*` sub-path entries in assignments.md to
include either proper specification links to the corresponding DIP-0013 sections
or an explicit placeholder note showing the spec is pending; specifically edit
the table row text that currently reads "`4'/*` - Asset Lock Address Topup
Funding" and "`5'/*` - Asset Lock Shielded Address Topup Funding" to append
"([details](../dip-0013.md#...))" when the DIP-13 anchors exist or add "(spec
pending)" in place of the link until those DIP-13 sections/anchors are finalized
so the omission is intentional and visible.
DIP-13 now documents two derivation methods for top up funding keys: - Not bound to identity: m/9'/coinType'/5'/2'/funding_index - Bound to specific identity: m/9'/coinType'/5'/2'/registration_index'/top_up_index DIP-9 assignments updated to show both sub-paths under subfeature 2'. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6 tasks
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dip-0013.md (1)
165-172:⚠️ Potential issue | 🟡 MinorRecovery steps are incomplete for unbound top-up keys.
The recovery procedure describes querying Dash Platform to check if the top-up has been applied and then performing an identity top-up transition. This workflow implicitly assumes a bound key where the hardened
registration index'identifies the target identity. For unbound keys there is no embedded identity reference, so the wallet cannot determine which identity to top up from the key path alone. The section should address this case — e.g., prompt the user to select an identity, or specify that unbound keys can only be reused if the originating identity is already known through other context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dip-0013.md` around lines 165 - 172, The recovery section assumes a bound top up funding key (using the hardened registration index) but misses the unbound-key case; update the text around "top up funding key", "registration index" and the "identity top up transition" (DIP11) to explicitly handle unbound keys by: detect whether the recovered top-up key is bound or unbound, and if unbound require explicit identity selection or external context before attempting an identity top up transition (per DIP11), or state that unbound keys may only be reused when the target identity is known from other sources; add language describing user prompt/workflow and the conditional check prior to initiating the top-up.
🧹 Nitpick comments (1)
dip-0013.md (1)
105-105: "top up" used as a compound modifier without a hyphen throughout the changed lines.LanguageTool flags "top up funding keys", "top up transaction", "top up index" etc. as needing a hyphen when the phrase attributively modifies a noun. The existing document heading "# Identity Top Up Funding keys" already sets an unhyphenated precedent, and the PR commit message explicitly standardizes to "top up" — so this is a style choice rather than a bug. That said, the grammar rule applies to compound-modifier positions and is consistent in standard technical writing.
✏️ Representative replacements (compound-modifier positions only)
-Identity Funding keys can either be for a registration or for a top up. +Identity Funding keys can either be for a registration or for a top-up. -Identity Funding keys used for top up are the third sub-feature +Identity Funding keys used for top-up are the third sub-feature -There are two derivation methods for top up funding keys: +There are two derivation methods for top-up funding keys: -the first unbound top up funding key +the first unbound top-up funding key -the top up index is a sequential non-hardened index for each top up transaction +the top-up index is a sequential non-hardened index for each top-up transaction -the first top up funding key +the first top-up funding key -30 address hashes of both unbound top up funding keys and bound top up funding keys +30 address hashes of both unbound top-up funding keys and bound top-up funding keysAlso applies to: 137-137, 142-142, 152-152, 159-160, 162-162, 174-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dip-0013.md` at line 105, The document uses "top up" without a hyphen when used as a compound modifier; update only the attributive instances to use "top-up" (e.g., change "top up funding keys" to "top-up funding keys", "top up transaction" to "top-up transaction", and "top up index" to "top-up index") while leaving non-attributive uses and the existing heading "Identity Top Up Funding keys" unchanged; search for the phrases "top up funding keys", "top up transaction", "top up index" and replace them where they directly modify a noun.
🤖 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-0013.md`:
- Around line 174-177: Reword the ambiguous guidance to explicitly state counts
and regeneration semantics: replace the sentence starting "It is recommended
that 30 address hashes of both unbound top up funding keys and bound top up
funding keys for each known identity be added..." with a version that clearly
says "30 address hashes of unbound top-up funding keys and 30 address hashes of
bound top-up funding keys for each known identity" (or, if the intent is
combined, explicitly state a single combined 30); also clarify the regeneration
clause that currently reads "If transactions containing 25 address hashes or
more have been found for either category, the filter should be regenerated with
30 more" to specify whether the 25/30 trigger applies per-identity for bound
keys or across all identities (e.g., "apply the 25-count trigger separately for
unbound keys and for bound keys per identity" or "apply the 25-count trigger
across the combined set"), so implementors know whether to count bound-key
matches per identity or globally.
---
Outside diff comments:
In `@dip-0013.md`:
- Around line 165-172: The recovery section assumes a bound top up funding key
(using the hardened registration index) but misses the unbound-key case; update
the text around "top up funding key", "registration index" and the "identity top
up transition" (DIP11) to explicitly handle unbound keys by: detect whether the
recovered top-up key is bound or unbound, and if unbound require explicit
identity selection or external context before attempting an identity top up
transition (per DIP11), or state that unbound keys may only be reused when the
target identity is known from other sources; add language describing user
prompt/workflow and the conditional check prior to initiating the top-up.
---
Duplicate comments:
In `@dip-0009/assignments.md`:
- Line 9: The entries for the new sub-paths `4'/*` and `5'/*` in assignments.md
lack specification links or status notes; update the `4'/*` and `5'/*` lines to
either include the appropriate DIP reference(s) (if specs exist) or explicitly
mark them as "spec pending" with a brief pointer to the related DIP or issue
that will define them, so readers know these subfeatures are unresolved rather
than accidentally omitted.
---
Nitpick comments:
In `@dip-0013.md`:
- Line 105: The document uses "top up" without a hyphen when used as a compound
modifier; update only the attributive instances to use "top-up" (e.g., change
"top up funding keys" to "top-up funding keys", "top up transaction" to "top-up
transaction", and "top up index" to "top-up index") while leaving
non-attributive uses and the existing heading "Identity Top Up Funding keys"
unchanged; search for the phrases "top up funding keys", "top up transaction",
"top up index" and replace them where they directly modify a noun.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
dip-0013.md (1)
105-105: "top up" used as a compound modifier should be hyphenated to "top-up".LanguageTool and standard English style require a hyphen when "top up" precedes a noun as a compound modifier (e.g., "top-up funding keys", "top-up transaction", "top-up index"). Affected lines: 105, 137, 142, 150, 152, 159, 160, 162, 174, 175. Note: the section headings (
## Top Up Funding keys) are title-case and their hyphenation style is a separate editorial choice.✏️ Proposed fixes (representative sample)
-Identity Funding keys can either be for a registration or for a top up. We will create for each +Identity Funding keys can either be for a registration or for a top-up. We will create for each-Identity Funding keys used for top up are the third sub-feature for identities. They are always +Identity Funding keys used for a top up are the third sub-feature for identities. They are always-There are two derivation methods for top up funding keys: +There are two derivation methods for top-up funding keys:-the case for top up funding keys. +the case for top-up funding keys.-the first unbound top up funding key for Dash would be at `m/9'/5'/5'/2'/0`. +the first unbound top-up funding key for Dash would be at `m/9'/5'/5'/2'/0`.-the top up index is a sequential non-hardened index for each top up transaction. +the top-up index is a sequential non-hardened index for each top-up transaction.-the first top up funding key for the first identity on Dash would be at +the first top-up funding key for the first identity on Dash would be at-It is recommended that 30 address hashes of unbound top up funding keys and 30 address hashes of -bound top up funding keys for each known identity be added to the bloom filter sent to peers. +It is recommended that 30 address hashes of unbound top-up funding keys and 30 address hashes of +bound top-up funding keys for each known identity be added to the bloom filter sent to peers.Also applies to: 137-137, 142-142, 150-152, 159-163, 174-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dip-0013.md` at line 105, Replace instances of the phrase "top up" used as a compound modifier with the hyphenated form "top-up" (e.g., change "top up funding keys", "top up transaction", "top up index" to "top-up funding keys", "top-up transaction", "top-up index") across the document (noting that the section heading "## Top Up Funding keys" is a title-case editorial choice and can be handled separately); specifically update the occurrences around the sentence starting "Identity Funding keys can either be for a registration or for a top up" and the other affected usages referenced in the comment so all compound modifiers use "top-up".
🤖 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-0013.md`:
- Line 178: Replace the typo phrase "found for one or one identities" in the DIP
text with "found for one or more identities" so the regeneration condition reads
correctly; search for the exact string "found for one or one identities" in
dip-0013.md and update it to "found for one or more identities".
---
Nitpick comments:
In `@dip-0013.md`:
- Line 105: Replace instances of the phrase "top up" used as a compound modifier
with the hyphenated form "top-up" (e.g., change "top up funding keys", "top up
transaction", "top up index" to "top-up funding keys", "top-up transaction",
"top-up index") across the document (noting that the section heading "## Top Up
Funding keys" is a title-case editorial choice and can be handled separately);
specifically update the occurrences around the sentence starting "Identity
Funding keys can either be for a registration or for a top up" and the other
affected usages referenced in the comment so all compound modifiers use
"top-up".
thephez
previously approved these changes
Feb 19, 2026
thephez
force-pushed
the
feat/dip9-asset-lock-subfeatures
branch
from
82f5bdb to
57d7014
Compare
thephez
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
4'(Asset Lock Address Topup Funding) and5'(Asset Lock Shielded Address Topup Funding) to the Identity Keys (5') feature in the DIP-9 assignments table0') and BLS (1') key types within the authentication subfeature (0')m/9'/5'/5'/0'/0'/0'/0'invitatation→invitation)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit