feat: Add CommitDiff Instruction for Efficient Account Updates

[snawaz]

It is the second PR in the current stack.

This PR introduces a new CommitDiff instruction that allows validators to commit changes to delegated accounts using a diff-based approach rather than sending the entire account data. This optimization significantly reduces the amount of data that needs to be transmitted when making small changes to large accounts.

Summary by CodeRabbit

Release Notes

• New Features

  • Added commit diff functionality enabling users to submit and process state updates to delegated accounts with validation, state management, nonce tracking, and delegation controls for enhanced security and operational control.

• Chores

  • Added strum library dependency for enum utility support.

[snawaz]

• feat: Optimize CommitDiff using efficient merge_diff_copy that avoids heap allocation  #113
• feat: Add CommitDiff Instruction for Efficient Account Updates #110 👈 (View in Graphite)
• feat: Add diff module that provides compute_diff, apply_diff_*, DiffSet #111
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Warning

Rate limit exceeded

@snawaz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 59 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between a76b7db and 3705937.

📒 Files selected for processing (3)

• src/args/commit_state.rs (2 hunks)
• src/instruction_builder/commit_diff.rs (1 hunks)
• src/processor/fast/commit_diff.rs (1 hunks)

Walkthrough

Introduces a new CommitDiff instruction type for applying diffs to delegated accounts. Adds supporting argument structures, a discriminator variant, an instruction builder, and a fast processor that validates arguments, deserializes diff data, constructs a DiffSet, applies the diff, and delegates to internal state processing.

Changes

Cohort / File(s)	Change Summary
Dependency management Cargo.toml	Added strum = { version = "0.27.2", features = ["derive"] } dependency for enum string conversion support.
Argument structures src/args/commit_state.rs	Added CommitDiffArgs struct with diff, nonce, lamports, allow_undelegation fields and CommitDiffArgsWithoutDiff for deserialization. Added SIZE_COMMIT_DIFF_ARGS_WITHOUT_DIFF constant.
Diff system src/diff/types.rs	Added try_new_from_borsh_vec() constructor helper to DiffSet for creating instances from Borsh-serialized buffers by skipping initial 4-byte header.
Discriminator src/discriminator.rs	Added CommitDiff = 16 enum variant to DlpDiscriminator, derived IntoStaticStr, and added name() method for static string conversion.
Instruction builder src/instruction_builder/commit_diff.rs	Added commit_diff() function that constructs a Solana Instruction, serializes arguments, computes PDAs, and assembles AccountMeta entries.
Instruction builder module src/instruction_builder/mod.rs	Added module declaration and public re-export for commit_diff.
Entry point src/lib.rs	Added match branch in fast_process_instruction to route CommitDiff discriminator to processor::fast::process_commit_diff().
Fast processor src/processor/fast/commit_diff.rs	Added process_commit_diff() function that validates accounts, deserializes arguments, constructs DiffSet, applies diff, and delegates to process_commit_state_internal().
Fast processor module src/processor/fast/mod.rs	Added module declaration and public re-export for commit_diff.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Instruction as commit_diff<br/>Instruction Builder
    participant EntryPoint as fast_process_<br/>instruction
    participant Processor as process_commit_diff
    participant DiffSet
    participant State as process_commit_<br/>state_internal

    Client->>Instruction: Create CommitDiff<br/>(validator, delegated_account, args)
    Instruction->>Instruction: Serialize CommitDiffArgs<br/>Compute PDAs
    Instruction-->>Client: Return Instruction
    
    Client->>EntryPoint: Send CommitDiff instruction
    EntryPoint->>EntryPoint: Match CommitDiff<br/>discriminator
    EntryPoint->>Processor: Call process_commit_diff
    
    Processor->>Processor: Validate accounts provided
    Processor->>Processor: Validate data length ≥<br/>SIZE_COMMIT_DIFF_ARGS_WITHOUT_DIFF
    Processor->>Processor: Split data: diff + args
    Processor->>Processor: Deserialize<br/>CommitDiffArgsWithoutDiff
    Processor->>DiffSet: Create from diff bytes<br/>try_new_from_borsh_vec()
    Processor->>Processor: Extract nonce, lamports,<br/>allow_undelegation
    Processor->>Processor: Unsafe borrow delegated<br/>account data
    Processor->>Processor: Apply diff via<br/>apply_diff_copy
    Processor->>State: Delegate to<br/>process_commit_state_internal
    State-->>Processor: Return result
    Processor-->>EntryPoint: Return ProgramResult
    EntryPoint-->>Client: Confirm instruction

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring extra attention:
  • Logic in process_commit_diff() around unsafe data borrowing and diff application pipeline
  • Correct field ordering in CommitDiffArgs (marked with safety comment as critical)
  • PDA computation and AccountMeta ordering in commit_diff() instruction builder
  • Integration path verification: discriminator routing through entry point to fast processor

Possibly related issues

• New Feature: Support "diff-based" commit strategy (instead of uploading full bytes) #100: Implements the diff-based commit feature proposal with CommitDiff discriminator, argument structures, instruction builder, and processor implementation.

Possibly related PRs

• feat: Add diff module that provides compute_diff, apply_diff_*, DiffSet #111: Modifies diff subsystem (src/diff/types.rs) which is directly extended by this PR's try_new_from_borsh_vec() helper and DiffSet usage in commit-diff processing.
• refactor: do not require system_program in instruction-accounts and add docs #102: Modifies src/discriminator.rs and DlpDiscriminator enum; this PR adds CommitDiff variant and IntoStaticStr derivation to the same enum.

Suggested reviewers

• thlorenz
• GabrielePicco

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description is significantly incomplete relative to the provided template. While the description is on-topic and provides useful context about the CommitDiff instruction's purpose and optimization benefits, it is missing the majority of required template sections. Specifically, it lacks the status/type/core change table, a formal problem statement, a solution section, deploy notes about the new strum dependency, and before/after structure. The description provides only a brief narrative explanation without the structured organization specified in the template. Given the number and importance of missing sections, the description fails to meet the template requirements.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "feat: Add CommitDiff Instruction for Efficient Account Updates" directly and clearly reflects the primary change in the changeset. The raw summary confirms that the PR introduces a new CommitDiff instruction across multiple files (args, discriminator, instruction builder, and processor modules), adds supporting data structures, and integrates it into the instruction processing pipeline. The title is concise, specific, and immediately conveys both what was added (CommitDiff Instruction) and its purpose (efficient account updates). The title accurately summarizes the main objective without being vague or misleading.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[GabrielePicco]

LGTM!

[coderabbitai]

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ea733e and 3099cc4.

⛔ Files ignored due to path filters (2)

• Cargo.lock is excluded by !**/*.lock
• tests/integration/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (10)

• Cargo.toml (1 hunks)
• src/args/commit_state.rs (2 hunks)
• src/diff/types.rs (1 hunks)
• src/discriminator.rs (2 hunks)
• src/instruction_builder/commit_diff.rs (1 hunks)
• src/instruction_builder/mod.rs (2 hunks)
• src/lib.rs (1 hunks)
• src/processor/fast/commit_diff.rs (1 hunks)
• src/processor/fast/mod.rs (1 hunks)
• tests/integration/programs/test-delegation/Cargo.toml (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-29T09:47:34.076Z

Learnt from: Dodecahedr0x
Repo: magicblock-labs/delegation-program PR: 90
File: src/instruction_builder/set_fees_receiver.rs:25-29
Timestamp: 2025-10-29T09:47:34.076Z
Learning: In instruction builder functions (files in src/instruction_builder/), using `.unwrap()` on serialization operations like `to_vec()` is standard practice and does not need to be replaced with `.expect()`.

Applied to files:

• src/diff/types.rs

🧬 Code graph analysis (6)

src/instruction_builder/mod.rs (1)

src/instruction_builder/commit_diff.rs (1)

• commit_diff (16-45)

src/args/commit_state.rs (3)

src/diff/types.rs (3)

• size_of (31-31)
• size_of (32-32)
• size_of (33-33)

src/state/commit_record.rs (1)

• size_of (37-37)

src/state/delegation_record.rs (1)

• size_of (40-40)

src/instruction_builder/commit_diff.rs (2)

src/discriminator.rs (1)

• to_vec (43-46)

src/pda.rs (6)

• commit_record_pda_from_delegated_account (122-128)
• commit_state_pda_from_delegated_account (114-120)
• delegation_metadata_pda_from_delegated_account (106-112)
• delegation_record_pda_from_delegated_account (98-104)
• program_config_from_program_id (161-167)
• validator_fees_vault_pda_from_validator (153-159)

src/processor/fast/commit_diff.rs (3)

src/processor/fast/commit_state.rs (1)

• process_commit_state_internal (108-282)

src/diff/algorithm.rs (1)

• apply_diff_copy (210-230)

src/diff/types.rs (1)

• try_new_from_borsh_vec (106-111)

src/processor/fast/mod.rs (1)

src/instruction_builder/commit_diff.rs (1)

• commit_diff (16-45)

src/lib.rs (1)

src/processor/fast/commit_diff.rs (1)

• process_commit_diff (42-94)

⏰ 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). (1)

• GitHub Check: lint

🔇 Additional comments (19)

src/instruction_builder/mod.rs (1)

4-4: LGTM!

The new commit_diff module is properly declared and publicly re-exported, following the existing pattern used by other instruction builder modules.

Also applies to: 21-21

Cargo.toml (1)

51-51: LGTM!

The strum dependency with the derive feature is correctly added to support the IntoStaticStr derive macro used in src/discriminator.rs.

src/processor/fast/mod.rs (1)

1-1: LGTM!

The new commit_diff module is properly declared and publicly re-exported, making process_commit_diff available through the fast processor path. This follows the existing pattern.

Also applies to: 9-9

src/lib.rs (1)

85-87: LGTM!

The CommitDiff discriminator is properly routed to the fast processor path, following the same pattern as other fast-path instructions like CommitState and Delegate.

src/diff/types.rs (1)

106-111: LGTM!

The try_new_from_borsh_vec helper correctly handles Borsh-serialized vector format by:

• Validating minimum buffer size (4 bytes for length prefix)
• Skipping the 4-byte length prefix before delegating to try_new
• Returning appropriate error on validation failure

src/instruction_builder/commit_diff.rs (1)

16-45: LGTM!

The instruction builder correctly:

• Serializes CommitDiffArgs using Borsh (.unwrap() is standard practice per learnings)
• Computes all necessary PDAs from the delegated account and validator
• Assembles the account metas in the correct order matching the processor
• Tags the instruction data with DlpDiscriminator::CommitDiff

src/discriminator.rs (3)

2-2: LGTM!

The IntoStaticStr derive from strum is properly imported and applied, enabling static string name conversion for discriminator variants.

Also applies to: 5-5

---

38-39: LGTM!

The CommitDiff variant is correctly added with discriminator value 16, following the sequential pattern from the existing variants.

---

48-50: LGTM!

The name() method provides convenient access to variant names as static strings, leveraging the IntoStaticStr conversion.

src/args/commit_state.rs (3)

1-2: LGTM!

The import is required for the size calculation below and is correctly placed.

---

43-52: LGTM!

The CommitDiffArgsWithoutDiff struct correctly mirrors the non-diff fields of CommitDiffArgs and is appropriately used for deserialization in the processor.

---

54-55: LGTM!

The size calculation correctly sums the sizes of the three fields (nonce + lamports + allow_undelegation) and follows the same pattern used elsewhere in the codebase.

src/processor/fast/commit_diff.rs (7)

1-10: LGTM!

All necessary imports are present and correctly organized.

---

11-41: LGTM!

The documentation is comprehensive and clearly describes the accounts, requirements, and processing steps.

---

53-55: LGTM!

Proper validation to ensure the instruction data contains at least the required fixed-size arguments.

---

57-67: LGTM!

The data parsing correctly implements the manual split strategy that aligns with the serialization format. The warning for empty diffs provides good observability without blocking potentially intentional no-op commits.

---

72-76: Acknowledge the unsafe operation with planned resolution.

The use of borrow_data_unchecked() bypasses Rust's borrow checker and could lead to undefined behavior if there are concurrent mutable borrows. However, based on the TODO comment and past review discussion, this is a known limitation that will be addressed with a zero-copy approach in PR #113.

For the current PR, ensure this is tracked and resolved in the follow-up.

Based on learnings (past review comments).

---

78-93: LGTM!

The function correctly constructs the internal arguments and delegates to the existing process_commit_state_internal function, effectively reusing the validated commit state processing logic. The diff-applied data is properly passed as commit_state_bytes.

---

47-51: Based on the web search, I need to verify the actual implementation of how process_commit_state_internal uses system::Transfer. Let me search the codebase more carefully:

Based on my analysis of the code and web searches, I can now provide a definitive answer:

The _system_program account is not actually used in this function.

Your Solana Program can transfer lamports from one account to another without 'invoking' the System program. The fundamental rule is that your program can transfer lamports from any account owned by your program to any account at all. When process_commit_state_internal calls system::Transfer.invoke() (as shown in the script output at lines 192-197 of commit_state.rs), Anchor knows that we will need the system program, so we do not need to pass it here in the accounts array passed to invoke().

The consistent pattern across all commit processors (commit_diff, commit_state, finalize, delegate, commit_state_from_buffer) of extracting but marking _system_program as unused suggests this account is included for consistency or future compatibility, but it is genuinely not required for the current implementation. The underscore prefix correctly indicates intentional non-use.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3099cc4 and a76b7db.

📒 Files selected for processing (1)

• src/processor/fast/commit_diff.rs (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/processor/fast/commit_diff.rs (3)

src/processor/fast/commit_state.rs (1)

• process_commit_state_internal (108-282)

src/diff/algorithm.rs (1)

• apply_diff_copy (210-230)

src/diff/types.rs (1)

• try_new_from_borsh_vec (106-111)

⏰ 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). (1)

• GitHub Check: test

🔇 Additional comments (8)

src/processor/fast/commit_diff.rs (8)

47-51: LGTM!

The account extraction correctly validates that exactly 9 accounts are provided and returns an appropriate error otherwise. The system program account is properly included for the CPI Transfer operation performed internally.

---

53-55: LGTM!

Proper validation ensures the data buffer is large enough before attempting to split and deserialize the fixed arguments.

---

57-57: LGTM!

The split correctly separates variable-length diff data from the fixed-size trailing arguments.

---

59-60: LGTM!

Proper deserialization with appropriate error handling.

---

62-62: LGTM!

DiffSet creation properly handles validation and error propagation.

---

72-76: Verify borrow safety and track the performance fix.

The TODO acknowledges the performance concerns with heap allocation. The use of borrow_data_unchecked() bypasses runtime borrow checking, which is safe here since this appears to be the first and only borrow of delegated_account data in this function. However, please verify:

1. No other concurrent borrows of delegated_account exist in the call stack
2. The zero-copy implementation in PR #113 properly addresses both the performance and safety concerns

Based on past review comments, the zero-copy approach in PR #113 will resolve these issues.

---

78-91: LGTM!

The CommitStateInternalArgs construction properly includes all necessary accounts and parameters for internal state processing.

---

93-93: LGTM!

Proper delegation to the internal state processor maintains good separation of concerns.