fix: delegated account evictions#1216
Draft
thlorenz wants to merge 22 commits into
Draft
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds predicate-driven LRU eviction (AddAccountOutcome / add_with_evict_filter), exposes a runtime capacity-eviction protection predicate in RemoteAccountProvider, and uses that predicate when registering subscriptions to avoid evicting delegated/undelegating accounts. It introduces an API to release a specific delegated-account subscription reason without tearing down tracking ownership, updates FetchCloner to register the protection predicate and call targeted delegated cleanup helpers during subscription updates and delegation resolution, updates account-removal handling to respect protection, and adds tests covering eviction and delegated-cleanup semantics. Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs`:
- Line 1622: The assertion currently uses removed_rx.try_recv().is_err(), which
accepts both Empty and Disconnected; change it to assert only the Empty variant
(e.g. assert!(matches!(removed_rx.try_recv(),
Err(std::sync::mpsc::TryRecvError::Empty))) or assert_eq!(removed_rx.try_recv(),
Err(std::sync::mpsc::TryRecvError::Empty))). Update the assertion at the line
referencing removed_rx and ensure the TryRecvError variant is either
fully-qualified or imported.
In `@magicblock-chainlink/src/remote_account_provider/mod.rs`:
- Around line 621-646: Replace the .expect() calls on the RwLock for
capacity_eviction_protection with proper error handling: change
set_capacity_eviction_protection to return a Result<(),
std::sync::PoisonError<std::sync::RwLockWriteGuard<_>>> and propagate lock
errors instead of panicking, and change capacity_eviction_protection_for to
return Result<CapacityEvictionProtection,
std::sync::PoisonError<std::sync::RwLockReadGuard<_>>> so callers can handle
poisoned locks; locate uses of capacity_eviction_protection.read()/write() in
set_capacity_eviction_protection and capacity_eviction_protection_for and
replace .expect(...) with map_err/and_then (or ? operator) to convert the
PoisonError into the function Result, preserving the current default behavior
only when the lock is successfully acquired.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c1e023cb-9f8f-4f68-b087-8b3da8c321e6
📒 Files selected for processing (8)
magicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/fetch_cloner/pipeline.rsmagicblock-chainlink/src/chainlink/fetch_cloner/tests.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-chainlink/src/remote_account_provider/errors.rsmagicblock-chainlink/src/remote_account_provider/lru_cache.rsmagicblock-chainlink/src/remote_account_provider/mod.rsmagicblock-chainlink/src/remote_account_provider/tests.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs`:
- Around line 620-625: The delegated-update cleanup paths are prematurely
dropping SubscriptionReason::UndelegationTracking; remove or guard calls to
cleanup_undelegation_tracking_for_delegated_account from the delegated-account
cleanup flows (where
cleanup_direct_subscription_for_delegated_account(pubkey).await is invoked) so
that only direct subscriptions are removed there, and only call
cleanup_undelegation_tracking_for_delegated_account when undelegation has been
observed as complete (i.e., in the bank-flip/undelegation-completion handling).
Update the three affected call sites that currently call both
cleanup_direct_subscription_for_delegated_account and
cleanup_undelegation_tracking_for_delegated_account to either drop the latter
call or add a clear predicate that verifies undelegation completion before
releasing SubscriptionReason::UndelegationTracking.
In `@magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs`:
- Around line 1640-1700: Add an end-to-end test that reproduces the
acquire-before-bank-update race by using setup/FetcherTestCtx to acquire an
UndelegationTracking subscription via
remote_account_provider.acquire_subscription, then deliver a delegated account
update that flows through fetch_cloner.process_subscription_update (instead of
calling cleanup_direct_subscription_for_delegated_account in isolation), and
only after that mark the bank account as undelegating so the code path that
currently releases UndelegationTracking is exercised; finally assert the
tracking subscription is still present (use
remote_account_provider.pubsub_client().subscriptions_union(),
release_subscription_with_mode(..., SubscriptionReleaseMode::All) and
assert_not_subscribed! as in the existing test to verify correct behavior).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d2357c9b-90e1-4620-aa57-58c230365a08
📒 Files selected for processing (4)
magicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/fetch_cloner/tests.rsmagicblock-chainlink/src/remote_account_provider/mod.rsmagicblock-chainlink/src/remote_account_provider/tests.rs
* master: feat: normalize gRPC backfill (#1201)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@magicblock-chainlink/src/remote_account_provider/mod.rs`:
- Around line 1245-1258: The code removes local tracking via
subscription_ownership.lock().await.remove(pubkey) and
lrucache_subscribed_accounts.remove(pubkey) before calling
pubsub_client.unsubscribe(*pubkey).await and then always returning
NoEvictableSubscriptionCapacity on failure; this can leave a live subscription
with no local record. Fix by performing the unsubscribe first (call
pubsub_client.unsubscribe(*pubkey).await) and only remove the ownership/LRU
entries after unsubscribe succeeds, or if you must remove first then re-insert
the entries on unsubscribe Err to restore state; in either case
surface/propagate the unsubscribe error instead of always mapping it to
NoEvictableSubscriptionCapacity so callers see teardown failures.
- Around line 1490-1523: The restore logic currently re-adds ownership and LRU
entries even when unsubscribe fails with
RemoteAccountProviderError::AccountSubscriptionDoesNotExist; move the block that
reacquires ownership (the code that locks subscription_ownership, checks
is_none_or(SubscriptionOwnership::is_empty), re-inserts ownership and loops for
released_count to call ownership.acquire(reason)) so it only runs in the Err
branch after confirming the error is not AccountSubscriptionDoesNotExist (i.e.,
perform the matches! check first, return Ok(false) for
AccountSubscriptionDoesNotExist, and only then restore ownership and LRU
handling for other errors), keeping the Ok(()) path unchanged and referencing
pubsub_client.unsubscribe, lrucache_subscribed_accounts, subscription_ownership,
SubscriptionOwnership::is_empty, released_count, and
AccountSubscriptionDoesNotExist.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c92c4af-39f6-4fa6-b8d8-a83c2e34f036
📒 Files selected for processing (4)
magicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/fetch_cloner/tests.rsmagicblock-chainlink/src/remote_account_provider/errors.rsmagicblock-chainlink/src/remote_account_provider/mod.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
magicblock-chainlink/src/remote_account_provider/mod.rs (2)
1491-1509:⚠️ Potential issue | 🟠 MajorClear the LRU entry when the upstream subscription is already missing.
The
AccountSubscriptionDoesNotExistbranch on Lines 1503-1509 no longer restores ownership, but it still leavespubkeyinlrucache_subscribed_accounts. That makesis_watching()and future capacity-eviction decisions treat a dead subscription as live. Remove the LRU entry before returningOk(false).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@magicblock-chainlink/src/remote_account_provider/mod.rs` around lines 1491 - 1509, When handling the Err branch from self.pubsub_client.unsubscribe in the match, remove the pubkey from lrucache_subscribed_accounts before returning Ok(false) so dead subscriptions don't count as watched; i.e., in the RemoteAccountProvider::... unsubscribe handling where you match RemoteAccountProviderError::AccountSubscriptionDoesNotExist(_), call self.lrucache_subscribed_accounts.remove(pubkey) (and optionally emit a trace log referencing pubkey and reason) and then return Ok(false).
620-644:⚠️ Potential issue | 🟠 MajorAvoid panicking on poisoned protection-lock access.
Lines 625-637 still use
.expect()in production code. Please switch these lock acquisitions to a recovery or propagation path instead of aborting the provider on a poisonedRwLock. As per coding guidelines, "Treat any usage of.unwrap()or.expect()in production Rust code as a MAJOR issue."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@magicblock-chainlink/src/remote_account_provider/mod.rs` around lines 620 - 644, The two methods set_capacity_eviction_protection and capacity_eviction_protection_for currently call .expect() on RwLock guards and must not panic on poisoning; change both to recover from poisoning instead: in set_capacity_eviction_protection use write().or_else(|poison| Ok(poison.into_inner())).map(|mut guard| *guard = Some(Arc::new(predicate))).expect only if absolutely necessary but prefer to propagate an error or log and continue; in capacity_eviction_protection_for use read().or_else(|poison| Ok(poison.into_inner())).map(|guard| guard.as_ref().map(|p| p(pubkey)).unwrap_or(CapacityEvictionProtection{delegated:false,undelegating:false})).unwrap_or_else(|_| CapacityEvictionProtection{delegated:false,undelegating:false}) so a poisoned lock returns the safe default rather than aborting.magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs (1)
704-712:⚠️ Potential issue | 🟠 MajorKeep
UndelegationTrackinguntil undelegation is actually complete.This path still drops
SubscriptionReason::UndelegationTrackingon any delegated update. But Lines 2384-2387 explicitly say that reason can be acquired before the bank reflectsundelegating, so a delegated update in that window will clear the protection and reopen the capacity-eviction race this PR is fixing. Keep this cleanup out of the generic delegated path, or gate it on confirmed undelegation completion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs` around lines 704 - 712, The delegated-update branch is prematurely removing SubscriptionReason::UndelegationTracking; instead, modify the delegated path (the block using account.delegated()) to NOT call cleanup_undelegation_tracking_for_redelegated_account(pubkey) unconditionally. Gate that cleanup on confirmed undelegation completion (i.e., verify the bank/account state shows undelegating/undelegation finished) or move the cleanup out of this generic delegated-update path so UndelegationTracking remains until undelegation is actually complete; keep cleanup_direct_subscription_for_delegated_account(pubkey) behavior as-is or adjust only if undelegation completion is confirmed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@magicblock-chainlink/src/remote_account_provider/mod.rs`:
- Around line 1213-1242: The eviction handler currently removes local ownership
before awaiting pubsub unsubscribe, which can leave local state inconsistent if
unsubscribe fails; in the AddAccountOutcome::Evicted branch, move the call to
self.subscription_ownership.lock().await.remove(&evicted) until after a
successful self.pubsub_client.unsubscribe(evicted).await, and if unsubscribe
returns Err then reinsert or leave the ownership entry intact (or explicitly
re-add it) and do not call self.send_removal_update(evicted).await;
alternatively propagate the unsubscribe error instead of proceeding so
listen_for_account_updates() won’t keep receiving updates for what you think is
evicted. Ensure changes reference the existing symbols:
AddAccountOutcome::Evicted handling, self.subscription_ownership.lock(),
self.pubsub_client.unsubscribe(...).await, and
self.send_removal_update(...).await.
---
Duplicate comments:
In `@magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs`:
- Around line 704-712: The delegated-update branch is prematurely removing
SubscriptionReason::UndelegationTracking; instead, modify the delegated path
(the block using account.delegated()) to NOT call
cleanup_undelegation_tracking_for_redelegated_account(pubkey) unconditionally.
Gate that cleanup on confirmed undelegation completion (i.e., verify the
bank/account state shows undelegating/undelegation finished) or move the cleanup
out of this generic delegated-update path so UndelegationTracking remains until
undelegation is actually complete; keep
cleanup_direct_subscription_for_delegated_account(pubkey) behavior as-is or
adjust only if undelegation completion is confirmed.
In `@magicblock-chainlink/src/remote_account_provider/mod.rs`:
- Around line 1491-1509: When handling the Err branch from
self.pubsub_client.unsubscribe in the match, remove the pubkey from
lrucache_subscribed_accounts before returning Ok(false) so dead subscriptions
don't count as watched; i.e., in the RemoteAccountProvider::... unsubscribe
handling where you match
RemoteAccountProviderError::AccountSubscriptionDoesNotExist(_), call
self.lrucache_subscribed_accounts.remove(pubkey) (and optionally emit a trace
log referencing pubkey and reason) and then return Ok(false).
- Around line 620-644: The two methods set_capacity_eviction_protection and
capacity_eviction_protection_for currently call .expect() on RwLock guards and
must not panic on poisoning; change both to recover from poisoning instead: in
set_capacity_eviction_protection use write().or_else(|poison|
Ok(poison.into_inner())).map(|mut guard| *guard =
Some(Arc::new(predicate))).expect only if absolutely necessary but prefer to
propagate an error or log and continue; in capacity_eviction_protection_for use
read().or_else(|poison| Ok(poison.into_inner())).map(|guard|
guard.as_ref().map(|p|
p(pubkey)).unwrap_or(CapacityEvictionProtection{delegated:false,undelegating:false})).unwrap_or_else(|_|
CapacityEvictionProtection{delegated:false,undelegating:false}) so a poisoned
lock returns the safe default rather than aborting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b23aa767-6b4f-4fbd-a847-bf5a951d4115
📒 Files selected for processing (3)
magicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/remote_account_provider/lru_cache.rsmagicblock-chainlink/src/remote_account_provider/mod.rs
Comment on lines
+1213
to
+1242
| AddAccountOutcome::Evicted(evicted) => { | ||
| trace!(evicted = %evicted, "Evicting account"); | ||
|
|
||
| // LRU eviction is a forced full removal. Drop all ownership reasons | ||
| // before awaiting on pubsub unsubscribe or removal notification so stale | ||
| // reasons cannot survive a later failure on this cold path. | ||
| self.subscription_ownership.lock().await.remove(&evicted); | ||
|
|
||
| // 1. Unsubscribe from the account directly (LRU has already removed it) | ||
| if let Err(err) = self.pubsub_client.unsubscribe(evicted).await | ||
| { | ||
| if matches!( | ||
| err, | ||
| RemoteAccountProviderError::AccountSubscriptionDoesNotExist( | ||
| _ | ||
| ) | ||
| ) { | ||
| debug!(evicted = %evicted, error = ?err, "Failed to unsubscribe from pubsub for evicted account"); | ||
| } else { | ||
| // Should we retry here? | ||
| warn!(evicted = %evicted, error = ?err, "Failed to unsubscribe from pubsub for evicted account"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 2. Inform upstream so it can remove it from the store. Failure | ||
| // to notify is non-fatal here because the LRU and pubsub state have | ||
| // already been updated consistently. | ||
| if let Err(err) = self.send_removal_update(evicted).await { | ||
| warn!(evicted = %evicted, error = ?err, "Failed to send removal update for evicted account"); | ||
| // 2. Inform upstream so it can remove it from the store. Failure | ||
| // to notify is non-fatal here because the LRU and pubsub state have | ||
| // already been updated consistently. | ||
| if let Err(err) = self.send_removal_update(evicted).await { | ||
| warn!(evicted = %evicted, error = ?err, "Failed to send removal update for evicted account"); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't finalize an eviction after unsubscribe fails.
On Line 1219 we drop local ownership before Line 1222 awaits unsubscribe(). If that unsubscribe fails transiently, the provider now believes the account is gone, send_removal_update() still tells upstream to evict it, and listen_for_account_updates() can still forward later updates from the still-live pubsub subscription. Please either roll local state back and suppress the removal update, or surface the teardown failure instead of continuing as a successful eviction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@magicblock-chainlink/src/remote_account_provider/mod.rs` around lines 1213 -
1242, The eviction handler currently removes local ownership before awaiting
pubsub unsubscribe, which can leave local state inconsistent if unsubscribe
fails; in the AddAccountOutcome::Evicted branch, move the call to
self.subscription_ownership.lock().await.remove(&evicted) until after a
successful self.pubsub_client.unsubscribe(evicted).await, and if unsubscribe
returns Err then reinsert or leave the ownership entry intact (or explicitly
re-add it) and do not call self.send_removal_update(evicted).await;
alternatively propagate the unsubscribe error instead of proceeding so
listen_for_account_updates() won’t keep receiving updates for what you think is
evicted. Ensure changes reference the existing symbols:
AddAccountOutcome::Evicted handling, self.subscription_ownership.lock(),
self.pubsub_client.unsubscribe(...).await, and
self.send_removal_update(...).await.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs`:
- Around line 712-720: The current code calls
cleanup_undelegation_tracking_for_redelegated_account(pubkey) and
cleanup_direct_subscription_for_delegated_account(pubkey) immediately for
delegated accounts, which can drop ownership before the local refresh succeeds;
instead, move these cleanup calls to occur only after a successful local refresh
(i.e. after handle_executable_sub_update(...) /
clone_account_with_ownership(...) completes successfully), or if you keep them
here ensure you restore the UndelegationTracking/DirectAccount on any refresh
error; adjust the await ordering so cleanup happens after the successful return
from handle_executable_sub_update or clone_account_with_ownership and reference
the same pubkey and self methods
(cleanup_undelegation_tracking_for_redelegated_account,
cleanup_direct_subscription_for_delegated_account) when moving/restoring logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5a5cd01d-73b7-4184-94a2-b734557190f9
📒 Files selected for processing (3)
magicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/fetch_cloner/tests.rsmagicblock-chainlink/src/chainlink/mod.rs
Comment on lines
+712
to
+720
| // Delegated non-undelegating subscription cleanup is the primary | ||
| // mechanism for removing known delegated accounts from direct | ||
| // subscription/LRU ownership; undelegation tracking owns protected | ||
| // subscriptions while undelegation is in progress. | ||
| if account.delegated() { | ||
| self.unsubscribe_from_delegated_account(pubkey).await; | ||
| self.cleanup_undelegation_tracking_for_redelegated_account(pubkey) | ||
| .await; | ||
| self.cleanup_direct_subscription_for_delegated_account(pubkey) | ||
| .await; |
Contributor
There was a problem hiding this comment.
Defer redelegation cleanup until the local refresh succeeds.
Lines 717-720 release both UndelegationTracking and DirectAccount before handle_executable_sub_update / clone_account_with_ownership runs. If that later refresh fails, the bank stays stale but the last subscription owner may already be gone, so the account can stop receiving the updates needed to recover. Move these releases after a successful local refresh, or restore them on the error path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs` around lines 712 -
720, The current code calls
cleanup_undelegation_tracking_for_redelegated_account(pubkey) and
cleanup_direct_subscription_for_delegated_account(pubkey) immediately for
delegated accounts, which can drop ownership before the local refresh succeeds;
instead, move these cleanup calls to occur only after a successful local refresh
(i.e. after handle_executable_sub_update(...) /
clone_account_with_ownership(...) completes successfully), or if you keep them
here ensure you restore the UndelegationTracking/DirectAccount on any refresh
error; adjust the await ordering so cleanup happens after the successful return
from handle_executable_sub_update or clone_account_with_ownership and reference
the same pubkey and self methods
(cleanup_undelegation_tracking_for_redelegated_account,
cleanup_direct_subscription_for_delegated_account) when moving/restoring logic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Prevent delegated accounts from being capacity-evicted while preserving the subscription cleanup path for accounts that are safely delegated and not undelegating.
Note: Avoiding delegated-account eviction races adds meaningful orchestration complexity:
cleanup must only drop direct delegated-account subscriptions when safe, while undelegation
must retain or restore protected subscription ownership. This PR handles that coordination, but
we may want to revisit whether dropping delegated-account subscriptions is still worth the
complexity now that gRPC subscriptions are working reliably.
Details
magicblock-chainlink
Adds capacity eviction protection for delegated accounts by allowing the remote account provider to consult the local account bank before evicting subscriptions. Delegated accounts that are currently undelegating are treated as protected so they remain subscribed while undelegation is in progress.
Delegated-account cleanup is routed through a silent direct-subscription release path. This lets the fetch cloner remove direct subscription ownership for accounts that are already delegated without disrupting undelegation-driven subscription ownership.
The subscription LRU now supports filtered capacity eviction so protected entries can be skipped while still freeing capacity from eligible subscriptions. Diagnostics and errors were expanded to make failed capacity eviction attempts easier to understand.
Tests
Adds coverage for silent delegated direct cleanup, protected capacity eviction, delegated fetch-cloner behavior, and the interaction between delegated cleanup and undelegation protection.
Summary by CodeRabbit
New Features
Bug Fixes
Tests