Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .agents/context/crates/magicblock-committor-service.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ IntentExecutorImpl::execute
-> TaskBuilderImpl::commit_tasks + finalize_tasks
-> fetch next commit nonces, delegation metadata, and diffable base accounts using max(remote_slot)
-> persist commit_id for each committed account
-> create commit, commit-finalize, undelegate, finalize, and action tasks
-> create commit, commit-finalize, finalize, explicit undelegate, and action tasks
-> TaskStrategist::build_execution_strategy
-> try single transaction when total task count <= 22 and it fits
-> optimize large tasks to buffers when needed
Expand All @@ -203,7 +203,7 @@ IntentExecutorImpl::execute
-> reset nonce cache for all committed pubkeys on errors, or only undelegated pubkeys on successful undelegation
```

Task building fetches `DelegationMetadata` for accounts whose base-layer flow may finalize. `FinalizeTask` and `CommitFinalizeTask` include the delegated account owner and `DelegationMetadata.rent_payer` so the Delegation Program can auto-undelegate owner-program requests during finalize, commit-finalize, and commit-finalize-from-buffer. The active task-building path derives the request PDA through the DLP instruction builders and does not fetch or decode `UndelegationRequest`; explicit undelegate tasks still pass `request_rent_payer = None` unless a future path deliberately restores request-account decoding.
Task building fetches `DelegationMetadata` for accounts whose base-layer flow may finalize. `FinalizeTask` and `CommitFinalizeTask` include the delegated account owner and `DelegationMetadata.rent_payer` so the Delegation Program can auto-undelegate owner-program and validator requests during finalize, commit-finalize, and commit-finalize-from-buffer. Validator-requested `CommitAndUndelegate` no longer emits a trailing `UndelegateTask`; post-undelegate base actions are still scheduled after the finalize task. The active task-building path derives the request PDA through the DLP instruction builders and does not fetch or decode `UndelegationRequest`; explicit undelegate tasks still pass `request_rent_payer = None` unless a future path deliberately restores request-account decoding.

Transaction fit is not only packet size. `TaskStrategist` currently checks whether a single-stage transaction or each two-stage transaction fits the wire size, optionally after switching commits to buffers and adding ALTs, but it does not split a transaction stage by compute units. Each commit, finalize, commit-finalize, and undelegate task currently advertises `120_000` CU, while Agave caps a transaction at `1_400_000` CU. Keep task bundles below that transaction-level cap unless the strategist and executor/output/persistence model are extended to split, record, and confirm multiple transactions for the affected stage.

Expand Down
28 changes: 15 additions & 13 deletions .agents/specs/validator-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,15 @@ This is a critical lifecycle invariant. Do not allow normal ER transactions to k

Owner-program requested undelegation is recorded by the Delegation Program in
`DelegationMetadata.undelegation_requester = OwnerProgram` and an
`UndelegationRequest` PDA. This marker is not a validator-side completion
signal: the validator must still commit/finalize the latest ER state. For
commit/finalize, commit-finalize, and commit-finalize-from-buffer paths, the
committor supplies the DLP auto-undelegation accounts using the delegated
account's owner program and `DelegationMetadata.rent_payer`; it derives the
request PDA for those instruction metas and does not need to fetch or decode
`UndelegationRequest` during task construction.
`UndelegationRequest` PDA. Validator-requested undelegation is recorded as
`DelegationMetadata.undelegation_requester = Validator`. These markers are not
validator-side completion signals: the validator must still commit/finalize the
latest ER state. For commit/finalize, commit-finalize, and
commit-finalize-from-buffer paths, the committor supplies the DLP
auto-undelegation accounts using the delegated account's owner program and
`DelegationMetadata.rent_payer`; it derives the request PDA for those
instruction metas and does not need to fetch or decode `UndelegationRequest`
during task construction.

The validator observes owner-program `UndelegationRequest` PDAs through
Chainlink and `magicblock-accounts`. Live request-account subscription updates
Expand All @@ -274,12 +276,12 @@ accounts with a filtered `getProgramAccounts` every configured interval
(`chainlink.undelegation-request-poll-interval`, default `5m`) and feeds the
decoded requests into the same `ScheduleCommitAndUndelegate` scheduling path.

If DLP sees `OwnerProgram` requester during a successful finalize path, it
finalizes the committed state and undelegates/closes the delegation/request PDAs
in the same instruction flow. An explicit undelegate instruction later in the
same transaction may no-op once ownership has already returned to the owner
program. `AlreadyUndelegated` remains a conflict for validator-requested
undelegation state, not a reason to drop an owner-program request.
If DLP sees `OwnerProgram` or `Validator` requester during a successful
finalize-style path, it finalizes the committed state and undelegates/closes the
delegation PDAs in the same instruction flow. Owner-program requests also close
the request PDA and reimburse its rent payer. Validator-requested
`CommitAndUndelegate` should rely on this auto-undelegation behavior and must
not append a separate explicit `Undelegate` task for the same account.

### Callback discriminator

Expand Down
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ magicblock-committor-program = { path = "./magicblock-committor-program", featur
magicblock-committor-service = { path = "./magicblock-committor-service" }
magicblock-config = { path = "./magicblock-config" }
magicblock-core = { path = "./magicblock-core" }
magicblock-delegation-program-api = { git = "https://github.com/magicblock-labs/delegation-program.git", rev = "72af1c7" }
magicblock-delegation-program-api = { git = "https://github.com/magicblock-labs/delegation-program.git", rev = "ccb7d57" }
magicblock-ledger = { path = "./magicblock-ledger" }
magicblock-magic-program-api = { path = "./magicblock-magic-program-api" }
magicblock-metrics = { path = "./magicblock-metrics" }
Expand Down
120 changes: 119 additions & 1 deletion magicblock-committor-service/src/tasks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,11 @@ mod serialization_safety_test {

use dlp_api::{
discriminator::DlpDiscriminator,
pda::undelegation_request_pda_from_delegated_account,
pda::{
fees_vault_pda, undelegate_buffer_pda_from_delegated_account,
undelegation_request_pda_from_delegated_account,
validator_fees_vault_pda_from_validator,
},
};
use magicblock_core::intent::CommittedAccount;
use magicblock_program::{
Expand Down Expand Up @@ -716,6 +720,120 @@ mod serialization_safety_test {
assert!(!ix.accounts[13].is_signer);
}

#[test]
fn test_finalize_task_includes_auto_undelegation_accounts() {
let validator = Pubkey::new_unique();
let delegated_account = Pubkey::new_unique();
let owner_program = Pubkey::new_unique();
let rent_reimbursement = Pubkey::new_unique();

let ix = FinalizeTask {
delegated_account,
owner_program,
rent_reimbursement,
}
.instruction(&validator);

assert_eq!(ix.program_id, dlp_api::id());
assert_eq!(ix.data, DlpDiscriminator::Finalize.to_vec());
assert_eq!(ix.accounts.len(), 13);
assert_eq!(
ix.accounts[6].pubkey,
validator_fees_vault_pda_from_validator(&validator)
);
assert_eq!(ix.accounts[8].pubkey, owner_program);
assert_eq!(
ix.accounts[9].pubkey,
undelegate_buffer_pda_from_delegated_account(&delegated_account)
);
assert_eq!(ix.accounts[10].pubkey, rent_reimbursement);
assert_eq!(ix.accounts[11].pubkey, fees_vault_pda());
assert_eq!(
ix.accounts[12].pubkey,
undelegation_request_pda_from_delegated_account(&delegated_account)
);
}

fn make_commit_finalize_task() -> CommitFinalizeTask {
let delegated_account = Pubkey::new_unique();
let owner_program = Pubkey::new_unique();
CommitFinalizeTask {
commit_id: 1,
allow_undelegation: true,
committed_account: CommittedAccount {
pubkey: delegated_account,
account: Account {
lamports: 1_000,
data: vec![1, 2, 3],
owner: owner_program,
executable: false,
rent_epoch: 0,
},
remote_slot: Default::default(),
},
delivery: CommitDelivery::StateInArgs,
rent_reimbursement: Pubkey::new_unique(),
}
}

#[test]
fn test_commit_finalize_task_includes_auto_undelegation_accounts() {
let validator = Pubkey::new_unique();
let task = make_commit_finalize_task();
let delegated_account = task.committed_account.pubkey;
let owner_program = task.committed_account.account.owner;
let rent_reimbursement = task.rent_reimbursement;

let ix = task.instruction(&validator);

assert_eq!(ix.program_id, dlp_api::id());
assert!(ix
.data
.starts_with(&DlpDiscriminator::CommitFinalize.to_vec()));
assert_eq!(ix.accounts.len(), 11);
assert_eq!(ix.accounts[6].pubkey, owner_program);
assert_eq!(
ix.accounts[7].pubkey,
undelegate_buffer_pda_from_delegated_account(&delegated_account)
);
assert_eq!(ix.accounts[8].pubkey, rent_reimbursement);
assert_eq!(ix.accounts[9].pubkey, fees_vault_pda());
assert_eq!(
ix.accounts[10].pubkey,
undelegation_request_pda_from_delegated_account(&delegated_account)
);
}

#[test]
fn test_commit_finalize_from_buffer_task_includes_auto_undelegation_accounts(
) {
let validator = Pubkey::new_unique();
let mut task = make_commit_finalize_task();
assert!(task.try_optimize_tx_size());
let delegated_account = task.committed_account.pubkey;
let owner_program = task.committed_account.account.owner;
let rent_reimbursement = task.rent_reimbursement;

let ix = task.instruction(&validator);

assert_eq!(ix.program_id, dlp_api::id());
assert!(ix
.data
.starts_with(&DlpDiscriminator::CommitFinalizeFromBuffer.to_vec()));
assert_eq!(ix.accounts.len(), 12);
assert_eq!(ix.accounts[7].pubkey, owner_program);
assert_eq!(
ix.accounts[8].pubkey,
undelegate_buffer_pda_from_delegated_account(&delegated_account)
);
assert_eq!(ix.accounts[9].pubkey, rent_reimbursement);
assert_eq!(ix.accounts[10].pubkey, fees_vault_pda());
assert_eq!(
ix.accounts[11].pubkey,
undelegation_request_pda_from_delegated_account(&delegated_account)
);
}

fn make_buffer_commit_task(
commit_id: u64,
allow_undelegation: bool,
Expand Down
47 changes: 8 additions & 39 deletions magicblock-committor-service/src/tasks/task_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{
tasks::{
commit_task::{CommitDelivery, CommitTask},
BaseActionTask, BaseActionTaskV1, BaseActionTaskV2, BaseTaskImpl,
CommitFinalizeTask, FinalizeTask, UndelegateTask,
CommitFinalizeTask, FinalizeTask,
},
};

Expand Down Expand Up @@ -172,9 +172,6 @@ impl TaskBuilderImpl {
[
intent_bundle.get_commit_intent_pubkeys(),
intent_bundle.get_undelegate_intent_pubkeys(),
intent_bundle
.intent_bundle
.get_commit_finalize_and_undelegate_intent_pubkeys(),
]
.into_iter()
.flatten()
Expand Down Expand Up @@ -360,21 +357,6 @@ impl TasksBuilder for TaskBuilderImpl {
.into()
}

// Helper to create an undelegate task
fn undelegate_task(
account: &CommittedAccount,
rent_reimbursement: &Pubkey,
request_rent_payer: Option<Pubkey>,
) -> BaseTaskImpl {
UndelegateTask {
delegated_account: account.pubkey,
owner_program: account.account.owner,
rent_reimbursement: *rent_reimbursement,
request_rent_payer,
}
.into()
}

// Helper to process commit types
fn create_finalize_tasks(
commit: &CommitType,
Expand Down Expand Up @@ -417,29 +399,16 @@ impl TasksBuilder for TaskBuilderImpl {
}
}

fn create_undelegate_tasks(
fn create_post_undelegate_action_tasks(
commit_and_undelegate: &CommitAndUndelegate,
delegation_metadata: &HashMap<Pubkey, DelegationMetadata>,
) -> TaskBuilderResult<Vec<BaseTaskImpl>> {
let accounts = commit_and_undelegate.get_committed_accounts();

let mut tasks = accounts
.iter()
.map(|account| {
let rent_reimbursement = rent_reimbursement(
delegation_metadata,
account.pubkey,
)?;
Ok(undelegate_task(account, &rent_reimbursement, None))
})
.collect::<TaskBuilderResult<Vec<_>>>()?;

) -> Vec<BaseTaskImpl> {
if let UndelegateType::WithBaseActions(actions) =
&commit_and_undelegate.undelegate_action
{
tasks.extend(TaskBuilderImpl::create_action_tasks(actions));
TaskBuilderImpl::create_action_tasks(actions).collect()
} else {
Vec::new()
}
Ok(tasks)
}

let mut tasks = Vec::new();
Expand Down Expand Up @@ -470,13 +439,13 @@ impl TasksBuilder for TaskBuilderImpl {
&value.commit_action,
&delegation_metadata,
)?);
tasks.extend(create_undelegate_tasks(value, &delegation_metadata)?);
tasks.extend(create_post_undelegate_action_tasks(value));
}

if let Some(ref value) =
intent_bundle.intent_bundle.commit_finalize_and_undelegate
{
tasks.extend(create_undelegate_tasks(value, &delegation_metadata)?);
tasks.extend(create_post_undelegate_action_tasks(value));
}
Comment on lines 445 to 449

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add direct test coverage for commit_finalize_and_undelegate in finalize_tasks.

task_strategist.rs gained tests for commit_and_undelegate (auto-undelegate + post-undelegate BaseAction ordering), but no equivalent test exercises this commit_finalize_and_undelegate branch through finalize_tasks() to confirm create_post_undelegate_action_tasks output for it. The mod.rs tests only cover CommitFinalizeTask::instruction() account layout, not this wiring 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-committor-service/src/tasks/task_builder.rs` around lines 445 -
449, Add direct test coverage for the `commit_finalize_and_undelegate` path in
`finalize_tasks()` so the wiring to `create_post_undelegate_action_tasks` is
exercised, not just `CommitFinalizeTask::instruction()` account layout. Add a
test in the same area as `finalize_tasks`/`task_builder` that builds an intent
bundle with `commit_bundle.commit_finalize_and_undelegate`, calls
`finalize_tasks()`, and asserts the returned tasks include the expected
post-undelegate BaseAction ordering. Use the existing
`create_post_undelegate_action_tasks`, `finalize_tasks`, and
`commit_finalize_and_undelegate` symbols to locate the branch and verify it
behaves like the `commit_and_undelegate` coverage.


Ok(tasks)
Expand Down
Loading
Loading