magicblock-labs / magicblock-labs/magicblock-validator

Switch failed undelegations to two-stage execution

Open
#1,421 0 comments 0 reactions 1 assignee Claimed by @snawaz View on GitHub
bug
Dominant language
Rust
Stars
58
Forks
58
Avg merge
22h 1m
Merged PRs (30d)
53

Description

## Introduction
Undelegations depend on user integration, hence they may fail and not because of our code.

## Problem
Some of the Intents get optimized to be executed in single tx, see SingleStageExecutor(SingleStageIntentExecutor in outbox branch).
As result the whole intent fails. This is a problem as `Commit` should still succeed.

This also could be a verctor of attack in case of forced undelegation. User maliciously creates failing undelegate callback, requests undelegation via Base layer and we fail to fulfill it.

## Definition of done
If undelegation fails account still has to be at least committed.

## Proposed fix
If intent was executed in SingleStage manner and failed on undelegation we should switch into two stage flow and at least commit(NOTE: otherwise case is already properly handled as it is TwoStage execution).

To properly do it outbox has to be properly patched `OutboxIntentBundle` via outbox-program, otherwise this won't be durable across restarts. For that `OutboxIntentBundleStatus` would have to be patched from `ExecutionStage::SingleStage` -> `ExecutionStage::TwoStage`. After that `IntentExecutionEngine` would be able to pick it up on retry and execute via `TwoStageIntentExecutor` flow automatically.

### Current obstacles
1. Patching is part of https://github.com/magicblock-labs/magicblock-validator/discussions/1422 and is not yet implemented. Ideally this shall be implemented after patching is enabled.
2. Switching `ExecutionStage::SingleStage` -> `ExecutionStage::TwoStage` requires TwoStage to hold `TwoStageProgress::Committing(PendingTransaction)`. The issue is that PendingTransaction - unknown as we didn't execute intent in TwoStage flow yet. IMO using PendingTransaction from SingleStage is incorrect as TwoStage shouldn't be aware of those, otherwise on restarts TwoStage should be aware that PendingTransaction could be from SingleStage and handle cases appropriatly. When TwoStage sees undelegation error it automatically assumes Finalize Stage(2nd transaction), but in our case that whould be tx from SingleStage. As a hacky soliution `TwoStageProgress::Committing(PendingTransaction::default())` could work as those signature and hashes don't exist we would retry from the start, but this costly as requires extra RPC calls. Maybe direction should be dictated by `OutboxIntentBundleStatus::Accepted`, where it would become OutboxIntentBundleStatus::Accepted(ExecutionStrategy) or something

**Commit / version**
master

**Deployment**
Local (native)

**OS / Arch**
_No response_

**Logs / Links**
_No response_

## Acceptance criteria

- [ ] The outcome is implemented and validated.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.