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
22 changes: 8 additions & 14 deletions apps/docs/operations/migration-keeper.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,22 +225,16 @@ chosen candidate:
before. The keeper does not duplicate the contract's ledger-gap math to
decide whether the cooldown has elapsed: if it hasn't, `migrate_adapter`
itself rejects the call with `MigrationCooldownNotMet` during simulation
(no fee, nothing sent), which is reported as a `failures` entry and
naturally retried on a later scheduled run.

This was deliberately simple rather than precise while `MIN_LEDGER_GAP`
was ~1 minute, since the cooldown had always long since elapsed by the
run after `begin_migration` fired. Now that it's ~1 day (#557), every
hourly run during that window hits the same rejection and reports it as
a `failures` entry, so a single migration currently produces roughly a
day's worth of expected-but-noisy failed runs before it can proceed.
Tracked as a follow-up to special-case `MigrationCooldownNotMet` as a
`skipped` outcome the same way a stale-adapter race already is.
(no fee, nothing sent), which is reported as a `skipped` outcome, not a
`failures` entry.

That is the intended waiting period, not an operational failure, so it
no longer pages anyone while the ~1-day cooldown (#557) is in progress.

A migration to a given candidate therefore now normally spans roughly a
day's worth of scheduled runs: the one that calls `begin_migration`, then
repeated (currently failing) attempts, and finally the run where
`migrate_adapter` succeeds once the on-chain snapshot is old enough.
day's worth of scheduled runs: the one that calls `begin_migration`, then a
quiet series of cooldown skips, and finally the run where `migrate_adapter`
succeeds once the on-chain snapshot is old enough.

## Retry And Failure Handling

Expand Down
11 changes: 11 additions & 0 deletions packages/stellar-sdk-helpers/src/keeper-tx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,17 @@ export function isStaleAdapterError(err: unknown): boolean {
return errorMessage(err).includes(STALE_ADAPTER_MESSAGE);
}

// MigrationCooldownNotMet (#20) is also a benign expected race rather than an
// operational failure: after begin_migration, the keeper intentionally waits
// for the 1-day ledger gap before it can migrate. Detecting that by the raw
// contract error code lets callers classify it as skipped instead of paging.
const MIGRATION_COOLDOWN_NOT_MET_ERROR =
/(?:^|\s)(?:MigrationCooldownNotMet|Error\(Contract, #20\))/;

export function isMigrationCooldownNotMetError(err: unknown): boolean {
return MIGRATION_COOLDOWN_NOT_MET_ERROR.test(rawErrorText(err));
}

/**
* Re-reads the vault's live `get_adapter()` and throws StaleAdapterError if
* it no longer matches what this run discovered. A cheap, best-effort guard
Expand Down
46 changes: 46 additions & 0 deletions packages/stellar-sdk-helpers/src/migration-keeper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1256,6 +1256,52 @@ describe("runMigrationKeeper", () => {
]);
});

it("treats a cooldown-in-progress MigrationCooldownNotMet rejection as a skip, not a failure", async () => {
// #557 lengthened the cooldown from ~1 minute to ~1 day. That made the
// expected waiting state last many scheduled runs, so treating it like a
// generic submission failure turns a healthy migration into a day of
// false-positive paging. It has to stay in skipped, alongside the other
// benign, expected races.
const server = makeServer({
simulateTransaction: vi.fn(async () => ({
kind: "error",
error: "Error(Contract, #20)",
})),
});
stellarMocks.getRpcServer.mockReturnValue(server);
stellarMocks.isSimulationError.mockImplementation(
(sim) => sim?.kind === "error"
);
mockLiveAdapterAndActiveSnapshot(
DISCOVERED_VAULT.currentAdapterId,
"CDEFINDEXADAPTER"
);
const rateSource = vi.fn(async ({ protocol }: { protocol: string }) =>
protocol === "blend" ? 500 : 700
);

const result = await runMigrationKeeper(CONFIG, {
logger: logger(),
discoverVaults: async () => ({
vaults: [DISCOVERED_VAULT],
failures: [],
}),
rateSource,
resolveCandidatePool: async () => "CDEFINDEXPOOL",
sleep: vi.fn(),
});

expect(result.migrations).toEqual([]);
expect(result.failures).toEqual([]);
expect(result.skipped).toMatchObject([
{
vaultId: "meridian-usdc",
reason: expect.stringContaining("MigrationCooldownNotMet; waiting"),
},
]);
expect(server.sendTransaction).not.toHaveBeenCalled();
});

it("keeps the stale-adapter skip reason informative even with real-length contract addresses", async () => {
// Regression test: sanitizeTxError redacts any message containing a
// 50+ char C-address to a generic fallback. A real Stellar address is
Expand Down
16 changes: 16 additions & 0 deletions packages/stellar-sdk-helpers/src/migration-keeper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
expectString,
isStaleAdapterError,
isTransientKeeperError,
isMigrationCooldownNotMetError,
submitKeeperOperation,
SubmissionInFlightError,
type KeeperRpcServer,
Expand Down Expand Up @@ -1239,6 +1240,21 @@ export async function runMigrationKeeper(
);
continue;
}
if (isMigrationCooldownNotMetError(err)) {
skipped.push({
vaultId: vault.vaultId,
reason:
"MigrationCooldownNotMet; waiting for the ledger-gap cooldown to elapse",
});
logger.info(
"[migration-keeper] migration skipped; ledger-gap cooldown not yet elapsed",
{
vaultId: vault.vaultId,
detail: errorMessage(err),
}
);
continue;
}
const { attempts, transient } = retryOutcome(err, isTransientKeeperError);
const failure: KeeperFailure = {
vaultId: vault.vaultId,
Expand Down
Loading