feat(platform)!: add the IdentityKeyLimitsUpdate state transition - #4807
Merged
Merged
Conversation
A MASTER key, or a CRITICAL authentication key without limits, can raise the total budget of one of the identity's keys (the remaining budget grows by the same amount) or move its expiry later. An update only ever loosens limits. State transition type 23, gated to protocol version 14; the identity revision is bumped and the proof of execution binds the rewritten key. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Adds Identity::update_key_limits, top_up_key_budget and extend_key_expiry to the Rust SDK, the IdentityKeyLimitsUpdate wrapper to wasm-dpp2, identityUpdateKeyLimits to wasm-sdk and identities.updateKeyLimits to js-evo-sdk, and documents the transition in the book and the protocol reference. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 121 files, which is 21 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (121)
You can disable this status message by setting the 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 |
Contributor
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-09-18T09:56:35.386Z |
Contributor
PR HygieneState: waiting-bots · commit
Self-review is an author attestation that you have read the diff: This report does not bypass CI or repository protection rules. |
Collaborator
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4807 +/- ##
============================================
+ Coverage 77.43% 79.77% +2.33%
============================================
Files 2944 2962 +18
Lines 424764 424638 -126
============================================
+ Hits 328916 338740 +9824
+ Misses 95848 85898 -9950
🚀 New features to boost your workflow:
|
QuantumExplorer
commented
Sep 17, 2026
QuantumExplorer
commented
Sep 17, 2026
QuantumExplorer
left a comment
QuantumExplorer
left a comment
Member
Author
There was a problem hiding this comment.
I found two remaining issues in the execution-proof and WASM API paths.
The proof of a key limits update shows the resulting key and revision, not the nonce, so it is classified as affected state and the SDKs wait for it as such. The Rust SDK skips contract-bound CRITICAL keys when it picks a signer. The wasm-dpp2 wrapper reads an undefined userFeeIncrease as the default. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
@coderabbitai review No review for |
Contributor
|
@thepastaclaw review No review for |
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
⚠️ DEGRADED — Final validation — Phase 1 + Phase 2
⚠️ DEGRADED review. The primary review models were unavailable (gpt-6-astraunavailable: All credentials for model gpt-6-astra are cooling down (last error: usage_limit_reached: The usage limit has been reache), so this review ran on stand-in models:gpt-5.6-luna→muse-spark-1.3-contributor,gpt-5.6-sol→muse-spark-1.3-contributor,gpt-5.6-terra→muse-spark-1.3-contributor,gpt-6-astra→muse-spark-1.3-contributor. Both review phases and the independent verifiers still ran, but on weaker models, with Phase 1 capped athigheffort. Treat the verdict as provisional; a full-strength re-review will run on the next push once the primary models are back.
New IdentityKeyLimitsUpdate transition is correctly wired across DPP, Drive, Drive-ABCI, and SDKs with no blocking defects. Three in-scope suggestions remain: a vacuous mempool assertion in the drive-abci test, a missing zero-amount guard in the SDK top-up path that would burn fees, and a silent saturating_sub in the Drive rewrite that should fail loudly.
🟡 3 suggestion(s)
Review provenance
Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: architecture-layering); reviewer 9: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: platform-versioning); reviewer 11: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: rust-quality); reviewer 12: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: security-auditor); final verifier: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: astra-verifier, role: final-verifier)
- Degraded mode:
gpt-6-astraunavailable: All credentials for model gpt-6-astra are cooling down (last error: usage_limit_reached: The usage limit has been reache (detected by probe, since 2026-09-18T05:22:01Z); stand-insgpt-5.6-luna→muse-spark-1.3-contributor,gpt-5.6-sol→muse-spark-1.3-contributor,gpt-5.6-terra→muse-spark-1.3-contributor,gpt-6-astra→muse-spark-1.3-contributor; Phase 1 effort capped athigh - Triage:
criticalbymuse-spark-1.3-contributor(standing in forgpt-6-astra) (effort low) — Large 120-file (+5349) change adding consensus-critical IdentityKeyLimitsUpdate validation and key-budget execution, e.g. packages/rs-drive/src/drive/identity/update/methods/update_identity_key_limits/v0/mod.rs. - Phase 1 reviewers:
gemini-3.8-flash-high— general (completed, effort high); agentphase1-reviewer,muse-spark-1.3-contributor— architecture-layering (completed, effort high); agentphase1-reviewer,muse-spark-1.3-contributor— ffi-engineer (completed, effort high); agentphase1-reviewer,muse-spark-1.3-contributor— platform-versioning (completed, effort high); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort high); agentphase1-reviewer,muse-spark-1.3-contributor— security-auditor (completed, effort high); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(lane failed),glm-5.3-flash(zai below 15% reserve: 5h 99% left, weekly 14% left) - Fresh verifier:
muse-spark-1.3-contributor(standing in forgpt-6-astra) — final-verifier; agentastra-verifier - Phase 2 reviewers:
muse-spark-1.3-contributor(standing in forgpt-6-astra) — general (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6-astra) — architecture-layering (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6-astra) — ffi-engineer (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6-astra) — platform-versioning (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6-astra) — rust-quality (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6-astra) — security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_key_limits_update/tests.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_key_limits_update/tests.rs:358-368: Mempool admission assertion passes vacuously for a limited signer
Lines 358-368 build an IdentityKeyLimitsUpdate signed by LIMITED_KEY_ID (key 2), which carries limits, and assert check_tx contains no 20015 (budget exhausted). But validate_state_transition_identity_signed_v1 refuses any limited signer of this transition with 20017 before the remaining budget is ever read. So check_tx returns 20017, the `all(code != 20015)` assertion passes trivially, and the comment's claim that the key is usable again is unproven. The Drive-level budget assertions on lines 344-351 already pin the top-up; prove mempool usability with a transition type limited keys may sign (e.g. Batch) or remove these lines.
In `packages/rs-sdk/src/platform/transition/update_identity_key_limits.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/transition/update_identity_key_limits.rs:149-153: top_up_key_budget with amount 0 guarantees a paid on-chain failure
With amount == 0, total_budget equals the current budget, which state validation rejects with 40221 (value not raised). Because that failure lands after the nonce check it is paid: the identity nonce bumps and fees are forfeited for a transition that could never succeed. The repo already guards this class client-side (shield_from_identity rejects amount == 0). Reject 0 up front in top_up_key_budget (and addBudget == Some(0) in the wasm adapter) to save a round trip and protect users from accidental fee burn.
In `packages/rs-drive/src/drive/identity/update/methods/update_identity_key_limits/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/identity/update/methods/update_identity_key_limits/v0/mod.rs:179-184: saturating_sub silently accepts a lowered total budget
Consensus state validation guarantees the new total exceeds the stored one, but this method rewrites the key first (lines 140-167) and only then adjusts the remaining budget. With saturating_sub, a direct-Drive caller passing a total below the stored value gets added = 0, skips the remaining-budget write, and leaves the rewritten lower total alongside the old higher remaining, breaking the remaining <= total invariant the module docs promise. The sibling budget-add path fails loudly with CorruptedDriveState on bad arithmetic; do the same here with checked_sub so a decrease errors instead of coercing into a no-op.
Out-of-scope follow-up suggestions (2)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Type-23 collision with #4760 needs merge-order coordination — Within this PR's base (v4.2-dev, last type 22) type 23 is the correct next discriminant, but the PR body notes #4760 on v4.3-dev also claims type 23. Whichever lands second must renumber.
- Follow-up: Coordinate merge order with #4760; the second landing PR renumbers its new transition type.
- Transition-specific signer rule lives in shared signature helper — Design preference, not a defect — the new matches!(IdentityKeyLimitsUpdate) branch sits directly beside the pre-existing contract-bounds Batch exception in the same shared v1 helper and follows that established pattern; the guard is safe for all other callers. Extracting a per-transition hook is a broader refactor outside this PR's scope.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
…swer the mempool with consensus codes The target key is read once, in transform_into_action, and carried in the action as it is stored, so the mempool refuses a version 0 or missing key with 40220 / 40209 instead of an internal error, the user pays one key read instead of two, and Drive rewrites the key without reading it again. Drive refuses a total that is not raised, or a limit the key does not have, as a corrupted state instead of coercing it with a saturating subtraction. Also: the proof carries the rewritten key with the revision rather than every key; the SDKs share raised_key_limits, which refuses a zero top-up, an expiry that is not later, and a limit the key lacks before signing; the wasm public key exposes totalBudget and expiresAt; the wasm-dpp2 spec covers toObject / fromObject / toJSON / fromJSON; the mempool re-admission test probes with a contract creation signed by the limited key on committed state; the expired-key error text fits both raise sites; IDENTITY_TRANSITION_TYPE lists the missing identity types; the docs name the contract-bounds rule and the smaller proof. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The transition no longer claims the identity's next revision, and the identity's revision is no longer bumped: an identity update needs the claim because clients allocate key ids, while this transition names an existing key and allocates nothing, so a stale copy of the identity cannot make it collide. Every top-up from a cached identity was a paid 40203 unless the client refetched first; now the client only needs to hold the signing key, and a total computed from a stale key is refused only when it no longer raises the stored one. The advanced structure stage and its version slot go, the signature stage no longer requests the revision, the action and the Drive operation carry no revision, and the proof is the rewritten key alone, verified without a revision pin. The wasm-dpp2 wrapper, its spec, the SDK docs, the book and the protocol reference follow. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Merged
6 tasks
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.
Issue being fixed or feature implemented
#4798 gave AUTHENTICATION keys a
total_budgetand anexpires_at, and #4802 the query for what is left. A spent or expiring key still had to be replaced: "Changing limits. There is no top-up and no extension. Register a new key."This adds
IdentityKeyLimitsUpdate, a state transition that raises the budget of one of the identity's keys or moves its expiry later.What was done?
Wire format (
rs-dpp)StateTransition::IdentityKeyLimitsUpdate(type 23, appended last),IdentityKeyLimitsUpdateTransitionV0 { identity_id, nonce, key_id, total_budget: Option<Credits>, expires_at: Option<TimestampMillis>, user_fee_increase, signature_public_key_id, signature }. Both limits carry the new absolute value, and what the wire carries is exactly what the execution proof shows; "top up by X" is SDK sugar. No identity revision is claimed or bumped: an identity update needs one because clients allocate key ids, while this transition names an existing key and allocates nothing, so a stale copy of the identity cannot make it collide.security_level_requirementis[MASTER, CRITICAL]; a CRITICAL signer must carry no limits itself (checked against state, see below). Active from protocol version 14 (active_version_range,IDENTITY_KEY_LIMITS_UPDATE_INITIAL_PROTOCOL_VERSION).IdentityPublicKeySettersV1(set_total_budget,set_expires_at).Rules. An update only ever loosens: a budget can grow, an expiry can move later, a limit the key does not have cannot be added; tightening is what disabling is for. This keeps the arithmetic safe (
remaining <= total_budgetalways holds, both grow by the same amount, so no overflow is possible).IdentityKeyLimitsUpdateEmptyError; 10537PublicKeyWithLimitsCannotUpdateKeyLimitsErrorIdentityPublicKeyLimitNotSetError; new 40221IdentityPublicKeyLimitNotRaisedError; 40219; all paidThe minimum fee reuses
state_transition_min_fees.identity_update(same shape of work), so the shipped fee tables are untouched.Drive
update_identity_key_limits(identity update methods v2,Nonebefore 14): the key is read once, intransform_into_action(so the mempool answers a bad target key with consensus codes), and carried in the action; Drive sets the limits on that copy, rewrites it withreplace_key_in_storage_operationspriced by the byte delta (a bigger varint, or an expiry that was absent), refreshes the key references (they carry the key's value hash, as the disable path does), andadd_to_identity_key_budget(new budget slot) raises the remaining budget by the difference. The action emitsUpdateIdentityNonce,UpdateIdentityKeyLimits.Clients
rs-sdk:UpdateIdentityKeyLimitsonIdentity(update_key_limits,top_up_key_budget,extend_key_expiry); the signing key defaults to the first MASTER, else the first CRITICAL key without limits and without contract bounds, that the signer holds. Resolves to the key as stored after the update.wasm-dpp2:IdentityKeyLimitsUpdatewrapper and the dispatch arms (type number 23, nonce, owner id,verifyPublicKey).wasm-sdk:identityUpdateKeyLimits({ identity, keyId, addBudget?, expiresAt?, signer, settings? }); JS speaks "add", the wasm layer computes the total from the identity's key.js-evo-sdk:identities.updateKeyLimits.wasm-dpp(legacy): the factory refuses the type with a message; the three new errors are mapped.Docs: a "Raising Limits" section (rules, decision diagram, Drive, proof) in
book/src/data-model/key-limits.md, the protocol reference, the enum listings inbook/src/state-transitions/lifecycle.md(which also stopped at type 14), the evo-sdk guide,v14.rs.Not included: Swift, Kotlin and FFI bindings; strategy-tests generation; lowering or removing limits (disable the key instead).
Note: #4760 on v4.3-dev also claims type 23; whichever lands second renumbers.
How Has This Been Tested?
rs-dpp: round trip, every limit field covered by the sig hash and the signature excluded, JSON and value wire shapes, the type tables, frozen error discriminants (17, 107, 108).cargo test -p dpp --all-features --lib -- identity_key_limits_update state_transition_types state_error signature_error umbrella(68 pass).rs-drive(update_identity_key_limits): total and remaining growing by the same amount (with prior spending), expiry alone, estimate at least the actual rewrite, the whole database hash-consistent after the reference refresh (visualize_verify_grovedb),add_to_identity_key_budgeton a budgeted key only, protocol version 13 inactive.cargo test -p drive --lib -- update_identity_key_limits(6 pass).rs-drive-abci(identity_key_limits_update/tests.rs, throughprocess_raw_state_transitions,check_txand the proof): a spent key topped up and admitted again; an expired key revived by an extension and refused while it stays expired; each refusal pinned to its code (paid ones bump the nonce); MASTER and unlimited CRITICAL accepted, limited (20017) and HIGH refused, the builder refusing HIGH up front; mempool admission; proof round trip (affected state) with a wrong total not verifying; protocol version 13 refused at decode.cargo test -p drive-abci --lib -- identity_key_limits_update(13 pass).dash-sdkoffline suite plus the signing-key selection unit tests (a contract-bound CRITICAL key is skipped),wasm-dpp2unit tests; newwasm-dpp2andjs-evo-sdkspecs added but not run locally (nonode_modulesin this checkout), nor the wasm-sdk functional tests.cargo fmt --all -- --check;cargo clippy ... --all-targets -- -D warningsover dpp, drive, drive-abci, platform-version, dash-sdk, wasm-dpp2, wasm-dpp;cargo clippy -p wasm-sdk --target wasm32-unknown-unknown(only the pre-existingDocumentPropertyTypewarning of v4.2-dev);cargo check --workspace --all-targets;cargo check -p drive --no-default-features --features verify.Breaking Changes
Consensus-breaking, gated to protocol version 14: a new state transition type, new consensus errors, a rewrite of a stored key. Nothing changes for protocol versions up to 13.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code