feat: add AccountSizeClass and compute account-data sizes for 8 instructions

[snawaz]

We plan to use SetLoadedAccountsDataSizeLimit when invoking DLP instructions (from the validator magicblock-labs/magicblock-validator#800 and elsewhere). Computing the exact loaded account data size per instruction would require fetching all accounts on-chain and summing their data_len, which is unnecessarily expensive and defeats the purpose of optimization.

The good news is that, fortunately, an exact value is not required. The runtime only needs a safe upper bound, even if that bound includes some conservative margin, not too large though.

This PR introduces a lightweight classification system called AccountSizeClass, which groups accounts into coarse-grained size buckets:

• Tiny — for all accounts <= 256 bytes
• Small — <= 1 KB
• Medium — <= 8 KB
• Large — <= 64 KB
• ExtraLarge — <= 256 KB
• Huge — <= 1 MB
• Dynamic(u32) — caller-supplied upper bound when size is truly variable, e.g data-len of delegated-account, or its diff.

Each instruction declares the size class of every account it touches. We then compute a deterministic upper bound on total loaded account data size by summing the maximum size of each class. This approach is simple, super efficient and flexible.

Summary by CodeRabbit

• New Features
  • Added account size classification system with hierarchical tiers (Tiny, Small, Medium, Large, ExtraLarge, Huge, Dynamic) for flexible account sizing.
  • Introduced size budget utilities across instruction operations including delegation, state commitment, and finalization to help optimize resource allocation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[snawaz]

• feat: add CommitFinalizeFromBuffer #137
• feat: add CommitFinalize #131
• feat: add AccountSizeClass and compute account-data sizes for 8 instructions #130 👈 (View in Graphite)
• main

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

[coderabbitai]

Actionable comments posted: 8

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d58eb95 and 1361f9f.

📒 Files selected for processing (9)

• src/account_size_class.rs
• src/instruction_builder/commit_diff.rs
• src/instruction_builder/commit_diff_from_buffer.rs
• src/instruction_builder/commit_state.rs
• src/instruction_builder/commit_state_from_buffer.rs
• src/instruction_builder/delegate.rs
• src/instruction_builder/finalize.rs
• src/instruction_builder/undelegate.rs
• src/lib.rs

🧰 Additional context used

🧬 Code graph analysis (6)

src/instruction_builder/finalize.rs (1)

src/account_size_class.rs (1)

• total_size_budget (60-62)

src/instruction_builder/commit_diff_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (60-62)

src/instruction_builder/delegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (60-62)

src/instruction_builder/commit_state_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (60-62)

src/instruction_builder/commit_diff.rs (1)

src/account_size_class.rs (1)

• total_size_budget (60-62)

src/instruction_builder/undelegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (60-62)

🔇 Additional comments (8)

src/instruction_builder/undelegate.rs (1)

12-12: LGTM!

The import correctly brings in the necessary types and functions for computing size budgets.

src/instruction_builder/commit_state_from_buffer.rs (1)

13-13: LGTM!

The import is correct for the new size budget functionality.

src/lib.rs (1)

34-36: LGTM!

The module declaration and re-export follow standard Rust patterns to expose the new AccountSizeClass and related utilities as part of the public API.

src/instruction_builder/commit_diff_from_buffer.rs (1)

13-13: LGTM!

The import is correct.

src/instruction_builder/finalize.rs (1)

11-11: LGTM!

The import is correct.

src/instruction_builder/commit_diff.rs (1)

48-60: Verify the size assumption for commit_state_pda.

The commit_state_pda (line 52) uses the same AccountSizeClass as delegated_account. This is the same PDA used in commit_state.rs. Confirm that this PDA is intentionally sized to match the delegated account across all usage sites.

src/account_size_class.rs (1)

42-62: LGTM!

The implementation of size_budget() as a const fn is appropriate, and the total_size_budget() helper correctly aggregates budgets using sum(). The use of local constants KB and MB improves readability.

src/instruction_builder/delegate.rs (1)

46-56: No issue found. The delegate_buffer_pda is correctly sized to match the delegated_account because it serves as a temporary storage buffer for the complete account data, which is later copied into the delegated account (see processor code line 225: copy_from_slice). This sizing matches the established pattern used for undelegate_buffer_pda, which is explicitly created with delegated_account.data_len(). The buffer is not a metadata-only PDA.

Likely an incorrect or invalid review comment.

[coderabbitai]

Actionable comments posted: 4

♻️ Duplicate comments (4)

src/instruction_builder/commit_diff_from_buffer.rs (1)

50-63: Verify PDA size class assumptions for commit_state_pda and commit_state_buffer.

This function assumes both commit_state_pda (Line 54) and commit_state_buffer (Line 58) have the same size class as delegated_account. While this may be intentional if these accounts store copies of the delegated account state, please verify these assumptions align with actual on-chain account sizes.

Note: Past review comments have already flagged the missing documentation for this public API.

src/instruction_builder/commit_state_from_buffer.rs (1)

54-67: Verify PDA size class assumptions for commit_state_pda and commit_state_buffer.

This function assumes both commit_state_pda (Line 58) and commit_state_buffer (Line 62) have the same size class as delegated_account. While this may be intentional if these accounts store copies of the delegated account state, please verify these assumptions align with actual on-chain account sizes.

Note: Past review comments have already flagged the missing documentation for this public API.

src/instruction_builder/undelegate.rs (1)

51-66: Documentation for public API still needed.

As noted in the previous review, this public function lacks documentation explaining its purpose, parameters, and return value. The suggestion to add documentation remains valid.

src/instruction_builder/finalize.rs (1)

38-49: Documentation for public API still needed.

The previous review noted that this public function lacks documentation. The suggestion to document the function's purpose, the delegated_account parameter, and the return value remains valid.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d58eb95 and 5f194b6.

📒 Files selected for processing (9)

• src/account_size_class.rs
• src/instruction_builder/commit_diff.rs
• src/instruction_builder/commit_diff_from_buffer.rs
• src/instruction_builder/commit_state.rs
• src/instruction_builder/commit_state_from_buffer.rs
• src/instruction_builder/delegate.rs
• src/instruction_builder/finalize.rs
• src/instruction_builder/undelegate.rs
• src/lib.rs

🧰 Additional context used

🧬 Code graph analysis (7)

src/instruction_builder/commit_diff_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_state_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/delegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/finalize.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_diff.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/undelegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_state.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

⏰ 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 (2)

src/lib.rs (1)

34-36: LGTM!

The module addition and re-export follow standard Rust patterns and correctly expose the new AccountSizeClass API at the crate root.

src/account_size_class.rs (1)

42-57: LGTM!

The size_budget implementation correctly returns the appropriate budget value for each variant. The use of const fn allows compile-time evaluation, and the constant definitions (KB, MB) improve readability.

[coderabbitai]

Actionable comments posted: 2

Fix all issues with AI Agents 🤖

In @src/account_size_class.rs:
- Around line 31-39: Add concise doc comments for the enum variants ExtraLarge,
Huge, and Dynamic(u32) in account_size_class.rs: explain typical example
accounts that fit ExtraLarge (e.g., token multisig or medium on-chain program
state) and Huge (e.g., large program accounts or aggregated metadata), and for
Dynamic(u32) describe when to prefer a custom byte limit (e.g., variable-sized
user data or when precise sizing is required) and note units are bytes; place
these as Rust doc comments (///) directly above the variants and include
one-line example use-cases or guidance on choosing Dynamic over fixed classes.

In @src/instruction_builder/call_handler.rs:
- Around line 43-56: call_handler_size_budget currently only sums the five fixed
accounts and ignores the always-present variable other_accounts, causing
underestimation; change its signature to accept an additional parameter (e.g.,
other_accounts: &[AccountSizeClass]) and include those entries when calling
total_size_budget (append other_accounts to the fixed list), update any
callers/tests to pass the appropriate slice (or provide a small helper overload
if needed), and update the function doc comment to state that other_accounts
must be provided so callers can accurately compute
SetLoadedAccountsDataSizeLimit.

♻️ Duplicate comments (5)

src/instruction_builder/commit_state.rs (1)

48-65: Expand documentation for public API.

The documentation is present but minimal. For a public API, consider documenting:

• The delegated_account parameter and its role
• The return value (size budget in bytes)
• That commit_state_pda conservatively uses the same size class as delegated_account (line 57) since the actual PDA size varies with committed data

This would help users understand the function's purpose and the conservative budgeting approach.

📝 Enhanced documentation example

 ///
 /// Returns accounts-data-size budget for commit_state instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - The size class of the delegated account. Also used conservatively
+///   for the commit_state_pda budget (actual PDA size varies with committed data).
+///
+/// # Returns
+/// Total size budget in bytes for all accounts in the instruction.
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///
 pub fn commit_size_budget(delegated_account: AccountSizeClass) -> u32 {

src/instruction_builder/commit_diff.rs (1)

48-65: Expand documentation for public API.

The documentation is present but minimal. For a public API, consider documenting:

• The delegated_account parameter and its role
• The return value (size budget in bytes)
• That commit_state_pda conservatively uses the same size class as delegated_account (line 57) since the actual PDA size depends on diff data

This would help users understand the function's purpose and the conservative budgeting approach.

📝 Enhanced documentation example

 ///
 /// Returns accounts-data-size budget for commit_diff instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - The size class of the delegated account. Also used conservatively
+///   for the commit_state_pda budget (actual PDA size varies with diff data).
+///
+/// # Returns
+/// Total size budget in bytes for all accounts in the instruction.
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///
 pub fn commit_diff_size_budget(delegated_account: AccountSizeClass) -> u32 {

src/instruction_builder/delegate.rs (1)

46-61: Expand documentation for public API.

The documentation is present but minimal. For a public API, consider documenting:

• The delegated_account parameter and its role
• The return value (size budget in bytes)
• That delegate_buffer_pda conservatively uses the same size class as delegated_account (line 56)

This would help users understand the function's purpose and usage.

📝 Enhanced documentation example

 ///
 /// Returns accounts-data-size budget for delegate instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - The size class of the delegated account. Also used
+///   for the delegate_buffer_pda budget.
+///
+/// # Returns
+/// Total size budget in bytes for all accounts in the instruction.
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///
 pub fn delegate_size_budget(delegated_account: AccountSizeClass) -> u32 {

src/instruction_builder/undelegate.rs (1)

51-71: Expand documentation for public API.

The documentation is present but minimal. For a public API, consider documenting:

• The delegated_account parameter and its role
• The return value (size budget in bytes)
• That multiple PDAs (undelegate_buffer_pda line 61, commit_state_pda line 62) conservatively use the same size class as delegated_account

This would help users understand the function's purpose and usage.

📝 Enhanced documentation example

 ///
 /// Returns accounts-data-size budget for undelegate instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - The size class of the delegated account. Also used conservatively
+///   for undelegate_buffer_pda and commit_state_pda budgets.
+///
+/// # Returns
+/// Total size budget in bytes for all accounts in the instruction.
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///
 pub fn undelegate_size_budget(delegated_account: AccountSizeClass) -> u32 {

src/account_size_class.rs (1)

59-61: Consider overflow protection or document assumptions.

The function sums u32 budget values without overflow checking. While overflow is unlikely with realistic account lists and predefined classes, using multiple Dynamic variants with large values could theoretically overflow.

🔎 Optional: Add saturating arithmetic

 pub fn total_size_budget(classes: &[AccountSizeClass]) -> u32 {
-    classes.iter().map(|f| f.size_budget()).sum()
+    classes
+        .iter()
+        .map(|f| f.size_budget())
+        .fold(0u32, |acc, x| acc.saturating_add(x))
 }

Or add documentation:

+/// Computes the total size budget by summing all individual budgets.
+///
+/// # Note
+/// Assumes the sum does not exceed `u32::MAX`. In practice, Solana's account
+/// limits and typical instruction account lists make overflow extremely unlikely.
 pub fn total_size_budget(classes: &[AccountSizeClass]) -> u32 {

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02c7ed0 and 293a606.

📒 Files selected for processing (10)

• src/account_size_class.rs
• src/instruction_builder/call_handler.rs
• src/instruction_builder/commit_diff.rs
• src/instruction_builder/commit_diff_from_buffer.rs
• src/instruction_builder/commit_state.rs
• src/instruction_builder/commit_state_from_buffer.rs
• src/instruction_builder/delegate.rs
• src/instruction_builder/finalize.rs
• src/instruction_builder/undelegate.rs
• src/lib.rs

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-10-16T10:04:26.576Z

Learnt from: taco-paco
Repo: magicblock-labs/delegation-program PR: 105
File: src/args/call_handler.rs:6-8
Timestamp: 2025-10-16T10:04:26.576Z
Learning: In `src/args/call_handler.rs`, the `data` field of the `CallHandlerArgs` struct has no maximum length constraint by design, as it needs to support flexible instruction formats including custom discriminators and arguments in any format.

Applied to files:

• src/instruction_builder/call_handler.rs

📚 Learning: 2025-10-16T10:04:25.110Z

Learnt from: taco-paco
Repo: magicblock-labs/delegation-program PR: 105
File: src/args/call_handler.rs:6-8
Timestamp: 2025-10-16T10:04:25.110Z
Learning: The `data` field in the `CallHandlerArgs` struct (src/args/call_handler.rs) is intentionally designed to accept any length without max length validation, allowing flexible instruction formats including custom discriminators and arguments.

Applied to files:

• src/instruction_builder/call_handler.rs

🧬 Code graph analysis (8)

src/instruction_builder/commit_diff.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/delegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_diff_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/undelegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/finalize.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_state.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_state_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/call_handler.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

⏰ 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 (9)

src/lib.rs (1)

34-36: LGTM!

The module declaration and re-export follow standard Rust patterns and are appropriately placed alongside other public modules.

src/account_size_class.rs (1)

42-57: LGTM! Clean and efficient implementation.

The const fn implementation is appropriate and enables compile-time evaluation. The budget mappings are clear and align with the documented size categories.

src/instruction_builder/call_handler.rs (1)

4-4: LGTM! Necessary imports for size budget functionality.

The imports are correctly structured and enable the new call_handler_size_budget function.

src/instruction_builder/commit_diff_from_buffer.rs (2)

13-13: LGTM! Correct imports for size budget functionality.

---

50-68: LGTM! Size budget correctly mirrors instruction accounts.

The function properly maps the 10 instruction accounts to their size classes. Using delegated_account for commit_state_pda and commit_state_buffer is appropriate since they store delegated account data.

src/instruction_builder/finalize.rs (2)

11-11: LGTM! Correct imports.

---

38-54: LGTM! Size budget correctly mirrors instruction accounts.

The function properly maps all 8 instruction accounts to their size classes, with appropriate documentation. Using delegated_account for commit_state_pda is correct since it stores delegated account state.

src/instruction_builder/commit_state_from_buffer.rs (2)

13-13: LGTM! Correct imports.

---

54-72: LGTM! Size budget correctly mirrors instruction accounts.

The function properly maps all 10 instruction accounts to their size classes, with clear documentation. The mapping is consistent with the similar commit_diff_from_buffer_size_budget function.

[coderabbitai]

Actionable comments posted: 3

Fix all issues with AI Agents 🤖

In @src/instruction_builder/finalize.rs:
- Around line 38-54: The doc comment for finalize_size_budget is missing
parameter and return-value details; update the function-level Rust doc for
finalize_size_budget to document the delegated_account parameter (explain it's
the AccountSizeClass used for the delegated account and commit_state_pda) and
clarify the return value is the total accounts-data-size budget in bytes (u32),
e.g., add a brief "Arguments" or parameter line for delegated_account and a
"Returns" line stating it returns the total size in bytes as u32.

In @src/instruction_builder/undelegate.rs:
- Around line 51-71: The undelegate_size_budget function correctly lists all 12
accounts with appropriate AccountSizeClass assignments and needs no changes;
keep the current implementation of undelegate_size_budget and its use of
delegated_account for the delegated_account, undelegate_buffer_pda, and
commit_state_pda, and leave the Tiny assignments for the fixed-size PDAs and
programs as-is.

♻️ Duplicate comments (5)

src/instruction_builder/delegate.rs (1)

46-61: Correct implementation with consistent pattern.

The budget calculation correctly matches the instruction's account list. The use of delegated_account size class for both the account itself and delegate_buffer_pda is appropriate since the buffer stores a copy of the account data.

The documentation follows the same pattern as other budget functions in this PR. While functional, parameter documentation would enhance clarity.

📝 Optional documentation enhancement

 ///
 /// Returns accounts-data-size budget for delegate instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - Size class for the delegated account and delegate_buffer_pda
+///
+/// # Returns
+/// Total size budget in bytes
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///
 pub fn delegate_size_budget(delegated_account: AccountSizeClass) -> u32 {

src/instruction_builder/commit_diff.rs (1)

48-65: Implementation is correct; consider addressing past documentation feedback.

The budget calculation accurately reflects the instruction's accounts. The delegated_account size class is correctly used for the read-only delegated_account (data must still be loaded) and conservatively for commit_state_pda (whose actual size varies with committed diff data).

A previous review suggested documenting why commit_state_pda uses the delegated_account size class (conservative upper bound for dynamic PDA). While the current docs are functional, adding this clarification would help users understand the budgeting strategy.

📝 Optional documentation enhancement addressing past feedback

 ///
 /// Returns accounts-data-size budget for commit_diff instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - Size class of the delegated account; also used conservatively
+///   for commit_state_pda whose actual size varies with diff data
+///
+/// # Returns
+/// Total size budget in bytes
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///
 pub fn commit_diff_size_budget(delegated_account: AccountSizeClass) -> u32 {

src/instruction_builder/commit_state.rs (1)

53-65: Verify commit_state_pda size assumption remains valid.

This function assumes commit_state_pda (line 57) uses the same size class as delegated_account. However, commit_state_pda stores variable-sized committed state data determined by the CommitStateArgs payload, not a fixed size matching the delegated account.

A previous review flagged this exact issue, and it was marked as "Fixed," but the implementation still uses delegated_account for the commit_state_pda budget. If this design is intentional (e.g., commit data is guaranteed to never exceed delegated account size), please add a comment explaining the invariant. Otherwise, consider adding a separate parameter for the commit state size class.

#!/bin/bash
# Check how commit_state_pda is sized in the processor to understand if it matches delegated_account size
rg -n "commit_state_pda" -B 5 -A 10 --type rust | grep -E "(realloc|resize|size|len|CommitStateArgs)"

src/instruction_builder/commit_diff_from_buffer.rs (1)

55-68: Verify size assumptions for commit_state_pda and commit_state_buffer.

Similar to commit_state_from_buffer.rs, this function assumes commit_state_pda (line 59) and commit_state_buffer (line 63) match the delegated_account size class. These accounts may have independent sizes:

• commit_state_pda stores variable committed state
• commit_state_buffer is caller-provided (line 21)

Consider adding parameters for these size classes or documenting the invariants that guarantee size equality.

src/account_size_class.rs (1)

59-61: Consider overflow protection or document assumptions.

The function sums u32 values without overflow checking. While Solana's practical account limits make overflow unlikely, it's theoretically possible with multiple Dynamic variants or many large accounts.

Consider either:

1. Using saturating arithmetic: fold(0u32, |acc, x| acc.saturating_add(x))
2. Documenting that callers must ensure reasonable budget values

Optional: Add overflow protection

 pub fn total_size_budget(classes: &[AccountSizeClass]) -> u32 {
-    classes.iter().map(|f| f.size_budget()).sum()
+    classes
+        .iter()
+        .map(|f| f.size_budget())
+        .fold(0u32, |acc, x| acc.saturating_add(x))
 }

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d58eb95 and 532856a.

📒 Files selected for processing (10)

• src/account_size_class.rs
• src/instruction_builder/call_handler.rs
• src/instruction_builder/commit_diff.rs
• src/instruction_builder/commit_diff_from_buffer.rs
• src/instruction_builder/commit_state.rs
• src/instruction_builder/commit_state_from_buffer.rs
• src/instruction_builder/delegate.rs
• src/instruction_builder/finalize.rs
• src/instruction_builder/undelegate.rs
• src/lib.rs

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-10-16T10:04:26.576Z

Learnt from: taco-paco
Repo: magicblock-labs/delegation-program PR: 105
File: src/args/call_handler.rs:6-8
Timestamp: 2025-10-16T10:04:26.576Z
Learning: In `src/args/call_handler.rs`, the `data` field of the `CallHandlerArgs` struct has no maximum length constraint by design, as it needs to support flexible instruction formats including custom discriminators and arguments in any format.

Applied to files:

• src/instruction_builder/call_handler.rs

📚 Learning: 2025-10-16T10:04:25.110Z

Learnt from: taco-paco
Repo: magicblock-labs/delegation-program PR: 105
File: src/args/call_handler.rs:6-8
Timestamp: 2025-10-16T10:04:25.110Z
Learning: The `data` field in the `CallHandlerArgs` struct (src/args/call_handler.rs) is intentionally designed to accept any length without max length validation, allowing flexible instruction formats including custom discriminators and arguments.

Applied to files:

• src/instruction_builder/call_handler.rs

🧬 Code graph analysis (8)

src/instruction_builder/finalize.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/call_handler.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_diff.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_state.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/delegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_state_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/undelegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

src/instruction_builder/commit_diff_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (59-61)

⏰ 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 (10)

src/lib.rs (1)

34-36: LGTM! Clean module integration.

The module addition and re-export follow standard Rust conventions and integrate cleanly with the existing codebase structure.

src/instruction_builder/commit_state.rs (1)

13-13: LGTM: Clean import for new functionality.

The import correctly brings in the necessary types for the size budget calculation.

src/instruction_builder/commit_state_from_buffer.rs (2)

13-13: LGTM: Import statement is correct.

---

54-58: Good documentation added.

The function documentation clearly explains the purpose and usage with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit.

src/instruction_builder/commit_diff_from_buffer.rs (2)

13-13: LGTM: Import is correct.

---

50-54: Good documentation.

The function documentation is clear and addresses the previous review feedback.

src/instruction_builder/call_handler.rs (2)

4-4: LGTM: Import is correct.

---

43-56: Well-designed API addressing previous feedback.

The function now correctly accounts for other_accounts by accepting a u32 parameter for their total byte size (line 48) and adding it to the budget (line 55). This addresses the previous review concern where the budget calculation was incomplete.

The use of u32 for other_accounts is appropriate since it represents a variable-length account list whose total size is known by the caller.

src/account_size_class.rs (2)

1-40: Well-structured enum with appropriate derives.

The AccountSizeClass enum is well-designed with:

• Correct derives including PartialEq, Eq (addressing previous feedback)
• Logical progression of size categories
• Good inline documentation with concrete examples for smaller variants

The documentation comments use "account size <= X" phrasing. While a previous review suggested clarifying these as "budget categories" rather than hard constraints, the current phrasing is clear enough in context.

---

42-57: Clean implementation with reasonable budget values.

The size_budget method implementation is straightforward and correct:

• Using const fn allows compile-time evaluation
• Budget values progress logically (256B → 1KB → 8KB → 64KB → 256KB → 1MB)
• Values align well with common Solana account size patterns

[coderabbitai]

Actionable comments posted: 2

Fix all issues with AI Agents 🤖

In @src/account_size_class.rs:
- Around line 4-12: The comment's example is ambiguous about why the subtraction
is needed; clarify that each helper like commit_diff_size_budget,
finalize_size_budget, and undelegate_size_budget already includes
DLP_PROGRAM_DATA_SIZE_CLASS, so when you combine N such helpers you must
subtract (N-1) copies of DLP_PROGRAM_DATA_SIZE_CLASS.size_budget() to avoid
double-counting — update the comment to state this explicitly and show the
corrected formula (e.g., sum_of_helpers - (N-1) *
DLP_PROGRAM_DATA_SIZE_CLASS.size_budget()), referencing the functions
commit_diff_size_budget, finalize_size_budget, undelegate_size_budget and the
constant DLP_PROGRAM_DATA_SIZE_CLASS.

♻️ Duplicate comments (6)

src/account_size_class.rs (2)

16-52: Consider clarifying that these are size budget categories, not account size constraints.

The comments state "account size <= X" for each variant, but these represent budget allocations for transaction cost estimation rather than hard constraints on actual account sizes. The size_budget() method returns a budget value used for sizing transactions. Consider rephrasing to clarify these are upper-bound budget categories (e.g., "budget class for accounts <= X bytes").

🤖 Prompt for AI Agents

In src/account_size_class.rs around lines 16-52, update the comments for each AccountSizeClass variant to clarify that these represent budget categories (upper-bound allocations in bytes for transaction cost estimation) rather than hard account-size constraints; change "account size <= X" phrasing to "budget for accounts <= X bytes" or similar, explicitly state units are bytes, and note that these values are used with SetLoadedAccountsDataSizeLimit for transaction sizing; keep variant comments concise while ensuring clarity that size_budget() returns the budget value associated with each category for use in compute budget instructions.

---

69-71: Add overflow protection to total_size_budget().

The function sums u32 budget values without overflow checking. While Solana's account limits make overflow unlikely in practice, using multiple Dynamic variants with large values could theoretically overflow. Consider using saturating_add or returning Option<u32> with checked_add.

🔎 Proposed fix using saturating arithmetic

 pub fn total_size_budget(classes: &[AccountSizeClass]) -> u32 {
-    classes.iter().map(|f| f.size_budget()).sum()
+    classes
+        .iter()
+        .map(|f| f.size_budget())
+        .fold(0u32, |acc, x| acc.saturating_add(x))
 }

Alternatively, return Option<u32> to signal overflow:

-pub fn total_size_budget(classes: &[AccountSizeClass]) -> u32 {
-    classes.iter().map(|f| f.size_budget()).sum()
+pub fn total_size_budget(classes: &[AccountSizeClass]) -> Option<u32> {
+    classes
+        .iter()
+        .map(|f| f.size_budget())
+        .try_fold(0u32, |acc, x| acc.checked_add(x))
 }

(Note: This would require updating all call sites to handle Option.)

src/instruction_builder/delegate.rs (1)

46-50: Enhance documentation with parameter and return value details.

The documentation explains the function's purpose but could be more complete. Consider adding:

• Description of the delegated_account parameter
• Clarification that the return value is in bytes
• Note that delegate_buffer_pda is budgeted at the same size class as delegated_account (since it stores a copy of the delegated account data)

🔎 Suggested documentation enhancement

 ///
 /// Returns accounts-data-size budget for delegate instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - Size class of the delegated account (also used for
+///   the delegate_buffer_pda, which stores a copy of the account data)
+///
+/// # Returns
+/// Total size budget in bytes for all accounts loaded by the delegate instruction.
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///

src/instruction_builder/commit_diff.rs (1)

48-52: Enhance documentation with parameter and return value details.

The documentation explains the function's purpose but could be more complete. Consider adding:

• Description of the delegated_account parameter and its role
• Clarification that the return value is in bytes
• Note that commit_state_pda is conservatively budgeted at the delegated_account size class (as its actual size varies with diff data)

🔎 Suggested documentation enhancement

 ///
 /// Returns accounts-data-size budget for commit_diff instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - Size class of the delegated account (also used conservatively
+///   for the commit_state_pda, whose actual size varies with diff data)
+///
+/// # Returns
+/// Total size budget in bytes for all accounts loaded by the commit_diff instruction.
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///

src/instruction_builder/commit_state.rs (1)

48-52: Enhance documentation with parameter and return value details.

The documentation explains the function's purpose but could be more complete. Consider adding:

• Description of the delegated_account parameter
• Clarification that the return value is in bytes
• Note that commit_state_pda is conservatively budgeted at the delegated_account size class (as its actual size varies with committed state data)

🔎 Suggested documentation enhancement

 ///
 /// Returns accounts-data-size budget for commit_state instruction.
 ///
+/// # Arguments
+/// * `delegated_account` - Size class of the delegated account (also used conservatively
+///   for the commit_state_pda, whose actual size varies with committed state data)
+///
+/// # Returns
+/// Total size budget in bytes for all accounts loaded by the commit_state instruction.
+///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///

src/instruction_builder/commit_state_from_buffer.rs (1)

54-73: Verify size assumptions for commit_state_pda and commit_state_buffer.

This function assumes both commit_state_pda (line 64) and commit_state_buffer (line 68) use the same size class as delegated_account. However:

1. commit_state_pda is dynamically sized based on the actual committed state data length, which can vary from delegated_account
2. commit_state_buffer is a caller-provided external account with no documented size guarantee relative to delegated_account

Without protocol invariants guaranteeing these size relationships, the computed budget may be inaccurate, potentially causing issues with SetLoadedAccountsDataSizeLimit.

Either document the protocol guarantees or refactor to accept separate size parameters for these accounts.

#!/bin/bash
# Search for documentation of size invariants between these accounts
rg -n "commit_state_pda|commit_state_buffer" --type rust src/processor/ -B 2 -A 5 | grep -i "size\|length\|resize\|realloc"

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d58eb95 and b0498df.

📒 Files selected for processing (10)

• src/account_size_class.rs
• src/instruction_builder/call_handler.rs
• src/instruction_builder/commit_diff.rs
• src/instruction_builder/commit_diff_from_buffer.rs
• src/instruction_builder/commit_state.rs
• src/instruction_builder/commit_state_from_buffer.rs
• src/instruction_builder/delegate.rs
• src/instruction_builder/finalize.rs
• src/instruction_builder/undelegate.rs
• src/lib.rs

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-10-16T10:04:26.576Z

Learnt from: taco-paco
Repo: magicblock-labs/delegation-program PR: 105
File: src/args/call_handler.rs:6-8
Timestamp: 2025-10-16T10:04:26.576Z
Learning: In `src/args/call_handler.rs`, the `data` field of the `CallHandlerArgs` struct has no maximum length constraint by design, as it needs to support flexible instruction formats including custom discriminators and arguments in any format.

Applied to files:

• src/instruction_builder/call_handler.rs

📚 Learning: 2025-10-16T10:04:25.110Z

Learnt from: taco-paco
Repo: magicblock-labs/delegation-program PR: 105
File: src/args/call_handler.rs:6-8
Timestamp: 2025-10-16T10:04:25.110Z
Learning: The `data` field in the `CallHandlerArgs` struct (src/args/call_handler.rs) is intentionally designed to accept any length without max length validation, allowing flexible instruction formats including custom discriminators and arguments.

Applied to files:

• src/instruction_builder/call_handler.rs

🧬 Code graph analysis (8)

src/instruction_builder/delegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (69-71)

src/instruction_builder/finalize.rs (1)

src/account_size_class.rs (1)

• total_size_budget (69-71)

src/instruction_builder/commit_state.rs (1)

src/account_size_class.rs (1)

• total_size_budget (69-71)

src/instruction_builder/commit_state_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (69-71)

src/instruction_builder/undelegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (69-71)

src/instruction_builder/call_handler.rs (1)

src/account_size_class.rs (1)

• total_size_budget (69-71)

src/instruction_builder/commit_diff_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (69-71)

src/instruction_builder/commit_diff.rs (1)

src/account_size_class.rs (1)

• total_size_budget (69-71)

⏰ 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 (12)

src/account_size_class.rs (2)

1-2: LGTM: Clear constants for size calculations.

The KB and MB constants are correctly defined and used consistently throughout the module.

---

55-67: LGTM: size_budget() implementation is correct.

The const fn implementation correctly maps each size class to its budget value in bytes, enabling compile-time evaluation where possible.

src/lib.rs (1)

34-36: LGTM: Module declaration and re-export follow Rust conventions.

The new account_size_class module is properly declared and its public API is re-exported, making the size budgeting primitives available throughout the crate and to external users.

src/instruction_builder/commit_diff.rs (2)

13-13: LGTM: Imports the necessary size budgeting primitives.

The import statement correctly brings in the required types and functions from the new account_size_class module.

---

53-66: LGTM: Budget calculation correctly accounts for all instruction accounts.

The function properly maps each account in the commit_diff instruction to a size class. The use of delegated_account for both the delegated account and commit_state_pda (line 58) is a conservative upper-bound estimate, appropriate for budgeting since the actual PDA size varies with diff data.

src/instruction_builder/delegate.rs (2)

12-12: LGTM: Imports the necessary size budgeting primitives.

The import statement correctly brings in the required types and functions.

---

51-62: LGTM: Budget calculation correctly accounts for all instruction accounts.

The function properly maps each account in the delegate instruction to a size class. Using delegated_account for both the delegated account and delegate_buffer_pda (line 57) is appropriate since the buffer stores a copy of the delegated account data.

src/instruction_builder/commit_state.rs (2)

13-13: LGTM: Imports the necessary size budgeting primitives.

The import statement correctly brings in the required types and functions.

---

53-66: LGTM: Budget calculation correctly accounts for all instruction accounts.

The function properly maps each account in the commit_state instruction to a size class. Using delegated_account for commit_state_pda (line 58) is a reasonable conservative upper-bound estimate for budgeting purposes, since the committed state typically reflects the delegated account's data. This approach ensures sufficient budget allocation while being transaction-cost efficient.

src/instruction_builder/undelegate.rs (1)

51-72: LGTM! Well-structured budget calculation.

The function correctly accounts for all 12 instruction accounts plus the program data, with appropriate size class assignments. Using delegated_account size class for the delegated account itself and derived PDAs (undelegate_buffer_pda, commit_state_pda) is appropriately conservative, while fixed-size PDAs and system accounts correctly use Tiny.

src/instruction_builder/finalize.rs (1)

38-55: LGTM! Correct budget calculation for finalize instruction.

The function properly accounts for all 8 instruction accounts with appropriate size class assignments. Using delegated_account size class for commit_state_pda is conservatively correct since the state grows with the delegated account.

src/instruction_builder/call_handler.rs (1)

43-57: LGTM! Properly handles variable accounts.

The function correctly accounts for all 5 base instruction accounts plus the program data, and appropriately adds the other_accounts parameter to handle the variable account list that's always present in the instruction. This properly addresses the previous review concern.

[GabrielePicco]

Thanks @snawaz, LGTM as a first version!

This could be further optimized by estimating more accurately, e.g.:

• DelegationRecord: 8 + 88 = 96 bytes (currently Tiny = 256)
• CommitRecord: 8 + 80 = 88 bytes (currently Tiny = 256)
• ValidatorFeesVault, ProgramConfig: likely similar small sizes

[coderabbitai]

Walkthrough

This PR introduces a hierarchical account size budget framework comprising a new AccountSizeClass enum with predefined tiers (Tiny through Huge and Dynamic), along with utilities for computing aggregate budgets. Corresponding size budget helper functions are added to eight instruction builder modules, each calculating the total data size for their respective instructions.

Changes

Cohort / File(s)	Summary
Account Size Class Framework src/account_size_class.rs	Introduces new public AccountSizeClass enum with variants: Tiny, Small, Medium, Large, ExtraLarge, Huge, and Dynamic(u32). Implements size_budget() method returning byte budgets for each tier. Adds total_size_budget() utility function and DLP_PROGRAM_DATA_SIZE_CLASS constant.
Module Exports src/lib.rs	Declares new account_size_class module and re-exports its public API via pub use account_size_class::*;.
Instruction Builder Budget Helpers src/instruction_builder/call_handler.rs, src/instruction_builder/commit_diff.rs, src/instruction_builder/commit_diff_from_buffer.rs, src/instruction_builder/commit_state.rs, src/instruction_builder/commit_state_from_buffer.rs, src/instruction_builder/delegate.rs, src/instruction_builder/finalize.rs, src/instruction_builder/undelegate.rs	Each module receives a new public size budget function (e.g., call_handler_size_budget(), commit_diff_size_budget()) that aggregates a predefined set of account size classes relevant to that instruction. Functions leverage the new total_size_budget() utility and accept delegated account size as a parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• feat: Add CommitDiff Instruction for Efficient Account Updates #110: Adds the commit_diff instruction builder; this PR extends that same module with a commit_diff_size_budget helper function.

Suggested reviewers

• GabrielePicco

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately and concisely describes the main change: introducing AccountSizeClass and computing account-data sizes for 8 instructions, directly matching the changeset content.
Description check	✅ Passed	The PR description is thorough and covers the problem, solution, and context well, but deviates from the template structure with missing Status/Type table, Before & After, Deploy Notes sections.
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/size-class

---

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.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI Agents

In @src/instruction_builder/commit_state_from_buffer.rs:
- Around line 54-73: The doc-comment for commit_state_from_buffer_size_budget
doesn't state that it conservatively assumes commit_state_buffer uses the same
AccountSizeClass as the delegated_account (caller-provided), so update the
function's documentation to explicitly note that commit_state_buffer is treated
as delegated_account for budgeting purposes and that this is a
conservative/over-budgeting assumption; reference
commit_state_from_buffer_size_budget, commit_state_buffer, and delegated_account
in the comment so callers understand the assumption and rationale.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d58eb95 and b96fb9d.

📒 Files selected for processing (10)

• src/account_size_class.rs
• src/instruction_builder/call_handler.rs
• src/instruction_builder/commit_diff.rs
• src/instruction_builder/commit_diff_from_buffer.rs
• src/instruction_builder/commit_state.rs
• src/instruction_builder/commit_state_from_buffer.rs
• src/instruction_builder/delegate.rs
• src/instruction_builder/finalize.rs
• src/instruction_builder/undelegate.rs
• src/lib.rs

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-10-16T10:04:26.576Z

Learnt from: taco-paco
Repo: magicblock-labs/delegation-program PR: 105
File: src/args/call_handler.rs:6-8
Timestamp: 2025-10-16T10:04:26.576Z
Learning: In `src/args/call_handler.rs`, the `data` field of the `CallHandlerArgs` struct has no maximum length constraint by design, as it needs to support flexible instruction formats including custom discriminators and arguments in any format.

Applied to files:

• src/instruction_builder/call_handler.rs

📚 Learning: 2025-10-16T10:04:25.110Z

Learnt from: taco-paco
Repo: magicblock-labs/delegation-program PR: 105
File: src/args/call_handler.rs:6-8
Timestamp: 2025-10-16T10:04:25.110Z
Learning: The `data` field in the `CallHandlerArgs` struct (src/args/call_handler.rs) is intentionally designed to accept any length without max length validation, allowing flexible instruction formats including custom discriminators and arguments.

Applied to files:

• src/instruction_builder/call_handler.rs

🧬 Code graph analysis (8)

src/instruction_builder/commit_state_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (73-75)

src/instruction_builder/commit_diff.rs (1)

src/account_size_class.rs (1)

• total_size_budget (73-75)

src/instruction_builder/delegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (73-75)

src/instruction_builder/undelegate.rs (1)

src/account_size_class.rs (1)

• total_size_budget (73-75)

src/instruction_builder/finalize.rs (1)

src/account_size_class.rs (1)

• total_size_budget (73-75)

src/instruction_builder/commit_diff_from_buffer.rs (1)

src/account_size_class.rs (1)

• total_size_budget (73-75)

src/instruction_builder/call_handler.rs (1)

src/account_size_class.rs (1)

• total_size_budget (73-75)

src/instruction_builder/commit_state.rs (1)

src/account_size_class.rs (1)

• total_size_budget (73-75)

⏰ 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 (11)

src/lib.rs (1)

34-36: LGTM!

The new module declaration and glob re-export follow the established pattern used elsewhere in this file (e.g., pub use diff::*). This cleanly exposes the AccountSizeClass API at the crate root.

src/instruction_builder/undelegate.rs (1)

51-72: LGTM!

The budget function correctly accounts for all 12 instruction accounts plus the DLP program data. The size class assignments are appropriate:

• delegated_account class used for the account itself and its derived PDAs (undelegate_buffer_pda, commit_state_pda) that scale with account size
• Tiny for fixed-size metadata PDAs and system accounts

src/instruction_builder/delegate.rs (1)

46-62: LGTM!

The budget function correctly maps all 7 instruction accounts plus the DLP program data. Using delegated_account class for both the account and delegate_buffer_pda is appropriate since the buffer needs to accommodate the full account data.

src/instruction_builder/finalize.rs (1)

38-55: LGTM!

The budget function correctly accounts for all 8 instruction accounts plus DLP program data. Using delegated_account class for commit_state_pda is a conservative and appropriate choice since committed state can scale with the delegated account size.

src/instruction_builder/commit_diff.rs (1)

48-66: LGTM!

The budget function correctly maps all 9 instruction accounts plus DLP program data. The size class assignments align with the account semantics, and using delegated_account class for commit_state_pda is appropriately conservative.

src/instruction_builder/commit_diff_from_buffer.rs (1)

13-13: LGTM: Imports are correct.

The imported types and functions are appropriately used by the new commit_diff_from_buffer_size_budget function.

src/instruction_builder/commit_state.rs (1)

13-13: LGTM: Imports are correct.

The imported types and functions are appropriately used by the new commit_size_budget function.

src/account_size_class.rs (4)

1-2: LGTM: Constants are correctly defined.

The KB and MB constants are properly defined and improve code readability throughout the module.

---

4-16: LGTM: Performance tip is well-documented.

The comment block clearly explains the optimization (lines 13-14 specifically address why subtraction is needed), and the DLP_PROGRAM_DATA_SIZE_CLASS constant is appropriately defined as a dynamic size class.

---

18-57: LGTM: Enum is well-structured and documented.

The AccountSizeClass enum provides a clear hierarchy of size categories with appropriate derives (PartialEq, Eq were added per past review). Documentation for each variant is adequate, and the decision to defer examples for ExtraLarge, Huge, and Dynamic until concrete use cases emerge is reasonable.

---

59-71: LGTM: Implementation is correct and efficient.

The size_budget method correctly maps each variant to its documented size, and the use of const fn enables compile-time evaluation, which is beneficial for performance.