feat(scenarios): add GoonFi state preparation - #13
Conversation
…y slot Addresses two review comments. A persisted override was re-queued with fetch_before_use intact, so every following slot pulled the whole account from mainnet again: one RPC per slot per override, and any field the override does not write was reset to mainnet's value, discarding what local transactions had written to it. fetch_before_use is now cleared on the re-queue, but only after the write succeeds, so a failed apply still retries next slot with the fetch. The re-queue replaces a copy of itself already queued for that slot instead of bailing out, which keeps one entry per id. persist also gains the ts-bindings attribute its sibling fetch_before_use already had, and the regenerated OverrideInstance.ts exposes it - the field was previously absent from the TS SDK entirely.
…ge numbers support
…eChain/surfpool into feat/bisonfi-support
…y slot Addresses two review comments. A persisted override was re-queued with fetch_before_use intact, so every following slot pulled the whole account from mainnet again: one RPC per slot per override, and any field the override does not write was reset to mainnet's value, discarding what local transactions had written to it. fetch_before_use is now cleared on the re-queue, but only after the write succeeds, so a failed apply still retries next slot with the fetch. The re-queue replaces a copy of itself already queued for that slot instead of bailing out, which keeps one entry per id. persist also gains the ts-bindings attribute its sibling fetch_before_use already had, and the regenerated OverrideInstance.ts exposes it - the field was previously absent from the TS SDK entirely.
…ge numbers support
…y slot Addresses two review comments. A persisted override was re-queued with fetch_before_use intact, so every following slot pulled the whole account from mainnet again: one RPC per slot per override, and any field the override does not write was reset to mainnet's value, discarding what local transactions had written to it. fetch_before_use is now cleared on the re-queue, but only after the write succeeds, so a failed apply still retries next slot with the fetch. The re-queue replaces a copy of itself already queued for that slot instead of bailing out, which keeps one entry per id. persist also gains the ts-bindings attribute its sibling fetch_before_use already had, and the regenerated OverrideInstance.ts exposes it - the field was previously absent from the TS SDK entirely.
…ge numbers support
Content was already applied in 26c7b5c, which lost its second parent to a reset and so left the branch looking unmerged - every later pull replayed the same conflicts. This records the parent; the tree is unchanged.
…LimeChain/surfpool into feat/kamino-protocol-support
The Kamino PR was squash-merged into the base as 272e238, which has no ancestry link to the feat/kamino-protocol-support history already in this branch - so every merge re-derived the same six conflicts from two unrelated lineages. 272e238's tree is byte-identical to the kamino tip merged in d243443, so this records the parent and leaves the tree alone.
GoonFi is a two-account dark AMM with no IDL: a 32-byte external price oracle plus a 2048-byte market carrying the reference band that guards it. - Four raw-layout templates over the two accounts (price, reference band, freshness, stale quote), plus a live market catalog read from program accounts. - Two builders. Price moves the oracle bid and ask together with the market's reference band and a persistent freshness stamp, because the deployed program rejects a decoupled move with 0x24. Liquidity resolves both token vaults from the market's own pointers and scales their balances. - Three MCP tools on the generic scenario path: list_goonfi_markets, create_goonfi_price_scenario and create_goonfi_liquidity_scenario. No protocol HTTP endpoints. - The Slot raw encoding now takes a width, so the 4-byte freshness field no longer clobbers the decay multiplier stored beside it. - Live gated tests against the deployed program. The shared live-test helper duplicates the Tessera branch's on purpose; they fold together when the branches converge.
|
Two improvements in the PR:
Tests could not run: Rustup attempted to write into a read-only toolchain directory. |
- Catalog discovery skips a market that fails validation with a warning instead of aborting the whole scan, so one obsolete market can no longer hide every valid one. An empty catalog from a program that does own markets stays an error. - Vault balances are read through TokenAccount::unpack and checked against the market's own mint: an owner-and-length check also admits an 82-byte mint, whose bytes at the amount offset would be misread as a balance. - GoonfiMarket is non_exhaustive, so a market and its oracle can only be paired through validate, which reads the oracle from the market's own pointer.
The catalog isolation issue from the earlier review is fixed in the latest commit. Review was static; tests were not run. |
`non_exhaustive` only blocked building a GoonfiMarket from scratch outside the crate; the fields stayed public, so a caller could still re-point a validated pair's oracle. Making both private leaves `validate` as the only way to pair a market with an oracle, and it reads that oracle from the market's own pointer.
Tests could not run: rustup attempted to write to a read-only directory. |
| pub fn validate( | ||
| address: Pubkey, | ||
| market_account: &Account, | ||
| oracle_account: &Account, | ||
| ) -> SurfpoolResult<Self> { | ||
| let oracle = Self::oracle_address(market_account)?; | ||
| validate_goonfi_oracle_layout(oracle_account)?; | ||
| Ok(Self { address, oracle }) | ||
| } |
There was a problem hiding this comment.
Validator permits mismatched markets
When a direct library caller passes a valid market account and oracle account with a different market pubkey, GoonfiMarket::validate stores that unchecked pubkey. The resulting scenario writes price and freshness to the oracle derived from the validated account but writes the reference band to the unrelated market, producing a rejected or misleading cross-market scenario.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/core/src/scenarios/protocols/goonfi/v1/price.rs
Line: 85-93
Comment:
**Validator permits mismatched markets**
When a direct library caller passes a valid market account and oracle account with a different market pubkey, `GoonfiMarket::validate` stores that unchecked pubkey. The resulting scenario writes price and freshness to the oracle derived from the validated account but writes the reference band to the unrelated market, producing a rejected or misleading cross-market scenario.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.9af2fc5 to
61a9091
Compare
Stacked on #8 — GoonFi is the same mechanism class as BisonFi and Tessera (PMM, no IDL) and reuses the raw-layout engine BisonFi introduced.
0x24. Liquidity resolves both token vaults from the market's own pointers and scales their balances through the genericspl-token-account-balancetemplate.list_goonfi_markets,create_goonfi_price_scenarioandcreate_goonfi_liquidity_scenario. No protocol HTTP endpoints and no custom RPC methods.Slotraw encoding now takes a width, so GoonFi's 4-byte freshness field no longer clobbers the decay multiplier stored beside it. Existing 8-byte slot fields are unchanged.0x1, a dislocated quote with a coupled band fills at the prepared price, an aged oracle rejects with0x15, and a price moved out of its band rejects with0x24.Greptile Summary
The PR adds GoonFi market discovery and scenario preparation using raw account layouts, including coupled price/reference updates, liquidity scaling, freshness handling, and MCP integrations. The latest changes isolate malformed catalog entries and restrict direct mutation of validated market descriptors, but the public validation constructor still permits mismatched market targets.
Confidence Score: 4/5
The PR is not yet safe to merge because direct users of the public GoonfiMarket validator can still construct a price scenario whose oracle and reference-band market do not correspond.
Making GoonfiMarket fields private blocks struct-literal mismatches, but validate still accepts and stores an independent market address while deriving only the oracle from the supplied market account; the builder consequently targets the two coupled halves of the scenario using values that can refer to different markets.
Files Needing Attention: crates/core/src/scenarios/protocols/goonfi/v1/price.rs
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Caller supplies market address] --> V[GoonfiMarket::validate] M[Market account] --> V O[Oracle account] --> V V -->|oracle derived from market bytes| P[Price and freshness overrides] V -->|unchecked address argument| R[Reference-band override] P --> X[Oracle account] R --> Y[Supplied market address] X -. may not belong to .-> YPrompt To Fix All With AI
Reviews (3): Last reviewed commit: "fix(goonfi): make the validated market p..." | Re-trigger Greptile