doc: Add low-level design doc for MIMD-0025 - #190
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe document defines MIMD-0025 bootstrap permissioning, domain-separated hashes, PDA layouts, account schemas, state types, instruction interfaces, payloads, and validation rules. It specifies verifier selection, commitment approval, state buffering, challenges, reveals, dispute resolution, timeouts, staking penalties, payout timelocks, finalization, expiration, and account closure. It also records validator-repository responsibilities and unresolved design parameters. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mimd-0025-impl.md`:
- Line 287: Update the “FinalizeCommitment” happy-path rule wording to say it
requires the window to be closed, replacing “requires closed window” while
preserving the existing approval requirement.
- Line 207: Update the RaiseChallenge specification and corresponding details at
the referenced challenge-flow section to require Active status, a current slot
strictly before challenge_window_end_slot, and no existing active challenge;
atomically set PendingCommitment.active_challenge during creation so concurrent
challengers cannot create multiple challenges and late challenges cannot block
finalization.
- Around line 208-210: Update the OperatorChallengeResponse rules to require the
submitted account_state_hash and optional state buffer to match
PendingCommitment before opening state. Treat any mismatch as an
operator-invalid outcome and ensure ResolvedOperator cannot finalize that
response; apply the same validation to the corresponding
ChallengerReveal-related rules.
- Around line 139-144: Update StateBuffer and the WriteStateBuffer flow to
retain immutable creation metadata and track covered byte ranges, rejecting
changed metadata, gaps, overlaps, out-of-bounds writes, and writes exceeding
total_len. Require dispute states that need buffer data to supply one, and
permit council resolution or finalization only when the buffer is finalized,
exactly covers total_len, and matches expected_data_hash; apply the same
validation to the referenced paths around the affected state transitions.
- Line 89: Persist a challenge-window extension counter on PendingCommitment,
initialize it when the commitment is created, and update ExtendChallengeWindow
to reject extensions once ProtocolConfig.max_window_extensions is reached.
Increment the counter with checked arithmetic and ensure every relevant
construction, serialization, and extension path carries the new field
consistently.
- Around line 196-200: The commitment flow must preserve configuration values
for in-flight commitments instead of reading updated protocol settings later.
Update the commitment data model and related instructions around
UpdateProtocolConfig so values such as match_penalty_bps, max_window_extensions,
and payout_timelock_slots are snapshotted when the commitment is created, or
bind the commitment to an immutable configuration version; ensure later
matching, expiration, and payout logic uses that frozen data.
- Around line 146-156: Define the missing ChallengeOutcome type with variants
covering invalid reveal, matching state, and challenger timeout outcomes. Update
each terminal branch in the challenge resolution flow, including invalid-reveal,
matching-state, and challenger-timeout handling, to atomically set the terminal
status and outcome, clear PendingCommitment.active_challenge, settle or return
challenger stake as specified, and apply the corresponding finalization
transition.
- Around line 212-213: Update the CouncilVote specification and implementation
to validate member_index bounds, require the signer to match
CouncilCase.member_identities[member_index], confirm the council epoch is
active, reject an already-set vote_bitmap bit, and enforce an open voting
deadline before applying the copied member weight. Apply the same validations to
the corresponding CouncilVote entry referenced later, preserving vote uniqueness
and signer identity guarantees.
- Around line 209-211: Update the state-transition rules for MarkOperatorTimeout
and MarkChallengerRevealTimeout to require the current slot to be at or after
the stored deadline and the commitment/challenge to be in the expected pending
status. Ensure successful operator responses or challenger reveals transition
the state so the corresponding timeout is rejected, making response/reveal and
timeout paths mutually exclusive and preventing premature slashing.
- Around line 83-89: Update ProtocolConfig to define a frozen withdrawal-delay
parameter used consistently by RequestStakeWithdrawal and WithdrawStake. Define
VerifierBond’s locked stake and specify how locked_lamports is increased,
maintained, and released for every outstanding slashable obligation, including
commitments covered by the affected sections. Ensure withdrawal eligibility
preserves sufficient reserved stake until all obligations settle.
- Around line 201-204: The CreateVerifierSetSnapshot flow must define and
enforce the cranker construction authority, validate active verifier bonds,
canonicalize membership ordering, reject duplicate entries, and reject
selected_verifier_count values larger than the resulting snapshot size before
storing it. Ensure the stored verifier_snapshot, including verifier_identity,
verifier_bond, weight, and hash, cannot be modified after creation and is the
authoritative source for later verifier selection and approval.
- Around line 231-234: Update ConsumeCommitmentRandomness to include the stored
VRF request ID alongside randomness and pending commitment. When handling the
message, require the commitment state to be AwaitingRandomness and verify its
stored vrf_request_id matches the supplied request ID before consuming it;
preserve one-time consumption semantics.
- Around line 31-42: Define challenge_window_id in PostCommitment before
computing state_commitment_hash, either by accepting and recording it when the
commitment is created or by deterministically deriving it from shared stored
fields; ensure the identical value is used in both the hash input and
PendingCommitment, and that ConsumeCommitmentRandomness uses that identifier
when starting the challenge window.
- Around line 169-181: Extend CouncilCase with a snapshot of the active council
thresholds needed by ResolveCouncilCase, including the NoQuorum resolution path.
Update ResolveCouncilCase to use the case snapshot, set the pending status/state
source for every resolved outcome, and apply the correct finalization stake
handling for OperatorCorrect, ChallengerCorrect, and NoQuorum.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d392353-0a38-4bca-a4ca-5542faf791a2
📒 Files selected for processing (1)
mimd-0025-impl.md
| StateBuffer { | ||
| role: OperatorFinalize | OperatorChallengeResponse | ChallengerReveal, | ||
| authority, account_pubkey, commit_id, | ||
| expected_data_hash, total_len, written_len, finalized, | ||
| data: Vec<u8>, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Require complete and immutable state-buffer writes.
WriteStateBuffer accepts offset, total len, and expected hash, but StateBuffer has no range or overlap tracking. The rules do not reject gaps, overlaps, changed metadata, out-of-bounds writes, or finalization before exactly total_len bytes are covered. A dispute state can also be supplied without a buffer. Make buffer metadata immutable after creation, enforce exact coverage and size limits, and require a finalized buffer with a matching hash before council resolution or finalization.
Also applies to: 205-206, 245-258
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` around lines 139 - 144, Update StateBuffer and the
WriteStateBuffer flow to retain immutable creation metadata and track covered
byte ranges, rejecting changed metadata, gaps, overlaps, out-of-bounds writes,
and writes exceeding total_len. Require dispute states that need buffer data to
supply one, and permit council resolution or finalization only when the buffer
is finalized, exactly covers total_len, and matches expected_data_hash; apply
the same validation to the referenced paths around the affected state
transitions.
| CouncilConfig { | ||
| authority, epoch, quorum_bps, supermajority_bps, voting_timeout_slots, | ||
| members: Vec<{ identity, weight, active }>, | ||
| } | ||
|
|
||
| CouncilCase { | ||
| challenge, council_epoch, opened_slot, voting_deadline_slot, | ||
| operator_votes, challenger_votes, abstain_votes, | ||
| vote_bitmap: Vec<u8>, | ||
| member_identities: Vec<Pubkey>, | ||
| member_weights: Vec<u64>, | ||
| outcome: Option<OperatorCorrect | ChallengerCorrect | NoQuorum>, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,200p'
echo "== target file existence and size =="
wc -l mimd-0025-impl.md 2>/dev/null || true
echo "== relevant line ranges =="
sed -n '140,370p' mimd-0025-impl.md 2>/dev/null || true
echo "== search symbols =="
rg -n "CouncilConfig|CouncilCase|ResolveCouncilCase|NoQuorum|FinalizeCommitment|state_source|quorum_bps|supermajority_bps" mimd-0025-impl.md || trueRepository: magicblock-labs/delegation-program
Length of output: 17106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== complete 230s around ResolveCouncilCase and FinalizeCommitment =="
sed -n '195,335p' mimd-0025-impl.md
echo "== council symbols in local source =="
rg -n "CouncilConfig|CouncilCase|ResolveCouncilCase|NoQuorum|FinalizeCommitment|state_source|quorum_bps|supermajority_bps|ResolvedOperatorState|ResolvedChallengerState" . --glob '!**/Cargo.lock' --glob '!target/**' || trueRepository: magicblock-labs/delegation-program
Length of output: 8843
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== earlier referenced 70-140 section =="
sed -n '60,135p' mimd-0025-impl.md
echo "== deterministic struct/data mapping probe =="
python3 - <<'PY'
from pathlib import Path
p = Path("mimd-0025-impl.md")
text = p.read_text()
checks = {
"CouncilConfig_includes_quorums": "CouncilConfig" in text and ("quorum_bps" in text or "supermajority_bps" in text),
"CouncilCase_outcome_exposes_NoQuorum": "NoQuorum" in text and "outcome: Option<OperatorCorrect | ChallengerCorrect | NoQuorum>" in text,
"ResolvedOperatorState_exists": "ResolvedOperatorState" in text,
"ResolvedChallengerState_exists": "ResolvedChallengerState" in text,
}
print(checks)
print("\nNoQuorum state_source:", "NoQuorum" in text and "NoQuorumState" in text)
print("CouncilConfig fields in CouncilCase:", "quorum_bps" in text[text.find("CouncilCase"):text.find("CouncilCase")+500])
PYRepository: magicblock-labs/delegation-program
Length of output: 3485
Make council resolution paths fully executable.
CouncilCase does not store the active council thresholds, but ResolveCouncilCase still needs them for OperatorCorrect, ChallengerCorrect, and NoQuorum. Add the missing snapshot for NoQuorum, plus the pending status/state source and finalization stake handling for each resolved outcome.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` around lines 169 - 181, Extend CouncilCase with a snapshot
of the active council thresholds needed by ResolveCouncilCase, including the
NoQuorum resolution path. Update ResolveCouncilCase to use the case snapshot,
set the pending status/state source for every resolved outcome, and apply the
correct finalization stake handling for OperatorCorrect, ChallengerCorrect, and
NoQuorum.
| | `OperatorChallengeResponse` | opened state metadata | operator signer, pending commitment, challenge, optional state buffer | Opens operator state and starts challenger reveal timeout. | | ||
| | `MarkOperatorTimeout` | none | cranker, pending commitment, challenge | Records non-response and waits for challenger reveal. | | ||
| | `ChallengerReveal` | opened state metadata, salt | challenger signer, pending commitment, challenge, optional buffer, fee vault, optional council case/config | Validates challenge preimage and either penalizes, dismisses, or opens council case. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Require the operator response to open the committed state.
OperatorChallengeResponse accepts new state metadata, but the rules do not require its account_state_hash to equal PendingCommitment.account_state_hash. ResolvedOperator can then finalize state that was never committed. Require the response and its buffer to match the committed state, or classify a mismatch as operator-invalid and disallow ResolvedOperator.
Also applies to: 245-250
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` around lines 208 - 210, Update the
OperatorChallengeResponse rules to require the submitted account_state_hash and
optional state buffer to match PendingCommitment before opening state. Treat any
mismatch as an operator-invalid outcome and ensure ResolvedOperator cannot
finalize that response; apply the same validation to the corresponding
ChallengerReveal-related rules.
| - `ChallengerReveal` has four terminal branches: | ||
| invalid hash, matching state, mismatch after operator response, valid reveal | ||
| after operator timeout. | ||
| - `FinalizeCommitment` on the happy path requires closed window, approval |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use one modal in the finalization rule.
Change “requires closed window” to “requires the window to be closed.”
🧰 Tools
🪛 LanguageTool
[style] ~287-~287: The double modal “requires closed” is nonstandard (only accepted in certain dialects). Consider “to be closed”.
Context: ...eCommitment` on the happy path requires closed window, approval threshold, no unreso...
(NEEDS_FIXED)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` at line 287, Update the “FinalizeCommitment” happy-path
rule wording to say it requires the window to be closed, replacing “requires
closed window” while preserving the existing approval requirement.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mimd-0025-impl.md (1)
58-98: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftFreeze canonical hash inputs.
Domain separation does not define the bytes that are hashed. Specify the hash algorithm, exact domain-tag bytes, field order, integer and enum encoding, option and vector encoding, and DA pointer encoding. Require operators, verifiers, challengers, and resolvers to use the same preimage format.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mimd-0025-impl.md` around lines 58 - 98, Update the Hashes section to freeze the canonical preimage format: specify the hash algorithm, exact domain-tag bytes, field order, integer and enum encodings, option/vector representations, and DA pointer encoding for every hash. State that operators, verifiers, challengers, and resolvers must use this identical serialization, and remove these items from the open parameters list while leaving unrelated parameters unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mimd-0025-impl.md`:
- Line 127: Define a fixed maximum for VerifierSetSnapshot.entries, including a
serialized-size bound, and enforce both limits before storing any snapshot.
Reject snapshots exceeding either limit while preserving the existing valid
snapshot flow.
- Around line 387-388: Define how dispute resolution obtains and immutably binds
the DA record, replay range, base-state identity, and deterministic
runtime/configuration to the commitment. Specify the exact DA-unavailable
outcome, including resulting state, stake handling, and payout transition,
instead of leaving it to the resolver policy.
- Line 389: Update the payout specification around PayoutTimelock and
ClaimPayout to define the funding account, slash and challenger payout amounts,
beneficiary derivation, and atomic settlement conditions. Require ClaimPayout to
verify current_slot >= unlock_slot and claimed == false before transferring
funds, then mark the timelock claimed as part of the same atomic operation.
- Around line 127-128: Validate approval configuration during both
initialization and updates: require selected_verifier_count to be greater than
zero and approval_threshold to be positive and no greater than
selected_verifier_count. Reject invalid configurations before they are stored or
used, while preserving valid configuration behavior.
- Around line 37-43: Update the RegisterOperator and RegisterVerifier
instruction flows to enforce permissioned-mode admission through a configured
signer, allowlist, or on-chain admission state, including validating the
authority before registration succeeds. Alternatively, revise the documented
bootstrap behavior so these registrations are explicitly permissionless and
remove the conflicting permissioned-mode claim.
- Line 106: Update the PDA account specifications in the ProtocolConfig table
and every other PDA definition to explicitly fix seed order, byte encoding for
Pubkey, u64, enum values, and role tags, plus maximum seed lengths. Ensure each
PDA has a complete deterministic derivation schema that clients can implement
consistently.
---
Outside diff comments:
In `@mimd-0025-impl.md`:
- Around line 58-98: Update the Hashes section to freeze the canonical preimage
format: specify the hash algorithm, exact domain-tag bytes, field order, integer
and enum encodings, option/vector representations, and DA pointer encoding for
every hash. State that operators, verifiers, challengers, and resolvers must use
this identical serialization, and remove these items from the open parameters
list while leaving unrelated parameters unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ef23319-7dad-4cdf-8c6e-8969d437a102
📒 Files selected for processing (1)
mimd-0025-impl.md
| operator_response_timeout_slots, | ||
| challenger_reveal_timeout_slots, | ||
| payout_timelock_slots, | ||
| selected_verifier_count, approval_threshold, max_window_extensions, |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Set a hard verifier snapshot limit.
VerifierSetSnapshot.entries is described as bounded, but no maximum entry count or serialized-size limit is defined. Add and enforce a fixed limit before storing a snapshot. This prevents oversized accounts and transactions from exceeding runtime limits.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` at line 127, Define a fixed maximum for
VerifierSetSnapshot.entries, including a serialized-size bound, and enforce both
limits before storing any snapshot. Reject snapshots exceeding either limit
while preserving the existing valid snapshot flow.
| selected_verifier_count, approval_threshold, max_window_extensions, | ||
| match_penalty_bps, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject impossible approval configurations.
Validate selected_verifier_count > 0 and 0 < approval_threshold <= selected_verifier_count during configuration initialization and updates. A zero threshold permits finalization without approvals. A threshold above the selected count makes happy-path finalization impossible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` around lines 127 - 128, Validate approval configuration
during both initialization and updates: require selected_verifier_count to be
greater than zero and approval_threshold to be positive and no greater than
selected_verifier_count. Reject invalid configurations before they are stored or
used, while preserving valid configuration behavior.
| - Bounded vector vs Merkleized verifier snapshots. | ||
| - Resolver no-decision policy. | ||
| - DA-unavailable or replay-insufficient policy. | ||
| - Operator slash amount and challenger payout amount. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Define payout funding and settlement.
PayoutTimelock and ClaimPayout are part of the interface, but the slash amount and challenger payout remain unspecified. Define the funding account, amount, beneficiary derivation, and atomic claim conditions. ClaimPayout must require current_slot >= unlock_slot and claimed == false before transferring funds and marking the timelock claimed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` at line 389, Update the payout specification around
PayoutTimelock and ClaimPayout to define the funding account, slash and
challenger payout amounts, beneficiary derivation, and atomic settlement
conditions. Require ClaimPayout to verify current_slot >= unlock_slot and
claimed == false before transferring funds, then mark the timelock claimed as
part of the same atomic operation.
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)
mimd-0025-impl.md (1)
72-83: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine canonical encoding for the state commitment hash.
state_commitment_hashnow includesverifier_registryandverifier_registry_hash, but their types and byte representations are not defined. Specify whetherverifier_registryis the PDA key, the full entry set, or an immutable snapshot identifier. Define field order, fixed-width encodings, option tags, and length delimiters before implementation. Otherwise, different implementations can produce different commitment hashes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mimd-0025-impl.md` around lines 72 - 83, Define the canonical byte encoding for every input to state_commitment_hash, especially verifier_registry and verifier_registry_hash: specify whether verifier_registry is a PDA key, full entry set, or immutable snapshot identifier, then document field order, fixed-width representations, option tags, and length delimiters. Ensure all implementations hash the identical canonical serialization before proceeding with implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mimd-0025-impl.md`:
- Line 257: Validate selected_verifier_index in the approval-update logic before
accessing selected_verifiers or approval_bitmap: ensure it is in range, the
signer matches the verifier at that index, and the corresponding bitmap bit
exists and is unset before setting it. Reject malformed input rather than
performing any update, covering the same checks in the logic around the
referenced approval-update lines.
---
Outside diff comments:
In `@mimd-0025-impl.md`:
- Around line 72-83: Define the canonical byte encoding for every input to
state_commitment_hash, especially verifier_registry and verifier_registry_hash:
specify whether verifier_registry is a PDA key, full entry set, or immutable
snapshot identifier, then document field order, fixed-width representations,
option tags, and length delimiters. Ensure all implementations hash the
identical canonical serialization before proceeding with implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b606e709-b1fd-4e86-810c-8b809ea262ec
📒 Files selected for processing (1)
mimd-0025-impl.md
| } | ||
|
|
||
| ApproveCommitment { | ||
| selected_verifier_index: u32, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate selected_verifier_index before updating approvals.
Require the index to be within selected_verifiers, require the signer to equal the entry at that index, and verify that the corresponding approval_bitmap bit exists and is unset before setting it. Otherwise, malformed input can cause an out-of-bounds access or record approval against the wrong slot.
Also applies to: 300-303
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` at line 257, Validate selected_verifier_index in the
approval-update logic before accessing selected_verifiers or approval_bitmap:
ensure it is in range, the signer matches the verifier at that index, and the
corresponding bitmap bit exists and is unset before setting it. Reject malformed
input rather than performing any update, covering the same checks in the logic
around the referenced approval-update lines.
4d825ab to
753a36a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mimd-0025-impl.md (1)
60-99: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSpecify the hash serialization before implementation.
challenge_hashis an on-chain commitment whose preimageChallengerRevealmust validate, but the spec leaves the hash algorithm and byte-level encoding open. Define the field order, integer/enum encoding, length prefixes, and hash algorithm for the domain tags used byaccount_state_hash,da_pointer_hash,state_commitment_hash, andchallenge_hash. Otherwise, honest operators, challengers, and resolvers can compute incompatible hashes from the same commitment/reveal data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mimd-0025-impl.md` around lines 60 - 99, Specify a canonical serialization for all hash preimages in the Hashes section before implementation: define the hash algorithm, exact field order, domain-tag encoding, integer and enum representations, byte/string length prefixes, and DA pointer encoding for account_state_hash, da_pointer_hash, state_commitment_hash, and challenge_hash. Ensure ChallengerReveal validation uses this same encoding so all participants derive identical commitments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mimd-0025-impl.md`:
- Around line 154-155: Define the withdrawal unlock rule for OperatorBond and
VerifierBond by adding a ProtocolConfig withdrawal delay or immutable
withdraw_available_slot, then update WithdrawStake to require current_slot to
reach the configured unlock value, the bond to be exiting, and no stake to
remain locked before allowing withdrawal.
- Line 144: Update PendingCommitment to store an immutable maximum extension
limit and an extensions_used counter initialized from
ProtocolConfig.max_window_extensions; update ExtendChallengeWindow to atomically
reject requests once the limit is reached and increment extensions_used only
after a successful extension, without treating challenge_window_id as the count.
- Around line 134-136: Enforce the configured minimum amounts from the
min_operator_bond, min_verifier_bond, and min_challenger_stake fields at every
corresponding registration and challenge instruction, including the referenced
sections. Reject underfunded or zero-stake inputs before state changes, and
ensure the validated amount is transferred or locked atomically with
registration or challenge creation.
---
Outside diff comments:
In `@mimd-0025-impl.md`:
- Around line 60-99: Specify a canonical serialization for all hash preimages in
the Hashes section before implementation: define the hash algorithm, exact field
order, domain-tag encoding, integer and enum representations, byte/string length
prefixes, and DA pointer encoding for account_state_hash, da_pointer_hash,
state_commitment_hash, and challenge_hash. Ensure ChallengerReveal validation
uses this same encoding so all participants derive identical commitments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6e82dba5-a903-4f06-a7f4-50c2a4e3b69b
📒 Files selected for processing (1)
mimd-0025-impl.md
| pub min_operator_bond: u64, | ||
| pub min_verifier_bond: u64, | ||
| pub min_challenger_stake: u64, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Enforce the configured minimum stake amounts.
The configuration defines minimum operator, verifier, and challenger stakes. The instruction contracts do not state lower-bound checks for the supplied amounts. An underfunded registration or zero-stake challenge can bypass economic protections or block finalization at low cost.
Require each amount to meet its configured minimum. Transfer or lock the amount atomically with registration or challenge creation.
Also applies to: 315-316, 326-326, 363-366
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` around lines 134 - 136, Enforce the configured minimum
amounts from the min_operator_bond, min_verifier_bond, and min_challenger_stake
fields at every corresponding registration and challenge instruction, including
the referenced sections. Reject underfunded or zero-stake inputs before state
changes, and ensure the validated amount is transferred or locked atomically
with registration or challenge creation.
| pub withdraw_requested_slot: Option<u64>, | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Define the withdrawal unlock condition.
OperatorBond and VerifierBond store withdraw_requested_slot, but ProtocolConfig defines no withdrawal delay or unlock slot. WithdrawStake also has no stated current-slot check. This leaves the stake-unlock rule implementation-dependent.
Add a withdrawal delay or store an immutable withdraw_available_slot. Require current_slot to reach that value, the bond to be exiting, and no stake to remain locked before withdrawal.
Also applies to: 162-163, 317-318
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` around lines 154 - 155, Define the withdrawal unlock rule
for OperatorBond and VerifierBond by adding a ProtocolConfig withdrawal delay or
immutable withdraw_available_slot, then update WithdrawStake to require
current_slot to reach the configured unlock value, the bond to be exiting, and
no stake to remain locked before allowing withdrawal.
73c9739 to
649a41d
Compare
a1f35b6 to
a313bb1
Compare
a313bb1 to
2fa8b2a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mimd-0025-impl.md`:
- Around line 341-346: Align the opened-state flow across
OperatorChallengeResponseData and ChallengerRevealData with FinalizeCommitment:
either reject None state_buffer values before marking the state resolved, or
introduce an explicit opened-state account and bind it through
FinalizeCommitmentStateSource. Ensure every resolved opened state supplies the
state buffer/account required by FinalizeCommitment.
- Line 334: Update PostCommitment and PostCommitmentData handling to bind the
instruction to the canonical CommitRecord: include the existing record account
and validate that its identity, target account, sequential nonce, and committed
lamports match the supplied commitment data. Ensure downstream consumers use the
validated CommitRecord values, or explicitly document and implement this path as
the replacement canonical source.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b6f4abef-52a2-417a-8f0c-67df125eb1b8
📒 Files selected for processing (1)
mimd-0025-impl.md
| | --- | --- | --- | | ||
| | `RequestStakeWithdrawal`<ul><li>ix-data: <code>actor_kind</code></li><li>accounts: <strong>actor signer, bond, config</strong></li></ul> | Operator or verifier | Marks bonded stake as exiting. The stake cannot be withdrawn until the configured delay passes and no locks remain. | | ||
| | `WithdrawStake`<ul><li>ix-data: <code>actor_kind</code></li><li>accounts: <strong>actor signer, bond, config</strong></li></ul> | Operator or verifier | Withdraws unlocked stake after the exit delay. Slashed or locked stake stays in the protocol. | | ||
| | `PostCommitment`<ul><li>ix-data: <code>commitment</code></li><li>accounts: <strong>operator, operator bond, pending commitment, delegated account, delegation record, config, verifier registry, DLP identity PDA, VRF queue/program</strong></li></ul> | Operator | Creates an `AwaitingRandomness` commitment, stores the current `registry_revision`, locks any commitment-local stake if needed, and requests VRF. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'PostCommitment|commit_record|commit_id|nonce|lamports' \
mimd-0025-impl.md \
dlp-api/src/state/commit_record.rs \
src/processor/fast/commit_state.rs \
src/processor/fast/commit_finalize.rsRepository: magicblock-labs/delegation-program
Length of output: 28775
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file list =="
git ls-files | rg '(^|/)(mimd-0025-impl\.md|commit_record\.rs|commit_state\.rs|commit_finalize\.rs|.*mimd.*\.rs|.*MIMD.*\.rs)$' || true
echo
echo "== PostCommitment references =="
rg -n 'PostCommitment|post_commitment|pending commitment|CommitRecord|state_commitment_hash|last_commit_id|commit_id|lamports' mimd-0025-impl.md src dlp-api -g '!target' -g '!**/*.lock' || true
echo
echo "== relevant mimd sections =="
sed -n '180,230p' mimd-0025-impl.md
sed -n '328,368p' mimd-0025-impl.md
sed -n '423,432p' mimd-0025-impl.mdRepository: magicblock-labs/delegation-program
Length of output: 36098
Bind PostCommitment to the canonical commit record.
PostCommitmentData supplies commit_id and lamports directly, but PostCommitment does not include the existing CommitRecord nor enforce equality with it. The canonical DLP path stores identity, target account, sequential nonce, and committed lamports in CommitRecord; include and check that record here, or document why this path replaces that canonical source and ensure downstream consumers use the source actually written.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` at line 334, Update PostCommitment and PostCommitmentData
handling to bind the instruction to the canonical CommitRecord: include the
existing record account and validate that its identity, target account,
sequential nonce, and committed lamports match the supplied commitment data.
Ensure downstream consumers use the validated CommitRecord values, or explicitly
document and implement this path as the replacement canonical source.
| | `OperatorChallengeResponse`<ul><li>ix-data: <code>state</code></li><li>accounts: <strong>operator signer, pending commitment, challenge, optional state buffer</strong></li></ul> | Operator | Opens the operator's claimed state for the challenged commitment and starts the challenger reveal timeout. | | ||
| | `MarkOperatorTimeout`<ul><li>ix-data: <code>empty</code></li><li>accounts: <strong>cranker, pending commitment, challenge</strong></li></ul> | Cranker | Records that the operator missed the response deadline. The challenger must still reveal the challenge preimage. | | ||
| | `ChallengerReveal`<ul><li>ix-data: <code>state, salt</code></li><li>accounts: <strong>challenger signer, pending commitment, challenge, optional buffer, fee vault</strong></li></ul> | Challenger | Verifies the challenge preimage and opened state. It slashes invalid reveals, penalizes matching reveals, or moves mismatches to resolver decision. | | ||
| | `MarkChallengerRevealTimeout`<ul><li>ix-data: <code>empty</code></li><li>accounts: <strong>cranker, pending commitment, challenge, fee vault</strong></li></ul> | Cranker | Slashes challenger stake when the reveal deadline passes without a valid reveal. | | ||
| | `ResolveDispute`<ul><li>ix-data: <code>decision</code></li><li>accounts: <strong>resolver signer, challenge, pending commitment, operator bond, fee vault, optional payout timelock</strong></li></ul> | Resolver multisig | Applies the multisig decision for a valid mismatch: operator correct or challenger correct. | | ||
| | `FinalizeCommitment`<ul><li>ix-data: <code>state_source</code></li><li>accounts: <strong>finalizer, pending commitment, delegated account, delegation record/metadata, state buffer, optional challenge, config</strong></li></ul> | Finalizer or cranker | Applies the final state after the happy path or after dispute resolution. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'OperatorChallengeResponse|ChallengerReveal|state_buffer|OpenedState|FinalizeCommitment' \
mimd-0025-impl.mdRepository: magicblock-labs/delegation-program
Length of output: 8596
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File outline =="
ast-grep outline mimd-0025-impl.md --match 'Instruction Data|Instruction Rules|Failure Scenarios' --view expanded || true
echo
echo "== Relevant specs around instruction data and finalize state source =="
sed -n '320,445p' mimd-0025-impl.md | cat -n
echo
echo "== Search for FinalizeCommitmentStateSource variants =="
rg -n -C 4 'FinalizeCommitmentStateSource|PendingOperatorState|ResolvedOperatorState|ResolvedChallengerState|WriteStateBuffer|FinalizeStateBuffer|state_buffer' mimd-0025-impl.mdRepository: magicblock-labs/delegation-program
Length of output: 15378
Make every resolved opened state finalizable.
OperatorChallengeResponseData and ChallengerRevealData allow state_buffer to be None, while FinalizeCommitment requires a state buffer with no alternate opened-state account. Reject missing buffers, or define an explicit opened-state account and bind it through FinalizeCommitmentStateSource.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mimd-0025-impl.md` around lines 341 - 346, Align the opened-state flow across
OperatorChallengeResponseData and ChallengerRevealData with FinalizeCommitment:
either reject None state_buffer values before marking the state resolved, or
introduce an explicit opened-state account and bind it through
FinalizeCommitmentStateSource. Ensure every resolved opened state supplies the
state buffer/account required by FinalizeCommitment.
925cdc2 to
bc4ff5e
Compare
bc4ff5e to
7fda1f5
Compare
fa86000 to
1209c5c
Compare
2 participants
Problem
What problem are you trying to solve?
Solution
How did you solve the problem?
Before & After Screenshots
Insert screenshots of example code output
BEFORE:
[insert screenshot here]
AFTER:
[insert screenshot here]
Other changes (e.g. bug fixes, small refactors)
Deploy Notes
Notes regarding deployment of the contained body of work. These should note any
new dependencies, new scripts, etc.
New scripts:
script: script detailsNew dependencies:
dependency: dependency detailsSummary by CodeRabbit