feat: use SetLoadedAccountsDataSizeLimit for tx containing DLP ix #800
Conversation
Manual Deploy AvailableYou can trigger a manual deploy of this PR branch to testnet: Alternative: Comment
Comment updated automatically when the PR is synchronized. |
📝 WalkthroughWalkthroughAdds per-task account-size budgeting by introducing Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (8)
🧰 Additional context used📓 Path-based instructions (2){magicblock-*,programs,storage-proto}/**⚙️ CodeRabbit configuration file
Files:
{test-*,tools}/**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-11-24T14:21:00.996ZApplied to files:
📚 Learning: 2025-10-26T16:54:39.084ZApplied to files:
📚 Learning: 2025-11-20T08:57:07.217ZApplied to files:
📚 Learning: 2025-11-21T11:03:26.756ZApplied to files:
📚 Learning: 2025-11-07T18:19:48.996ZApplied to files:
📚 Learning: 2025-10-26T08:49:31.543ZApplied to files:
📚 Learning: 2025-11-07T13:20:13.793ZApplied to files:
📚 Learning: 2025-12-03T09:36:01.527ZApplied to files:
📚 Learning: 2025-10-21T14:00:54.642ZApplied to files:
📚 Learning: 2025-12-03T09:33:48.707ZApplied to files:
📚 Learning: 2025-10-14T09:56:14.047ZApplied to files:
🧬 Code graph analysis (5)magicblock-committor-service/src/tasks/buffer_task.rs (3)
magicblock-committor-service/src/tasks/mod.rs (2)
magicblock-committor-service/src/tasks/utils.rs (2)
magicblock-committor-service/src/tasks/args_task.rs (2)
magicblock-committor-service/src/tasks/task_strategist.rs (1)
🔇 Additional comments (8)
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: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktest-integration/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.tomlmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/utils.rs
🧰 Additional context used
📓 Path-based instructions (1)
{magicblock-*,programs,storage-proto}/**
⚙️ CodeRabbit configuration file
{magicblock-*,programs,storage-proto}/**: Treat any usage of.unwrap()or.expect()in production Rust code as a MAJOR issue.
These should not be categorized as trivial or nit-level concerns.
Request proper error handling or explicit justification with invariants.
Files:
magicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/utils.rsmagicblock-committor-service/src/tasks/task_strategist.rs
🧠 Learnings (7)
📓 Common learnings
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 585
File: magicblock-committor-service/src/tasks/buffer_task.rs:111-115
Timestamp: 2025-10-26T08:49:31.543Z
Learning: In the magicblock-committor-service, compute units returned by the `compute_units()` method in task implementations (such as `BufferTask`, `ArgsTask`, etc.) represent the compute budget for a single task. Transactions can comprise multiple tasks, and the total compute budget for a transaction is computed as the sum of the compute units of all tasks included in that transaction.
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
📚 Learning: 2025-10-26T08:49:31.543Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 585
File: magicblock-committor-service/src/tasks/buffer_task.rs:111-115
Timestamp: 2025-10-26T08:49:31.543Z
Learning: In the magicblock-committor-service, compute units returned by the `compute_units()` method in task implementations (such as `BufferTask`, `ArgsTask`, etc.) represent the compute budget for a single task. Transactions can comprise multiple tasks, and the total compute budget for a transaction is computed as the sum of the compute units of all tasks included in that transaction.
Applied to files:
magicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/utils.rsmagicblock-committor-service/src/tasks/task_strategist.rs
📚 Learning: 2025-12-03T09:36:01.527Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/src/remote_account_provider/mod.rs:1350-1353
Timestamp: 2025-12-03T09:36:01.527Z
Learning: Repo: magicblock-labs/magicblock-validator
File: magicblock-chainlink/src/remote_account_provider/mod.rs
Context: consolidate_fetched_remote_accounts
Learning: For unexpected result counts (>2), the project prefers logging an error and returning an empty Vec over panicking; acceptable during development per maintainer (Dodecahedr0x).
Applied to files:
magicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/utils.rs
📚 Learning: 2025-11-07T13:20:13.793Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
Applied to files:
magicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/utils.rs
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
magicblock-committor-service/src/tasks/mod.rs
📚 Learning: 2025-11-24T14:21:00.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: Cargo.toml:58-58
Timestamp: 2025-11-24T14:21:00.996Z
Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.
Applied to files:
Cargo.toml
📚 Learning: 2025-10-26T16:54:39.084Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: test-manual/Cargo.toml:0-0
Timestamp: 2025-10-26T16:54:39.084Z
Learning: In the magicblock-validator repository, use git branch references (not commit hashes or tags) for the helius-laserstream dependency to allow automatic updates when the branch is pushed to.
Applied to files:
Cargo.toml
🧬 Code graph analysis (5)
magicblock-committor-service/src/tasks/mod.rs (2)
magicblock-committor-service/src/tasks/args_task.rs (1)
accounts_size_budget(185-201)magicblock-committor-service/src/tasks/buffer_task.rs (1)
accounts_size_budget(134-140)
magicblock-committor-service/src/tasks/args_task.rs (2)
magicblock-committor-service/src/tasks/buffer_task.rs (1)
accounts_size_budget(134-140)magicblock-committor-service/src/tasks/mod.rs (1)
accounts_size_budget(88-88)
magicblock-committor-service/src/tasks/buffer_task.rs (2)
magicblock-committor-service/src/tasks/args_task.rs (1)
accounts_size_budget(185-201)magicblock-committor-service/src/tasks/mod.rs (1)
accounts_size_budget(88-88)
magicblock-committor-service/src/tasks/utils.rs (4)
magicblock-committor-service/src/tasks/args_task.rs (2)
compute_units(175-183)accounts_size_budget(185-201)magicblock-committor-service/src/tasks/buffer_task.rs (2)
compute_units(128-132)accounts_size_budget(134-140)magicblock-committor-service/src/tasks/mod.rs (3)
compute_units(86-86)compute_units(270-272)accounts_size_budget(88-88)magicblock-committor-service/src/compute_budget.rs (1)
compute_unit_price(190-199)
magicblock-committor-service/src/tasks/task_strategist.rs (1)
magicblock-committor-service/src/tasks/utils.rs (2)
tasks_accounts_size_budget(131-138)budget_instructions(140-154)
🔇 Additional comments (8)
magicblock-committor-service/src/tasks/buffer_task.rs (2)
134-140: LGTM!The implementation of
accounts_size_budgetforBufferTaskis correct and follows a consistent pattern. UsingAccountSizeClass::Hugefor the Commit variant is appropriate for large account operations.
1-4: Adddlpas a dependency toCargo.toml.The imports from the
dlpcrate cannot be resolved becausedlpis not listed as a dependency inmagicblock-committor-service/Cargo.toml. This will cause a compilation error. Thedlpcrate must be added as a workspace or path dependency to enable the imports ofCommitStateFromBufferArgs,commit_size_budget, andAccountSizeClass.⛔ Skipped due to learnings
Learnt from: Dodecahedr0x Repo: magicblock-labs/magicblock-validator PR: 639 File: Cargo.toml:58-58 Timestamp: 2025-11-24T14:21:00.996Z Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.magicblock-committor-service/src/tasks/args_task.rs (1)
185-201: LGTM!The implementation correctly handles all
ArgsTaskTypevariants:
- Commit, CommitDiff, Undelegate, and Finalize appropriately use the corresponding size budget functions with
AccountSizeClass::Huge- BaseAction returns 0, which is correct since base actions don't require account data size budgets
The implementation is consistent with the
BufferTaskpattern.magicblock-committor-service/src/tasks/task_strategist.rs (2)
227-232: LGTM!The update correctly computes the accounts size budget using
tasks_accounts_size_budgetand passes it as the third parameter tobudget_instructions. This aligns with the new three-parameter signature and maintains the existing logic flow.
265-270: LGTM!The update correctly mirrors the changes in
attempt_lookup_tables, computing the size budgets and passing them tobudget_instructions. The implementation is consistent and correct.magicblock-committor-service/src/tasks/utils.rs (3)
131-138: LGTM: Pattern mirrorstasks_compute_unitscorrectly.The implementation correctly aggregates
accounts_size_budget()across all tasks using the same pattern astasks_compute_units. The approach is consistent with existing learnings that confirm tasks return budgets for individual tasks, and transaction-level budgets are computed as sums.Based on learnings: compute units (and similarly, accounts size budgets) represent the budget for a single task, with the total computed as the sum across all tasks in a transaction.
69-69: LGTM: Correctly integrates accounts size budget into transaction assembly.The call to
Self::tasks_accounts_size_budget(tasks)properly computes the aggregated budget and passes it tobudget_instructions. The integration is consistent with the existing pattern for compute units.
140-154: Verify behavior whenaccounts_size_budgetis zero.The new instruction
ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(accounts_size_budget)is always included in the budget instructions, even when the budget might be zero. From the relevant code snippets,ArgsTaskType::BaseActionreturns0foraccounts_size_budget(), meaning a transaction composed entirely ofBaseActiontasks would pass0to this instruction.Verify whether:
- Passing
0is valid forset_loaded_accounts_data_size_limit- A
0value means "no limit", "default limit", or causes an error- Whether this instruction should be conditionally included only when
accounts_size_budget > 0#!/bin/bash # Description: Check Solana SDK documentation and usage patterns for set_loaded_accounts_data_size_limit # to understand behavior when passed 0 or if there are minimum value requirements. # Search for set_loaded_accounts_data_size_limit implementation and usage rg -n -A 10 -B 2 'set_loaded_accounts_data_size_limit' --type rust # Look for any validation or special handling of 0 values rg -n -A 5 'set_loaded_accounts_data_size_limit.*\b0\b' --type rust # Search for documentation or comments about this instruction rg -n -B 5 -A 10 'SetLoadedAccountsDataSizeLimit' --type rust
snawaz
force-pushed
the
snawaz/size-budget
branch
from
8fc14ec to
557f4f1
Compare
snawaz
force-pushed
the
snawaz/size-budget
branch
2 times, most recently
from
ddd40d3 to
c44bdbb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
magicblock-committor-service/src/tasks/mod.rs (1)
88-89: Documentation could be more comprehensive.While documentation has been added, it could still be enhanced per the previous review suggestion to include:
- That this represents a per-task budget (analogous to
compute_units())- The unit is bytes
- The relationship between compute budget and accounts data size budget
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktest-integration/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.tomlmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/utils.rstest-integration/test-committor-service/tests/test_ix_commit_local.rs
🧰 Additional context used
📓 Path-based instructions (2)
{magicblock-*,programs,storage-proto}/**
⚙️ CodeRabbit configuration file
{magicblock-*,programs,storage-proto}/**: Treat any usage of.unwrap()or.expect()in production Rust code as a MAJOR issue.
These should not be categorized as trivial or nit-level concerns.
Request proper error handling or explicit justification with invariants.
Files:
magicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/utils.rsmagicblock-committor-service/src/tasks/args_task.rs
{test-*,tools}/**
⚙️ CodeRabbit configuration file
Usage of
.unwrap()or.expect()in test code is acceptable and may be treated as trivial.
Files:
test-integration/test-committor-service/tests/test_ix_commit_local.rs
🧠 Learnings (10)
📓 Common learnings
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 585
File: magicblock-committor-service/src/tasks/buffer_task.rs:111-115
Timestamp: 2025-10-26T08:49:31.543Z
Learning: In the magicblock-committor-service, compute units returned by the `compute_units()` method in task implementations (such as `BufferTask`, `ArgsTask`, etc.) represent the compute budget for a single task. Transactions can comprise multiple tasks, and the total compute budget for a transaction is computed as the sum of the compute units of all tasks included in that transaction.
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
📚 Learning: 2025-10-26T08:49:31.543Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 585
File: magicblock-committor-service/src/tasks/buffer_task.rs:111-115
Timestamp: 2025-10-26T08:49:31.543Z
Learning: In the magicblock-committor-service, compute units returned by the `compute_units()` method in task implementations (such as `BufferTask`, `ArgsTask`, etc.) represent the compute budget for a single task. Transactions can comprise multiple tasks, and the total compute budget for a transaction is computed as the sum of the compute units of all tasks included in that transaction.
Applied to files:
magicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/utils.rsmagicblock-committor-service/src/tasks/args_task.rs
📚 Learning: 2025-12-03T09:36:01.527Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/src/remote_account_provider/mod.rs:1350-1353
Timestamp: 2025-12-03T09:36:01.527Z
Learning: Repo: magicblock-labs/magicblock-validator
File: magicblock-chainlink/src/remote_account_provider/mod.rs
Context: consolidate_fetched_remote_accounts
Learning: For unexpected result counts (>2), the project prefers logging an error and returning an empty Vec over panicking; acceptable during development per maintainer (Dodecahedr0x).
Applied to files:
magicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/utils.rsmagicblock-committor-service/src/tasks/args_task.rs
📚 Learning: 2025-11-07T13:20:13.793Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
Applied to files:
magicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/buffer_task.rs
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
magicblock-committor-service/src/tasks/buffer_task.rs
📚 Learning: 2025-11-24T14:21:00.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: Cargo.toml:58-58
Timestamp: 2025-11-24T14:21:00.996Z
Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.
Applied to files:
Cargo.toml
📚 Learning: 2025-10-26T16:54:39.084Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: test-manual/Cargo.toml:0-0
Timestamp: 2025-10-26T16:54:39.084Z
Learning: In the magicblock-validator repository, use git branch references (not commit hashes or tags) for the helius-laserstream dependency to allow automatic updates when the branch is pushed to.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-20T08:57:07.217Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 650
File: magicblock-chainlink/src/submux/subscription_task.rs:13-99
Timestamp: 2025-11-20T08:57:07.217Z
Learning: In the magicblock-validator repository, avoid posting review comments that merely confirm code is correct or matches intended behavior without providing actionable feedback, suggestions for improvement, or identifying potential issues. Such confirmatory comments are considered unhelpful noise by the maintainers.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-21T11:03:26.756Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 664
File: magicblock-chainlink/src/testing/mod.rs:342-370
Timestamp: 2025-11-21T11:03:26.756Z
Learning: In the magicblock-validator codebase, avoid leaving review comments that merely acknowledge code is correct or well-structured when there is no actionable suggestion, improvement, or issue to flag. Only comment when there is something specific to recommend, fix, or clarify.
Applied to files:
Cargo.toml
📚 Learning: 2025-12-03T09:33:48.707Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: test-integration/test-committor-service/tests/test_ix_commit_local.rs:867-881
Timestamp: 2025-12-03T09:33:48.707Z
Learning: Repo: magicblock-labs/magicblock-validator PR: 639
Context: test-integration/test-committor-service/tests/test_ix_commit_local.rs (ix_commit_local)
Learning: The PhotonIndexer used for compressed account fetches (get_compressed_account) has built‑in retry logic (defaults to ~10 attempts), so tests should not add separate retry loops around compressed fetches unless there’s a specific need.
Applied to files:
test-integration/test-committor-service/tests/test_ix_commit_local.rs
🧬 Code graph analysis (5)
magicblock-committor-service/src/tasks/task_strategist.rs (1)
magicblock-committor-service/src/tasks/utils.rs (2)
tasks_accounts_size_budget(131-138)budget_instructions(140-154)
magicblock-committor-service/src/tasks/mod.rs (2)
magicblock-committor-service/src/tasks/args_task.rs (1)
accounts_size_budget(185-201)magicblock-committor-service/src/tasks/buffer_task.rs (1)
accounts_size_budget(134-140)
magicblock-committor-service/src/tasks/buffer_task.rs (2)
magicblock-committor-service/src/tasks/args_task.rs (1)
accounts_size_budget(185-201)magicblock-committor-service/src/tasks/mod.rs (1)
accounts_size_budget(89-89)
magicblock-committor-service/src/tasks/utils.rs (3)
magicblock-committor-service/src/tasks/args_task.rs (2)
compute_units(175-183)accounts_size_budget(185-201)magicblock-committor-service/src/tasks/buffer_task.rs (2)
compute_units(128-132)accounts_size_budget(134-140)magicblock-committor-service/src/tasks/mod.rs (3)
compute_units(86-86)compute_units(271-273)accounts_size_budget(89-89)
magicblock-committor-service/src/tasks/args_task.rs (2)
magicblock-committor-service/src/tasks/buffer_task.rs (1)
accounts_size_budget(134-140)magicblock-committor-service/src/tasks/mod.rs (1)
accounts_size_budget(89-89)
🔇 Additional comments (10)
test-integration/test-committor-service/tests/test_ix_commit_local.rs (2)
104-114: LGTM—test boundary adjustments reflect new budget instruction.The test value adjustments (679→671, 680→673) and the accompanying comment accurately document the transaction size boundaries after adding the third budget instruction for
SetLoadedAccountsDataSizeLimit. The strategy transition from Args to FromBuffer at the new boundary is correct.
111-111: Fix typo in comment.Line 111 contains a typo: "execeds" should be "exceeds".
🔎 Proposed fix
- // of size 1644 (which is the max limit), but while the size of raw bytes for 671 is within - // 1232 limit, the size for 672 execeds by 1 (1233). That is why we used + // of size 1644 (which is the max limit), but while the size of raw bytes for 671 is within + // 1232 limit, the size for 672 exceeds by 1 (1233). That is why we usedLikely an incorrect or invalid review comment.
magicblock-committor-service/src/tasks/task_strategist.rs (2)
227-232: LGTM—accounts size budget computation and propagation.The addition of
size_budgetscomputation viatasks_accounts_size_budget()and its propagation tobudget_instructions()correctly extends the budgeting pipeline to include account data size limits. The implementation follows the established pattern for compute budget aggregation.Based on learnings, this budget represents the sum of per-task account data size budgets, analogous to how compute units are aggregated.
265-270: LGTM—consistent implementation across call sites.The size budget computation and propagation pattern is correctly duplicated in this second call site (
collect_lookup_table_keys), ensuring consistency in how budget instructions are constructed throughout the codebase.magicblock-committor-service/src/tasks/buffer_task.rs (2)
1-4: LGTM—necessary imports added.The imports correctly bring in
commit_size_budgetandAccountSizeClassfrom thedlpcrate, which are required for the newaccounts_size_budget()implementation.
134-140: LGTM—accounts size budget implementation for BufferTask.The implementation of
accounts_size_budget()correctly returnscommit_size_budget(AccountSizeClass::Huge)for Commit tasks, consistent with the pattern used inArgsTask. SinceBufferTaskTypecurrently only has theCommitvariant, the match statement appropriately covers all cases.Based on learnings, this budget represents the account data size limit for a single task, mirroring how
compute_units()represents the compute budget per task.magicblock-committor-service/src/tasks/utils.rs (3)
131-138: LGTM!The
tasks_accounts_size_budgethelper function follows the same clean pattern astasks_compute_units, properly aggregating per-task budgets. The implementation is consistent with the existing codebase patterns.
69-69: LGTM!The integration correctly computes the total accounts size budget using the new helper function and passes it to
budget_instructions. This properly enables account data size limit budgeting for task transactions.
140-154: Theset_loaded_accounts_data_size_limitAPI method is available and correctly implemented.The Solana ComputeBudgetInstruction API (v2.2) provides
set_loaded_accounts_data_size_limit(bytes: u32)as documented. The implementation correctly adds the third instruction and updates the function signature accordingly. This is a breaking change to the public API ofbudget_instructions.magicblock-committor-service/src/tasks/args_task.rs (1)
1-9: These budget functions do not appear to exist in themagicblock-delegation-programcrate.The instruction_builder module in the delegation-program repository exports builders for
delegate,undelegate,commit_state,finalize, andinit_validator_fees_vault— but does not exportcommit_size_budget,commit_diff_size_budget,finalize_size_budget, orundelegate_size_budget. The code in lines 185–201 actively calls these non-existent functions, which will cause compilation failures. Either these functions must be added to the delegation-program crate's instruction_builder module, or the imports and usages need to reference the correct functions that actually exist.⛔ Skipped due to learnings
Learnt from: bmuddha Repo: magicblock-labs/magicblock-validator PR: 578 File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27 Timestamp: 2025-10-21T14:00:54.642Z Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
snawaz
force-pushed
the
snawaz/size-budget
branch
from
c44bdbb to
120eca0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktest-integration/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.tomlmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/utils.rstest-integration/test-committor-service/tests/test_ix_commit_local.rs
🧰 Additional context used
📓 Path-based instructions (2)
{magicblock-*,programs,storage-proto}/**
⚙️ CodeRabbit configuration file
{magicblock-*,programs,storage-proto}/**: Treat any usage of.unwrap()or.expect()in production Rust code as a MAJOR issue.
These should not be categorized as trivial or nit-level concerns.
Request proper error handling or explicit justification with invariants.
Files:
magicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/utils.rs
{test-*,tools}/**
⚙️ CodeRabbit configuration file
Usage of
.unwrap()or.expect()in test code is acceptable and may be treated as trivial.
Files:
test-integration/test-committor-service/tests/test_ix_commit_local.rs
🧠 Learnings (12)
📓 Common learnings
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 585
File: magicblock-committor-service/src/tasks/buffer_task.rs:111-115
Timestamp: 2025-10-26T08:49:31.543Z
Learning: In the magicblock-committor-service, compute units returned by the `compute_units()` method in task implementations (such as `BufferTask`, `ArgsTask`, etc.) represent the compute budget for a single task. Transactions can comprise multiple tasks, and the total compute budget for a transaction is computed as the sum of the compute units of all tasks included in that transaction.
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
📚 Learning: 2025-10-26T08:49:31.543Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 585
File: magicblock-committor-service/src/tasks/buffer_task.rs:111-115
Timestamp: 2025-10-26T08:49:31.543Z
Learning: In the magicblock-committor-service, compute units returned by the `compute_units()` method in task implementations (such as `BufferTask`, `ArgsTask`, etc.) represent the compute budget for a single task. Transactions can comprise multiple tasks, and the total compute budget for a transaction is computed as the sum of the compute units of all tasks included in that transaction.
Applied to files:
magicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/utils.rs
📚 Learning: 2025-11-07T13:20:13.793Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
Applied to files:
magicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/mod.rs
📚 Learning: 2025-12-03T09:36:01.527Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/src/remote_account_provider/mod.rs:1350-1353
Timestamp: 2025-12-03T09:36:01.527Z
Learning: Repo: magicblock-labs/magicblock-validator
File: magicblock-chainlink/src/remote_account_provider/mod.rs
Context: consolidate_fetched_remote_accounts
Learning: For unexpected result counts (>2), the project prefers logging an error and returning an empty Vec over panicking; acceptable during development per maintainer (Dodecahedr0x).
Applied to files:
magicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/utils.rs
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
magicblock-committor-service/src/tasks/buffer_task.rs
📚 Learning: 2025-12-03T09:33:48.707Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: test-integration/test-committor-service/tests/test_ix_commit_local.rs:867-881
Timestamp: 2025-12-03T09:33:48.707Z
Learning: Repo: magicblock-labs/magicblock-validator PR: 639
Context: test-integration/test-committor-service/tests/test_ix_commit_local.rs (ix_commit_local)
Learning: The PhotonIndexer used for compressed account fetches (get_compressed_account) has built‑in retry logic (defaults to ~10 attempts), so tests should not add separate retry loops around compressed fetches unless there’s a specific need.
Applied to files:
test-integration/test-committor-service/tests/test_ix_commit_local.rs
📚 Learning: 2025-10-14T09:56:14.047Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 564
File: test-integration/programs/flexi-counter/src/processor/call_handler.rs:122-125
Timestamp: 2025-10-14T09:56:14.047Z
Learning: The file test-integration/programs/flexi-counter/src/processor/call_handler.rs contains a test smart contract used for integration testing, not production code.
Applied to files:
test-integration/test-committor-service/tests/test_ix_commit_local.rs
📚 Learning: 2025-11-24T14:21:00.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: Cargo.toml:58-58
Timestamp: 2025-11-24T14:21:00.996Z
Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.
Applied to files:
Cargo.toml
📚 Learning: 2025-10-26T16:54:39.084Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: test-manual/Cargo.toml:0-0
Timestamp: 2025-10-26T16:54:39.084Z
Learning: In the magicblock-validator repository, use git branch references (not commit hashes or tags) for the helius-laserstream dependency to allow automatic updates when the branch is pushed to.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-20T08:57:07.217Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 650
File: magicblock-chainlink/src/submux/subscription_task.rs:13-99
Timestamp: 2025-11-20T08:57:07.217Z
Learning: In the magicblock-validator repository, avoid posting review comments that merely confirm code is correct or matches intended behavior without providing actionable feedback, suggestions for improvement, or identifying potential issues. Such confirmatory comments are considered unhelpful noise by the maintainers.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-21T11:03:26.756Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 664
File: magicblock-chainlink/src/testing/mod.rs:342-370
Timestamp: 2025-11-21T11:03:26.756Z
Learning: In the magicblock-validator codebase, avoid leaving review comments that merely acknowledge code is correct or well-structured when there is no actionable suggestion, improvement, or issue to flag. Only comment when there is something specific to recommend, fix, or clarify.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-07T18:19:48.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 614
File: test-integration/programs/schedulecommit/Cargo.toml:19-20
Timestamp: 2025-11-07T18:19:48.996Z
Learning: In Solana programs (crates depending on solana-program), the empty feature flags `custom-heap = []` and `custom-panic = []` are standard declarations required to avoid compiler warnings. These should not be flagged as unused features in Cargo.toml files for Solana program crates.
Applied to files:
Cargo.toml
🧬 Code graph analysis (4)
magicblock-committor-service/src/tasks/buffer_task.rs (2)
magicblock-committor-service/src/tasks/args_task.rs (1)
accounts_size_budget(185-205)magicblock-committor-service/src/tasks/mod.rs (1)
accounts_size_budget(89-89)
magicblock-committor-service/src/tasks/task_strategist.rs (1)
magicblock-committor-service/src/tasks/utils.rs (2)
tasks_accounts_size_budget(131-138)budget_instructions(140-154)
magicblock-committor-service/src/tasks/mod.rs (2)
magicblock-committor-service/src/tasks/args_task.rs (1)
accounts_size_budget(185-205)magicblock-committor-service/src/tasks/buffer_task.rs (1)
accounts_size_budget(134-140)
magicblock-committor-service/src/tasks/args_task.rs (2)
magicblock-committor-service/src/tasks/buffer_task.rs (1)
accounts_size_budget(134-140)magicblock-committor-service/src/tasks/mod.rs (1)
accounts_size_budget(89-89)
🔇 Additional comments (7)
magicblock-committor-service/src/tasks/task_strategist.rs (1)
227-232: LGTM! Consistent integration of account size budgets.The changes correctly compute the total accounts size budget by summing individual task budgets (line 227) and pass it to
budget_instructions(lines 228-232). This follows the established pattern for compute unit budgeting and aligns with the learning that transactions comprise multiple tasks with summed budgets.The same pattern is correctly applied in
collect_lookup_table_keys(lines 265-270).magicblock-committor-service/src/tasks/buffer_task.rs (1)
134-140: Verify the size class choice forBufferTaskis intentional.
BufferTask::CommitusesAccountSizeClass::Huge, whileArgsTask::CommitusesAccountSizeClass::Dynamicbased on actual account data length. More importantly,ArgsTask::Commitoptimizes directly toBufferTask::Commit(see args_task.rs lines 130-137), meaning the same task data transitions from a precise Dynamic size budget to a conservative Huge size budget.This inconsistency could affect transaction sizing and batching differently than expected. Confirm whether this behavior is intentional or if
BufferTaskshould useAccountSizeClass::Dynamic(data.len() as u32)to maintain consistency withArgsTask.magicblock-committor-service/src/tasks/utils.rs (3)
131-138: LGTM! Consistent implementation pattern.The new helper function correctly mirrors the pattern of
tasks_compute_units, summing account size budgets across all tasks. The implementation is clean and consistent with the established codebase patterns.
66-70: LGTM! Call site correctly updated.The call to
budget_instructionshas been properly updated to pass the accounts size budget using the newtasks_accounts_size_budgethelper function. The three arguments now align with the updated function signature.
140-154: All call sites have been updated correctly for the breaking API change.The function signature change (adding
accounts_size_budgetparameter and expanding the return array from 2 to 3 instructions) has been properly propagated across the codebase. All three call sites now pass the required three arguments:
magicblock-committor-service/src/tasks/utils.rs:66magicblock-committor-service/src/tasks/task_strategist.rs:228magicblock-committor-service/src/tasks/task_strategist.rs:266Return value handling is correct at all sites—the
[Instruction; 3]array is properly used as slice references (&budget_instructions) where expected. No error handling issues detected in the file.test-integration/test-committor-service/tests/test_ix_commit_local.rs (2)
104-106: LGTM! Test boundary adjustment aligned with new budgeting.The test has been appropriately adjusted from 679 to 671 bytes to reflect the transaction size limits under the new account size budgeting implementation. This ensures the test correctly validates the Args strategy boundary.
109-115: LGTM! Well-documented boundary test.This test correctly validates the transition from Args to FromBuffer commit strategy. The detailed comment (lines 110-113) effectively explains the boundary condition nuance and why 673 bytes was chosen instead of 672. This documentation will help future maintainers understand the precise boundary behavior.
snawaz
force-pushed
the
snawaz/size-budget
branch
from
120eca0 to
0ee286d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
Cargo.toml (1)
102-102: Consider removing the commented-out local path dependency.The commented line serves as a developer convenience but may cause confusion in the codebase. Since the git dependency on line 101 is the canonical approach, removing the commented alternative keeps the manifest clean.
magicblock-committor-service/src/tasks/mod.rs (1)
88-90: Add documentation for the new trait method.The
accounts_size_budgetmethod lacks documentation. Add a doc comment explaining:
- The purpose of this method (per-task account data size limit)
- What the returned
u32value represents (bytes of account data size budget)- How it relates to
compute_units()(both are per-task budgets)Based on learnings, this budget represents the account data size limit for a single task, similar to how
compute_units()represents the compute budget for a single task.🔎 Suggested documentation
+ /// Returns the accounts data size budget required for this task in bytes. + /// This budget limits the total size of account data that can be loaded + /// for this task's instruction execution. Similar to `compute_units()`, + /// this represents the budget for a single task. fn accounts_size_budget(&self) -> u32;magicblock-committor-service/src/tasks/args_task.rs (1)
197-197:BaseActionshould compute a conservative account size budget or document why zero-sizing is safe.
BaseActiontasks includeaccount_metas_per_program(accounts that will be loaded), yetaccounts_size_budget()returns0, meaning noset_loaded_accounts_data_size_limit()is enforced for these instructions.Since only
ShortAccountMeta(metadata only) is available at transaction-building time, actual account data cannot be inspected to compute size likeCommitandCommitDiffdo. However, similar tasks (Undelegate,Finalize) conservatively useAccountSizeClass::Huge. Either adopt the same conservative approach forBaseAction, or explicitly document why zero-sizing is safe.🔎 Proposed fix using conservative sizing
- ArgsTaskType::BaseAction(_) => 0, + ArgsTaskType::BaseAction(_) => { + // BaseAction loads accounts from account_metas_per_program, + // but actual data size is unknown at build time. + // Use conservative Huge budget to avoid runtime failures. + dlp::instruction_builder::base_action_size_budget(AccountSizeClass::Huge) + }Note: Verify that
base_action_size_budgetexists in the dlp crate, or use an appropriate alternative size budget function.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktest-integration/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.tomlmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/mod.rsmagicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/utils.rstest-integration/test-committor-service/tests/test_ix_commit_local.rs
🧰 Additional context used
📓 Path-based instructions (2)
{magicblock-*,programs,storage-proto}/**
⚙️ CodeRabbit configuration file
{magicblock-*,programs,storage-proto}/**: Treat any usage of.unwrap()or.expect()in production Rust code as a MAJOR issue.
These should not be categorized as trivial or nit-level concerns.
Request proper error handling or explicit justification with invariants.
Files:
magicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/utils.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/mod.rs
{test-*,tools}/**
⚙️ CodeRabbit configuration file
Usage of
.unwrap()or.expect()in test code is acceptable and may be treated as trivial.
Files:
test-integration/test-committor-service/tests/test_ix_commit_local.rs
🧠 Learnings (12)
📓 Common learnings
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 585
File: magicblock-committor-service/src/tasks/buffer_task.rs:111-115
Timestamp: 2025-10-26T08:49:31.543Z
Learning: In the magicblock-committor-service, compute units returned by the `compute_units()` method in task implementations (such as `BufferTask`, `ArgsTask`, etc.) represent the compute budget for a single task. Transactions can comprise multiple tasks, and the total compute budget for a transaction is computed as the sum of the compute units of all tasks included in that transaction.
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
📚 Learning: 2025-10-26T08:49:31.543Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 585
File: magicblock-committor-service/src/tasks/buffer_task.rs:111-115
Timestamp: 2025-10-26T08:49:31.543Z
Learning: In the magicblock-committor-service, compute units returned by the `compute_units()` method in task implementations (such as `BufferTask`, `ArgsTask`, etc.) represent the compute budget for a single task. Transactions can comprise multiple tasks, and the total compute budget for a transaction is computed as the sum of the compute units of all tasks included in that transaction.
Applied to files:
magicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/task_strategist.rsmagicblock-committor-service/src/tasks/utils.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/mod.rs
📚 Learning: 2025-11-07T13:20:13.793Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
Applied to files:
magicblock-committor-service/src/tasks/buffer_task.rsmagicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/mod.rs
📚 Learning: 2025-11-24T14:21:00.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: Cargo.toml:58-58
Timestamp: 2025-11-24T14:21:00.996Z
Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.
Applied to files:
Cargo.toml
📚 Learning: 2025-10-26T16:54:39.084Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: test-manual/Cargo.toml:0-0
Timestamp: 2025-10-26T16:54:39.084Z
Learning: In the magicblock-validator repository, use git branch references (not commit hashes or tags) for the helius-laserstream dependency to allow automatic updates when the branch is pushed to.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-20T08:57:07.217Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 650
File: magicblock-chainlink/src/submux/subscription_task.rs:13-99
Timestamp: 2025-11-20T08:57:07.217Z
Learning: In the magicblock-validator repository, avoid posting review comments that merely confirm code is correct or matches intended behavior without providing actionable feedback, suggestions for improvement, or identifying potential issues. Such confirmatory comments are considered unhelpful noise by the maintainers.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-21T11:03:26.756Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 664
File: magicblock-chainlink/src/testing/mod.rs:342-370
Timestamp: 2025-11-21T11:03:26.756Z
Learning: In the magicblock-validator codebase, avoid leaving review comments that merely acknowledge code is correct or well-structured when there is no actionable suggestion, improvement, or issue to flag. Only comment when there is something specific to recommend, fix, or clarify.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-07T18:19:48.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 614
File: test-integration/programs/schedulecommit/Cargo.toml:19-20
Timestamp: 2025-11-07T18:19:48.996Z
Learning: In Solana programs (crates depending on solana-program), the empty feature flags `custom-heap = []` and `custom-panic = []` are standard declarations required to avoid compiler warnings. These should not be flagged as unused features in Cargo.toml files for Solana program crates.
Applied to files:
Cargo.toml
📚 Learning: 2025-12-03T09:36:01.527Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/src/remote_account_provider/mod.rs:1350-1353
Timestamp: 2025-12-03T09:36:01.527Z
Learning: Repo: magicblock-labs/magicblock-validator
File: magicblock-chainlink/src/remote_account_provider/mod.rs
Context: consolidate_fetched_remote_accounts
Learning: For unexpected result counts (>2), the project prefers logging an error and returning an empty Vec over panicking; acceptable during development per maintainer (Dodecahedr0x).
Applied to files:
magicblock-committor-service/src/tasks/args_task.rsmagicblock-committor-service/src/tasks/mod.rs
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
magicblock-committor-service/src/tasks/args_task.rs
📚 Learning: 2025-12-03T09:33:48.707Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: test-integration/test-committor-service/tests/test_ix_commit_local.rs:867-881
Timestamp: 2025-12-03T09:33:48.707Z
Learning: Repo: magicblock-labs/magicblock-validator PR: 639
Context: test-integration/test-committor-service/tests/test_ix_commit_local.rs (ix_commit_local)
Learning: The PhotonIndexer used for compressed account fetches (get_compressed_account) has built‑in retry logic (defaults to ~10 attempts), so tests should not add separate retry loops around compressed fetches unless there’s a specific need.
Applied to files:
test-integration/test-committor-service/tests/test_ix_commit_local.rs
📚 Learning: 2025-10-14T09:56:14.047Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 564
File: test-integration/programs/flexi-counter/src/processor/call_handler.rs:122-125
Timestamp: 2025-10-14T09:56:14.047Z
Learning: The file test-integration/programs/flexi-counter/src/processor/call_handler.rs contains a test smart contract used for integration testing, not production code.
Applied to files:
test-integration/test-committor-service/tests/test_ix_commit_local.rs
🧬 Code graph analysis (5)
magicblock-committor-service/src/tasks/buffer_task.rs (3)
magicblock-committor-service/src/persist/types/commit_strategy.rs (1)
args(17-23)magicblock-committor-service/src/tasks/args_task.rs (1)
accounts_size_budget(185-205)magicblock-committor-service/src/tasks/mod.rs (1)
accounts_size_budget(89-89)
magicblock-committor-service/src/tasks/task_strategist.rs (3)
magicblock-committor-service/src/compute_budget.rs (1)
default(15-21)magicblock-table-mania/src/compute_budget.rs (1)
default(90-97)programs/magicblock/src/schedule_transactions/transaction_scheduler.rs (1)
default(25-36)
magicblock-committor-service/src/tasks/utils.rs (3)
magicblock-committor-service/src/tasks/args_task.rs (2)
compute_units(175-183)accounts_size_budget(185-205)magicblock-committor-service/src/tasks/buffer_task.rs (2)
compute_units(128-132)accounts_size_budget(134-140)magicblock-committor-service/src/tasks/mod.rs (3)
compute_units(86-86)compute_units(271-273)accounts_size_budget(89-89)
magicblock-committor-service/src/tasks/args_task.rs (2)
magicblock-committor-service/src/tasks/buffer_task.rs (1)
accounts_size_budget(134-140)magicblock-committor-service/src/tasks/mod.rs (1)
accounts_size_budget(89-89)
magicblock-committor-service/src/tasks/mod.rs (2)
magicblock-committor-service/src/tasks/args_task.rs (1)
accounts_size_budget(185-205)magicblock-committor-service/src/tasks/buffer_task.rs (1)
accounts_size_budget(134-140)
🔇 Additional comments (4)
test-integration/test-committor-service/tests/test_ix_commit_local.rs (2)
104-105: LGTM!The test name and parameter adjustment accurately reflect the recalibrated size boundary for the Args strategy after introducing the accounts_size_budget feature.
109-114: LGTM! Helpful boundary explanation.The updated test correctly reflects the strategy transition from Args to FromBuffer at the 671/673 boundary. The comment clearly explains why 672 cannot be used (exceeds the 1232-byte raw limit despite the same encoded size), which aids future maintainability.
magicblock-committor-service/src/tasks/utils.rs (2)
131-138: LGTM! Consistent pattern.The new
tasks_accounts_size_budgetfunction correctly follows the established pattern oftasks_compute_units, summing individual task budgets to compute the total for a transaction. This consistency aids maintainability.Based on learnings, this pattern of summing per-task budgets for transactions is established in the codebase.
69-69: LGTM!The integration of
tasks_accounts_size_budgetintoassemble_tasks_txis correct and follows the established pattern for compute units.
snawaz
force-pushed
the
snawaz/size-budget
branch
2 times, most recently
from
2400d01 to
e702030
Compare
snawaz
force-pushed
the
snawaz/size-budget
branch
from
e702030 to
aa2ba9f
Compare
Solana runtime allocates 64 MB of buffer space to load all accounts for a transaction. In most cases, we do not need such a large buffer, yet we still pay the cost in terms of compute units (CU).
This PR uses
SetLoadedAccountsDataSizeLimitwhen invoking DLP instructions to reduce unnecessary account data allocation and improve compute efficiency.The DLP program now exposes a coarse-grained account size classification API that allows callers to compute a safe upper bound on the total loaded account data without fetching accounts on-chain. This PR consumes that API to:
SetLoadedAccountsDataSizeLimitcompute-budget instruction alongside existing compute-budget instructions.No behavioral changes are introduced to instruction execution.