Skip to content

Conversation

@taco-paco
Copy link
Contributor

@taco-paco taco-paco commented Nov 11, 2025

Summary by CodeRabbit

  • Chores
    • Enhanced RPC transaction configuration settings for improved transaction retrieval and metrics calculation across transaction processing paths.

@github-actions
Copy link

Manual Deploy Available

You can trigger a manual deploy of this PR branch to testnet:

Deploy to Testnet 🚀

Alternative: Comment /deploy on this PR to trigger deployment directly.

⚠️ Note: Manual deploy requires authorization. Only authorized users can trigger deployments.

Comment updated automatically when the PR is synchronized.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

Walkthrough

Modified RPC transaction handling in the intent executor module by introducing RpcTransactionConfig to configure commitment level and maximum supported transaction version for get_transaction calls across SingleStage and TwoStage execution paths, plus metrics computation.

Changes

Cohort / File(s) Summary
RPC Transaction Configuration
magicblock-committor-service/src/intent_executor/mod.rs
Added import for RpcTransactionConfig; created config object with commitment and max_supported_transaction_version set to 0; updated all get_transaction calls (SingleStage, TwoStage commit/finalize signatures, and metrics path) to pass Some(config) instead of None

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the configuration values (commitment level, transaction version) are correct for the use case
  • Confirm all affected RPC call sites (SingleStage, TwoStage paths, metrics) are properly updated
  • Check that the config construction aligns with expected behavior in both success and failure scenarios

Suggested reviewers

  • bmuddha
  • GabrielePicco

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: fetching CUs for metrics' directly and clearly summarizes the main change: fixing the computation unit (CU) fetching mechanism for metrics collection by introducing RpcTransactionConfig.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 fix/base-layer-ix/cu-metrics-fetch

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ed8faa and ee9c43b.

📒 Files selected for processing (1)
  • magicblock-committor-service/src/intent_executor/mod.rs (3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/intent_executor/mod.rs
📚 Learning: 2025-10-21T13:06:38.900Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/http/get_program_accounts.rs:17-25
Timestamp: 2025-10-21T13:06:38.900Z
Learning: The magicblock validator does not support ledger forking, so commitment-based state queries (processed/confirmed/finalized) are not applicable. RPC methods can safely ignore commitment and minContextSlot parameters from Solana RPC config objects.

Applied to files:

  • magicblock-committor-service/src/intent_executor/mod.rs
⏰ 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). (3)
  • GitHub Check: Build Project
  • GitHub Check: run_make_ci_lint
  • GitHub Check: run_make_ci_test
🔇 Additional comments (3)
magicblock-committor-service/src/intent_executor/mod.rs (3)

26-26: LGTM - Required import added.

The import of RpcTransactionConfig is necessary for configuring transaction fetching in the metrics function below.


691-695: Correct fix - Config properly addresses v0 transaction fetching.

The configuration is correctly constructed:

  • max_supported_transaction_version: Some(0) is essential for fetching v0 transactions (used throughout the codebase at line 456), which would otherwise be rejected by the RPC.
  • commitment maintains consistency with the RPC client configuration.

This resolves the CU metrics fetching issue described in the PR.


699-715: LGTM - Config correctly applied to all execution paths.

The RpcTransactionConfig is properly passed to all get_transaction calls:

  • SingleStage: Line 700
  • TwoStage: Lines 711 and 714 (commit and finalize signatures)

This ensures compute unit metrics are successfully fetched for both execution strategies. Error handling at lines 728-736 appropriately manages any fetch failures.


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

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

Copy link
Contributor

@bmuddha bmuddha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@taco-paco taco-paco merged commit c94263e into master Nov 12, 2025
19 of 20 checks passed
@taco-paco taco-paco deleted the fix/base-layer-ix/cu-metrics-fetch branch November 12, 2025 05:33
thlorenz added a commit that referenced this pull request Nov 13, 2025
* master:
  fix: fetching CUs for metrics (#623)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants