Skip to content

feat(scenarios): add GoonFi state preparation - #13

Open
92Infinitus92 wants to merge 43 commits into
feat/bisonfi-supportfrom
feat/goonfi-support
Open

feat(scenarios): add GoonFi state preparation#13
92Infinitus92 wants to merge 43 commits into
feat/bisonfi-supportfrom
feat/goonfi-support

Conversation

@92Infinitus92

@92Infinitus92 92Infinitus92 commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #8 — GoonFi is the same mechanism class as BisonFi and Tessera (PMM, no IDL) and reuses the raw-layout engine BisonFi introduced.

  • Two raw-layout accounts per market: a 32-byte external price oracle (bid, ask, freshness, decay) and the 2048-byte market holding the reference band that guards it. Four templates over the pair — price, reference band, freshness, stale quote — plus a live catalog read from program accounts and validated at creation.
  • Two builders. Price moves the oracle bid and ask together with the market's reference band and a persistent freshness stamp, since the deployed program rejects a decoupled move with 0x24. Liquidity resolves both token vaults from the market's own pointers and scales their balances through the generic spl-token-account-balance template.
  • Three MCP tools on the generic scenario path: list_goonfi_markets, create_goonfi_price_scenario and create_goonfi_liquidity_scenario. No protocol HTTP endpoints and no custom RPC methods.
  • The Slot raw 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.
  • The oracle carries no discriminator, so the raw-layout owner predicate is what keeps a write out of a foreign 32-byte account, and every builder resolves the oracle through the market's own pointer rather than trusting a caller-supplied address.
  • Verified against the deployed program on a fork: drained vaults reject with 0x1, a dislocated quote with a coupled band fills at the prepared price, an aged oracle rejects with 0x15, and a price moved out of its band rejects with 0x24.
  • The shared live-test helper duplicates the Tessera branch's on purpose; they fold together when the branches converge.

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.

  • Adds GoonFi market and oracle raw-layout templates.
  • Adds live market discovery and price/liquidity scenario builders.
  • Exposes three GoonFi MCP tools and supporting tests.
  • Adds width-aware slot encoding for the four-byte freshness field.

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

Filename Overview
crates/core/src/scenarios/protocols/goonfi/v1/price.rs Adds coupled GoonFi price preparation, but its public validator still permits an unchecked market address to be paired with an oracle derived from another market account.
crates/core/src/scenarios/protocols/goonfi/v1/markets.rs Adds live market discovery and now isolates malformed or unresolved individual catalog entries instead of failing the complete catalog.
crates/core/src/scenarios/protocols/goonfi/v1/liquidity.rs Adds validated vault resolution, proportional balance preparation, and persistent oracle freshness for liquidity scenarios.
crates/types/src/scenarios.rs Extends raw slot encoding with explicit widths so four-byte GoonFi freshness writes preserve adjacent fields.
crates/mcp/src/surfpool/mod.rs Exposes GoonFi market listing and price/liquidity scenario creation through the generic MCP scenario path.

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 .-> Y
Loading

Fix all with Greploop Fix All in Claude Code Fix All in Codex Fix All in Cursor

Prompt To Fix All With AI
### Issue 1
crates/core/src/scenarios/protocols/goonfi/v1/price.rs:85-93
**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.

Reviews (3): Last reviewed commit: "fix(goonfi): make the validated market p..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

…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.
…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.
…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.
bakasura980 and others added 11 commits August 26, 2026 12:37
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.
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.
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Two improvements in the PR:

  • markets.rs:146: One missing oracle/mint or invalid market aborts the entire catalog. Skip invalid entries and report diagnostics so an obsolete market cannot hide all valid markets.
  • liquidity.rs:178: Vault validation accepts any token-program-owned account with at least 72 bytes—including an 82-byte mint. Unpack a token account and verify its mint matches the corresponding market mint before reading its balance.

Tests could not run: Rustup attempted to write into a read-only toolchain directory.

@92Infinitus92 92Infinitus92 self-assigned this Sep 9, 2026
Comment thread crates/core/src/scenarios/protocols/goonfi/v1/markets.rs Outdated
Comment thread crates/core/src/scenarios/protocols/goonfi/v1/price.rs
- 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.
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
  • [P2] Validated market/oracle pairing remains mutableprice.rs:65. #[non_exhaustive] prevents external struct literals, but callers can still assign market.oracle = another_oracle after validation. The builder then modifies that oracle alongside the original market’s reference band, producing an inconsistent scenario. Make both fields private and expose read-only getters.

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.
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
  • [P2] Account validation still lacks address bindingprice.rs:90. validate checks the supplied oracle’s layout but cannot establish that it belongs to the market’s oracle pointer. Private fields do not close this gap for direct library callers. Accept keyed accounts and compare addresses, or fetch the referenced account within validation.

  • [P2] Liquidity scaling can use another vault’s balanceliquidity.rs:99. Any token account with the expected mint passes validation. Passing another market’s vault calculates the percentage from its balance, then writes that amount to this market’s vault—potentially increasing liquidity during a requested drain. Validate supplied vault addresses against the market pointers and add a mismatch regression test.

Tests could not run: rustup attempted to write to a read-only directory.

Comment on lines +85 to +93
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 })
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Fix in Claude Code Fix in Codex Fix in Cursor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants