[92Infinitus92]

This fixes mismatched state between Phoenix Eternal accounts after a scenario plays. The templates used to write values straight into Phoenix's accounts, so the price map, the order books and the trader index could disagree. Now they run Phoenix's own instructions on a copy of the local VM and write every changed account back all or none, so the accounts stay consistent with each other.

• phoenix-market-move moves a market the way mainnet does: every oracle key reports the price, every maker re-centres its spline, and the uncross crank matches crossed orders.
• phoenix-oracle-price moves only the oracle readings, in ticks or by a percentage, and leaves the book and the makers' quotes at their old prices.
• phoenix-trader-collateral sets a trader's collateral. A hot trader first has its resting orders cancelled, so Phoenix reads the written account.
• Twelve configuration templates run Phoenix's admin instructions as the role that signs them on mainnet: risk factors, liquidation size, open interest cap, funding, fees, market and exchange status, withdrawals, trader fees and capabilities. The stop-loss and permission templates stay IDL writes.
• list_phoenix_trader_positions (MCP) reports a trader's positions and margin, and the margin after given price moves.
• Every Phoenix override keeps the markets usable in the local VM: it raises stale thresholds, restamps old readings, rewinds funding timestamps and extends spline lifespans.

svm.rs only changes the Phoenix arm, to write all or none, and exposes one IDL encoding helper. The bundled IDL is updated.

Tests: the live tests behind integration-tests send real Eternal and Hawkeye transactions from the deployed programs against mainnet accounts, keeper liquidations included. cargo test -p surfpool-core --features integration-tests -- phoenix scenarios passes, and so do the core and MCP suites.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; all three previous findings are addressed, and no new blocking issue was found.

Summary

Phoenix scenarios now prepare state through Phoenix instructions on a separate VM copy, then write the changed accounts back together.

• Phoenix market scenarios move prices, quotes, and crossed orders through separate steps.
• Trader scenarios set effective collateral and MCP reports positions and margin.
• Phoenix admin scenarios change market and exchange settings through Phoenix instructions.
• Phoenix scenario changes run on a local VM copy before accounts are written back.

_{Reviews (6) · Last reviewed commit: "fix(scenarios/phoenix): land the collate..." · Reviewed by Greptile}

[greptile-apps]

Failed account stays changed

write_all_or_none does not restore the account whose write failed. With SQLite or PostgreSQL, set_account_inner can change the VM account and then fail while saving its account indexes. This loop restores only earlier successful writes, leaving the failed account changed while related Phoenix accounts are put back. Later trades or margin checks can read that mixed state.

Save the current account's old value before attempting its write, and include it in rollback.

Knowledge Base Used: State and Surfnet execution

[92Infinitus92]

@greptileai Agreed. We'll push the current account's old value into written before attempting its write, so an account whose write changes the VM and then fails on the index update is put back with the others. If the write fails before touching the VM, putting it back is a no-op, since set_scenario_override_account returns early when the account is unchanged. We don't have a storage double that fails only on the index update, so we'd ship this without a new unit test. Does that cover it, or would you recommend another approach?

[greptile-apps]

Yes, that covers the failure mode. Capture the live VM snapshot immediately before each set_scenario_override_account call and append (pubkey, before) to the rollback journal before attempting the write; on failure, walk that journal in reverse. If the write failed before mutating the VM, restoring the current snapshot is a no-op, while a partial mutation is still reverted. Keep the Option<Account> snapshot so a partially-created account can be removed by restoring None.

One additional guard: do not silently discard rollback failures. Preserve the original write error, but include any rollback error in the returned error (or log it at minimum), since an unsuccessful rollback means the all-or-none guarantee could not be fulfilled. Given the lack of a storage fault-injection seam, I would not require a new unit test for this narrowly scoped journaling fix.

[greptile-apps]

Margin prediction lacks coverage

The new marginAfterMoves calculation has no test that runs it with a real trader and nonempty moves. The MCP test only assigns a made-up view and checks its JSON. The prediction runs only oracle_reports, while phoenix-market-move also moves splines and runs the uncross crank, so a mismatch could go unnoticed.

Add a live test that compares the reported margin with the margin after playing the matching overrides, including a hot trader.

[92Infinitus92]

@greptileai Agreed, we'll add a live test. It takes two or three traders from the GlobalTraderIndex, so hot ones, and computes their collateral from marginAfterMoves for a 2% and a 10% move. It plays the collateral override at slot 0 and the matching phoenix-market-move overrides at slot 1, then checks that each trader is healthy before the move, liquidatable and between backstop and maintenance after it, within 5% of the band from the prediction, and liquidated by a keeper. It fails when no eligible trader is found. To test the tool's own code rather than a copy of it, we'd split trader_positions into the part that builds a VM copy from the surfnet and the part that computes on a given VM, and call the second from the test. Does that address it?

[greptile-apps]

Yes, that addresses the coverage gap. A live test with hot traders, nonempty 2%/10% moves, the collateral override, matching market-move overrides, and keeper liquidation exercises the full prediction-to-execution path rather than only testing fabricated view data. Splitting the surfnet/VM-copy setup from the computation is also the right way to ensure the test calls the production trader_positions logic instead of reproducing it.

Please make the selected move adverse to each trader’s position (or cover both directions), and keep the test failing when no eligible traders are found. The assertions you listed should catch both stale/inconsistent market state and an inaccurate margin prediction.

[92Infinitus92]

@greptileai

[greptile-apps]

Collateral target is ignored

When the local index still lists a trader whose fetched account has cleared its HOT bit, collateral_writes discards the index record and writes only the Trader account. Hawkeye still reads the listed record in this state, so Play succeeds without reaching the requested effective collateral. A trader meant to become liquidatable can stay healthy.

Choose the collateral field by index membership, not the account's HOT bit. Restore coverage for the case previously tested by collateral_stress_follows_the_index_when_the_hot_flag_lags.

[92Infinitus92]

Choosing the index record alone does not fix this. Checked on mainnet data before changing anything.

How the state arises: the local index, buffer and books are loaded together on the first Phoenix load, and a Trader account only when an override first touches it. A trader that left the index upstream in between is cold in its account but still listed locally.

What each program reads in that state:

• Eternal reads the Trader account. With the index record far below zero, a keeper's ForceCancelRiskIncreasing is refused as not liquidatable.
• Hawkeye reads the index record.
• The current code writes the account. The keeper's force cancel then copies the stale index record over it, and the liquidation is refused.

Proposed fix: when the local index lists the trader and its account is cold, write the index record as for any hot trader, and set the HOT flag on the local Trader account. The account then matches the index, buffer and books it was loaded with. A trader whose flag and index agree takes the same path as today.

Checked:

• A real mainnet trader that went cold 30 s after the index was loaded. With the current code the target misses and the liquidation is refused. With the fix it lands exactly on the target, and the keeper's force cancel and liquidation both land.
• A local trader with SOL and BTC positions, lagged the same way. Deposit, withdraw, new orders, the collateral and cancel templates and a keeper liquidation all match a trader without the lag, number for number.

Is setting the HOT flag on the local account acceptable here, or is there a case where it leaves the trader in a state Phoenix itself would not produce?

[greptile-apps]

Yes. In the mixed-snapshot state described, setting the local Trader account’s HOT capability is the coherent repair.

The invariant is not merely “prefer the index record”; a trader listed in the local GlobalTraderIndex must be treated as hot throughout the simulated transaction:

• Hawkeye reads collateral from the index record.
• Eternal’s force-cancel and liquidation paths read the Trader account.
• The collateral override should therefore use the index record as the collateral source, update that record, and also write the Trader account with the HOT bit set and matching collateral.
• Both account writes should be returned together.

That reconstructs the state Phoenix would have while the trader remains active, rather than leaving a cold Trader account paired with an active index entry. Set only the HOT capability bit and preserve the other flags; continue rejecting spline makers.

The opposite-direction guard should remain: if the Trader account is HOT but absent from the local index, refuse the operation because there is no active index record from which to reconstruct state.

Given the mainnet reproduction and local equivalence test, I do not see a Phoenix-produced state that this repair would introduce. The earlier concern that selecting the index record alone was sufficient was incomplete—the lagged HOT flag is the missing part.