Skip to content

Migration keeper can starve a completable migration under alternating candidate rates #699

Description

@collinsezedike

Summary

The migration keeper (packages/stellar-sdk-helpers/src/migration-keeper.ts) re-derives the best rate candidate from scratch on every run, with no memory of an already-begun migration. If the top candidate flips between two adapters across consecutive scheduled runs, each run overwrites the prior begin_migration snapshot and resets the ledger-gap cooldown, so a migration can in principle never reach migrate_adapter despite a real, sustained improvement opportunity existing throughout.

Reproduction

  1. Run 1: candidate A clears minImprovementBps over the current adapter's rate. Keeper calls begin_migration(A), snapshotting A.
  2. Before run 2 (an hour later on the documented schedule), rates shift so candidate B is now best.
  3. Run 2 reads get_migration_snapshot(), sees adapter = A != best.adapterId = B, and calls begin_migration(B), overwriting A's snapshot and resetting the cooldown.
  4. If rates keep alternating between A and B across runs, no single candidate's snapshot survives long enough to reach MIN_LEDGER_GAP with a matching "best" on a later run.

Impact

Not a fund-safety issue: the vault's position is never at risk, and the contract-side stability/slippage checks in migrate_adapter still hold. The impact is purely a liveness gap: an automated migration that should happen never does, so the vault keeps earning at the old adapter's rate.

Proposed fix

Before re-deriving "best" each run, check whether an active begin_migration snapshot already exists for this vault. If it does and that snapshot's adapter still clears minImprovementBps against the current rate, prefer completing it over switching to a fresher "best" candidate. Only let the snapshot lapse (and pick a new candidate) once the snapshotted adapter's rate genuinely stops clearing the threshold.

Scope

Field Value
Area SDK (migration keeper)
Protocol affected None
Network Both
Breaking change? No

Additional Context

Found during review of #606 (two-phase migration with ledger-gap stability check). Deferred rather than folded into that PR to keep its scope to the security fix it was already carrying.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingmediumRequires familiarity with the Meridian codebase or relevant tooling; expect 4–8 hourssdkInvolves Blend or DeFindex SDK helpers in packages/stellar-sdk-helpers

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions