fix: refresh stuck undelegating accounts that are closed on-chain

[thlorenz]

Summary

Refreshes stuck accounts that were marked as undelegating in the bank but are actually closed
on-chain. Previously, such accounts would remain in their stale state indefinitely. Now, the
system detects when an undelegating account has no delegation record and is closed on-chain,
then refreshes it by replacing it with an empty account.

Details

Enhanced undelegation refresh logic

The should_refresh_undelegating_in_bank_account method now returns a RefreshDecision enum instead of a boolean, allowing for more granular control over how accounts should be refreshed:

• RefreshDecision::No: Account is valid, no refresh needed
• RefreshDecision::Yes: Account needs to be refreshed
• RefreshDecision::YesAndMarkEmptyIfNotFound: Account should be refreshed and marked as empty if not found on-chain

When an undelegating account has no delegation record in the delegation ledger, the method now performs a lightweight check to verify if the account still exists on-chain. If the account is closed, the method returns YesAndMarkEmptyIfNotFound, signaling that the account should be replaced with an empty account in the bank.

Updated fetch and clone deduplication

The fetch_and_clone_accounts_with_dedup method now collects accounts that need to be marked as empty if not found and merges them with any existing such accounts before delegating to the main fetch implementation.

Test coverage

Added test_fetch_and_clone_undelegating_account_that_is_closed_on_chain to verify that accounts marked as undelegating that are closed on-chain are properly detected and replaced with empty accounts.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of undelegating accounts that were closed on-chain: such accounts are now refreshed and marked empty in bank state while preserving subscription status.
  • Refined refresh logic to more accurately decide when to re-fetch or retain undelegating accounts, reducing stale or incorrect states.

• Tests

  • Added a test verifying closed undelegating accounts are refreshed as empty and remain subscribed.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces an internal RefreshDecision enum with variants No, Yes, and YesAndMarkEmptyIfNotFound, and changes should_refresh_undelegating_in_bank_account to return RefreshDecision instead of bool. Updates fetch_and_clone_accounts_with_dedup and related call sites to handle the new variants, logging and counting unstuck undelegations, collecting pubkeys to extra_mark_empty when YesAndMarkEmptyIfNotFound, and preserving accounts on No. Merges extra_mark_empty with existing mark_empty_when_not_found before calling fetch_and_clone_accounts. Adds a test verifying an undelegating account closed on-chain is refreshed as empty and remains subscribed.

Suggested reviewers

• bmuddha
• GabrielePicco
• taco-paco

✨ 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/unstuck-empty-accounts

---

📜 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 266e732 and e7bfbd7.

📒 Files selected for processing (1)

• magicblock-chainlink/src/chainlink/fetch_cloner.rs (7 hunks)

🧰 Additional context used

🧠 Learnings (4)

📚 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/chainlink/fetch_cloner.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/src/chainlink/fetch_cloner.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/fetch_cloner.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/src/chainlink/fetch_cloner.rs

🔇 Additional comments (7)

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

77-81: LGTM!

The RefreshDecision enum cleanly encapsulates the three possible outcomes for undelegating account refresh decisions. The naming is clear and self-documenting.

---

1139-1145: Logic looks correct for the two-phase approach.

The method now returns YesAndMarkEmptyIfNotFound when no delegation record exists, deferring the actual on-chain existence check to the subsequent fetch operation. This is an appropriate design since fetch_and_clone_accounts will handle the case where the account is not found on chain by creating an empty account entry.

However, note that this will trigger a refetch even for accounts that still exist on chain but simply lack a delegation record (e.g., accounts that were never delegated or have an unusual state). This is likely acceptable given the context of undelegating accounts.

---

1221-1237: LGTM!

The match handling correctly distinguishes between the three decision outcomes:

• Yes/YesAndMarkEmptyIfNotFound: Account needs refresh (logs metric, accumulates pubkey for marking empty if applicable)
• No: Account stays in bank, excluded from fetch

The control flow ensures accounts needing refresh are not added to in_bank, so they pass through to fetch_new for re-fetching.

---

1270-1283: Clean merging of mark-empty sources.

The logic correctly combines the caller-provided mark_empty_if_not_found with the internally-detected extra_mark_empty pubkeys, handling the Option conversion appropriately.

---

3007-3072: Comprehensive test for the new functionality.

The test correctly validates the fix:

1. Sets up an undelegating account in bank with no on-chain presence
2. Verifies the account is detected as closed and replaced with an empty account
3. Confirms the subscription is maintained for future updates

This aligns well with the PR objective of "refresh stuck undelegating accounts that are closed on-chain."

---

1208-1208: LGTM!

Clean declaration for collecting pubkeys that need empty marking.

---

1161-1161: Minor edge case consideration for RefreshDecision::Yes.

When returning Yes (undelegation completed but account not marked for empty-if-not-found), there's a theoretical race condition: if the account gets closed on chain between the delegation record check and the actual fetch, it will end up in not_found_on_chain instead of being replaced with an empty account.

This is likely acceptable since:

1. The race window is small
2. Callers can check FetchAndCloneResult::not_found_on_chain
3. Subsequent fetches would correctly detect and handle the closed state

---

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

📜 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 7834bfb and 266e732.

📒 Files selected for processing (1)

• magicblock-chainlink/src/chainlink/fetch_cloner.rs (7 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 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/chainlink/fetch_cloner.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/src/chainlink/fetch_cloner.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/src/chainlink/fetch_cloner.rs

🧬 Code graph analysis (1)

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

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

• inc_unstuck_undelegation_count (582-584)

magicblock-chainlink/src/testing/utils.rs (1)

• random_pubkey (21-23)

🔇 Additional comments (2)

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

1218-1293: LGTM!

The decision handling logic correctly:

1. Tracks accounts to mark empty via extra_mark_empty
2. Keeps accounts needing refresh in pubkeys (by not adding to in_bank)
3. Merges both mark_empty sources before calling fetch_and_clone_accounts

The metric increment for both Yes and YesAndMarkEmptyIfNotFound variants accurately tracks unstuck undelegations.

---

3018-3082: LGTM!

The test comprehensively validates the new behavior:

1. Sets up an undelegating account in bank with no on-chain counterpart
2. Verifies the account is replaced with an empty account (zero lamports, empty data, system program owner)
3. Confirms the account remains subscribed for future updates

This properly exercises the YesAndMarkEmptyIfNotFound code path.

[GabrielePicco]

LGTM!