diff --git a/.agents/context/crates/magicblock-chainlink.md b/.agents/context/crates/magicblock-chainlink.md index 3595c482d..6315274ad 100644 --- a/.agents/context/crates/magicblock-chainlink.md +++ b/.agents/context/crates/magicblock-chainlink.md @@ -75,6 +75,7 @@ Important methods: - `ensure_accounts(pubkeys, mark_empty_if_not_found, fetch_origin)`: fetches/clones accounts but returns only fetch/clone status. - `fetch_accounts(pubkeys, fetch_origin)`: ensures accounts and then reads them from the local bank. - `accounts_delegated_on_base_and_er(pubkeys, fetch_origin)`: checks that each account is DLP-owned on base and represented as delegated/DLP-owned locally. +- `account_delegation_statuses(pubkeys, fetch_origin)`: returns base-layer delegation plus explicit account-on-ER status (`missing`, `delegated`, or `not_delegated`) for owner-program undelegation request logs. - `undelegation_requested(pubkey)`: called by committor/account flows before an account is undelegated so Chainlink keeps watching for base-layer completion. - `fetch_undelegation_requests()`: scans base-layer Delegation Program accounts for active `UndelegationRequest` PDAs using filtered `getProgramAccounts` and returns decoded `ObservedUndelegationRequest`s for `magicblock-accounts`. - `fetch_count()` / `is_watching()`: mainly observability/testing helpers. diff --git a/.agents/context/crates/magicblock-services.md b/.agents/context/crates/magicblock-services.md index a56a399f0..adf93cab2 100644 --- a/.agents/context/crates/magicblock-services.md +++ b/.agents/context/crates/magicblock-services.md @@ -118,12 +118,13 @@ service.stop() For each observed request, the service: 1. verifies the request PDA matches the delegated account; -2. asks Chainlink to materialize the delegated account if it is missing locally; +2. asks Chainlink to materialize the delegated account when it is delegated on base but missing locally; 3. checks the delegated account is delegated on base and ER; 4. best-effort notifies Chainlink with `undelegation_requested`; 5. submits a local validator-signed `ScheduleCommitAndUndelegate` transaction with the validator as payer/signer, `MAGIC_CONTEXT_PUBKEY`, and each delegated account as writable non-signer. Transient delegation-check and local-scheduling failures are retried three times with short exponential backoff. Invalid request PDAs and non-delegated accounts are skipped. +Skipped request logs must identify whether the delegated account was not DLP-owned on base, was missing locally, or was present locally but not delegated, so operators can distinguish invalid/stale requests from ER materialization and local-state issues. ## Runtime flows @@ -217,10 +218,12 @@ The undelegation request service is a trigger: it only schedules the local Magic The service validates the request PDA from delegated account before scheduling. It must not trust request-local owner, rent payer, or commit nonce fields; current request data carries the delegated account and expiry slot only. -Before checking base/ER delegation readiness, the service calls Chainlink `ensure_accounts` for the delegated account. This lets polling recover valid requests for accounts that are delegated on base but not yet present in the ER bank, instead of skipping the request until unrelated traffic clones the account. +Before deciding whether base/ER delegation readiness is terminally false, the service calls Chainlink `ensure_accounts` for accounts that are delegated on base but missing in the ER bank. This lets polling recover valid requests for accounts that are delegated on base but not yet present locally, instead of skipping the request until unrelated traffic clones the account. Successful materialization of this missing-on-ER case is intentionally logged at error level with an `alert` field because it should be rare and operator-visible. The service logs expired requests but still schedules normal undelegation when the delegated account is still valid. This preserves the best chance of committing ER state and clearing lifecycle state instead of leaving timeout/rollback handling to a stale request. +When the delegated-account readiness check fails, the service logs `delegated_on_base`, `account_on_er`, and a bounded `skip_reason`. `delegated_on_base_missing_on_er` indicates a valid-looking base delegation whose ER account was absent at request-processing time; `delegated_on_base_not_delegated_on_er` indicates the account was present on ER but not represented as delegated/DLP-owned; `not_delegated_on_base` indicates an invalid or stale request from the validator's current base-layer view. Successfully scheduled requests log at info level with the request PDA, delegated account, and final readiness fields. + ## Important invariants 1. `schedule` must return exactly one `Result` for each input callback, preserving input order. diff --git a/magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs b/magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs index 727cf4b4b..9ff7ceccc 100644 --- a/magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs +++ b/magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs @@ -2317,6 +2317,9 @@ where warn!( request_pda = %observed.request_pda, delegated_account = %observed.delegated_account, + observed_slot = observed.observed_slot, + expires_at_slot = observed.expires_at_slot, + drop_reason = "no_active_subscribers", "Dropped observed DLP undelegation request because no subscribers are active" ); } diff --git a/magicblock-chainlink/src/chainlink/mod.rs b/magicblock-chainlink/src/chainlink/mod.rs index 111f62377..2f5c7b80f 100644 --- a/magicblock-chainlink/src/chainlink/mod.rs +++ b/magicblock-chainlink/src/chainlink/mod.rs @@ -67,6 +67,66 @@ pub struct ObservedUndelegationRequest { pub observed_slot: u64, } +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum AccountStatusOnEr { + /// The account is missing from the ER bank, so its ER delegation state is unknown. + #[default] + Missing, + /// The account is present on ER and represented as delegated. + Delegated, + /// The account is present on ER and is not represented as delegated. + NotDelegated, +} + +impl AccountStatusOnEr { + pub fn is_delegated(&self) -> bool { + matches!(self, Self::Delegated) + } + + pub fn as_str(&self) -> &'static str { + match self { + Self::Missing => "missing", + Self::Delegated => "delegated", + Self::NotDelegated => "not_delegated", + } + } +} + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct AccountDelegationStatus { + pub delegated_on_base: bool, + pub account_on_er: AccountStatusOnEr, +} + +impl AccountDelegationStatus { + #[deprecated( + note = "use AccountDelegationStatus directly; this bool treats missing-on-ER as not delegated" + )] + pub fn delegated_on_base_and_er(&self) -> bool { + self.delegated_on_base && self.account_on_er.is_delegated() + } + + pub fn not_ready_reason(&self) -> Option<&'static str> { + #[allow(deprecated)] + let delegated_on_base_and_er = self.delegated_on_base_and_er(); + if delegated_on_base_and_er { + None + } else if !self.delegated_on_base { + Some("not_delegated_on_base") + } else { + Some(match self.account_on_er { + AccountStatusOnEr::Missing => "delegated_on_base_missing_on_er", + AccountStatusOnEr::Delegated => { + "delegated_on_base_and_er_mismatch" + } + AccountStatusOnEr::NotDelegated => { + "delegated_on_base_not_delegated_on_er" + } + }) + } + } +} + // ----------------- // Chainlink // ----------------- @@ -153,18 +213,41 @@ impl } } + #[deprecated( + note = "use AccountDelegationStatus directly; this bool treats missing-on-ER as not delegated" + )] pub async fn accounts_delegated_on_base_and_er( &self, pubkeys: &[Pubkey], fetch_origin: AccountFetchOrigin, ) -> ChainlinkResult> { + Ok(self + .account_delegation_statuses(pubkeys, fetch_origin) + .await? + .into_iter() + .map(|status| { + #[allow(deprecated)] + let delegated_on_base_and_er = + status.delegated_on_base_and_er(); + delegated_on_base_and_er + }) + .collect()) + } + + pub async fn account_delegation_statuses( + &self, + pubkeys: &[Pubkey], + fetch_origin: AccountFetchOrigin, + ) -> ChainlinkResult> { match self { Self::Enabled(chainlink) => { chainlink - .accounts_delegated_on_base_and_er(pubkeys, fetch_origin) + .account_delegation_statuses(pubkeys, fetch_origin) .await } - Self::Disabled => Ok(vec![false; pubkeys.len()]), + Self::Disabled => { + Ok(vec![AccountDelegationStatus::default(); pubkeys.len()]) + } } } @@ -542,13 +625,35 @@ impl } #[instrument(skip(self, pubkeys))] + #[deprecated( + note = "use AccountDelegationStatus directly; this bool treats missing-on-ER as not delegated" + )] pub async fn accounts_delegated_on_base_and_er( &self, pubkeys: &[Pubkey], fetch_origin: AccountFetchOrigin, ) -> ChainlinkResult> { + Ok(self + .account_delegation_statuses(pubkeys, fetch_origin) + .await? + .into_iter() + .map(|status| { + #[allow(deprecated)] + let delegated_on_base_and_er = + status.delegated_on_base_and_er(); + delegated_on_base_and_er + }) + .collect()) + } + + #[instrument(skip(self, pubkeys))] + pub async fn account_delegation_statuses( + &self, + pubkeys: &[Pubkey], + fetch_origin: AccountFetchOrigin, + ) -> ChainlinkResult> { let Some(fetch_cloner) = self.fetch_cloner() else { - return Ok(vec![false; pubkeys.len()]); + return Ok(vec![AccountDelegationStatus::default(); pubkeys.len()]); }; let remote_accounts = fetch_cloner .fetch_remote_accounts(pubkeys, fetch_origin) @@ -567,14 +672,24 @@ impl .map(|(pubkey, remote_account)| { let delegated_on_base = remote_account.is_owned_by_delegation_program(); - let delegated_on_er = self - .accounts_bank - .get_account(pubkey) - .is_some_and(|account| { - account.delegated() + let account_on_er = match self.accounts_bank.get_account(pubkey) + { + None => AccountStatusOnEr::Missing, + Some(account) => { + // Q: do we need to compare the owner? isn't delegated() alone enough? + if account.delegated() || account.owner().eq(&dlp_api::id()) - }); - delegated_on_base && delegated_on_er + { + AccountStatusOnEr::Delegated + } else { + AccountStatusOnEr::NotDelegated + } + } + }; + AccountDelegationStatus { + delegated_on_base, + account_on_er, + } }) .collect()) } diff --git a/magicblock-committor-service/src/service.rs b/magicblock-committor-service/src/service.rs index ba7a0a419..f2bac19bf 100644 --- a/magicblock-committor-service/src/service.rs +++ b/magicblock-committor-service/src/service.rs @@ -377,12 +377,15 @@ where let results = join_all( bundles.iter().map(|b| b.get_all_committed_pubkeys()).map( |pubkeys| async move { - self.chainlink + #[allow(deprecated)] + let result = self + .chainlink .accounts_delegated_on_base_and_er( &pubkeys, AccountFetchOrigin::GetAccount, ) - .await + .await; + result }, ), ) diff --git a/magicblock-services/src/undelegation_request_service.rs b/magicblock-services/src/undelegation_request_service.rs index 0742cfae3..e53938264 100644 --- a/magicblock-services/src/undelegation_request_service.rs +++ b/magicblock-services/src/undelegation_request_service.rs @@ -2,7 +2,9 @@ use std::{fmt, sync::Arc, time::Duration}; use dlp_api::pda::undelegation_request_pda_from_delegated_account; use magicblock_account_cloner::ChainlinkCloner; -use magicblock_chainlink::{ObservedUndelegationRequest, ProdChainlink}; +use magicblock_chainlink::{ + AccountStatusOnEr, ObservedUndelegationRequest, ProdChainlink, +}; use magicblock_core::{ link::transactions::TransactionSchedulerHandle, traits::LatestBlockProvider, }; @@ -111,6 +113,7 @@ impl UndelegationRequestService { Err(broadcast::error::RecvError::Lagged(skipped)) => { error!( skipped_count = skipped, + skip_reason = "broadcast_receiver_lagged", "Lagged behind undelegation request updates" ); continue; @@ -278,33 +281,14 @@ impl UndelegationRequestService { return Ok(()); } - if let Err(err) = chainlink - .ensure_accounts( - &[request.delegated_account], - None, - AccountFetchOrigin::GetAccount, - ) - .await - { - error!( - request_pda = %request.request_pda, - delegated_account = %request.delegated_account, - error = ?err, - "Failed to materialize requested undelegation account" - ); - return Err(ObservedUndelegationRequestError::Transient( - "failed to materialize requested undelegation account", - )); - } - - let delegated = match chainlink - .accounts_delegated_on_base_and_er( + let mut delegation_status = match chainlink + .account_delegation_statuses( &[request.delegated_account], AccountFetchOrigin::GetAccount, ) .await { - Ok(value) => value.into_iter().next().unwrap_or(false), + Ok(value) => value.into_iter().next().unwrap_or_default(), Err(err) => { error!( request_pda = %request.request_pda, @@ -317,11 +301,72 @@ impl UndelegationRequestService { )); } }; - if !delegated { - debug!( + + if delegation_status.delegated_on_base + && delegation_status.account_on_er == AccountStatusOnEr::Missing + { + if let Err(err) = chainlink + .ensure_accounts( + &[request.delegated_account], + None, + AccountFetchOrigin::GetAccount, + ) + .await + { + error!( + request_pda = %request.request_pda, + delegated_account = %request.delegated_account, + error = ?err, + "Failed to materialize requested undelegation account" + ); + return Err(ObservedUndelegationRequestError::Transient( + "failed to materialize requested undelegation account", + )); + } + + error!( + request_pda = %request.request_pda, + delegated_account = %request.delegated_account, + delegated_on_base = delegation_status.delegated_on_base, + account_on_er = delegation_status.account_on_er.as_str(), + alert = "materialized_missing_er_account_for_undelegation_request", + "Materialized delegated account for undelegation request because it was delegated on base but missing on ER" + ); + + delegation_status = match chainlink + .account_delegation_statuses( + &[request.delegated_account], + AccountFetchOrigin::GetAccount, + ) + .await + { + Ok(value) => value.into_iter().next().unwrap_or_default(), + Err(err) => { + error!( + request_pda = %request.request_pda, + delegated_account = %request.delegated_account, + error = ?err, + "Failed to verify materialized undelegation account" + ); + return Err(ObservedUndelegationRequestError::Transient( + "failed to verify materialized undelegation account", + )); + } + }; + } + + let delegated_on_base_and_er = delegation_status.delegated_on_base + && delegation_status.account_on_er.is_delegated(); + if !delegated_on_base_and_er { + warn!( request_pda = %request.request_pda, delegated_account = %request.delegated_account, - "Skipping request because account is not delegated on base and ER" + delegated_on_base = delegation_status.delegated_on_base, + account_on_er = delegation_status.account_on_er.as_str(), + skip_reason = delegation_status + .not_ready_reason() + .unwrap_or("delegation_status_ready"), + "Skipping observed undelegation request because delegated account is not ready" ); return Ok(()); } @@ -356,7 +401,9 @@ impl UndelegationRequestService { info!( request_pda = %request.request_pda, delegated_account = %request.delegated_account, - "Scheduled requested undelegation via ScheduleCommitAndUndelegate" + delegated_on_base = delegation_status.delegated_on_base, + account_on_er = delegation_status.account_on_er.as_str(), + "Processed observed undelegation request and scheduled undelegation" ); Ok(()) }