Repository navigation
feat(scenarios): add PancakeSwap CLMM templates - #24
92Infinitus92 wants to merge 13 commits into
Conversation
ff84d74 to
d41bc68
Compare
51b8819 to
38e7797
Compare
Shared IDL definitions match Raydium’s exactly. Tests could not run because rustup attempted to write to a read-only directory. |
|
No confirmed bugs found in the PR changes. The three core account layouts match Raydium’s, and the new IDL discriminators are correct. One improvement: the materializer test omits I couldn’t run the Rust tests: rustup attempted to write to a read-only directory. |
|
No concrete bugs found in the specified PR changes. One improvement: add an offline account fixture for the byte-level materializer test in pancakeswap/mod.rs. Currently, that coverage requires mainnet access and Validation: IDL layouts and discriminators checked; diff whitespace checks passed. Rust tests could not run because rustup attempted to write to a read-only directory. |
|
No concrete bugs found in the PR changes. The PDA definitions and One improvement: add an offline fixture test for the Review was static; tests were not run. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR adds PancakeSwap CLMM templates (pancakeswap-clmm-custom, -pool-state, -amm-config), its vendored IDL, a how-to README, registry wiring and tests. Unit tests check that all 22 fee-tier PDAs derive to their documented addresses. Live tests fetch real mainnet accounts: one round-trips a pool, one checks each AmmConfig against its tier, and one confirms an override changes only the targeted bytes.
Overall the PR is clean and mirrors the Raydium CLMM setup closely. The IDL address matches the program id, the status bit meanings match the program, and the template count and protocol assertions are updated consistently.
Issues
- Unrelated CI change:
.github/workflows/claude-review.ymlis not mentioned in the PR description and has nothing to do with PancakeSwap.- It should go in its own PR so it can be reviewed separately.
- It pastes the PR title and body straight into the prompt (
${{ github.event.pull_request.body }}), so anyone who can open a same-repo PR can inject instructions into the review. The read-only token and limitedallowedToolscontain the damage, but the risk should be stated in that PR.
- Duplicated
llm_context: the PRICE/TICK, TICK ARRAYS, LIQUIDITY RULE and VAULTS blocks appear word for word in both pool templates, and are probably close copies of the Raydium v3 text too. If the engine supports shared/protocol-level context or YAML anchors (&/*), using them would stop these copies from drifting apart. - Owner check: the README admits that a PancakeSwap template pointed at a Raydium pool writes it without complaint.
pancakeswap-clmm-pool-statetakes any pubkey, so a cheap check would help: reject the override, or at least warn, when the forked account's owner is notHpNfyc…. This could be a follow-up. - Minor README gap: in the "Number formats" table, the
sqrt_price_x64andliquidityrows have an empty Example cell. Fill them in or drop the column for those rows.
| smaller than token_mint_1's (compare the decoded bytes, not the base58 string). | ||
| The pool PDA is ["pool", amm_config, token_mint_0, token_mint_1] in that exact | ||
| order, so swapping the two mints derives a different, almost certainly empty, | ||
| address. token_mint_0/token_mint_1 are plain property_ref seeds here and the |
There was a problem hiding this comment.
The template doesn't check the mint order, and the README admits a reversed pair silently derives an empty address. With fetchBeforeUse: true, does the materializer then write a brand-new account at that address, or fail? If it writes one, the user ends up with a bogus 1544-byte account that no swap uses. Either way the scenario 'succeeds' while doing nothing useful. It's worth confirming what happens, and ideally the resolver should sort the two property_ref mints (or reject unsorted ones) for CLMM pool seeds.
| - path: status | ||
| label: Pool Status | ||
| description: "Disable bitmask (u8): bit0 open/increase liquidity, bit1 decrease liquidity, bit2 collect fee, bit3 collect reward, bit4 swap; 0 = all enabled" | ||
| llm_context: | |
There was a problem hiding this comment.
Lines 345–383 repeat lines 245–299 of the custom template almost word for word, and those are close copies of the Raydium v3 text. Consider a YAML anchor (llm_context: &clmm_ctx) or shared protocol-level context so fixes to the liquidity/tick rules land in one place.
|
|
||
| #[test] | ||
| fn pancakeswap_template_derives_a_live_pool() { | ||
| let registry = TemplateRegistry::new(); |
There was a problem hiding this comment.
The test name says derives_a_live_pool, but it only does an offline PDA derivation. Two suggestions:
- Rename it, e.g.
pancakeswap_custom_template_derives_known_pool_address. - Add a comment naming the pair.
CGiwPp7b…is not documented anywhere else in the PR, so it's unclear which pool1Piz3kL…is and why the mints are in this order.
| const SOL_USDC_POOL: Pubkey = | ||
| Pubkey::from_str_const("DJNtGuBGEQiUCWE8F981M2C3ZghZt2XLD8f2sQdZ6rsZ"); | ||
|
|
||
| const LIQUIDITY_RANGE: std::ops::Range<usize> = 237..253; |
There was a problem hiding this comment.
These hard-coded offsets (237..253, 253..269, 269..273, 389) duplicate the IDL layout. A cheap guard would help:
- Assert the live pool's length is 1544 (as the yaml documents) and that it starts with the
PoolStatediscriminator. - Then a layout upgrade fails with a clear message, not as an unexplained diff of bytes outside the overridden fields.
|
|
||
| | You'll see | It means | Example | | ||
| |---|---|---| | ||
| | `sqrt_price_x64` | `sqrt(price_raw) x 2^64`, as a decimal **string** | | |
There was a problem hiding this comment.
The Example cells for sqrt_price_x64 and liquidity are empty. Add a sample value (e.g. the SOL/USDC pool's current sqrt price) or remove the column for these rows.
| Pull request title and body: | ||
| ---- | ||
| ${{ github.event.pull_request.title }} | ||
| ${{ github.event.pull_request.body }} |
There was a problem hiding this comment.
This workflow is unrelated to the PancakeSwap feature and should move to its own PR. It also puts the untrusted PR body directly into the model prompt. The read-only token and restricted tools limit the impact, but prompt injection can still steer the posted review. Consider stating that risk, or leaving the body out.
a9eba96 to
691c199
Compare
…he liquidity rule to the custom template
4e3b1ea to
1fd2035
Compare
There was a problem hiding this comment.
Overview. This PR adds PancakeSwap CLMM support: a vendored IDL, three templates (pancakeswap-clmm-custom, -pool-state and -amm-config), a README, and unit and integration tests. It also adds a collection-level llm_context that is prepended to each template's own context, and it expands the description of the MCP call_surfnet_rpc params.
What I checked.
- The hard-coded byte offsets match the Raydium CLMM
PoolState/AmmConfiglayout:PoolState: mints at 73 and 105,token_vault_0at 137,liquidityat 237,sqrt_price_x64at 253,tick_currentat 269,statusat 389, size 1544.AmmConfig:trade_fee_rateat 47,tick_spacingat 51, size 117.
- The PDA seeds and the price/tick formulas are consistent.
- The tick-crossing rule in the README is correct (up:
old < t <= new, add; down:new < t <= old, subtract). - The tests are well targeted. Deriving every fee tier's address offline, and asserting that only the targeted bytes change, are good guards.
Concerns and nits.
- Out-of-scope workflow.
.github/workflows/claude-review.ymlis unrelated to PancakeSwap and isn't mentioned in the PR body. Consider splitting it into its own PR. As written it is reasonably hardened (read-only token for the Claude job, fork PRs skipped, pinned SHAs). However, the PR title and body are interpolated straight into the prompt, so a same-repo author can steer the review text. - Shared
llm_contextreaches every template. It is now also prepended topancakeswap-clmm-amm-config, which gets the full pool-search and price-shock guidance (see the inline comment). - Tests depend on the public mainnet RPC. The integration tests rely on
api.mainnet-beta.solana.com, which is often rate-limited. They are feature-gated, so this is acceptable, but it's worth noting for CI flakiness.
Overall this looks correct and ready, apart from the scope and nit items above.
| description: Select a token mint address from verified tokens | ||
| source: verified_tokens | ||
|
|
||
| llm_context: | |
There was a problem hiding this comment.
This collection-level llm_context is now prepended to every template, including pancakeswap-clmm-amm-config. That template doesn't need the pool-search, fetchBeforeUse and price/liquidity guidance, which costs tokens and may lead an agent to look up pools when it only wants to change fees. Consider moving the PRICE paragraph into the two pool templates' own llm_context, or noting here that the pool guidance only applies to the pool templates.
| let account_type = entry | ||
| .idl_account_name | ||
| .unwrap_or_else(|| default_account_type.clone()); | ||
| let llm_context = match (&self.llm_context, entry.llm_context) { |
There was a problem hiding this comment.
Nit: when only own is present, it is returned untrimmed. When both are present, shared is trimmed but own is not. The result is fine for YAML block scalars, but (None, own) => own, (shared, None) => shared.clone() would read more clearly than own.or_else(|| shared.clone()) inside a catch-all arm.
| pub method: String, | ||
| #[schemars(description = "The parameters to pass to the RPC method")] | ||
| #[schemars( | ||
| description = "The parameters to pass to the RPC method. Wrap every value at every depth by its type: a string as {\"String\":\"...\"}, a whole number as {\"Number\":5}, a boolean as {\"Bool\":true}, a list as {\"Array\":[...]} and an object as {\"Object\":{\"key\":<wrapped value>}}; object keys stay plain. Example for getProgramAccounts with a filter: [{\"String\":\"<program id>\"},{\"Object\":{\"encoding\":{\"String\":\"base64\"},\"filters\":{\"Array\":[{\"Object\":{\"memcmp\":{\"Object\":{\"offset\":{\"Number\":0},\"bytes\":{\"String\":\"<base58>\"}}}}}]}}}]" |
There was a problem hiding this comment.
The description lists String/Number/Bool/Array/Object but leaves out Null, which is a unit variant serialized as the plain string "Null". That one is easy to get wrong. It also doesn't say that Number is i64, so u64 values above i64::MAX and all fractional numbers must be passed as strings. Mentioning both would save the LLM a failed call.
| are numerically smaller, not the smaller base58 string. A reversed pair derives an empty address | ||
| and nothing checks it. | ||
|
|
||
| **Check the owner.** The layouts match Raydium's, so a PancakeSwap template pointed at a Raydium |
There was a problem hiding this comment.
This warns about the owner mismatch, but nothing in the templates enforces it. Since the IDL is resolved by account owner, a pancakeswap-clmm-pool-state override on a Raydium pool silently succeeds. If the engine can compare the template's program id with the fetched owner, a warning there would turn this limitation into a detectable error. Worth a follow-up issue if it's out of scope here.
| fee_rate_bps: 10 | ||
| derived_address: 5szFcjKA7UU93nhgb3fE1o1cfcurwpFQEzywZyb2qAYF | ||
| - id: index_11 | ||
| label: "Fee tier 0.15% (tick spacing 10)" |
There was a problem hiding this comment.
Ou ou ou, let's move these hardcoded values to the front-end please, and here to make it generic so one could provide it from externally
There was a problem hiding this comment.
Overview
Adds PancakeSwap CLMM templates (YAML and vendored IDL), a collection-level llm_context that gets prepended to each template's own context, live integration tests, and a shared diff_indices test helper. It also bundles some unrelated changes: a new claude-review.yml workflow and a rewrite of the MCP call_surfnet_rpc params description.
I checked the hard-coded byte offsets against the PoolState layout and they're correct (amm_config 9, mints 73/105, vault0 137, liquidity 237, sqrt_price 253, tick 269, status 389). The README math also checks out: the sqrt_price_x64 example for 100 USDC/SOL, the tick formula, and the rule for which ticks are crossed and what happens to liquidity_net. The llm_context merge in to_override_templates is simple and has a unit test.
Issues
- The PR description no longer matches the code. It lists a
pancakeswap-clmm-customtemplate and "22 fee tiers by config index, address derived". After 22d0247 there are only 2 templates, andamm-configis addressed by a plain pubkey. Please update the body. The test-plan line "check every fee tier against its live AmmConfig" is out of date too: the tests now check only the SOL/USDC pool's config. - Unrelated changes. The CI workflow (
.github/workflows/claude-review.yml) and the MCP schema description are separate concerns. They'd be easier to review and revert as their own PRs. The workflow puts the PR body straight into the prompt; fork PRs are skipped, so this only matters for same-repo branches. - The shared
llm_contextrepeats itself. The collection context (pool lookup plus the PRICE rule) is now prepended toamm-configas well. Both that and the template's own context explain how to readamm_configat offset 9, so there's some duplication. That's fine, but it's worth knowing before you add more templates. - Minor: CLMM pools store mints ordered so that
mint0 < mint1. Telling the LLM to sort the two mints by bytes and run onegetProgramAccountscall would replace the "try both orders" instruction and halve the RPC calls.
|
|
||
| PancakeSwap's concentrated-liquidity AMM on Solana, a fork of Raydium CLMM with the same account | ||
| layouts. Two templates: a pool, and the fee tier a pool points to. Units and rules match | ||
| [Raydium CLMM](../raydium/v3/README.md); only the program, pools and fee tiers differ. |
There was a problem hiding this comment.
../raydium/v3/README.md doesn't exist in this branch: protocols/raydium/v3/ only has idl.json and overrides.yaml. If #21 adds that README, make sure this PR merges after it. Otherwise the link is broken, and the line "Units and rules match Raydium CLMM" sends readers to nothing.
|
|
||
| Set fetchBeforeUse: true. | ||
|
|
||
| PRICE: do not move a pool's price by hand. sqrt_price_x64, tick_current and liquidity must change |
There was a problem hiding this comment.
The collection-level context tells the LLM never to move the price by hand, yet pancakeswap-clmm-pool-state exposes sqrt_price_x64, tick_current and liquidity as editable properties right below. An LLM (or a user in the form UI) gets mixed signals. Consider one of these:
- drop
sqrt_price_x64andtick_currentfrom the template's properties, keeping only the coupled preset path in Studio; or - state the coupling inside the property descriptions too (e.g. on
liquidity: "must be recomputed from tick arrays when the price moves"), so non-LLM users see the warning.
| PancakeSwap CLMM, program HpNfyc2Saw7RKkQd8nEL4khUcuPhQ7WwY1B2qjx8jxFq, a fork of Raydium CLMM. | ||
|
|
||
| FIND A POOL BY PAIR: resolve both mints with get_token_address, then call getProgramAccounts on the | ||
| program with filters dataSize 1544 and memcmp of the two mints at offsets 73 and 105, once in each |
There was a problem hiding this comment.
CLMM requires mint0 < mint1 (by bytes), so the order is deterministic. Telling the LLM to sort the two mints and run a single getProgramAccounts call is cheaper and less error-prone than "once in each order (only one returns pools)".
|
|
||
| **The destination tick array must exist.** A swap resumes from the array covering the new tick, | ||
| PDA `["tick_array", pool, start_index as i32 big-endian]` with 60 ticks of `tick_spacing` each. | ||
| Studio refuses a move onto a missing array. |
There was a problem hiding this comment.
"Studio refuses a move onto a missing array" describes behaviour in another repo (surfpool-web-ui#22) that may not be merged or released yet. Consider linking the PR or phrasing this as "the Studio preset checks…" so the README doesn't promise something Surfpool itself doesn't do.
There was a problem hiding this comment.
Summary
Adds PancakeSwap CLMM (a Raydium CLMM fork) as a YAML + IDL protocol with two templates (pancakeswap-clmm-pool-state, pancakeswap-clmm-amm-config). It also adds a collection-level llm_context that is prepended to each template's own context, moves diff_indices into tests/helpers, expands the call_surfnet_rpc params description, and adds live mainnet integration tests.
I checked the hard-coded values against the Raydium layout and they're correct:
- PoolState offsets:
amm_config9, mints 73/105,token_vault_0137,liquidity237,sqrt_price_x64253,tick_current269,status389. Total size 1544. - AmmConfig: size 117.
- Status bits: bit 4 is swap.
- Tick array PDA: big-endian
start_index. - Tick formula:
floor(2·ln(sqrt/2^64)/ln(1.0001)). - Crossing rules:
old < t <= newgoing up,new < t <= oldgoing down.
The code is in good shape. Remaining points:
- The PR description is out of date. It lists a third template,
pancakeswap-clmm-custom, and sayspancakeswap-clmm-amm-configcovers "22 fee tiers by config index, address derived". After 22d0247 there are only two templates, and the AmmConfig is addressed by pubkey read from the pool. Please update the body so reviewers and release notes match the code. - Unrelated change in this PR.
.github/workflows/claude-review.ymlhas nothing to do with PancakeSwap and should go in its own PR. Within it, the PR title and body are put straight into the prompt. Fork PRs are skipped, so the exposure is limited, but untrusted text is still being placed into an instruction prompt. - Collection-level
llm_contexthas noraw_layout: falsecoverage. The new unit test only covers araw_layout: truecollection. That's fine for testing the merge logic. PancakeSwap is the only YAML that uses the field so far, so the change is low risk. - The "known limitation" could be guarded. The PancakeSwap template writes Raydium pools without complaint. If the engine already knows the template's program id, a warning or an owner check at materialization time would turn that README caveat into a guard.
| let account_type = entry | ||
| .idl_account_name | ||
| .unwrap_or_else(|| default_account_type.clone()); | ||
| let llm_context = match (&self.llm_context, entry.llm_context) { |
There was a problem hiding this comment.
Merging the contexts looks right. When only the shared context exists, it's cloned once per template. That's fine, but consider documenting on the struct field that whitespace is trimmed only in the (Some, Some) case. A shared context without a trailing newline would be joined differently from one with it, and the asymmetry isn't obvious.
| pub method: String, | ||
| #[schemars(description = "The parameters to pass to the RPC method")] | ||
| #[schemars( | ||
| description = "The parameters to pass to the RPC method. Wrap every value at every depth by its type: a string as {\"String\":\"...\"}, a whole number as {\"Number\":5}, a boolean as {\"Bool\":true}, a list as {\"Array\":[...]} and an object as {\"Object\":{\"key\":<wrapped value>}}; object keys stay plain. Example for getProgramAccounts with a filter: [{\"String\":\"<program id>\"},{\"Object\":{\"encoding\":{\"String\":\"base64\"},\"filters\":{\"Array\":[{\"Object\":{\"memcmp\":{\"Object\":{\"offset\":{\"Number\":0},\"bytes\":{\"String\":\"<base58>\"}}}}}]}}}]" |
There was a problem hiding this comment.
The description lists String, Number, Bool, Array and Object, but not Null. JsonValue::Null is a unit variant, so it serializes as the bare string "Null", not {"Null":...}. An LLM following this text has no way to pass a null (for example an optional positional param). Add null as "Null". It's also worth noting that Number is i64, so fractional values aren't accepted.
| whose vault holds the most; name it. If neither order returns a pool, recheck the mints once, then | ||
| ask for the pool address. A pool address the user gives is used as is. | ||
|
|
||
| Set fetchBeforeUse: true. |
There was a problem hiding this comment.
fetchBeforeUse: true is a scenario/override flag. Here it sits in the collection-wide context next to the getProgramAccounts recipe, so it applies to both templates. That's intended, but say where the flag goes (on the override instance), as the per-template contexts do for account. Otherwise the LLM may put it inside values.
| #[tokio::test] | ||
| async fn pool_amm_config_at_offset_9_is_a_live_amm_config_that_round_trips() { | ||
| let pool = fetch(&[SOL_USDC_POOL]).await.remove(0).data; | ||
| let config_address = Pubkey::try_from(&pool[AMM_CONFIG_RANGE]).expect("32 bytes"); |
There was a problem hiding this comment.
The pool and its AmmConfig are fetched in two separate requests. That's harmless here because only the address is used. The fetch doc comment says it fetches "in one request, so every account returned is from the same slot", but that guarantee doesn't hold across these two calls.
| `DJNtGuBGEQiUCWE8F981M2C3ZghZt2XLD8f2sQdZ6rsZ` (0.01% fee, tick spacing 10). Pools are also listed | ||
| at `https://solana.pancakeswap.finance/liquidity-pools/`. | ||
|
|
||
| **Check the owner.** The layouts match Raydium's, so a PancakeSwap template pointed at a Raydium |
There was a problem hiding this comment.
This documents the IDL-by-owner limitation but offers no mitigation. Consider stating the PancakeSwap program id inline here (HpNfyc2S...) as the owner to check, so readers know what to compare owner against.
2 participants
Adds PancakeSwap (Solana) CLMM state prep. Stacked on #21: PancakeSwap is a Raydium CLMM
fork with identical
PoolState,AmmConfigandTickArrayStatelayouts, so the templatesmirror the Raydium CLMM ones under the PancakeSwap program id.
Heads up on the size: 1854 lines of the diff are the vendored IDL.
Templates:
pancakeswap-clmm-pool-state,pancakeswap-clmm-amm-config(22 fee tiers byconfig index, address derived) and
pancakeswap-clmm-custom.YAML and IDL only, like Kamino: no protocol Rust and no MCP tool. The price/tick coupling,
the tick array PDA recipe and the "stay inside the current tick array" rule live in the
pool-state template's
llm_contextand in the README; the Studio card computes the shockclient-side (LimeChain/surfpool-web-ui#22).
Tests: the two IDLs were compared field by field; live tests round-trip a real mainnet
pool, check every fee tier against its live
AmmConfig, and apply overrides through thematerializer asserting only the targeted bytes change. Verified on a forked surfnet:
a 0.5 shock made a real
swap_v2fill at half the mainnet quote.Known limitation (in the README): the engine picks the IDL by account owner, so a
PancakeSwap template pointed at a Raydium CLMM pool writes the same bytes.