fix(chainlink): prevent ephermal account from being evicted#1323
Conversation
📝 WalkthroughWalkthroughThe PR prevents ephemeral accounts from being evicted via the Assessment against linked issues
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}
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/remote_account_provider/mod.rs`:
- Around line 1471-1474: The early return of `Ok(())` when skipping an ephemeral
account subscription is being treated as a successful registration by the caller
`acquire_subscription_with_mode`, which allows ownership to be recorded without
an actual `pubsub_client.subscribe()` call. This creates inconsistency where
later non-ephemeral reacquires can add the pubkey to the LRU assuming ownership
exists, but no upstream subscription does. Either return a distinct outcome (not
`Ok(())`) from the ephemeral check in the
`capacity_eviction_protection_for(pubkey).ephemeral` block to differentiate
skipped ephemeral subscriptions, or move this ephemeral short-circuit check
above any ownership or LRU mutations in the function. Additionally, add a
regression test that verifies the behavior when reacquiring the same pubkey as
non-ephemeral after an initial ephemeral acquire.
🪄 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: 838a0378-30ff-4763-97a3-cc79976f8ab6
📒 Files selected for processing (5)
magicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/fetch_cloner/tests.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-chainlink/src/remote_account_provider/lru_cache.rsmagicblock-chainlink/src/remote_account_provider/mod.rs
| if self.capacity_eviction_protection_for(pubkey).ephemeral { | ||
| trace!(pubkey = %pubkey, "Skipping subscription for ephemeral account"); | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
Don't treat skipped ephemeral acquires as successful subscriptions.
Returning Ok(()) here is still consumed as a real registration by acquire_subscription_with_mode, so ownership can be recorded without an actual pubsub_client.subscribe(). That leaves a later acquire free to re-add the pubkey to the LRU via the existing-ownership fast path while no upstream subscription exists, and rollback/release paths then operate on a nonexistent subscription. Please return a distinct outcome here, or move the ephemeral short-circuit above ownership/LRU mutation, and add a regression test for an ephemeral → non-ephemeral reacquire.
🤖 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 1471 -
1474, The early return of `Ok(())` when skipping an ephemeral account
subscription is being treated as a successful registration by the caller
`acquire_subscription_with_mode`, which allows ownership to be recorded without
an actual `pubsub_client.subscribe()` call. This creates inconsistency where
later non-ephemeral reacquires can add the pubkey to the LRU assuming ownership
exists, but no upstream subscription does. Either return a distinct outcome (not
`Ok(())`) from the ephemeral check in the
`capacity_eviction_protection_for(pubkey).ephemeral` block to differentiate
skipped ephemeral subscriptions, or move this ephemeral short-circuit check
above any ownership or LRU mutations in the function. Additionally, add a
regression test that verifies the behavior when reacquiring the same pubkey as
non-ephemeral after an initial ephemeral acquire.
1 participant
Summary
Prevent validator-native ephemeral accounts from entering chainlink's subscription eviction pipeline, so
EvictAccountis never submitted for them. This implements the proper caller-side fix for #1302, following the same layered approach used for delegated accounts in #1216.Changes in
magicblock-chainlink:should_schedule_bank_eviction()and use it in the removal listener and stale subscription-update path to skip delegated, undelegating, and ephemeral bank state.CapacityEvictionProtectionwith anephemeralflag so LRU capacity eviction skips live ephemeral entries (predicate usesephemeral && owner != defaultso post-eviction tombstones and normal cloned accounts are unaffected).Breaking Changes
Test Plan
cargo test -p magicblock-chainlink --lib ephemeralcargo test -p magicblock-chainlink --lib test_should_schedule_bank_evictionNew tests:
test_should_schedule_bank_eviction— helper covers normal, delegated, undelegating, and ephemeral accountstest_subscribe_account_removals_skips_ephemeral_and_evicts_normal— removal listener skips ephemeral, still evicts normal cloned accountstest_ephemeral_account_is_protected_from_capacity_eviction— LRU evicts a non-protected candidate instead of a live ephemeral entrytest_acquire_subscription_skips_ephemeral_account— no pubsub or LRU entry for ephemeral acquiretest_stale_subscription_update_does_not_notify_removal_for_ephemeral— stale account-sub updates do not enqueue removal for ephemeral; still do for normal accountstest_lru_cache_skips_ephemeral_protected_candidate_and_evicts_next— LRU filter unit testCloses #1302 (caller-side fix; on-chain guard from #1301 remains as defense in depth)
Summary by CodeRabbit
Bug Fixes
Tests