fix: distinguish ER account presence from delegation status#1411
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
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:
📝 WalkthroughWalkthroughThe Chainlink facade now models base-layer and ER delegation state with structured status types. Account resolution classifies missing, delegated, and present-but-not-delegated accounts, including offline defaults. The undelegation service uses these statuses for readiness checks, materializes missing accounts, and logs bounded skip reasons. Additional logs identify broadcast lag and dropped requests caused by absent subscribers. Documentation describes the new API and logging 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 |
120ee1a to
af07722
Compare
![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-services/src/undelegation_request_service.rs`:
- Around line 306-318: Differentiate logging severity in the
!delegation_status.delegated_on_base_and_er() branch: retain warn! for the
actionable not_delegated_on_base case, but use debug! or info! for
delegated_on_base_missing_on_er and other benign ER-materialization races. Use
delegation_status.not_ready_reason() or the corresponding status fields to
select the level while preserving the existing context and early return.
🪄 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: 3b01da6b-f226-4080-8673-864db6e52fc5
📒 Files selected for processing (5)
.agents/context/crates/magicblock-chainlink.md.agents/context/crates/magicblock-services.mdmagicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-services/src/undelegation_request_service.rs
af07722 to
5358558
Compare
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 @.agents/context/crates/magicblock-chainlink.md:
- Line 78: The AGENTS.md index must reflect all updated .agents documentation
entrypoints. Update entries for .agents/context/crates/magicblock-chainlink.md
lines 78-78, .agents/context/crates/magicblock-services.md lines 126-126, and
.agents/context/crates/magicblock-services.md lines 222-223 to describe the new
Chainlink facade entrypoint, readiness skip-log contract, and bounded
readiness-reason documentation respectively.
🪄 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: 16d655c5-c782-4a2e-b7ae-ff797a05cd4f
📒 Files selected for processing (5)
.agents/context/crates/magicblock-chainlink.md.agents/context/crates/magicblock-services.mdmagicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-services/src/undelegation_request_service.rs
5358558 to
f48681b
Compare
9d836d6 to
da09233
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.agents/context/crates/magicblock-services.md:
- Around line 98-116: Update the UndelegationRequestService API documentation to
include the required undelegation_request_poll_interval argument in the new(...)
example, and describe start() as potentially spawning both subscription and
polling workers rather than exactly one background task. Keep the remaining
service behavior and ownership descriptions unchanged.
In `@magicblock-chainlink/src/chainlink/mod.rs`:
- Around line 303-310: Update the Chainlink `try_new` constructor to reuse the
request sender owned by `FetchCloner` instead of creating a separate
`broadcast::channel`, ensuring
`InnerChainlink::subscribe_undelegation_requests()` receives live events. Adjust
the constructor wiring or signature as needed to pass the shared sender, and add
a constructor-level test verifying subscribers receive undelegation requests.
- Around line 654-667: Update the account status classification in the visible
accounts-bank lookup to use account.delegated() alone when deciding
AccountStatusOnEr::Delegated. Remove the account.owner() == dlp_api::id()
condition so normalized DLP-owned accounts with delegation cleared are reported
as AccountStatusOnEr::NotDelegated; leave Missing handling unchanged.
In `@magicblock-services/src/undelegation_request_service.rs`:
- Around line 370-390: Add a shared in-flight guard keyed by undelegation
request PDA to the service state, and have both the live processor started in
start() and spawn_undelegation_request_poll_processor() claim the guard before
scheduling ScheduleCommitAndUndelegate. Skip requests already claimed, and
release or clear the guard when processing finishes or the request is no longer
valid so future legitimate requests can proceed.
🪄 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: 925da263-6150-4a81-b291-7682e7203f98
📒 Files selected for processing (5)
.agents/context/crates/magicblock-chainlink.md.agents/context/crates/magicblock-services.mdmagicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-services/src/undelegation_request_service.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🤖 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 @.agents/context/crates/magicblock-services.md:
- Around line 98-116: Update the UndelegationRequestService API documentation to
include the required undelegation_request_poll_interval argument in the new(...)
example, and describe start() as potentially spawning both subscription and
polling workers rather than exactly one background task. Keep the remaining
service behavior and ownership descriptions unchanged.
In `@magicblock-chainlink/src/chainlink/mod.rs`:
- Around line 303-310: Update the Chainlink `try_new` constructor to reuse the
request sender owned by `FetchCloner` instead of creating a separate
`broadcast::channel`, ensuring
`InnerChainlink::subscribe_undelegation_requests()` receives live events. Adjust
the constructor wiring or signature as needed to pass the shared sender, and add
a constructor-level test verifying subscribers receive undelegation requests.
- Around line 654-667: Update the account status classification in the visible
accounts-bank lookup to use account.delegated() alone when deciding
AccountStatusOnEr::Delegated. Remove the account.owner() == dlp_api::id()
condition so normalized DLP-owned accounts with delegation cleared are reported
as AccountStatusOnEr::NotDelegated; leave Missing handling unchanged.
In `@magicblock-services/src/undelegation_request_service.rs`:
- Around line 370-390: Add a shared in-flight guard keyed by undelegation
request PDA to the service state, and have both the live processor started in
start() and spawn_undelegation_request_poll_processor() claim the guard before
scheduling ScheduleCommitAndUndelegate. Skip requests already claimed, and
release or clear the guard when processing finishes or the request is no longer
valid so future legitimate requests can proceed.
🪄 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: 925da263-6150-4a81-b291-7682e7203f98
📒 Files selected for processing (5)
.agents/context/crates/magicblock-chainlink.md.agents/context/crates/magicblock-services.mdmagicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-services/src/undelegation_request_service.rs
🛑 Comments failed to post (4)
.agents/context/crates/magicblock-services.md (1)
98-116: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the
UndelegationRequestServiceAPI documentation.The implementation’s constructor also requires
undelegation_request_poll_interval, but the documented call omits it. Additionally,start()may spawn both subscription and polling workers, not one background task. Correct these examples before release so callers understand the required configuration and runtime behavior.🤖 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 @.agents/context/crates/magicblock-services.md around lines 98 - 116, Update the UndelegationRequestService API documentation to include the required undelegation_request_poll_interval argument in the new(...) example, and describe start() as potentially spawning both subscription and polling workers rather than exactly one background task. Keep the remaining service behavior and ownership descriptions unchanged.magicblock-chainlink/src/chainlink/mod.rs (2)
303-310: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Share the FetchCloner request sender in
try_new.
FetchCloner::newcreates its own broadcast channel, while this constructor creates a second channel. Subscribers fromInnerChainlink::subscribe_undelegation_requests()therefore receive no live events when this constructor is used. Reuse the FetchCloner’s sender or require the shared sender here; add a constructor-level test.🤖 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/mod.rs` around lines 303 - 310, Update the Chainlink `try_new` constructor to reuse the request sender owned by `FetchCloner` instead of creating a separate `broadcast::channel`, ensuring `InnerChainlink::subscribe_undelegation_requests()` receives live events. Adjust the constructor wiring or signature as needed to pass the shared sender, and add a constructor-level test verifying subscribers receive undelegation requests.
654-667: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the target file and search related symbols echo "== Outline ==" ast-grep outline magicblock-chainlink/src/chainlink/mod.rs --view expanded || true echo "== Search related symbols ==" rg -n "AccountStatusOnEr|delegated\(|dlp_api::id\(|unresolved-DLP|unresolved DLP|NotDelegated|Delegated" magicblock-chainlink/src/chainlink/mod.rs magicblock-chainlink/src -g '!**/target/**' || true echo "== Relevant slices from the target file ==" sed -n '600,720p' magicblock-chainlink/src/chainlink/mod.rsRepository: magicblock-labs/magicblock-validator
Length of output: 34018
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== DLP-owned but not delegated paths ==" sed -n '1320,1375p' magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs echo "== Nearby delegation handling in fetch_cloner pipeline ==" sed -n '330,390p' magicblock-chainlink/src/chainlink/fetch_cloner/pipeline.rs echo "== Tests around DLP-owned accounts and delegation state ==" sed -n '5930,6010p' magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs echo "== More tests around unresolved/delegation state transitions ==" sed -n '2238,2270p' magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs echo "== Exact source for remote_account owner/delegation helpers ==" sed -n '80,120p' magicblock-chainlink/src/remote_account_provider/remote_account.rs sed -n '250,285p' magicblock-chainlink/src/remote_account_provider/remote_account.rsRepository: magicblock-labs/magicblock-validator
Length of output: 10793
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== AccountDelegationStatus and consumers ==" sed -n '70,125p' magicblock-chainlink/src/chainlink/mod.rs sed -n '210,255p' magicblock-chainlink/src/chainlink/mod.rs echo "== Search for account_delegation_statuses consumers ==" rg -n "account_delegation_statuses\(|delegated_on_base_and_er\(|not_ready_reason\(" magicblock-chainlink/src -g '!**/target/**' || true echo "== Undelegation/request logic around this state ==" sed -n '700,780p' magicblock-chainlink/src/chainlink/mod.rsRepository: magicblock-labs/magicblock-validator
Length of output: 7067
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== not_ready_reason consumers ==" rg -n "not_ready_reason\(" magicblock-chainlink/src -g '!**/target/**' || true echo "== delegated_on_base_and_er consumers ==" rg -n "delegated_on_base_and_er\(" magicblock-chainlink/src -g '!**/target/**' || true echo "== AccountStatusOnEr::NotDelegated usages ==" rg -n "AccountStatusOnEr::NotDelegated|not_delegated_on_er|delegated_on_base_not_delegated_on_er|delegated_on_base_and_er_mismatch" magicblock-chainlink/src -g '!**/target/**' || trueRepository: magicblock-labs/magicblock-validator
Length of output: 1413
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Tests that mention account_delegation_statuses or not_ready_reason ==" rg -n "account_delegation_statuses|not_ready_reason|delegated_on_base_not_delegated_on_er|delegated_on_base_and_er_mismatch|not_delegated_on_base_missing_on_er" magicblock-chainlink/src/chainlink/mod.rs magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs -g '!**/target/**' || true echo "== Surrounding tests near the target area in mod.rs ==" sed -n '880,960p' magicblock-chainlink/src/chainlink/mod.rsRepository: magicblock-labs/magicblock-validator
Length of output: 4273
Treat DLP ownership and delegation as separate states here.
normalize_unresolved_dlp_clone_request()clearsdelegated()while keepingowner() == dlp_api::id(), so this branch still reports those accounts asAccountStatusOnEr::Delegatedand hides the newNotDelegatedcase. Useaccount.delegated()alone, or add a test/invariant for the DLP-owned exception.🤖 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/mod.rs` around lines 654 - 667, Update the account status classification in the visible accounts-bank lookup to use account.delegated() alone when deciding AccountStatusOnEr::Delegated. Remove the account.owner() == dlp_api::id() condition so normalized DLP-owned accounts with delegation cleared are reported as AccountStatusOnEr::NotDelegated; leave Missing handling unchanged.magicblock-services/src/undelegation_request_service.rs (1)
370-390: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map relevant files and symbols. git ls-files 'magicblock-services/src/*' | sed -n '1,200p' printf '\n--- search ---\n' rg -n "subscribe_undelegation_requests|spawn_undelegation_request_poll_processor|undelegation_request_processor|ScheduleCommitAndUndelegate|undelegation" magicblock-services/src -SRepository: magicblock-labs/magicblock-validator
Length of output: 5743
🏁 Script executed:
#!/bin/bash set -euo pipefail file=magicblock-services/src/undelegation_request_service.rs wc -l "$file" printf '\n--- 90-380 ---\n' sed -n '90,380p' "$file" | cat -nRepository: magicblock-labs/magicblock-validator
Length of output: 13378
🏁 Script executed:
#!/bin/bash set -euo pipefail file=magicblock-services/src/undelegation_request_service.rs printf -- '--- 291-410 ---\n' sed -n '291,410p' "$file" | cat -n printf -- '\n--- guard/state search ---\n' rg -n "in-flight|inflight|dedup|duplicate|undelegation_requested|request_pda|ScheduleCommitAndUndelegate|poll_interval" magicblock-services/src -SRepository: magicblock-labs/magicblock-validator
Length of output: 8438
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect chainlink tracking and request fetching to see whether they dedupe already. rg -n "fn undelegation_requested|undelegation_requested\\(|fetch_undelegation_requests|subscribe_undelegation_requests|observed_undelegation" . -SRepository: magicblock-labs/magicblock-validator
Length of output: 3482
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect chainlink implementations around the relevant methods. sed -n '700,780p' magicblock-chainlink/src/chainlink/mod.rs | cat -n printf '\n--- fetch_cloner ---\n' sed -n '330,390p' magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs | cat -nRepository: magicblock-labs/magicblock-validator
Length of output: 5879
🏁 Script executed:
#!/bin/bash set -euo pipefail printf -- '--- fetch_cloner remainder ---\n' sed -n '390,470p' magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs | cat -n printf -- '\n--- tests search ---\n' rg -n "same_slot|duplicate|undelegation request|ScheduleCommitAndUndelegate|undelegation_requested" magicblock-chainlink/tests test-integration -SRepository: magicblock-labs/magicblock-validator
Length of output: 6230
Deduplicate live and polling undelegation processing
magicblock-services/src/undelegation_request_service.rs:370-390start()runs both processors, but neither path shares an in-flight guard keyed by request PDA before schedulingScheduleCommitAndUndelegate. The same request can be observed by both loops and enqueue duplicate local intents until the on-chain request disappears.🤖 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-services/src/undelegation_request_service.rs` around lines 370 - 390, Add a shared in-flight guard keyed by undelegation request PDA to the service state, and have both the live processor started in start() and spawn_undelegation_request_poll_processor() claim the guard before scheduling ScheduleCommitAndUndelegate. Skip requests already claimed, and release or clear the guard when processing finishes or the request is no longer valid so future legitimate requests can proceed.
13222e9 to
c5cf78b
Compare
c5cf78b to
6ef7468
Compare
da09233 to
61e875d
Compare
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 855-858: Replace the `.expect()` on `request` in the
`CloneClaim::Owner` handling with explicit error handling that preserves the
existing ownership-claim flow and avoids panicking when the request is absent.
Return or propagate an appropriate error using the surrounding function’s
established error type and control flow.
In `@magicblock-committor-service/src/service.rs`:
- Around line 380-388: Replace the deprecated
`Chainlink::accounts_delegated_on_base_and_er` call in the surrounding
account-fetch flow with the `account_delegation_statuses` API, adapting its
arguments and result handling to preserve the existing behavior, then remove the
`#[allow(deprecated)]` attribute.
🪄 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: 2444b2c1-60c8-457b-b5c2-45aa305e7bfb
📒 Files selected for processing (6)
.agents/context/crates/magicblock-chainlink.md.agents/context/crates/magicblock-services.mdmagicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-committor-service/src/service.rsmagicblock-services/src/undelegation_request_service.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 855-858: Replace the `.expect()` on `request` in the
`CloneClaim::Owner` handling with explicit error handling that preserves the
existing ownership-claim flow and avoids panicking when the request is absent.
Return or propagate an appropriate error using the surrounding function’s
established error type and control flow.
In `@magicblock-committor-service/src/service.rs`:
- Around line 380-388: Replace the deprecated
`Chainlink::accounts_delegated_on_base_and_er` call in the surrounding
account-fetch flow with the `account_delegation_statuses` API, adapting its
arguments and result handling to preserve the existing behavior, then remove the
`#[allow(deprecated)]` attribute.
🪄 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: 2444b2c1-60c8-457b-b5c2-45aa305e7bfb
📒 Files selected for processing (6)
.agents/context/crates/magicblock-chainlink.md.agents/context/crates/magicblock-services.mdmagicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-committor-service/src/service.rsmagicblock-services/src/undelegation_request_service.rs
🛑 Comments failed to post (2)
magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs (1)
855-858: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove
.expect()and handle the error gracefully or justify with invariants.As per path instructions, using
.expect()or.unwrap()in production code must be treated as a major issue. Whilerequestis only taken in theCloneClaim::Ownerbranch which currently always returns and doesn't loop back (making thisexpecttheoretically safe), please replace this with proper error handling or provide explicit justification for this invariant to avoid potential panics if the loop structure changes in the future.🛡️ Proposed fix
- if self.local_account_satisfies_clone_request( - request - .as_ref() - .expect("request must be present before ownership claim"), - ) { + let Some(req_ref) = request.as_ref() else { + return Err(ClonerError::FailedToCloneRegularAccount( + pubkey, + Box::new(ClonerError::CommittorServiceError( + "missing request before ownership claim".to_string(), + )), + )); + }; + if self.local_account_satisfies_clone_request(req_ref) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let Some(req_ref) = request.as_ref() else { return Err(ClonerError::FailedToCloneRegularAccount( pubkey, Box::new(ClonerError::CommittorServiceError( "missing request before ownership claim".to_string(), )), )); }; if self.local_account_satisfies_clone_request(req_ref) {🤖 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 855 - 858, Replace the `.expect()` on `request` in the `CloneClaim::Owner` handling with explicit error handling that preserves the existing ownership-claim flow and avoids panicking when the request is absent. Return or propagate an appropriate error using the surrounding function’s established error type and control flow.Source: Path instructions
magicblock-committor-service/src/service.rs (1)
380-388: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider migrating to
account_delegation_statusesdirectly.Since
accounts_delegated_on_base_and_eris being deprecated in this PR, consider switching this call to the newaccount_delegation_statusesAPI to avoid the#[allow(deprecated)]attribute.♻️ Proposed refactor
- #[allow(deprecated)] - let result = self - .chainlink - .accounts_delegated_on_base_and_er( - &pubkeys, - AccountFetchOrigin::GetAccount, - ) - .await; - result + let statuses = self + .chainlink + .account_delegation_statuses( + &pubkeys, + AccountFetchOrigin::GetAccount, + ) + .await; + statuses.map(|s| { + s.into_iter() + .map(|status| status.delegated_on_base && status.account_on_er.is_delegated()) + .collect::<Vec<bool>>() + })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let statuses = self .chainlink .account_delegation_statuses( &pubkeys, AccountFetchOrigin::GetAccount, ) .await; statuses.map(|s| { s.into_iter() .map(|status| status.delegated_on_base && status.account_on_er.is_delegated()) .collect::<Vec<bool>>() })🤖 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-committor-service/src/service.rs` around lines 380 - 388, Replace the deprecated `Chainlink::accounts_delegated_on_base_and_er` call in the surrounding account-fetch flow with the `account_delegation_statuses` API, adapting its arguments and result handling to preserve the existing behavior, then remove the `#[allow(deprecated)]` attribute.
6ef7468 to
5df5201
Compare
0abab77 to
e31d2f3
Compare
5df5201 to
d48e2a6
Compare
e31d2f3 to
fee54b7
Compare
d48e2a6 to
8df6616
Compare
7df4dcf to
c90c4f4
Compare
fee54b7 to
7454fa5
Compare
c90c4f4 to
a9d793e
Compare
f863bd9 to
81bd61b
Compare
a9d793e to
0e5845c
Compare
0e5845c to
c3f6035
Compare
* master: fix: distinguish ER account presence from delegation status (#1411) fix: refuse impossible sized intents (#1390) feat: Impl UndelegationRequest backfill loop (#1378) fix: increase RPC fetch retries to survive RPC lag behind pubsub tip (#1425) feat: Schedule undelegation from observed DLP requests (#1357) feat: add id for actions (#1409) release: 0.13.7 (#1423) refactor: move LatestBlockInner into core to be able to expose subscribe in LatestBlockProvider trait (#1405) Fix buffer preparation signature aliasing (#1420) feat: add ephemeral-system-program that handles ephemeral account lifecycle (#1415) fix: do not warn about the first false positive blockhash divergence (#1418)
2 participants
This PR deprecates
accounts_delegated_on_base_and_er()as it incorrectly treats "missing-account-on-ER" as "not delegated" and provides better API that separatesMissingfromNotDelegated. Seeenum AccountStatusOnEr.Summary by CodeRabbit
New Features
Bug Fixes
Observability