feat: persist all accounts #648

GabrielePicco · 2025-11-20T05:01:25Z

Summary by CodeRabbit

Refactor
- Simplified transaction fee handling logic to improve reliability and code maintainability.
- Enhanced account management for automatic airdrop functionality with updated processing order.

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

github-actions · 2025-11-20T05:01:34Z

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 · 2025-11-20T05:01:36Z

Walkthrough

The ensure_transaction_accounts function in the chainlink module was refactored to simplify feepayer escrow cloning logic into a single condition, consolidate account marking behavior by applying mark_empty_if_not_found to all collected pubkeys at once, and reorder the auto-airdrop logic to execute after account marking.

Changes

Cohort / File(s)	Change Summary
Feepayer escrow and account marking refactor `magicblock-chainlink/src/chainlink/mod.rs`	Simplified feepayer escrow cloning decision into single condition; consolidated `mark_empty_if_not_found` from per-key basis to all pubkeys; reordered auto-airdrop logic to execute after account marking

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Logic consolidation: Verify that the unified clone_escrow condition correctly captures all scenarios previously handled by separate per-key logic
Account marking behavior: Ensure consolidating mark_empty_if_not_found to operate on all pubkeys simultaneously preserves intended empty-account handling
Airdrop logic ordering: Confirm that reordering auto-airdrop after account marking doesn't alter fund flow or error handling behavior
Edge cases: Check for any interactions between feepayer presence in bank, delegation status, and subsequent airdrop operations

Possibly related issues

Chainlink performance/maintenance optimizations #581: Directly related—both changes modify the ensure_transaction_accounts function with refactors to escrow cloning/marking logic and account-marking/airdrop behavior sequencing

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 'persist all accounts' directly relates to the core change: updating the account-marking logic to mark all collected pubkeys instead of per-key marking.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch picco/persist-all-accounts

📜 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 1a58df6 and 91693ba.

📒 Files selected for processing (1)

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

🧰 Additional context used

🧠 Learnings (4)

📓 Common 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

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

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.890Z
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/src/chainlink/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/chainlink/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-chainlink/src/chainlink/mod.rs

🧬 Code graph analysis (1)

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

magicblock-processor/tests/fees.rs (1)

ephemeral_balance_pda_from_payer (20-26)

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

lamports (79-85)

⏰ 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_test
GitHub Check: run_make_ci_lint

🔇 Additional comments (3)

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

247-251: LGTM! Clean simplification of the escrow cloning logic.

The use of is_none_or elegantly combines the two conditions (account missing or not delegated) into a single, readable expression.

254-258: LGTM! Correct conditional addition of balance PDA.

The balance PDA is appropriately added only when escrow cloning is needed, with helpful trace logging.

268-292: LGTM! Appropriate reordering of auto-airdrop logic.

Moving the auto-airdrop logic after account marking/cloning is logical. The conditional check for zero lamports and error handling remain intact.

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

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90ac7f8 and 1a58df6.

📒 Files selected for processing (2)

configs/ephem-devnet.toml (1 hunks)
magicblock-chainlink/src/chainlink/mod.rs (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common 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

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

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.890Z
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/src/chainlink/mod.rs

🧬 Code graph analysis (1)

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

magicblock-processor/tests/fees.rs (1)

ephemeral_balance_pda_from_payer (20-26)

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

lamports (79-85)

⏰ 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: run_make_ci_test
GitHub Check: run_make_ci_lint
GitHub Check: Build Project

🔇 Additional comments (4)

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

247-251: LGTM: Cleaner escrow cloning logic.

The simplified condition using is_none_or clearly expresses when escrow cloning is needed: when the feepayer doesn't exist in the bank or when it exists but isn't delegated. This is more readable than the previous approach.

269-293: LGTM: Auto-airdrop logic preserved correctly.

The auto-airdrop flow is well-structured and correctly positioned after account provisioning:

Checks if auto-airdrop is configured

Retrieves current feepayer lamports from the bank

Performs best-effort airdrop if the balance is zero

Logs errors without failing the operation

The "best-effort" approach with non-blocking error handling is appropriate for this feature.

254-258: No issues found; implementation is correct.

The function call properly uses the correct signature (2 parameters) and index value:

ephemeral_balance_pda_from_payer(feepayer, 0) correctly derives the balance PDA with index 0 (for feepayer), which aligns with codebase patterns where index 0 represents the feepayer's balance PDA and higher indices (e.g., 1/ACTOR_ESCROW_INDEX) represent delegated actor escrows.

configs/ephem-devnet.toml (1)

2-2: No issues found. Configuration is properly handled.

Verification confirms:

✓ RPC endpoint https://rpc.magicblock.app/devnet is accessible and responds correctly to RPC calls

✓ Custom cluster types are properly handled throughout the codebase via RemoteCluster enum variants and remote_cluster_from_remote() conversion function

✓ Endpoint provides expected devnet-compatible behavior (responds to standard Solana RPC methods)

The configuration change integrates correctly with existing cluster type handling logic.

magicblock-chainlink/src/chainlink/mod.rs

thlorenz

LGTM

* master: feat: persist all accounts (#648)

## Summary by CodeRabbit * **Refactor** * Simplified transaction fee handling logic to improve reliability and code maintainability. * Enhanced account management for automatic airdrop functionality with updated processing order. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

* master: feat: use latest svm version (#657) chore: update solana account (#660) fix: better transaction diagnostics & rent exemption check (#642) chore: add access-control-max-age header to cors (#654) fix(aperture): prevent racy getLatestBlockhash (#649) fix: await until sub is established and perform them in parallel (#650) feat: persist all accounts (#648)

feat: persist all accounts

1a58df6

GabrielePicco requested review from bmuddha and thlorenz November 20, 2025 05:01

coderabbitai bot reviewed Nov 20, 2025

View reviewed changes

magicblock-chainlink/src/chainlink/mod.rs Outdated Show resolved Hide resolved

GabrielePicco added 2 commits November 20, 2025 09:28

chore: revert config

fd1b907

chore: avoid allocation

91693ba

thlorenz approved these changes Nov 20, 2025

View reviewed changes

GabrielePicco merged commit bb08ebd into master Nov 20, 2025
18 checks passed

GabrielePicco deleted the picco/persist-all-accounts branch November 20, 2025 06:56

thlorenz added a commit that referenced this pull request Nov 20, 2025

Merge branch 'master' into thlorenz/await-subs

be7479e

* master: feat: persist all accounts (#648)

coderabbitai bot mentioned this pull request Nov 22, 2025

feat: account fetch metrics fine-grained by RPC origin #677

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: persist all accounts #648

feat: persist all accounts #648

Uh oh!

GabrielePicco commented Nov 20, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

github-actions bot commented Nov 20, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 20, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

thlorenz left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: persist all accounts #648

feat: persist all accounts #648

Uh oh!

Conversation

GabrielePicco commented Nov 20, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

github-actions bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Manual Deploy Available

Uh oh!

coderabbitai bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thlorenz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GabrielePicco commented Nov 20, 2025 •

edited by coderabbitai bot

Loading

github-actions bot commented Nov 20, 2025 •

edited

Loading

coderabbitai bot commented Nov 20, 2025 •

edited

Loading

thlorenz left a comment •

edited

Loading