refactor: do not require system_program in instruction-accounts and add docs

[snawaz]

This PR does the following things:

• Remove the requirement of system_program: AccountInfo as it is not required by both Pinocchio SDK as well as Solana SDK, though this PR doesn't touch the older implementations that use Solana SDK).
• Add the docs for the newly reimplemented instructions (see feat: optimize delegate instruction using Pinocchio for lower CU consumption #94).
  • Also notice the new doc does not mention [] the system program for all, except one in which I couldn't remove it, because downstream APIs require it.
• Remove the unnecessary TryFrom for discriminator, as it is automatically implemented by TryFromPrimitive.

Summary by CodeRabbit

• Refactor

  • Streamlined instruction parsing by using a single-byte discriminator.
  • Removed redundant runtime program identity checks to reduce false failures.
  • Consolidated upfront account validation with clearer “not enough accounts” errors.

• Documentation

  • Added detailed, user-oriented explanations for commit, delegate, finalize, and undelegate flows, including required accounts and steps.

• Style

  • Minor cleanup of unused checks and parameters to simplify code paths.

• Tests

  • No changes to end-user behavior; fast path remains unaffected.

[snawaz]

• refactor: do not require system_program in instruction-accounts and add docs #102 👈 (View in Graphite)
• cleanup: remove older impl: delegate, commit_*, finalize, undelegate #103
• fix: correct the conditions in requires.rs and return granular error #101
• main

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

[coderabbitai]

Walkthrough

Removes 8-byte discriminator conversion and uses first-byte discriminator in slow path. Eliminates system program validation across fast-path processors. Adjusts account destructuring accordingly, adding an unused system program placeholder where needed. Adds documentation comments. Updates undelegate function signature to accept an extra data slice.

Changes

Cohort / File(s)	Summary
Discriminator handling src/discriminator.rs, src/lib.rs	Removed TryFrom<[u8; 8]> for discriminator; slow path now derives discriminator from tag[0]. Removed program_id equality guard in slow path. Error paths updated accordingly.
Fast commit-state src/processor/fast/commit_state.rs, src/processor/fast/commit_state_from_buffer.rs	Dropped system program checks and related imports. Removed system_program from internal args. Introduced single upfront accounts destructuring including unused _system_program. Added doc comments.
Fast delegate src/processor/fast/delegate.rs	Added extensive docs. Removed commented-out system program check. No functional changes otherwise.
Fast finalize src/processor/fast/finalize.rs	Removed require_program system program validation; account slot now _system_program and ignored. Added doc comments.
Fast undelegate src/processor/fast/undelegate.rs	Added docs. Added _data: &[u8] param to public function. Removed system program validation and related import. Core flow unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Runtime
  participant Program as Program::slow_process_instruction
  participant Decoder as Discriminator::try_from(first_byte)

  Runtime->>Program: Invoke with input data
  Program->>Program: Read 8-byte tag
  Program->>Decoder: try_from(tag[0])
  alt valid discriminator
    Program-->>Runtime: Dispatch to handler
  else invalid
    Program-->>Runtime: Err(InvalidInstructionData)
  end

Loading

sequenceDiagram
  autonumber
  participant Runtime
  participant FastProc as Fast-path Processor (delegate/commit_state/finalize/undelegate)

  Runtime->>FastProc: Invoke with Accounts[], Data
  FastProc->>FastProc: Destructure Accounts (includes _system_program placeholder)
  note right of FastProc: System program not validated
  FastProc->>FastProc: Load/verify required accounts
  FastProc-->>Runtime: Perform action or return error

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat: optimize commit instruction using Pinocchio for lower CU consumption #96: Prior changes to fast commit_state introducing system_program handling that this PR now removes.
• feat: optimize commit-from-buffer instruction using Pinocchio for lower CU consumption #97: Adjustments to commit_state_from_buffer routing/args that align with this PR’s removal of system_program from internal args.
• feat: optimize finalize instruction using Pinocchio for lower CU consumption #98: Fast finalize implementation that previously included system_program checks now directly impacted by their removal here.

Suggested reviewers

• thlorenz
• GabrielePicco

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The PR description does not follow the repository’s required template: it omits the status table, the Problem and Solution headings, and other structured sections specified in the template, instead presenting a free-form list of changes without contextual framing.	Please update the description to match the repository template by adding the status table, filling in the Problem and Solution sections, and including any other required headings such as Deploy Notes or Other Changes.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly and concisely summarizes the main refactoring effort—removing the system_program requirement and adding documentation—and reflects the core changes without extraneous details, making it easy for reviewers to understand the primary purpose at a glance.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch snawaz/refactor

---

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

[GabrielePicco]

LGTM, few suggested nits

[coderabbitai]

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 (3)

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

55-57: System program account still mandatory.

Pattern matching still expects a trailing _system_program, so calls that omit it (per the new docs) will immediately fail with NotEnoughAccountKeys. Drop the extra binding.

Apply this diff:

-    let [payer, delegated_account, owner_program, delegate_buffer_account, delegation_record_account, delegation_metadata_account, _system_program] =
+    let [payer, delegated_account, owner_program, delegate_buffer_account, delegation_record_account, delegation_metadata_account] =
         accounts

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

56-60: Finalize still requires unused system program account.

Even though the instruction no longer documents system_program, the slice pattern still needs it, so a caller following the new spec will see NotEnoughAccountKeys. Remove the extra slot.

Apply this diff:

-    let [validator, delegated_account, commit_state_account, commit_record_account, delegation_record_account, delegation_metadata_account, validator_fees_vault, _system_program] =
+    let [validator, delegated_account, commit_state_account, commit_record_account, delegation_record_account, delegation_metadata_account, validator_fees_vault] =
         accounts

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

67-71: Commit-state fast path still demands _system_program.

The account matcher keeps an unused _system_program, so callers supplying only the documented eight accounts will fail with NotEnoughAccountKeys. Drop the unused binding.

Apply this diff:

-    let [validator, delegated_account, commit_state_account, commit_record_account, delegation_record_account, delegation_metadata_account, validator_fees_vault, program_config_account, _system_program] =
+    let [validator, delegated_account, commit_state_account, commit_record_account, delegation_record_account, delegation_metadata_account, validator_fees_vault, program_config_account] =
         accounts

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1836012 and 6d0ac94.

📒 Files selected for processing (7)

• src/discriminator.rs (0 hunks)
• src/lib.rs (1 hunks)
• src/processor/fast/commit_state.rs (2 hunks)
• src/processor/fast/commit_state_from_buffer.rs (1 hunks)
• src/processor/fast/delegate.rs (1 hunks)
• src/processor/fast/finalize.rs (1 hunks)
• src/processor/fast/undelegate.rs (1 hunks)

💤 Files with no reviewable changes (1)

• src/discriminator.rs

🧰 Additional context used

🧬 Code graph analysis (3)

src/processor/fast/finalize.rs (2)

src/processor/fast/utils/requires.rs (3)

• require_initialized_validator_fees_vault (223-244)
• require_owned_pda (33-44)
• require_signer (49-57)

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

• to_pinocchio_program_error (14-18)

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

src/processor/fast/utils/requires.rs (2)

• require_owned_pda (33-44)
• require_signer (49-57)

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

src/processor/fast/utils/requires.rs (5)

• require_initialized_validator_fees_vault (223-244)
• require_owned_pda (33-44)
• require_program_config (248-269)
• require_signer (49-57)
• require_uninitialized_pda (135-152)

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

• to_pinocchio_program_error (14-18)

⏰ 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: install

🔇 Additional comments (3)

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

35-76: Excellent documentation!

The comprehensive documentation clearly describes accounts, requirements, and the undelegation flow. The TODO comment on line 50 appropriately explains why the system program remains required (downstream API dependencies), which aligns with the PR objectives stating this is the one instruction where it couldn't be removed.

---

82-86: System program validation removed appropriately.

The removal of the runtime require_program check for system_program is appropriate. The system program is still required in the accounts array and used in CPI operations (lines 210, 252, 281-286, 316, 326). The Pinocchio/Solana runtime will validate it during CPI invocation, so the explicit upfront check is redundant. This reduces validation overhead while maintaining security, though it defers error detection until CPI time.

---

77-81: Verified call sites updated for new _data parameter; no further action required.