feat: account fetch metrics fine-grained by RPC origin

[thlorenz]

Summary

Make account fetch metrics fine-grained by RPC origin

Details

Split account fetch metrics into origin-specific counters to track which
RPC method initiated each fetch:

• Replace ACCOUNT_FETCHES_FOUND_COUNT with:

  • ACCOUNT_FETCHES_GET_ACCOUNT_FOUND_COUNT
  • ACCOUNT_FETCHES_GET_MULTIPLE_ACCOUNTS_FOUND_COUNT
  • ACCOUNT_FETCHES_SEND_TRANSACTION_FOUND_COUNT

• Replace ACCOUNT_FETCHES_NOT_FOUND_COUNT with:

  • ACCOUNT_FETCHES_GET_ACCOUNT_NOT_FOUND_COUNT
  • ACCOUNT_FETCHES_GET_MULTIPLE_ACCOUNTS_NOT_FOUND_COUNT
  • ACCOUNT_FETCHES_SEND_TRANSACTION_NOT_FOUND_COUNT

Propagate AccountFetchOrigin through all fetch layers (Aperture →
Chainlink → FetchCloner → RemoteAccountProvider) and update auxiliary
fetch paths (delegation records, program data) to use parent request's
origin for accurate metrics categorization.

This enables dashboards to sum these metrics to recover previous totals
while gaining visibility into fetch patterns by RPC method.

Summary by CodeRabbit

• New Features

  • Added a public AccountFetchOrigin enum and explicit origin tagging for account fetches.

• Refactor

  • Public account-fetch APIs updated to accept and propagate fetch origin.
  • Metrics switched to per-origin counters and now record fetches by origin.

• Tests

  • Updated unit and integration tests to call the origin-aware APIs and supply appropriate origin variants.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

This change introduces an AccountFetchOrigin enum and threads it through account-fetching APIs across multiple crates (magicblock-aperture, magicblock-chainlink, remote_account_provider, magicblock-metrics, tests, and integrations). Many public and internal function signatures now accept an additional fetch_origin: AccountFetchOrigin parameter and propagate it into fetch, clone, and metrics calls. Metrics counters for account fetches were converted to per-origin counter vectors and are now incremented using the provided origin.

Possibly related PRs

• magicblock-labs/magicblock-validator PR 664 — Modifies the same fetch/cloner paths (fetch_and_clone_accounts, fetch_and_clone_accounts_with_dedup, should_refresh_undelegating_in_bank_account) that were extended here to accept and propagate AccountFetchOrigin.
• magicblock-labs/magicblock-validator PR 648 — Alters chainlink account-ensuring logic (ensure_transaction_accounts / ensure_accounts) in the same call sites where signatures were changed to accept AccountFetchOrigin.
• magicblock-labs/magicblock-validator PR 621 — Introduces subscription/reconnection and account-fetch metrics changes that are extended here by making metrics per-origin and adding AccountFetchOrigin plumbing.

Suggested reviewers

• bmuddha

✨ 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 thlorenz/fetch-metrics

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2245742 and e6acc90.

📒 Files selected for processing (1)

• magicblock-metrics/src/metrics/types.rs (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

magicblock-metrics/src/metrics/types.rs (3)

magicblock-committor-service/src/intent_executor/error.rs (1)

• value (111-118)

magicblock-committor-service/src/intent_executor/mod.rs (1)

• value (74-82)

magicblock-committor-service/src/types.rs (1)

• value (23-29)

🔇 Additional comments (4)

magicblock-metrics/src/metrics/types.rs (4)

72-77: LGTM! Enum definition is clean and appropriate.

The derives are well-chosen for a metric label type, and the variants correctly map to the three RPC methods mentioned in the PR objectives.

---

79-88: Well done! Previous review feedback properly addressed.

The as_str() method successfully eliminates the duplication that was flagged in the previous review, matching the existing Outcome::as_str() pattern in this file.

---

90-94: LGTM! Display implementation correctly delegates to as_str().

This follows the DRY principle and keeps the string mapping in one place.

---

96-100: LGTM! LabelValue implementation correctly delegates to as_str().

This completes the refactor suggested in the previous review, ensuring both Display and LabelValue share the same string mapping.

---

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

[github-actions]

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]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

magicblock-chainlink/src/chainlink/fetch_cloner.rs (1)

1863-1960: Tests exercise the new origin parameter for GetAccount; consider adding coverage for other origins

All updated tests in this module use AccountFetchOrigin::GetAccount, which validates the main ensure/fetch flows. If you want direct unit coverage for GetMultipleAccounts and SendTransaction origins (beyond higher‑level integration tests), adding targeted cases here or in a nearby module would improve confidence that no origin is accidentally mis‑wired.

Also applies to: 2191-2260, 2356-2362, 2426-2456, 2503-2530, 2579-2590, 2621-2660, 2729-2735, 2821-2827, 2903-2913

magicblock-chainlink/src/chainlink/mod.rs (1)

313-329: Origin‑aware ensure_accounts/fetch_accounts API looks consistent

Adding a fetch_origin: AccountFetchOrigin parameter to ensure_accounts, fetch_accounts, and fetch_accounts_common, and threading it through to fetch_and_clone_accounts_with_dedup, makes origin an explicit part of the public contract. This is a breaking but coherent change; as long as all internal and Aperture call sites are updated, the design is sound.

Also applies to: 335-357, 366-372, 397-402

magicblock-chainlink/src/remote_account_provider/mod.rs (1)

521-577: Refetch path may perform a redundant RPC fetch and double‑count metrics

In try_get_multi_until_slots_match, the refetch block calls:

self.fetch(pubkeys.to_vec(), None, self.chain_slot(), fetch_origin);

before entering the retry loop, but at that point there are no pending entries in fetching_accounts for these pubkeys, so this fetch call’s results are never delivered—only RPC traffic and metrics are generated. The first try_get_multi call inside the loop will then set up channels and spawn its own fetch, which actually resolves callers.

Consider removing this standalone self.fetch(...) and relying on the loop’s try_get_multi calls, or restructuring so the forced refetch reuses the same pending‑request mechanism instead of firing a “blind” fetch.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cc6810 and 3d4145f.

📒 Files selected for processing (12)

• magicblock-aperture/src/requests/http/mod.rs (3 hunks)
• magicblock-chainlink/src/chainlink/fetch_cloner.rs (32 hunks)
• magicblock-chainlink/src/chainlink/mod.rs (8 hunks)
• magicblock-chainlink/src/lib.rs (1 hunks)
• magicblock-chainlink/src/remote_account_provider/mod.rs (19 hunks)
• magicblock-chainlink/tests/01_ensure-accounts.rs (9 hunks)
• magicblock-chainlink/tests/03_deleg_after_sub.rs (3 hunks)
• magicblock-chainlink/tests/08_subupdate-ordering.rs (2 hunks)
• magicblock-chainlink/tests/basics.rs (3 hunks)
• magicblock-chainlink/tests/utils/test_context.rs (2 hunks)
• magicblock-metrics/src/metrics/mod.rs (4 hunks)
• magicblock-metrics/src/metrics/types.rs (1 hunks)

🧰 Additional context used

🧠 Learnings (8)

📚 Learning: 2025-11-07T14:20:31.457Z

Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 621
File: magicblock-chainlink/src/remote_account_provider/chain_pubsub_actor.rs:457-495
Timestamp: 2025-11-07T14:20:31.457Z
Learning: In magicblock-chainlink/src/remote_account_provider/chain_pubsub_client.rs, the unsubscribe closure returned by PubSubConnection::account_subscribe(...) resolves to () (unit), not a Result. Downstream code should not attempt to inspect an unsubscribe result and can optionally wrap it in a timeout to guard against hangs.

Applied to files:

• magicblock-chainlink/src/lib.rs
• magicblock-chainlink/tests/01_ensure-accounts.rs
• magicblock-chainlink/tests/08_subupdate-ordering.rs
• magicblock-chainlink/tests/03_deleg_after_sub.rs
• magicblock-chainlink/src/chainlink/fetch_cloner.rs
• magicblock-chainlink/src/remote_account_provider/mod.rs

📚 Learning: 2025-10-26T16:53:29.820Z

Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: magicblock-chainlink/src/remote_account_provider/mod.rs:134-0
Timestamp: 2025-10-26T16:53:29.820Z
Learning: In magicblock-chainlink/src/remote_account_provider/mod.rs, the `Endpoint::separate_pubsub_url_and_api_key()` method uses `split_once("?api-key=")` because the api-key parameter is always the only query parameter right after `?`. No additional query parameter parsing is needed for this use case.

Applied to files:

• magicblock-chainlink/src/lib.rs
• magicblock-chainlink/tests/basics.rs
• magicblock-chainlink/tests/03_deleg_after_sub.rs
• magicblock-aperture/src/requests/http/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-chainlink/src/lib.rs
• magicblock-chainlink/tests/03_deleg_after_sub.rs
• magicblock-aperture/src/requests/http/mod.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:

• magicblock-chainlink/src/lib.rs

📚 Learning: 2025-11-19T09:34:37.917Z

Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 621
File: test-integration/test-chainlink/tests/ix_remote_account_provider.rs:62-63
Timestamp: 2025-11-19T09:34:37.917Z
Learning: In test-integration/test-chainlink/tests/ix_remote_account_provider.rs and similar test files, the `_fwd_rx` receiver returned by `init_remote_account_provider()` is intentionally kept alive (but unused) to prevent "receiver dropped" errors on the sender side. The pattern `let (remote_account_provider, _fwd_rx) = init_remote_account_provider().await;` should NOT be changed to `let (remote_account_provider, _) = ...` because dropping the receiver would cause send() operations to fail.

Applied to files:

• magicblock-chainlink/tests/basics.rs
• magicblock-chainlink/tests/01_ensure-accounts.rs
• magicblock-chainlink/tests/08_subupdate-ordering.rs
• magicblock-chainlink/src/chainlink/mod.rs
• magicblock-chainlink/tests/03_deleg_after_sub.rs
• magicblock-aperture/src/requests/http/mod.rs
• magicblock-chainlink/tests/utils/test_context.rs
• magicblock-chainlink/src/chainlink/fetch_cloner.rs
• magicblock-chainlink/src/remote_account_provider/mod.rs

📚 Learning: 2025-11-18T08:47:39.702Z

Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/tests/04_redeleg_other_separate_slots.rs:158-165
Timestamp: 2025-11-18T08:47:39.702Z
Learning: In magicblock-chainlink tests involving compressed accounts, `set_remote_slot()` sets the slot of the `AccountSharedData`, while `compressed_account_shared_with_owner_and_slot()` sets the slot of the delegation record. These are two different fields and both calls are necessary.

Applied to files:

• magicblock-chainlink/tests/basics.rs
• magicblock-chainlink/tests/01_ensure-accounts.rs
• magicblock-chainlink/tests/03_deleg_after_sub.rs
• magicblock-chainlink/tests/utils/test_context.rs
• magicblock-chainlink/src/chainlink/fetch_cloner.rs
• magicblock-chainlink/src/remote_account_provider/mod.rs

📚 Learning: 2025-11-19T11:31:24.218Z

Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: test-integration/test-chainlink/tests/ix_01_ensure-accounts.rs:85-114
Timestamp: 2025-11-19T11:31:24.218Z
Learning: In the magicblock-validator codebase, compressed delegation records are never subscribed to by design. Tests for compressed delegation flows should focus on verifying that the delegated account (e.g., counter PDA) is cloned as delegated and not subscribed, but do not need to check for subscription to delegation-record-like PDAs in the compressed path because subscriptions to compressed delegation records never occur.

Applied to files:

• magicblock-chainlink/tests/01_ensure-accounts.rs
• magicblock-chainlink/tests/03_deleg_after_sub.rs
• magicblock-chainlink/src/chainlink/fetch_cloner.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-chainlink/src/remote_account_provider/mod.rs

🧬 Code graph analysis (3)

magicblock-metrics/src/metrics/mod.rs (1)

magicblock-chainlink/src/remote_account_provider/mod.rs (1)

• new (278-335)

magicblock-chainlink/src/chainlink/fetch_cloner.rs (2)

magicblock-chainlink/src/remote_account_provider/remote_account.rs (2)

• delegated (95-101)
• slot (203-215)

magicblock-chainlink/src/remote_account_provider/program_account.rs (1)

• get_loaderv3_get_program_data_address (37-41)

magicblock-chainlink/src/remote_account_provider/mod.rs (1)

magicblock-metrics/src/metrics/mod.rs (3)

• set_monitored_accounts_count (469-471)
• inc_account_fetches_found (515-528)
• inc_account_fetches_not_found (530-546)

🔇 Additional comments (23)

magicblock-chainlink/src/lib.rs (1)

9-9: Re-exporting AccountFetchOrigin from crate root is appropriate

Publicly re-exporting AccountFetchOrigin from magicblock-chainlink aligns with how tests and callers use the type and avoids leaking the metrics module path into higher layers. Looks good.

magicblock-chainlink/tests/basics.rs (1)

4-4: Consistent origin tagging for ensure_accounts in basics tests

Using AccountFetchOrigin::GetMultipleAccounts for all ensure_accounts calls keeps the test behavior unchanged while aligning with the updated API and origin-aware metrics. The fetch-count expectations and clone assertions remain valid.

Also applies to: 48-55, 60-67, 103-110, 118-125

magicblock-aperture/src/requests/http/mod.rs (1)

13-13: Correct mapping of HTTP helpers to AccountFetchOrigin variants

Importing AccountFetchOrigin and using:

• GetAccount in read_account_with_ensure, and
• GetMultipleAccounts in read_accounts_with_ensure

correctly tags fetches by the initiating RPC method while preserving existing timing and error-handling behavior. This is exactly what the per-origin metrics need.

Also applies to: 105-111, 131-137

magicblock-metrics/src/metrics/types.rs (1)

76-95: AccountFetchOrigin enum and Display implementation look solid

The AccountFetchOrigin enum is minimal and well-scoped, and the Display mapping to "get_multiple_accounts", "get_account", and "send_transaction" provides clear, stable strings for metric labels and logs. Trait bounds and visibility are appropriate.

magicblock-chainlink/tests/03_deleg_after_sub.rs (1)

7-7: Origin tagging in deleg-after-sub test matches updated API

Bringing AccountFetchOrigin into scope and passing AccountFetchOrigin::GetMultipleAccounts to ensure_accounts keeps the delegation flow test behavior the same while satisfying the new signature. Metric attribution remains consistent with other multi-account-style tests.

Also applies to: 57-64, 86-93

magicblock-chainlink/tests/08_subupdate-ordering.rs (1)

2-2: Subupdate-ordering test updated correctly for AccountFetchOrigin

The added AccountFetchOrigin import and use of AccountFetchOrigin::GetMultipleAccounts in the initial ensure_accounts call integrate cleanly with the new API without altering the test’s ordering assertions.

Also applies to: 59-66

magicblock-chainlink/tests/01_ensure-accounts.rs (1)

9-9: Ensure-accounts tests correctly adapted to AccountFetchOrigin

Updating all ensure_accounts invocations in this file to include AccountFetchOrigin::GetMultipleAccounts matches the new API and maintains existing test coverage across all scenarios (non-existing, delegated, other-authority, undelegating, and invalid delegation record). The failure-case test that inspects the raw Result still behaves as intended.

Also applies to: 39-46, 70-77, 106-113, 147-154, 192-199, 247-254, 295-302, 338-344

magicblock-metrics/src/metrics/mod.rs (3)

8-10: Publicly re‑exporting AccountFetchOrigin is appropriate

Making AccountFetchOrigin available from the metrics crate aligns with how other metric types are exposed and keeps downstream imports consistent.

---

197-237: Per‑origin account‑fetch counters are well‑named and correctly registered

The six new counters cleanly separate found/not‑found by origin and are all registered in register(), so they can fully replace the old aggregate counters while still allowing dashboards to sum back to totals.

Also applies to: 365-370

---

515-545: Origin‑aware dispatch for found/not‑found metrics looks correct

Matching on AccountFetchOrigin and incrementing the corresponding per‑origin counter is straightforward and exhaustive; this keeps compiler pressure on future enum extensions and aligns with how fetch_origin is threaded from callers.

magicblock-chainlink/src/chainlink/fetch_cloner.rs (6)

15-16: Importing AccountFetchOrigin at this level is the right abstraction point

FetchCloner is where high‑level fetch flows are orchestrated, so depending on the shared AccountFetchOrigin type here (rather than re‑defining an origin enum) keeps origin semantics centralized.

---

287-297: Confirm intent for subscription‑driven fetches being classified as GetAccount

Both the LoaderV3 program‑data path and delegation‑record companion fetch on subscription updates use AccountFetchOrigin::GetAccount. If metrics are meant to reflect only HTTP RPC origins, this is reasonable as a “closest fit”; if you eventually want to distinguish subscription‑triggered fetches, this would be the main place to introduce a dedicated origin.

Also applies to: 360-366

---

493-515: Delegation‑record fetch now correctly carries the caller’s origin

Adding fetch_origin to fetch_and_parse_delegation_record and passing it through to try_get_multi_until_slots_match ensures delegation‑record lookups are counted under the same origin as the parent ensure/fetch call, without changing functional behavior.

Also applies to: 1112-1126

---

571-612: Core fetch‑and‑clone flow propagates fetch_origin consistently

fetch_and_clone_accounts now forwards fetch_origin both to try_get_multi and to the delegated/program‑data companion fetch tasks, so all underlying RPC activity (including secondary lookups) is tagged with the initiating RPC origin.

Also applies to: 737-752, 866-908

---

1161-1197: Deduplicated fetch path keeps origin context intact

fetch_and_clone_accounts_with_dedup passes fetch_origin through to should_refresh_undelegating_in_bank_account and the final fetch_and_clone_accounts call, so both refresh checks and actual remote fetches share the correct origin label without impacting dedup semantics.

Also applies to: 1239-1245

---

1296-1352: Companion‑fetch helper tasks correctly forward fetch_origin

The task_to_fetch_with_delegation_record, task_to_fetch_with_program_data, and task_to_fetch_with_companion helpers now pass fetch_origin into try_get_multi_until_slots_match, ensuring all two‑account companion fetches are attributed to the right origin in metrics.

magicblock-chainlink/src/chainlink/mod.rs (2)

11-11: Bringing AccountFetchOrigin into the Chainlink API is appropriate

Chainlink is the main public surface for fetch/ensure operations, so importing the shared origin type here is the right place to define origin semantics for upstream callers.

---

274-278: ensure_transaction_accounts correctly tags fetches as SendTransaction

Passing AccountFetchOrigin::SendTransaction into ensure_accounts ensures all account fetches performed during transaction processing (including delegation and program‑data lookups) are attributed to the sendTransaction origin in metrics.

magicblock-chainlink/src/remote_account_provider/mod.rs (5)

49-55: Metric/origin imports are structured cleanly

Importing the metric increment helpers alongside AccountFetchOrigin keeps all fetch‑related metrics concerns localized here without widening the public surface of RemoteAccountProvider.

---

507-519: try_get wrapper correctly forwards origin into try_get_multi

Extending try_get with fetch_origin and delegating to try_get_multi(&[pubkey], None, fetch_origin) preserves the single‑result invariant while ensuring even single‑account fetches are origin‑aware for metrics.

---

622-667: try_get_multi origin threading and fetch invocation are consistent

Adding fetch_origin to try_get_multi and passing it through to self.fetch(pubkeys.to_vec(), mark_empty_if_not_found, min_context_slot, fetch_origin) means every RPC call behind this method is counted under the correct origin; subscription override logic and pending‑request handling remain unchanged.

---

852-858: Fetch metrics correctly separate RPC success from found/not‑found by origin

Extending fetch to accept fetch_origin and then calling:

• inc_account_fetches_success(pubkeys.len() as u64) for any successful RPC response, and
• inc_account_fetches_found(fetch_origin, found_count) / inc_account_fetches_not_found(fetch_origin, not_found_count)

cleanly distinguishes RPC‑level success from on‑chain presence, with per‑origin breakdown matching how mark_empty_if_not_found is treated.

Also applies to: 1000-1037

---

1190-1195: Tests updated to supply AccountFetchOrigin::GetAccount

All RemoteAccountProvider tests now pass AccountFetchOrigin::GetAccount into try_get and try_get_multi_until_slots_match, which keeps existing behavior while validating the new parameter plumb‑through. This should be enough for this module; broader origin combinations are better exercised at the Chainlink/Aperture levels.

Also applies to: 1246-1249, 1342-1351, 1382-1387, 1420-1425, 1559-1665

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

test-integration/test-chainlink/tests/ix_programs.rs (1)

4-12: AccountFetchOrigin wiring in program-cloning tests is sound, with a minor semantic consideration

All ensure_accounts invocations now correctly pass the new origin parameter, and using AccountFetchOrigin::GetAccount works fine for these program-cloning scenarios, including the single-program tests.

For the multi-program cases (e.g., ixtest_clone_multiple_programs_v1_v2_v3), if your intent is to model a batched account/program fetch akin to a getMultipleAccounts-style RPC for metrics, you might consider using the corresponding origin there. Otherwise, the current choice is perfectly acceptable for the behaviours asserted in these tests.

Also applies to: 439-442, 467-470, 496-499, 523-526, 575-578, 603-606

test-integration/test-chainlink/tests/ix_full_scenarios.rs (1)

2-10: Use GetMultipleAccounts origin for batched fetch_accounts calls to align metrics with actual RPC semantics

Both fetch_accounts(&all_pubkeys, …) calls operate on batches of pubkeys but use AccountFetchOrigin::GetAccount. Since the fetch_origin parameter affects only metrics recording (not RPC call behavior), this causes the batched account fetches to be tagged under single-account counters, misrepresenting observability. Change to AccountFetchOrigin::GetMultipleAccounts at lines 165-169 and 213-217 for accurate metrics classification.

test-integration/test-chainlink/tests/ix_exceed_capacity.rs (1)

2-7: Update AccountFetchOrigin to GetMultipleAccounts for multi-account operations

The tests exercise reading multiple accounts but attribute them to AccountFetchOrigin::GetAccount, which affects metrics attribution. The GetMultipleAccounts variant exists for this purpose and should be used when fetching multiple pubkeys.

Update lines 45-48, 77-84, and the similar call earlier in the test (around line 44) to use AccountFetchOrigin::GetMultipleAccounts for semantic accuracy and correct metric categorization.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3d4145f and 03679ef.

⛔ Files ignored due to path filters (1)

• test-integration/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (9)

• test-integration/test-chainlink/src/test_context.rs (1 hunks)
• test-integration/test-chainlink/tests/ix_01_ensure-accounts.rs (4 hunks)
• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs (4 hunks)
• test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.rs (2 hunks)
• test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs (2 hunks)
• test-integration/test-chainlink/tests/ix_exceed_capacity.rs (3 hunks)
• test-integration/test-chainlink/tests/ix_full_scenarios.rs (3 hunks)
• test-integration/test-chainlink/tests/ix_programs.rs (7 hunks)
• test-integration/test-chainlink/tests/ix_remote_account_provider.rs (7 hunks)

🧰 Additional context used

🧠 Learnings (7)

📚 Learning: 2025-11-19T09:34:37.917Z

Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 621
File: test-integration/test-chainlink/tests/ix_remote_account_provider.rs:62-63
Timestamp: 2025-11-19T09:34:37.917Z
Learning: In test-integration/test-chainlink/tests/ix_remote_account_provider.rs and similar test files, the `_fwd_rx` receiver returned by `init_remote_account_provider()` is intentionally kept alive (but unused) to prevent "receiver dropped" errors on the sender side. The pattern `let (remote_account_provider, _fwd_rx) = init_remote_account_provider().await;` should NOT be changed to `let (remote_account_provider, _) = ...` because dropping the receiver would cause send() operations to fail.

Applied to files:

• test-integration/test-chainlink/src/test_context.rs
• test-integration/test-chainlink/tests/ix_full_scenarios.rs
• test-integration/test-chainlink/tests/ix_01_ensure-accounts.rs
• test-integration/test-chainlink/tests/ix_exceed_capacity.rs
• test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs
• test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.rs
• test-integration/test-chainlink/tests/ix_programs.rs
• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_remote_account_provider.rs

📚 Learning: 2025-11-07T14:20:31.457Z

Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 621
File: magicblock-chainlink/src/remote_account_provider/chain_pubsub_actor.rs:457-495
Timestamp: 2025-11-07T14:20:31.457Z
Learning: In magicblock-chainlink/src/remote_account_provider/chain_pubsub_client.rs, the unsubscribe closure returned by PubSubConnection::account_subscribe(...) resolves to () (unit), not a Result. Downstream code should not attempt to inspect an unsubscribe result and can optionally wrap it in a timeout to guard against hangs.

Applied to files:

• test-integration/test-chainlink/src/test_context.rs
• test-integration/test-chainlink/tests/ix_full_scenarios.rs
• test-integration/test-chainlink/tests/ix_01_ensure-accounts.rs
• test-integration/test-chainlink/tests/ix_exceed_capacity.rs
• test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs
• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_remote_account_provider.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-chainlink/tests/ix_01_ensure-accounts.rs
• test-integration/test-chainlink/tests/ix_exceed_capacity.rs
• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs

📚 Learning: 2025-11-18T08:47:39.702Z

Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/tests/04_redeleg_other_separate_slots.rs:158-165
Timestamp: 2025-11-18T08:47:39.702Z
Learning: In magicblock-chainlink tests involving compressed accounts, `set_remote_slot()` sets the slot of the `AccountSharedData`, while `compressed_account_shared_with_owner_and_slot()` sets the slot of the delegation record. These are two different fields and both calls are necessary.

Applied to files:

• test-integration/test-chainlink/tests/ix_exceed_capacity.rs
• test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs
• test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.rs
• test-integration/test-chainlink/tests/ix_programs.rs
• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_remote_account_provider.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:

• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_remote_account_provider.rs

📚 Learning: 2025-11-19T11:31:24.218Z

Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: test-integration/test-chainlink/tests/ix_01_ensure-accounts.rs:85-114
Timestamp: 2025-11-19T11:31:24.218Z
Learning: In the magicblock-validator codebase, compressed delegation records are never subscribed to by design. Tests for compressed delegation flows should focus on verifying that the delegated account (e.g., counter PDA) is cloned as delegated and not subscribed, but do not need to check for subscription to delegation-record-like PDAs in the compressed path because subscriptions to compressed delegation records never occur.

Applied to files:

• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs

📚 Learning: 2025-10-26T16:53:29.820Z

Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: magicblock-chainlink/src/remote_account_provider/mod.rs:134-0
Timestamp: 2025-10-26T16:53:29.820Z
Learning: In magicblock-chainlink/src/remote_account_provider/mod.rs, the `Endpoint::separate_pubsub_url_and_api_key()` method uses `split_once("?api-key=")` because the api-key parameter is always the only query parameter right after `?`. No additional query parameter parsing is needed for this use case.

Applied to files:

• test-integration/test-chainlink/tests/ix_remote_account_provider.rs

🔇 Additional comments (7)

test-integration/test-chainlink/src/test_context.rs (1)

201-212: Wire AccountFetchOrigin through TestContext::ensure_account correctly

Using the new ensure_accounts signature with magicblock_chainlink::AccountFetchOrigin::GetAccount for this single-account helper is consistent and keeps behaviour unchanged aside from origin tagging.

test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs (1)

2-7: Origin parameter usage in delegation-after-subscribe test is consistent

Importing AccountFetchOrigin and passing AccountFetchOrigin::GetAccount for all ensure_accounts calls (each operating on a single pubkey) matches the test’s intent and the updated API; nothing else needs changing here.

Also applies to: 24-27, 39-42, 63-66

test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.rs (1)

7-12: Correct AccountFetchOrigin wiring for redelegation (separate slots)

The added AccountFetchOrigin import and using AccountFetchOrigin::GetAccount in the lone ensure_accounts call align with the single-account flow under test and the new signature.

Also applies to: 37-40

test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs (1)

7-11: Consistent origin tagging in same-slot redelegation test

Using AccountFetchOrigin::GetAccount for the single-account ensure_accounts call is consistent with the rest of the delegation tests and the updated API; no further changes needed here.

Also applies to: 36-39

test-integration/test-chainlink/tests/ix_01_ensure-accounts.rs (1)

2-8: AccountFetchOrigin usage in basic ensure-accounts tests is appropriate

Each ensure_accounts call here operates on a single pubkey, and tagging them with AccountFetchOrigin::GetAccount fits the scenarios under test while satisfying the new signature; nothing else stands out.

Also applies to: 20-24, 45-49, 75-79

test-integration/test-chainlink/tests/ix_remote_account_provider.rs (2)

18-18: LGTM!

The import of AccountFetchOrigin is correctly added to support the updated API signatures.

---

67-70: LGTM!

The try_get() calls correctly use AccountFetchOrigin::GetAccount, which properly categorizes these single-account fetches in the metrics system.

Also applies to: 123-126, 143-146

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

test-integration/test-chainlink/tests/ix_remote_account_provider.rs (1)

170-170: Use AccountFetchOrigin::GetMultipleAccounts for multi-account fetches.

All three try_get_multi calls use AccountFetchOrigin::GetAccount, which misclassifies these multi-account fetches in the metrics system. They should use GetMultipleAccounts to properly increment the ACCOUNT_FETCHES_GET_MULTIPLE_ACCOUNTS_*_COUNT metrics.

Apply this diff to all three calls:

-            .try_get_multi(&all_pubkeys, None, AccountFetchOrigin::GetAccount)
+            .try_get_multi(&all_pubkeys, None, AccountFetchOrigin::GetMultipleAccounts)

Also applies to: 197-197, 226-226

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03679ef and bf818f6.

📒 Files selected for processing (5)

• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs (4 hunks)
• test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.rs (2 hunks)
• test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs (2 hunks)
• test-integration/test-chainlink/tests/ix_full_scenarios.rs (3 hunks)
• test-integration/test-chainlink/tests/ix_remote_account_provider.rs (7 hunks)

🧰 Additional context used

🧠 Learnings (6)

📚 Learning: 2025-11-19T09:34:37.917Z

Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 621
File: test-integration/test-chainlink/tests/ix_remote_account_provider.rs:62-63
Timestamp: 2025-11-19T09:34:37.917Z
Learning: In test-integration/test-chainlink/tests/ix_remote_account_provider.rs and similar test files, the `_fwd_rx` receiver returned by `init_remote_account_provider()` is intentionally kept alive (but unused) to prevent "receiver dropped" errors on the sender side. The pattern `let (remote_account_provider, _fwd_rx) = init_remote_account_provider().await;` should NOT be changed to `let (remote_account_provider, _) = ...` because dropping the receiver would cause send() operations to fail.

Applied to files:

• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_full_scenarios.rs
• test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs
• test-integration/test-chainlink/tests/ix_remote_account_provider.rs
• test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.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:

• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_remote_account_provider.rs

📚 Learning: 2025-11-18T08:47:39.702Z

Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/tests/04_redeleg_other_separate_slots.rs:158-165
Timestamp: 2025-11-18T08:47:39.702Z
Learning: In magicblock-chainlink tests involving compressed accounts, `set_remote_slot()` sets the slot of the `AccountSharedData`, while `compressed_account_shared_with_owner_and_slot()` sets the slot of the delegation record. These are two different fields and both calls are necessary.

Applied to files:

• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs
• test-integration/test-chainlink/tests/ix_remote_account_provider.rs
• test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.rs

📚 Learning: 2025-11-07T14:20:31.457Z

Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 621
File: magicblock-chainlink/src/remote_account_provider/chain_pubsub_actor.rs:457-495
Timestamp: 2025-11-07T14:20:31.457Z
Learning: In magicblock-chainlink/src/remote_account_provider/chain_pubsub_client.rs, the unsubscribe closure returned by PubSubConnection::account_subscribe(...) resolves to () (unit), not a Result. Downstream code should not attempt to inspect an unsubscribe result and can optionally wrap it in a timeout to guard against hangs.

Applied to files:

• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_full_scenarios.rs
• test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs
• test-integration/test-chainlink/tests/ix_remote_account_provider.rs
• test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.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-chainlink/tests/ix_03_deleg_after_sub.rs
• test-integration/test-chainlink/tests/ix_full_scenarios.rs

📚 Learning: 2025-11-19T11:31:24.218Z

Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: test-integration/test-chainlink/tests/ix_01_ensure-accounts.rs:85-114
Timestamp: 2025-11-19T11:31:24.218Z
Learning: In the magicblock-validator codebase, compressed delegation records are never subscribed to by design. Tests for compressed delegation flows should focus on verifying that the delegated account (e.g., counter PDA) is cloned as delegated and not subscribed, but do not need to check for subscription to delegation-record-like PDAs in the compressed path because subscriptions to compressed delegation records never occur.

Applied to files:

• test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs

🔇 Additional comments (4)

test-integration/test-chainlink/tests/ix_03_deleg_after_sub.rs (1)

6-6: LGTM! Correct origin for single-account fetches.

The test correctly uses AccountFetchOrigin::GetAccount for all ensure_accounts calls, which is appropriate since the test fetches a single account (pubkeys = [counter_pda]).

Also applies to: 24-28, 40-43, 64-67

test-integration/test-chainlink/tests/ix_07_redeleg_us_same_slot.rs (1)

10-10: LGTM! Correct origin for single-account fetch.

The test correctly uses AccountFetchOrigin::GetAccount for the ensure_accounts call with a single account (pubkeys = [counter_pda]).

Also applies to: 36-39

test-integration/test-chainlink/tests/ix_remote_account_provider.rs (1)

67-70: LGTM! Correct origin for single-account fetches.

All try_get calls correctly use AccountFetchOrigin::GetAccount for fetching individual accounts.

Also applies to: 122-125, 141-144

test-integration/test-chainlink/tests/ix_06_redeleg_us_separate_slots.rs (1)

10-10: LGTM! Correct origin for single-account fetch.

The test correctly uses AccountFetchOrigin::GetAccount for the ensure_accounts call with a single account (pubkeys = [counter_pda]).

Also applies to: 36-39

[bmuddha]

LGTM

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf818f6 and 2245742.

📒 Files selected for processing (2)

• magicblock-metrics/src/metrics/mod.rs (3 hunks)
• magicblock-metrics/src/metrics/types.rs (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

magicblock-metrics/src/metrics/types.rs (3)

magicblock-committor-service/src/intent_executor/error.rs (1)

• value (111-118)

magicblock-committor-service/src/intent_executor/mod.rs (1)

• value (74-82)

magicblock-committor-service/src/types.rs (1)

• value (23-29)

magicblock-metrics/src/metrics/mod.rs (2)

magicblock-chainlink/src/remote_account_provider/mod.rs (1)

• new (278-335)

magicblock-aperture/src/state/subscriptions.rs (4)

• new (342-355)
• new (410-413)
• count (335-337)
• count (370-372)

🔇 Additional comments (3)

magicblock-metrics/src/metrics/mod.rs (3)

8-10: Public re-export of AccountFetchOrigin is appropriate

Re-exporting AccountFetchOrigin (alongside the existing metric types) from metrics keeps the public surface coherent and matches how other metric-related types are exposed, which is useful for downstream crates that need to tag origin without depending on types directly.

---

197-213: Per-origin account-fetch counters are wired correctly

Converting ACCOUNT_FETCHES_FOUND_COUNT/ACCOUNT_FETCHES_NOT_FOUND_COUNT to IntCounterVec with a single "origin" label cleanly supports the per-RPC-origin breakdown described in the PR. The metric names and help strings stay stable, and the low, enum-backed cardinality for origin is safe for Prometheus.

---

487-500: Origin-aware helper functions correctly dispatch to labeled counters

The updated helpers now take fetch_origin: AccountFetchOrigin and use with_label_values(&[fetch_origin.value()]) before inc_by(count). This centralizes label handling and ensures callers can’t accidentally drift from the agreed string representation while still preserving the previous call pattern (just with the added origin).