Repository navigation
feat(scenarios): add Project 0 support - state templates - #34
92Infinitus92 wants to merge 25 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
Adds Project 0 (marginfi v2) templates: 6 for Bank and 2 for MarginfiAccount, on a vendored 0.1.12 IDL with WrappedI80F48.value retyped to i128. It also adds the Pyth Pro receiver template, a collection-level llm_context that is prepended to each template's own, and enum variants accepted by name in json_to_txtx_value_for_idl_type. diff_indices moves to the shared test helpers.
I checked the fixed-point and rate constants against 2^48 and u32::MAX: 0.8, 0.9, 1.1, 1.25, 0.3 and 0.95 weights, 10 SOL in shares, 50% and 80% utilization, and 10% and 100% rates. All are correct. Base58 "4" is indeed byte value 3. The tests are strong: live round-trips, per-template byte-isolation checks, and LiteSVM runs of the deployed program.
Overall this looks good. A few minor points:
- Unrelated workflow file:
.github/workflows/claude-review.ymlcomes from the "ci: claude code review" commit and has nothing to do with this feature. Drop it from the PR or split it out. - PR body vs. template text: the PR body says the stablecoin circuit breaker trips at "about 0.7%". The
project0-bank-oraclellm_contextsays 50 bps for USDC. Make the two agree. - Protocol naming: this protocol is registered as
"Project 0", with a space and capital letters. The others use slugs (pyth,kamino-lend,drift). Callers usingby_protocolor tag-style lookups now need the exact string with the space. Considerproject0so naming stays consistent.
| } | ||
| } | ||
|
|
||
| /// The vendored IDL retypes `WrappedI80F48.value` from `[u8; 16]` to `i128`. Writing back the |
There was a problem hiding this comment.
This doc paragraph ("The vendored IDL retypes WrappedI80F48.value… pins the byte order") describes fixed_point_fields_read_as_little_endian_i128 at line 219. It got merged into the doc comment of llm_context_bank_lookup_finds_the_native_sol_and_usdc_banks, so rustdoc attaches it to the wrong test. Move it down to the right test.
| template.llm_context.as_deref().is_some_and( | ||
| |context| context.contains("MFv2hWf31Z9kbCa1snEPYctwafyhdvnV7FZnsebVacA") | ||
| ), | ||
| "{} must say how to find its account on the program", |
There was a problem hiding this comment.
This assertion always passes now, because the collection-level llm_context, which mentions the program id, is prepended to every template. It doesn't check that any template has its own guidance. To keep the intent, also assert that each template's context is longer than the shared prefix, for example by checking for a template-specific marker.
| .collect::<SurfpoolResult<Vec<_>>>()?; | ||
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. |
There was a problem hiding this comment.
Looks right. Two small points:
- The guard scans
idl_typeslinearly for every string value. That's fine for these IDL sizes. - When a struct-variant name is passed as a bare string, this arm doesn't match, so the input falls through to the generic object arm. That arm's error probably won't say the variant needs fields. A targeted error such as "variant
Limitedrequires fields" would make the failure clearer in Studio and for LLM callers.
| 1. Resolve the token's mint with get_token_address. | ||
| 2. Call getProgramAccounts with two positional params: the program id | ||
| "MFv2hWf31Z9kbCa1snEPYctwafyhdvnV7FZnsebVacA", then the config object | ||
| {"encoding":"base58","dataSlice":{"offset":610,"length":32},"filters":[ |
There was a problem hiding this comment.
This example shows the getProgramAccounts config as plain JSON. The MCP call_surfnet_rpc params description, changed in this PR, requires every value to be type-wrapped ({"String":...}, {"Object":...}). An LLM may follow this example literally and send unwrapped params. Either say the call goes through call_surfnet_rpc with values wrapped as its schema describes, or show the wrapped form.
There was a problem hiding this comment.
Summary
This PR adds eight Project 0 (marginfi v2, 0.1.12) templates: six for the bank and two for the account. It also adds the pyth-pro-price-feed-v2 template, a collection-level llm_context that goes before each template's own context, and lets IDL enum fields take a unit variant by name (e.g. "ReduceOnly").
Overall: the PR is in good shape.
- Fixed-point examples: I spot-checked the weights, prices and shares (0.8, 0.9, 1.1, 1.25, 0.3, 0.95, 10 SOL × 2^48). All are correctly scaled by 2^48.
- Rate encodings: the u32 rate and utilization values check out.
OracleSetupfilter:PythPushOracleis index 3 in the bundled IDL, so the"4"(base58) filter in the lookup matches.- Tests: the fixed-point offsets are covered by live tests, and the behavioural LiteSVM tests are a strong addition.
- Refactors: the
llm_contextmerge and the shareddiff_indiceshelper are clean.
Points to consider
- Out-of-scope workflow:
.github/workflows/claude-review.yml(commit2afc0c3) has nothing to do with Project 0. Consider moving it to its own PR so it gets its own review. Fork PRs are also skipped, which is worth knowing. - Lookup filter and retries: the bank lookup only returns banks with
oracle_setup == PythPushOracle. For a token whose only bank uses a Switchboard or staked setup, step 3 tells the LLM to treat the empty result as a mistyped mint and retry. That retry is wasted (inline comment). - Share examples: the "10 SOL of shares" examples assume a share value of 1.0. Live banks have accrued interest, so following the example as-is gives a different token amount (inline comment).
| {"memcmp":{"offset":41,"bytes":"4qp6Fx6tnZkY5Wropq9wUYgtFxXKwE6viZxFHg3rdAG8"}}, | ||
| {"memcmp":{"offset":8,"bytes":"<mint>"}},{"memcmp":{"offset":609,"bytes":"4"}}]}. | ||
| Offset 0 is the Bank account type, 41 the main group and 8 the mint. Byte 609 equal to 3 | ||
| ("4" in base58) keeps the bank priced straight from a Pyth push oracle and leaves out the |
There was a problem hiding this comment.
The 609 == 3 filter drops every bank priced through SwitchboardPull, StakedWithPythPush or a wrapped venue. For a token whose only main-group bank uses one of those setups, step 3 treats the empty result as a mistyped mint and makes the LLM retry. Suggest saying in step 3 that an empty result can also mean the bank uses another oracle setup, and that the filter can be rerun without the 609 memcmp (then read config.oracle_setup to decide).
| llm_context: | | ||
| The borrow rate is a piecewise-linear curve from zero_util_rate at 0% utilization, through up | ||
| to 5 points, to hundred_util_rate at 100%. Points stay in ascending util order and a point with | ||
| util 0 is unused. Rates are out of 1000% (100% APR = 429496729), utilization out of 100% |
There was a problem hiding this comment.
The example "2814749767106560000000000 is 10 SOL of shares" holds only when asset_share_value is 1.0. Live banks have accrued interest, so their share value is above 1.0. The account-balance example adds "(share value 1.0)"; adding the same caveat here keeps the LLM from copying the number as a token amount.
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. | ||
| (IdlType::Defined { name, .. }, serde_json::Value::String(variant)) |
There was a problem hiding this comment.
Nit: when the string is not a unit variant of the enum (a typo, or a variant that has fields), this guard is false and the value falls through to the generic arms. The error then reads as a type mismatch rather than "unknown variant X for enum Y". A dedicated error arm that lists the valid variants would make bad Studio/LLM input much easier to debug.
0bde0f9 to
5fb2c01
Compare
There was a problem hiding this comment.
Summary
This PR adds Project 0 (marginfi v2, 0.1.12) state templates: six for Bank and two for MarginfiAccount. It also adds a collection-level llm_context that goes before each template's own context, and it lets a unit enum variant be passed by its name ("ReduceOnly") as well as {"ReduceOnly": null}. The duplicated diff_indices helper now lives in one shared place.
Assessment: The PR is well built and I'm fine with merging it.
- Enum change: it is narrow. It only applies when the defined type is an enum and the variant has no fields. Unknown names and variants with data still return an error, and the unit test covers both cases.
- Fixed-point example values: I recomputed them all (0.8, 0.9, 1.1, 1.25, 0.3 and 0.95 times 2^48, plus 10 SOL of shares) and they are correct. The interest-rate scaling examples are also correct.
- Tests: the live tests are thorough. They round-trip real accounts, write each template property alone and check that only its own bytes change, and run the deployed program in LiteSVM. One risk: they fetch the current mainnet program and accounts. A program upgrade past 0.1.12 that changes the Bank layout will break CI until the IDL is updated. That is fine behind
integration-tests, but it's worth knowing.
Minor points:
- Protocol name: the key is
"Project 0", with a space and capitals, while most keys are lowercase slugs (kamino-scope,drift). It matches other display-style names (Phoenix Eternal), so this is only a note on consistency. - Unmentioned YAML field: the new collection
llm_contextfield incrates/typesis a schema addition that is not mentioned incrates/core/src/scenarios/README.md. Consider documenting it there for other protocol authors.
| ("4" in base58) keeps the bank priced straight from a Pyth push oracle and leaves out the | ||
| Kamino-, Drift- and JupLend-wrapped banks of the same mint. The result's pubkey is the bank, | ||
| and its data is the bank's price account (config.oracle_keys.0). | ||
| 3. No result means a mistyped mint or no such bank. Copy the mint again and repeat once, then |
There was a problem hiding this comment.
The oracle_setup == 3 filter (line 20) also leaves out native banks priced through Switchboard Pull (4) and staked-SOL banks (5), not only the Kamino, Drift and JupLend wrapped ones. A mint whose only bank is Switchboard-priced returns no result, and step 3 then tells the model to blame the mint or say no bank exists. Consider adding: "or the bank uses another oracle; drop the 609 filter to list every bank of the mint". Then the model can still find the bank before it has to ask the user.
| template.llm_context.as_deref().is_some_and( | ||
| |context| context.contains("MFv2hWf31Z9kbCa1snEPYctwafyhdvnV7FZnsebVacA") | ||
| ), | ||
| "{} must say how to find its account on the program", |
There was a problem hiding this comment.
This assertion always passes, because every template inherits the program id from the collection-level llm_context. It no longer checks anything about a single template. If the goal is to guard the shared lookup, one assertion on the collection or on any one template states that more honestly. Otherwise, check something specific to each template.
5fb2c01 to
5c34984
Compare
There was a problem hiding this comment.
What the PR does
It adds eight Project 0 (marginfi v2, 0.1.12) state templates: six for Bank (weights, limits, status, oracle and circuit breaker, interest rate curve, shares) and two for MarginfiAccount (balance slot, authority). It also makes three small generic changes:
- a collection-level
llm_contextthat is placed in front of each template's own context - enum fields accept the bare variant name, e.g.
"ReduceOnly" diff_indicesmoves into the shared test helpers
Overall assessment
This is solid work. The generic changes are small and have tests. I checked the fixed-point examples in the YAML and they are all correct: 0.8, 0.9, 1.25, 1.1, 0.3 and 0.95 times 2^48, the 10 SOL and 8 SOL share examples, and 1.25 × 1.1. The u32 utilization and rate encodings are also right. The getProgramAccounts recipe matches the Bank layout: group at 41, oracle_setup at 609 and oracle_keys[0] at 610. Base58 "4" does decode to byte 3, which is PythPushOracle. The round-trip and offset tests on live accounts, plus the LiteSVM runs of the deployed program, give good evidence that the risk engine actually reads the bytes these templates write.
Minor points
- Duplicated lookup context. The shared bank/wallet lookup (about 30 lines) is copied into all 8 templates'
llm_context. Any client that sends the whole template catalog to an LLM pays for that text 8 times. That's fine for now, but consider exposing the collection context once at the protocol level later. - Time fields: docs vs tests. The
llm_contextsays to leave the Pyth time fields unset. The behavioural tests setpublish_timeandprev_publish_timeexplicitly, which LiteSVM needs because it has no upstream clock. A one-line comment in the test explaining why would stop readers seeing it as a contradiction. - Stacking. The PR says it depends on #33, but
pyth-pro-price-feed-v2already seems to be in the base. Check that the merge order is still right.
| @@ -0,0 +1,322 @@ | |||
| protocol: Project 0 | |||
There was a problem hiding this comment.
protocol: Project 0 has a space and capitals, but the other collections use slugs such as kamino-lend and drift. This name becomes the by_protocol key used by tests, Studio and the MCP, so a slug such as project0 would be easier to pass on the CLI or in URLs and would match the rest of the registry. The display name can still be "Project 0" elsewhere.
| 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.
The merge logic is fine. One inconsistency: when a template has no context of its own, the shared text is returned untrimmed; when both exist, it is trim_end()-ed. Nothing breaks today, and the test pins the current output. Normalizing both branches the same way would make the result predictable if a YAML author uses |+ or leaves trailing blank lines.
| .collect::<SurfpoolResult<Vec<_>>>()?; | ||
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. |
There was a problem hiding this comment.
Nice, small change. The guard requires v.fields.is_none(), so a variant declared with an empty field list ("fields": []) is not matched by name. Some IDL generators emit that for unit-like variants. Consider also accepting Some(IdlDefinedFields::Tuple(v)) if v.is_empty() and the empty Named case.
|
|
||
| let found = program_accounts(UiAccountEncoding::Base58, ORACLE_SLICE, &filters).await; | ||
| let banks: Vec<String> = found.iter().map(|(pubkey, _)| pubkey.to_string()).collect(); | ||
| assert_eq!(banks, [JITOSOL_BANK], "expected the one jitoSOL bank"); |
There was a problem hiding this comment.
This exact-match assertion depends on mainnet: it fails as soon as a second jitoSOL bank (e.g. a Kamino-wrapped one) is added to the group. The same applies to found.len() == 1 at line 211. Asserting contains(JITOSOL_BANK) would still check the recipe while surviving new banks, unless the exact count is what you want to pin.
…escribe both ways past it
There was a problem hiding this comment.
Summary
This PR adds Project 0 (marginfi v2) templates: 6 Bank templates and 2 MarginfiAccount templates. It also:
- bundles a vendored IDL where
WrappedI80F48.valueis retyped asi128; - adds a collection-level
llm_contextthat is put in front of each template's own context; - lets a unit enum variant be given by its name in the IDL value conversion;
- moves
diff_indicesinto the shared test helpers.
The core changes are small and well tested. The enum arm runs before the struct/Object arm, rejects variants that carry fields, and has its own unit test.
I checked the fixed-point examples (0.3, 0.8, 0.9, 1.1, 1.25, $0.95, $20, the 10 SOL and 8 SOL share amounts, and 1.25 × 1.1). The u32 rate and utilization scales and the base58 "4" for byte value 3 are also correct.
Issues
- Circuit breaker guidance contradicts itself. The oracle template's
llm_contextand the README say the breaker "is on whencb_deviation_bps_tiers.0is above 0". A few lines later the template says "Only all five at 10000 switch it off". An LLM could reasonably set tier 0 to0and expect the breaker to be off. Please settle on one rule (see the inline comment). - Threshold mismatch. The PR body says "about 0.7%" for stablecoins, but the template and README say 50 bps (0.5%) for USDC.
- Brittle live tests.
DEPLOYED_SLOTis compared for exact equality, so every behavioural test fails on any program upgrade.- The jitoSOL fallback test expects exactly one bank. Adding a second bank to the group breaks it.
- These tests only run with
integration-tests, so this is acceptable, but expect churn. Consider soft-skipping when the slot doesn't match, or matching the jitoSOL bank with "contains" instead of exact equality.
project0-account-balanceonly exposes slot 0. Studio/form users can only edit slot 0; changing the slot means rewriting the path by hand. Slot 0 is usually the highestbank_pkposition that is already in use, so the form's default path pushes users towards overwriting a live position, which the template's own guidance warns against.
| like a slow move it has adapted to. A bank whose breaker is off ignores both fields. | ||
| - CHANGE THE THRESHOLDS: set the three config.cb_deviation_bps_tiers entries and the two | ||
| config.cb_window_max_*_bps caps. Use this to test the breaker itself, with a price just | ||
| under or just over tier 1, or to model the admin loosening it. Only all five at 10000 |
There was a problem hiding this comment.
This contradicts line 183 ("It is on when config.cb_deviation_bps_tiers.0 is above 0"), which suggests that setting tier 0 to 0 disables the breaker. Here the text says only all five at 10000 turn it off. Please state one rule, ideally the one checked against the program (if tier0 == 0 does disable it, say so here as the simple off switch). The same wording is in the README.
| An account holds 16 fixed balance slots. Replace the 0 in the paths with the slot you edit. | ||
|
|
||
| RULES TO KEEP: | ||
| - Active slots are kept sorted by bank_pk, highest first, compared as raw bytes. Pick the slot |
There was a problem hiding this comment.
The template's properties are hard-coded to slot 0. Most live accounts already use slot 0 (the highest bank_pk), so the default Studio form edits an existing position, while this context tells users to take a free slot. Consider exposing a couple of slots, or warning more clearly that slot 0 is usually already in use.
| let program_data = fetch(&[PROGRAM_DATA]).await.remove(0).data; | ||
| let deployed_slot = u64::from_le_bytes(program_data[4..12].try_into().unwrap()); | ||
| assert_eq!( | ||
| deployed_slot, DEPLOYED_SLOT, |
There was a problem hiding this comment.
An exact slot equality check makes every behavioural test fail as soon as Project 0 upgrades, even when the layout doesn't change. That may be intentional as a re-verify tripwire. If so, consider failing with a clearer message, or skipping only the LiteSVM tests, so the IDL round-trip tests still give a signal.
|
|
||
| let found = program_accounts(UiAccountEncoding::Base58, ORACLE_SLICE, &filters).await; | ||
| let banks: Vec<String> = found.iter().map(|(pubkey, _)| pubkey.to_string()).collect(); | ||
| assert_eq!(banks, [JITOSOL_BANK], "expected the one jitoSOL bank"); |
There was a problem hiding this comment.
Asserting exactly one jitoSOL bank breaks if the group ever adds a second (for example, wrapped) jitoSOL bank. Checking that JITOSOL_BANK is among the results would be more robust.
…project0 # Conflicts: # crates/core/src/scenarios/registry.rs
…ct 0 recipes and tests
406b66c to
d7de4f1
Compare
There was a problem hiding this comment.
Summary
This PR adds eight Project 0 (marginfi v2) state templates on the bundled 0.1.12 IDL. It also adds a collection-level llm_context, lets an enum field take a unit variant by name, and moves diff_indices into a shared test helper. The live tests round-trip mainnet accounts and check that each property writes only its own bytes, and LiteSVM tests run the deployed program.
I recomputed the fixed-point examples in overrides.yaml by hand: the 2^48 weights, $0.95, $20, the share totals, the 1.1× share value, and the interest-rate u32 encodings. They are all correct. The base58 of byte 3 is "4", as the text says. The bank offsets used in the lookups are pinned by live tests. I could not run the IDL layout script here, so I did not check the discriminators or offsets independently of those tests.
The llm_context merge in scenarios.rs and its unit test look fine. The enum-by-name arm only matches unit variants, so a variant with fields still needs its data.
Overall the change looks solid. My comments are minor:
- The CI change (
show_full_output) has nothing to do with Project 0 and has security implications, so it deserves its own review. - The wallet lookup recipe leaves
<group>unexplained. - Without the
integration-testsfeature, nothing uses the shareddiff_indices, so clippy will warn about dead code. - Some test fragility: tests fail on any program redeploy (pinned
DEPLOYED_SLOT), on mainnet address churn, and on a possible u128 overflow inconfidence_scaled_to. The deploy-slot pin is intentional, but whoever maintains the tests should know about it. - Nit:
version: v0.1.12while the directory isv1. Other protocols keep the two in sync; check that Studio and the registry don't key on the version.
| # which needs `id-token: write` and would get write access. | ||
| github_token: ${{ github.token }} | ||
| # Prints the full SDK output, including API errors, to the public log. | ||
| show_full_output: true |
There was a problem hiding this comment.
This change has nothing to do with Project 0 and has security implications. show_full_output: true prints the full SDK transcript to a public Actions log, including tool outputs and any file contents the agent reads. On a public repo that can expose more than API errors. Consider moving it, together with the action bump, into its own PR, or limiting it to debug runs (for example, behind runner.debug == '1').
|
|
||
| FINDING A WALLET'S ACCOUNT: getProgramAccounts on the same program with the config object | ||
| {"encoding":"base64","dataSlice":{"offset":0,"length":0},"filters":[ | ||
| {"memcmp":{"offset":0,"bytes":"CKkRR4La3xu"}},{"memcmp":{"offset":8,"bytes":"<group>"}}, |
There was a problem hiding this comment.
The wallet lookup uses a <group> placeholder but never says what it is. The bank lookup above hard-codes the main group 4qp6Fx6tnZkY5Wropq9wUYgtFxXKwE6viZxFHg3rdAG8. Write it here too, or say "the main group above", so the LLM doesn't have to guess or ask.
| .collect::<SurfpoolResult<Vec<_>>>()?; | ||
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. |
There was a problem hiding this comment.
The guard only accepts variants with v.fields.is_none(). An IDL that writes a unit variant as "fields": [] won't match, and the string falls through to the struct arm, which gives a confusing error. Consider also accepting an empty field list here, for example with v.fields.as_ref().is_none_or(|f| f.is_empty()) or the equivalent for IdlDefinedFields.
| }; | ||
|
|
||
| /// Returns the byte indices whose values differ between two buffers. | ||
| pub(super) fn diff_indices(left: &[u8], right: &[u8]) -> Vec<usize> { |
There was a problem hiding this comment.
helpers is always compiled, but every caller (kamino, pump, project0) is behind cfg(feature = "integration-tests"). Without that feature nothing uses diff_indices, so cargo clippy --workspace --all-targets will report it as dead code. Gate it with the same cfg, or add #[cfg_attr(not(feature = "integration-tests"), allow(dead_code))].
| fn confidence_scaled_to(data: &[u8], price: i64) -> [(String, serde_json::Value); 2] { | ||
| let at = | ||
| |offset: usize| u64::from_le_bytes(data[offset..offset + 8].try_into().unwrap()) as u128; | ||
| let scaled = |conf: u128, from: u128| serde_json::json!((conf * price as u128 / from) as u64); |
There was a problem hiding this comment.
conf * price as u128 can overflow, and a negative price would wrap when cast to u128. The current test prices are small and positive, so this is fine today. It would be more robust to compute in i128 or with checked_mul, so a future test with a large or odd price fails clearly instead of producing a garbage confidence.
0c64259 to
d7de4f1
Compare
There was a problem hiding this comment.
PR #34: Project 0 (marginfi v2) state templates
The PR adds eight Project 0 templates (six Bank, two MarginfiAccount) on the bundled 0.1.12 IDL. It also makes three small supporting changes:
- a collection-level
llm_contextthat is put in front of each template's own context; - unit enum variants can now be set by name (
"ReduceOnly"); diff_indicesmoves into the shared test helpers.
Overall it looks solid. I checked the following and found them correct:
- Fixed-point examples: 0.8, 0.9, 1.1, 1.25, 0.3, $0.95, $20, the share amounts and 1.25 × 1.1 all match their 2^48-scaled values.
- u32 rate and utilization examples: they are correct.
- Lookup filter offsets: discriminator at 0, mint at 8, group at 41,
oracle_setupat 609 andoracle_keysat 610."4"is base58 for byte 3, which isPythPushOracle. BankOperationalStatevariants: the ones listed inproject0-bank-statusmatch the IDL.- Template counts: the registry count and per-protocol counts in the tests add up (71 + 8 = 79).
- Enum-by-name arm in
svm.rs: it only matches field-less variants of a type that is actually an enum. Data-carrying variants and unknown names still fail, and the new unit test covers both.
Minor points:
- Circuit breaker threshold mismatch: the PR body says stablecoin banks trip at about 0.7%. The
project0-bank-oraclellm_context says 50 bps for USDC. One of them should be made consistent with the other, or the gap explained (for example, the oracle's confidence widening the threshold). - Contradictory "off" condition: the oracle template says the breaker is on when
cb_deviation_bps_tiers.0is above 0. Later it says only setting all five values to 10000 turns it off. See the inline comment. - Shared context is repeated in every template: the collection
llm_contextis copied into each template's context, so the same ~30-line lookup recipe shows up eight times when an LLM is given all Project 0 templates. That's acceptable, but worth knowing about for prompt size.
| like a slow move it has adapted to. A bank whose breaker is off ignores both fields. | ||
| - CHANGE THE THRESHOLDS: set the three config.cb_deviation_bps_tiers entries and the two | ||
| config.cb_window_max_*_bps caps. Use this to test the breaker itself, with a price just | ||
| under or just over tier 1, or to model the admin loosening it. Only all five at 10000 |
There was a problem hiding this comment.
Two statements here pull against each other. Line 183 says the breaker is on when cb_deviation_bps_tiers.0 > 0, which suggests setting tier 1 to 0 turns it off. This line says only setting all five to 10000 turns it off, and that 'tier 1 alone still trips the cache crank'.
An LLM following line 183 could set tier 1 to 0 and expect the breaker to be off. Please state one rule for disabling it, for example: "Setting tier 1 to 0 does not disable it; set all five to 10000." Alternatively, reword line 183 as a way to detect whether the breaker is on, not a way to switch it off.
| bank's price account, config.oracle_keys.0. | ||
|
|
||
| CIRCUIT BREAKER: most banks have one, among them the stablecoins, SOL and its liquid staking | ||
| tokens, BTC and ETH. It is on when config.cb_deviation_bps_tiers.0 is above 0. Such a bank |
There was a problem hiding this comment.
Small mismatch with the PR description. This says USDC trips at 50 bps; the PR body says stablecoins reject once the price moves 'about 0.7%'. If the difference comes from confidence or the second tier, say so here, because the LLM will use this number to pick prices that stay just under or just over the threshold.
…project0 # Conflicts: # crates/core/src/scenarios/registry.rs
There was a problem hiding this comment.
Project 0 (marginfi v2) state templates
This PR adds 8 templates for Project 0 (6 for Bank, 2 for MarginfiAccount) on top of the bundled 0.1.12 IDL. It also adds:
- an
llm_contextthat applies to the whole collection, - support for passing a unit enum variant by its name, as a string,
- a shared
diff_indicestest helper, - live tests against mainnet and behavioural tests that run the deployed program in LiteSVM.
Overall: the PR is well built. I spot-checked these values in the YAML and they are all correct:
- the 2^48 fixed-point examples: 0.8, 0.9, 1.1, 1.25, 0.3, 0.95, $20, 10 SOL and 8 SOL of shares, and 1.25 × 1.1,
- the u32 rate and utilization encodings,
- the base58
"4"→ 3 used for the oracle-setup filter.
The enum-by-name arm comes before the object arm and only fires for unit variants that exist. Strings that don't match still go to the old fallback, so existing behaviour is unchanged.
Minor points:
- Mismatched circuit-breaker figure: the PR body says stablecoin banks trip at "about 0.7%", but the template's
llm_contextsays 50 bps for USDC. Please make the two agree, or say that the 0.7% includes the confidence adjustment. - Edge of the
llm_contextmerge incrates/types/src/scenarios.rs: when a template has no context of its own, the shared context is used without trimming. When both exist, the shared one is trimmed. This is harmless and matches the test, but it is slightly asymmetric. - Unhelpful error for a bad variant name: a string with a typo, such as
"Reduceonly", falls through tojson_to_txtx_valueas a plainValue::String. The resulting encoder error probably won't name the valid variants. It would help users and LLMs to return an explicit "unknown variant X of enum Y; expected one of …" error when the defined type is an enum.
| ); | ||
| assert!( | ||
| template.llm_context.as_deref().is_some_and( | ||
| |context| context.contains("MFv2hWf31Z9kbCa1snEPYctwafyhdvnV7FZnsebVacA") |
There was a problem hiding this comment.
This check no longer tests anything per template. The collection-level llm_context already contains the program id and is now prepended to every template's context, so the assertion passes even if a template's own llm_context is empty or missing. To keep the intent ("each template says how to find its account"), assert on the collection-level context once. Alternatively, drop this check and keep only the address/constants checks.
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. | ||
| (IdlType::Defined { name, .. }, serde_json::Value::String(variant)) |
There was a problem hiding this comment.
Suggestion: when the defined type is an enum and the string matches no unit variant, return a clear error such as unknown variant 'X' for enum 'OperationalState'; expected one of …. Today it falls through to json_to_txtx_value and becomes a Value::String, and the encoder then fails with a less helpful message. A clearer error would make LLM/editor retries much easier. The same applies to a variant that has fields: say that it needs its data.
| CIRCUIT BREAKER: most banks have one, among them the stablecoins, SOL and its liquid staking | ||
| tokens, BTC and ETH. It is on when config.cb_deviation_bps_tiers.0 is above 0. Such a bank | ||
| rejects borrow, withdraw and liquidation with error 6604 (CircuitBreakerPriceJump) once its | ||
| price, net of the oracle's confidence, is that many basis points (50 for USDC, 600 for SOL) |
There was a problem hiding this comment.
This says the USDC breaker trips at 50 bps. The PR description says stablecoins trip at "about 0.7%". Please reconcile the two numbers, or explain that the effective threshold differs because the price is taken net of confidence, so readers and LLMs don't pick the wrong distance to clear the breaker.
| like a slow move it has adapted to. A bank whose breaker is off ignores both fields. | ||
| - CHANGE THE THRESHOLDS: set the three config.cb_deviation_bps_tiers entries and the two | ||
| config.cb_window_max_*_bps caps. Use this to test the breaker itself, with a price just | ||
| under or just over tier 1, or to model the admin loosening it. Only all five at 10000 |
There was a problem hiding this comment.
"tier 1 alone still trips the cache crank at the higher tiers" is hard to follow, and an LLM is likely to misread it. Consider rewording it to say what actually happens. For example: "Raising only tier 1 is not enough: tiers 2–3 and the 24h caps still halt the bank; set all five to 10000 to disable the breaker."
There was a problem hiding this comment.
This PR adds the Project 0 (marginfi v2) templates: eight on Bank and MarginfiAccount, built on a bundled 0.1.12 IDL in which WrappedI80F48.value is retyped to i128. It also adds three general changes:
- a collection-level
llm_context; - unit enum variants accepted by name in
json_to_txtx_value_for_idl_type; - the shared
diff_indicestest helper.
Overall this is solid work.
- Tests: they cover the risky parts. Live round-trips pin the layout, and the little-endian
i128write-back pins the retyped IDL. A per-property byte-range test catches offset bugs. The LiteSVM runs execute the deployed program for the weight, price and circuit-breaker levers. - Numbers in the guidance: I recomputed the fixed-point examples and the u32 rate and utilization scales in
overrides.yaml(0.8, 0.9, 1.1, 1.25, 0.3, 0.95, $20, 10 SOL and 8 SOL of shares, 1.25×1.1). All of them are correct. - Enum-by-name conversion: it is narrowly scoped to unit variants, and the guard falls through to an error for unknown or data-carrying variants.
Main issue: the guidance on when the circuit breaker is on doesn't match the program. The IDL documents flags bit 11 (CIRCUIT_BREAKER_ENABLED) as the switch, and the USDC test asserts that bit. The README and llm_context instead tell the LLM the breaker is on when cb_deviation_bps_tiers[0] > 0. They also say that setting all five thresholds to 10000 is the only way to turn it off. See the inline comments.
Minor points:
- The integration tests assert a pinned
DEPLOYED_SLOT, so any program upgrade fails the whole live suite. That is intentional and the failure message is clear, but it is worth knowing for CI. - The
project0-bank-oracledescription says USDC trips at 50 bps, while the PR body says about 0.7%. The difference is presumably the confidence-net adjustment. Mentioning it in the context would avoid confusing an LLM that compares the two figures.
| bank's price account, config.oracle_keys.0. | ||
|
|
||
| CIRCUIT BREAKER: most banks have one, among them the stablecoins, SOL and its liquid staking | ||
| tokens, BTC and ETH. It is on when config.cb_deviation_bps_tiers.0 is above 0. Such a bank |
There was a problem hiding this comment.
This says the breaker is on when config.cb_deviation_bps_tiers.0 is above 0. The bundled IDL documents flags bit 11 (CIRCUIT_BREAKER_ENABLED) as the switch instead, and usdc_breaker_accepts_a_move_once_its_reference_prices_follow asserts flags >> 11 & 1, not the tier.
A bank can have tiers configured while the flag is clear. In that case this guidance makes the LLM add needless reference overrides and wrongly warn about error 6604.
Suggest telling it to check flags & 2048; getAccountInfo with jsonParsed exposes flags. The same wording is in README.md:105 and in the project0-bank-risk-weights context at line 88.
| like a slow move it has adapted to. A bank whose breaker is off ignores both fields. | ||
| - CHANGE THE THRESHOLDS: set the three config.cb_deviation_bps_tiers entries and the two | ||
| config.cb_window_max_*_bps caps. Use this to test the breaker itself, with a price just | ||
| under or just over tier 1, or to model the admin loosening it. Only all five at 10000 |
There was a problem hiding this comment.
The statement that only all five thresholds at 10000 switch the breaker off doesn't match the program. The off switch is flags bit 11, CIRCUIT_BREAKER_ENABLED.
Setting the thresholds to 10000 only makes them very loose. It also leaves the cache crank's tier and window state in play. Clearing the flag bit is the direct way to turn the breaker off.
Consider exposing flags (or documenting it) for the switch-off case. README.md:128 has the same wording.
| address: | ||
| type: pubkey | ||
| llm_context: | | ||
| Use this template to change how old a price the bank accepts, or to let a price move past |
There was a problem hiding this comment.
The IDL says cb_reference_price and cb_window_reference_price are in the multiplier-adjusted effective-price domain. For the native SOL and USDC banks the multiplier is 1, so price × 2^48 is right.
The context also says LST banks have a breaker. For those banks, an LLM following "set it to the new price" literally would write the raw Pyth price and trip the breaker.
Worth a sentence: for banks with a price multiplier (LST/wrapped), the reference is the effective price, not the raw feed price.
| .collect::<SurfpoolResult<Vec<_>>>()?; | ||
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. |
There was a problem hiding this comment.
Nit: the guard scans all of idl_types to find the enum, and the struct arm below does the same lookup again. This is fine for correctness.
If the IDL defines two types with the same name (it shouldn't), the guard matches on any of them while the encoder uses the first. Using idl_types.iter().find(|t| &t.name == name) once, then matching on its ty, would keep both paths consistent.
There was a problem hiding this comment.
Summary
This PR adds eight Project 0 (marginfi v2) state templates, covering Bank risk, limits, status, oracle and circuit breaker, interest rate and shares, plus MarginfiAccount position and authority. It also bundles the 0.1.12 IDL with WrappedI80F48.value retyped as i128. Three smaller changes come with it:
- A collection-level
llm_contextthat is put in front of each template's own context. - Unit enum variants can be passed by name in IDL overrides.
- The duplicated
diff_indicestest helper moves intotests/helpers.rs.
Assessment
The change is well built and well tested. The live tests round-trip real accounts and check that each property writes only its own bytes, and the behavioural tests run the deployed program in LiteSVM.
I recomputed the fixed-point examples in the YAML and they are all correct: 0.8, 0.9, 1.25, 1.1 and 0.3 weights; $0.95 and $20 reference prices; the 10 SOL and 8 SOL share amounts; 1.375 × 2^48; and the u32 utilization and rate encodings. Base58 "4" → byte 3 for the Pyth push oracle filter is also correct.
Minor points
- Fragile live tests: the integration tests depend on mutable mainnet state.
DEPLOYED_SLOTfails on any program upgrade, which is intended, andACCOUNTfails if that specific user account closes or drops its SOL/USDC positions. It would help to say in the module docs that these are expected maintenance points, and to name the positions the behavioural tests need, so a failure points straight to replacing the fixture. - Protocol name:
"Project 0"contains a space and is used as theby_protocolkey, while most collections use slug-style names (kamino-lend,drift). Check that nothing downstream (Studio filters, MCP params, URLs) expects a slug. - Hard-coded offsets: the shared
llm_contextlookup hard-codes byte offsets 41, 609 and 610 that belong to v0.1.12. That's fine for now, but these strings should be updated whenever the IDL is bumped. The round-trip and lookup tests do cover this.
| .collect::<SurfpoolResult<Vec<_>>>()?; | ||
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. |
There was a problem hiding this comment.
Nit: v.fields.is_none() treats a variant declared with "fields": [] as not being a unit variant, even though Borsh encodes it exactly like one. Anchor's generator leaves fields out, so bundled IDLs work, but hand-edited or third-party IDLs may not. Consider v.fields.as_ref().map_or(true, |f| f.is_empty()), which needs a small match because IdlDefinedFields has Named and Tuple variants.
| llm_context: | | ||
| config.operational_state is an enum. Pass the variant name, e.g. "ReduceOnly". | ||
|
|
||
| Variants: Paused, Operational, ReduceOnly, KilledByBankruptcy, Uninitialized, |
There was a problem hiding this comment.
This list puts KilledByBankruptcy, Uninitialized and CircuitBroken next to the useful states without any guidance. An LLM could pick one of them for a vague request like "break the bank", and some are probably hard or impossible to undo through normal instructions. Consider marking which variants are meant for scenarios (Paused, Operational, ReduceOnly, ReduceOnlyWithBorrowingPower) and saying what the others imply.
| An account holds 16 fixed balance slots. Replace the 0 in the paths with the slot you edit. | ||
|
|
||
| RULES TO KEEP: | ||
| - Active slots are kept sorted by bank_pk, highest first, compared as raw bytes. Pick the slot |
There was a problem hiding this comment.
The descending bank_pk ordering rule is important. If it's wrong, health checks fail with a confusing remaining-accounts mismatch rather than a clear error. If the behavioural tests don't already write a second slot out of order, a small negative test would lock this rule against the deployed program.
| const GROUP: &str = "4qp6Fx6tnZkY5Wropq9wUYgtFxXKwE6viZxFHg3rdAG8"; | ||
| const USDC_BANK: &str = "2s37akK2eyBbp8DZgCm7RtsaEz8eJP3Nxd4urLHQv7yB"; | ||
| const SOL_BANK: &str = "CCKtUs6Cgwo4aaQUmBPmyoApH2gUDErxNZCAntD6LYGh"; | ||
| const ACCOUNT: &str = "CGnzJGWscJAasjwngK1Jixx4xHfxF5EZaMje6uRtnbMy"; |
There was a problem hiding this comment.
This test depends on one live user account keeping a particular SOL-collateral / USDC-debt shape. If the owner closes or rebalances it, the behavioural tests will fail for reasons unrelated to surfpool. Consider asserting the expected slot layout early, with a message like "ACCOUNT no longer holds SOL deposit + USDC borrow; pick a new fixture", the same way the DEPLOYED_SLOT check does.
| 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.
Only the shared context has its trailing whitespace trimmed, so a template whose own context ends in several newlines keeps them. The test covers the common case. A short doc comment on the field saying that templates without their own context get the shared one as-is would make the asymmetry obvious.
There was a problem hiding this comment.
Summary
This PR adds Project 0 (marginfi v2) support:
- Bundled files: the 0.1.12 IDL, with
WrappedI80F48.valueretyped toi128, and 8 override templates (6 Bank, 2 MarginfiAccount). - Docs: a how-to README.
- Collection-wide context: a
llm_contextfield that is put in front of each template's own context. - Enum by name: a small change so unit enum variants can be given by name (e.g.
"ReduceOnly"). - Shared helper:
diff_indicesmoves intotests::helpers. - Tests: live mainnet tests and LiteSVM tests that run the deployed program.
I checked the fixed-point examples in overrides.yaml and the README by hand: 0.8, 0.9, 1.25, 1.1, 0.3, 0.95 and 20 × 2^48, the 10 SOL / 8 SOL / 500 USDC share amounts, the 1.375 aged share value, and the u32 rate and utilisation encodings. All are correct. The base58 "4" for byte value 3 is also right, and the u16/u32 types match the IDL.
The design looks sound overall:
- The enum-by-name branch only matches unit variants and otherwise falls back to the existing path, so other inputs are unaffected.
- The
llm_contextmerge is simple and has a test. - The write-only-its-own-bytes test and the deployed-program tests give good confidence that the offsets are right and that the program actually reads them.
Points that don't belong to a single line:
- Repeated context: the collection-level
llm_context(about 30 lines of lookup recipe) is added to all 8 templates. Anyget_override_templatesresponse that lists Project 0 therefore carries 8 copies, which costs tokens for MCP/LLM clients. Consider returning the shared context once at the collection or protocol level, or dropping it from the list view. - Behavioural tests fail on any upgrade:
project0_elfassertsDEPLOYED_SLOT, so every behavioural test fails whenever the program is upgraded, even when the layout is unchanged. The failure message is clear, so this looks deliberate. It's still worth stating in the README so the failures aren't mistaken for flakes. - Stacked PR: this branch includes merges from the #33 branch, so merge #33 first as the description says.
| if idl_types.iter().any(|t| { | ||
| &t.name == name | ||
| && matches!(&t.ty, IdlTypeDefTy::Enum { variants } | ||
| if variants.iter().any(|v| &v.name == variant && v.fields.is_none())) |
There was a problem hiding this comment.
Nit: this only treats fields: None as a unit variant. Some IDL generators write unit variants as "fields": []. Accepting those too would make the name form work for them as well:
if variants.iter().any(|v| &v.name == variant && v.fields.as_ref().map_or(true, |f| f.is_empty())))That assumes the encoder accepts {"Variant": null} for such variants, so it's worth checking that first.
| - path: config.cb_window_max_up_bps | ||
| label: Circuit breaker 24h cap up (bps) | ||
| description: >- | ||
| Largest rise from cb_window_reference_price the breaker allows inside its window |
There was a problem hiding this comment.
In the IDL, cb_window_max_up_bps / cb_window_max_down_bps say that 0 uses the CB_WINDOW_MAX_*_BPS default. A user (or an LLM) who writes 0 to "turn the cap off" would get the default cap instead. Please add that note to both descriptions, next to the existing "all five at 10000" guidance.
| - risk-engine | ||
| - defi | ||
|
|
||
| llm_context: | |
There was a problem hiding this comment.
This collection-level llm_context is added in front of each of the 8 templates, so the same ~30-line lookup recipe appears 8 times in every template listing. If the MCP output for listing templates is size-sensitive, consider sending the shared context once per protocol instead.
| async fn project0_elf() -> Vec<u8> { | ||
| let program_data = fetch(&[PROGRAM_DATA]).await.remove(0).data; | ||
| let deployed_slot = u64::from_le_bytes(program_data[4..12].try_into().unwrap()); | ||
| assert_eq!( |
There was a problem hiding this comment.
Every behavioural test depends on this check, so any redeploy of the program, even one that leaves the layout unchanged, makes them fail. That's reasonable as a guard, but please mention it in the Project 0 README's testing note so maintainers know to bump DEPLOYED_SLOT after checking the new deploy.
There was a problem hiding this comment.
Summary
This PR adds 8 Project 0 (marginfi v2) templates on a vendored 0.1.12 IDL, where WrappedI80F48.value is retyped to i128. It also adds a collection-level llm_context that is put in front of each template's own context, and lets a unit enum variant be written by its name (e.g. "ReduceOnly"). Tests cover live round-trips, per-property byte isolation, the lookup recipes, and LiteSVM runs of the deployed program.
Overall it is solid and carefully tested. I checked the bundled IDL's sha256 against the README, plus every fixed-point example (0.8, 0.9, 1.25, 1.1, 0.3, $0.95, $20, the 10 SOL and 500 USDC shares, and the 1.375 liability example) and the u32 rate/utilization examples. All of them are correct. The enum-by-name conversion is narrowly scoped: unit variants only, and variants with fields still fall through to an error. It has a focused test.
Minor points:
- Shared
llm_contextsize: the bank and wallet lookup block is now put in front of all 8 templates, including the two MarginfiAccount ones. That repeats a long block in everyget_override_templatesresponse. This is acceptable, but worth knowing if template payload size matters for LLM context. - Brittle tests: the live tests are pinned to
DEPLOYED_SLOTand to one specific mainnet MarginfiAccount and jitoSOL bank. That looks intentional, but these will fail on the next program upgrade or account change. Consider marking them#[ignore]or running them in a separate CI job so unrelated PRs aren't blocked. - Breaker test vs. recommended method:
usdc_breaker_accepts_a_move_once_its_reference_prices_followcomputes the reference as price × 2^48 / 10^|exp|. Thellm_contextrecommends a different default: scale the bank's currentcb_reference_priceby the price move. Also testing that approach would back up the guidance the LLM is actually given. The current formula also assumes a negative exponent.
|
|
||
| ``` | ||
| project0-bank-risk-weights on the SOL bank | ||
| config.asset_weight_maint.value: "84442493013197" # 0.3 |
There was a problem hiding this comment.
This recipe only lowers asset_weight_maint to 0.3 and leaves asset_weight_init at its live value (e.g. 0.8). That breaks the rule stated on the template property ("Keep it at or above the borrowing weight"). The YAML example and the behavioural test both set init and maint together. Suggest adding config.asset_weight_init.value: "84442493013197" here as well, so users don't end up with init > maint.
| self.load_protocol_overrides( | ||
| PROJECT0_V1_IDL_CONTENT, | ||
| PROJECT0_V1_OVERRIDES_CONTENT, | ||
| "Project 0", |
There was a problem hiding this comment.
The protocol key is "Project 0", with a space and capitals. Other collections use slug keys (kamino-lend, drift, pump, ...). The key is what by_protocol and any protocol filter in RPC, MCP or Studio match on, so a mixed-case name with a space is easy to get wrong. It is also inconsistent with the project0-* template ids. Consider project0 as the key and keeping "Project 0" as the display name.
There was a problem hiding this comment.
Overview
This PR adds eight Project 0 (marginfi v2) state templates (six for Bank, two for MarginfiAccount) on top of a bundled 0.1.12 IDL. In that IDL, WrappedI80F48.value is retyped to i128, so fixed-point fields take decimal strings. It makes three generic changes:
- Shared
llm_context: a collection-levelllm_contextthat is placed before each template's own. - Enum variants by name: a unit enum variant can be passed as its name, e.g.
"ReduceOnly". - Shared test helper:
diff_indicesmoves intotests/helpers.rs.
Assessment
The PR is in good shape. I spot-checked the following and found them correct:
- Fixed-point examples: 0.8, 0.9, 1.1, 1.25, 0.3, $0.95 and $20 scaled by 2^48, plus the 10 SOL and 8 SOL share values.
- Interest-rate encodings: 100% APR = 429496729, 10% = 42949672, 80% utilization = 3435973836.
- Bank lookup filter: in base58,
"4"is byte 3. - README hash: the bundled IDL's sha256 matches the value in the README (
bcab69…).
The tests go beyond byte offsets. They run the deployed program in LiteSVM and show that the weight, price and circuit-breaker levers actually take effect. The per-property "touches only its own bytes" test is also a good guard.
Minor notes
- Protocol key: the key
"Project 0"(display name with a space) follows thePhoenix Eternal/PumpSwapstyle rather than the slug style (kamino-*,pump-amm). There is precedent either way, but the convention is mixed. Anything callingby_protocolhas to match it exactly. - Deploy-slot pin:
DEPLOYED_SLOTmakes the whole integration suite fail on any program upgrade. That is intentional and the message explains it, but expect CI noise each time Project 0 redeploys. - Test helper:
confidence_scaled_tocastsprice as u128. That's fine for the positive prices used here, but a negative price would silently wrap.
| .collect::<SurfpoolResult<Vec<_>>>()?; | ||
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. |
There was a problem hiding this comment.
When the string doesn't match a unit variant (a typo, or a variant that has fields such as "Limited"), the call falls through to json_to_txtx_value and passes a bare Value::String to the encoder. The error then comes from deep inside the encoder and doesn't say what was expected. The new test only checks is_err(). Since this path exists for editor/LLM input, consider an explicit arm: when name resolves to an Enum and the JSON is a string, return an error that lists the accepted unit variants, and says when the variant needs data.
There was a problem hiding this comment.
Overview
This PR adds Project 0 (marginfi v2) support: 8 templates for Bank and MarginfiAccount built on a bundled 0.1.12 IDL, where WrappedI80F48.value is retyped to i128. It also adds three small generic changes:
- a collection-level
llm_contextthat is placed before each template's own context - unit enum variants accepted by name (
"ReduceOnly") in the IDL value conversion - a shared
diff_indicestest helper
Assessment
The PR is solid and well tested. I checked the following, and they are correct:
- Fixed-point examples: 0.8, 0.9, 1.25, 1.1, 0.3, $0.95, $20, the 1.375 share value and the 8 SOL/10 SOL share examples all match ×2^48.
- Rate encodings: u32 utilization and rate values are right.
- Lookup filter: the base58
"4"→ byte 3 (PythPushOracle) filter is right. - Hashes: the IDL sha256 in the README matches the bundled file.
The enum-by-name arm only fires for unit variants that exist, so it can't affect strings meant for other Defined types. The new test covers the unknown-variant and variant-with-fields cases. The live tests (round-trip, one test per template property, and LiteSVM runs of the real program for health and the circuit breaker) are good.
Minor points (inline)
- Protocol key:
"Project 0"contains a space and differs from the slug-style keys the other protocols use. - Only slot/point 0 exposed: the templates expose only balance slot 0 and curve point 0. Editing any other index depends on an LLM or client building the path by hand.
curve_typefoot-gun:curve_typeis editable, but the context tells users to keep it at 1.- Test brittleness: the behavioural tests hard-fail on any program redeploy, and the depeg reference math assumes a negative exponent.
Merge order: this PR depends on #33 (pyth-core-compatible-price-feed-v2), so #33 needs to merge first.
| self.load_protocol_overrides( | ||
| PROJECT0_V1_IDL_CONTENT, | ||
| PROJECT0_V1_OVERRIDES_CONTENT, | ||
| "Project 0", |
There was a problem hiding this comment.
Nit: this protocol key has a space and capitals ("Project 0"). Other collections use slug keys (kamino-lend, drift, ...). by_protocol lookups and any Studio grouping or URLs built from this key will need the exact display string. Consider a slug such as project0, with the display name kept elsewhere, unless Studio already expects this form.
| label: Kink rate | ||
| description: >- | ||
| Borrow rate at this curve point, out of 1000% as u32 (100% = 429496729). Example: 42949672 | ||
| - "config.interest_rate_config.curve_type" |
There was a problem hiding this comment.
curve_type is exposed as an editable property, but the context below says "Keep curve_type at 1; 0 is the retired legacy curve". If nothing needs to change it, dropping it from properties avoids a foot-gun in the Studio editor.
| properties: | ||
| - "config.interest_rate_config.zero_util_rate" | ||
| - "config.interest_rate_config.hundred_util_rate" | ||
| - path: config.interest_rate_config.points.0.util |
There was a problem hiding this comment.
Only points.0 (and balances.0 in project0-account-balance) is declared. The editor therefore shows one point/slot, and "change the index for other points" works only for callers who build paths by hand. Real accounts often put the position you want in slot ≥1, so this may be worth noting in the description, or the most common indices could be exposed.
| let program_data = fetch(&[PROGRAM_DATA]).await.remove(0).data; | ||
| let deployed_slot = u64::from_le_bytes(program_data[4..12].try_into().unwrap()); | ||
| assert_eq!( | ||
| deployed_slot, DEPLOYED_SLOT, |
There was a problem hiding this comment.
A hard assert_eq! on DEPLOYED_SLOT makes both behavioural tests fail on every Project 0 upgrade. That already happened once (67f8a12). If the goal is to flag that re-verification is needed, that's fine. Otherwise, consider checking the layout offsets the tests rely on, or printing a clear skip reason, so CI with integration-tests isn't red for unrelated PRs.
| ), | ||
| ], | ||
| ); | ||
| let reference = ((depeg as i128) << 48) / 10i128.pow(exponent.unsigned_abs()); |
There was a problem hiding this comment.
10i128.pow(exponent.unsigned_abs()) assumes a negative exponent. With a positive exponent it would divide where it should multiply. It is fine for USDC (-8), but an assert!(exponent < 0) would make the assumption explicit.
…project0 # Conflicts: # crates/core/src/surfnet/svm.rs
…hed for Project 0
There was a problem hiding this comment.
I found one documentation inconsistency worth fixing before merge. Otherwise the PR looks solid.
What the PR does
- Adds Project 0 (marginfi v2, 0.1.12) support: the IDL, 8 Bank and MarginfiAccount templates, and a how-to README.
- Adds a shared
llm_contextfor the whole collection, which is placed before each template's own context. - Lets unit enum variants be set by name (e.g.
"ReduceOnly") instead of{"ReduceOnly": null}. - Moves
diff_indicesinto the shared test helpers.
What I checked
- Scaled numbers in the YAML: every example matches its 2^48 value: 0.8, 0.9, 1.1, 1.25, 0.3, $0.95, $20, 10 SOL and 8 SOL of shares, and the 1.1× debt-share example. The utilization and rate values on the u32 scales (50% = 2147483647, 100% APR = 429496729) are also correct.
- Bank lookup filter: the base58
"4"filter does match byte value 3. - IDL hash: the bundled IDL's sha256 matches the README.
- Enum-by-name arm in
svm.rs: it is narrow. It only applies to fieldless variants that exist in the IDL, and its test covers unknown variants and variants that carry data. - Shared
llm_context: the merge logic is straightforward and has a test. - Integration tests: they compare against the deployed program in LiteSVM, which checks more than byte offsets alone would.
Issues
- Contradictory keep-alive text (inline comment): the last commit updated the keep-alive wording in the README and in
project0-bank-risk-weights, butproject0-bank-oraclestill says the other banks' prices age. - PR description vs README: the PR description says the bundled IDL has "one change", but the README lists two. The second one is the
addressfield being filled in. Worth aligning. - Test maintenance:
project0_elfasserts on a fixedDEPLOYED_SLOT. Any mainnet upgrade will fail the integration suite until someone bumps it. That's deliberate, but it's worth noting.
|
|
||
| A price is stale once the Surfnet clock is more than config.oracle_max_age seconds past its | ||
| publish_time. Lower oracle_max_age to make the bank refuse a price sooner, or raise it (max | ||
| 65535) to accept older prices. A price account a pyth-core-compatible-price-feed-v2 override |
There was a problem hiding this comment.
This contradicts the last commit (7205f10). The README's "Prices on a fork" section and the project0-bank-risk-weights context now say that once an override plays, keep-alive keeps every loaded Pyth Core Compatible price published. This line still says only the overridden account is republished and "the other banks' prices age". An LLM reading this template could wrongly decide it has to raise oracle_max_age on the other banks. Suggested wording: "Once a pyth-core-compatible-price-feed-v2 override plays, every loaded Pyth Core Compatible price account is republished every slot; a bank priced by another oracle still ages."
…blished in project0-bank-oracle
There was a problem hiding this comment.
Summary
This PR adds Project 0 (marginfi v2) support: the bundled 0.1.12 IDL with WrappedI80F48.value retyped to i128, 8 Bank/MarginfiAccount templates, a collection-level llm_context shared by every template in a collection, enum fields that take a variant name, a shared diff_indices test helper, and live and LiteSVM tests.
Overall it's solid and well tested. I checked by hand all the fixed-point constants in the YAML and README: 0.8, 0.9, 1.25, 1.1, 0.3, $0.95, $20, the 10 SOL / 8 SOL / 500 USDC share counts, 1.375, and the u32 rate and utilization encodings. All of them are correct. The bundled IDL's sha256 matches the README.
Notes
- Description mismatch: the PR body says the IDL has "one change", but the README (line 7) lists two: the
i128retype and the filled-inaddress. Please align them. - Brittle test pin:
DEPLOYED_SLOTis hard-coded intests/project0/mod.rs, sorisk_engine_*andusdc_breaker_*fail on every program upgrade. That looks intentional ("re-verify the templates"), and commit 67f8a12 already had to follow a redeploy. Still, a CI job runningintegration-testswill go red with no code change. Consider documenting that, or gating these two tests separately. - Live-state assumptions: the behavioural tests assume things about current mainnet state, e.g. that the USDC bank has breaker bit 11 set and that
ACCOUNTkeeps its layout and slots. Clear skip or failure messages when those assumptions break would make the tests easier to maintain. - Naming:
protocol: Project 0contains a space, while most protocol keys are single tokens (kamino-lend,drift, …).Phoenix Eternalsets a precedent, so it's fine, butby_protocol("Project 0")callers in Studio/MCP need the exact string.
| description: Override one balance slot of a Project 0 MarginfiAccount | ||
| idl_account_name: MarginfiAccount | ||
| properties: | ||
| - "lending_account.balances.0.active" |
There was a problem hiding this comment.
Only slot 0 is declared as a property. The llm_context ("Replace the 0 in the paths") and the README's "Build a position from scratch" recipe both write lending_account.balances.1.*. That works because IDL templates accept any path. But slot 1+ fields have no label or description in Studio or get_override_templates, and the form editor can't reach them. Consider declaring a second slot, or saying in the template description that other slots are edited by path.
| .collect::<SurfpoolResult<Vec<_>>>()?; | ||
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. |
There was a problem hiding this comment.
Looks good. One nit: a string naming a variant with fields (e.g. "Limited"), or an unknown name, falls through to the later arms and fails with a generic type-mismatch error. An explicit error such as "variant Limited of Status has fields; pass {"Limited": [...]}" or "unknown variant" would make the failure much clearer for editors and LLM callers.
| 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.
Minor: a template whose llm_context is empty or whitespace-only (llm_context: "") yields "shared\n\n". Filtering out blank own (e.g. entry.llm_context.filter(|s| !s.trim().is_empty())) would keep the output clean.
| {"encoding":"base58","dataSlice":{"offset":610,"length":32},"filters":[ | ||
| {"memcmp":{"offset":0,"bytes":"QnTef4UXSzF"}}, | ||
| {"memcmp":{"offset":41,"bytes":"4qp6Fx6tnZkY5Wropq9wUYgtFxXKwE6viZxFHg3rdAG8"}}, | ||
| {"memcmp":{"offset":8,"bytes":"<mint>"}},{"memcmp":{"offset":609,"bytes":"4"}}]}. |
There was a problem hiding this comment.
The 609 == 3 filter depends on the OracleSetup discriminant (PythPushOracle = 3), and the 610 slice on the current Bank layout. Since program upgrades already change the Bank size, it's worth noting next to these offsets that they were verified for 0.1.12, so a future IDL bump updates them together. The live llm_context_bank_lookup_* tests catch drift, but only when they're run.
There was a problem hiding this comment.
Summary
This PR adds Project 0 (marginfi v2, v0.1.12) support: the bundled IDL, 8 override templates for Bank and MarginfiAccount, and a README. It also makes three small cross-cutting changes:
- A collection-level
llm_contextthat is placed before each template's own context. - Unit enum variants can now be passed by name in IDL overrides.
- The
diff_indicestest helper is moved into the shared helpers module.
Overall the PR is solid. I checked all the fixed-point examples against the 2^48 scaling (0.8, 0.9, 1.1, 1.25, 0.3, $0.95, $20, 10 SOL and 500 USDC shares, liability ×1.1) and they are correct. The rate and utilization encodings (u32::MAX/10, u32::MAX/2) are correct too. So are the getProgramAccounts offsets and filters: group at 41, mint at 8, oracle_setup at 609 where "4" in base58 is 3, oracle_keys.0 at 610, and MarginfiAccount group/authority at 8/40. The enum-by-name match arm is limited to unit variants that exist in the IDL, and it has a focused unit test.
Minor points
- Docs mismatch: the PR body says the bundled IDL differs from the published one by "one change". The README (line 7) says "two changes":
WrappedI80F48.valueandaddress. One of them should be updated. - Flaky live tests: the
llm_context_*_lookup_*tests callgetProgramAccountson the Project 0 program through the default public RPC. Public mainnet endpoints often reject or rate-limit GPA calls on large programs. Consider documenting thatSURFPOOL_TEST_RPC_URLis effectively required for these tests, or skipping them when the RPC returns a GPA-disabled error, so they don't panic. - Duplicated lookup guidance: the collection
llm_context(bank and wallet lookup) is now placed before every template's context, includingproject0-account-authorityandproject0-bank-interest-rate. That is fine, but it makes every Project 0 template's context noticeably longer. Keep this in mind if prompt size becomes a concern. - Hard-coded offsets: offsets 609/610 sit inside
configand are hard-coded in the YAML. They are only checked by the network-gated tests. A non-network unit test that derives these offsets from the bundled IDL would catch drift when the IDL is next bumped.
| if idl_types.iter().any(|t| { | ||
| &t.name == name | ||
| && matches!(&t.ty, IdlTypeDefTy::Enum { variants } | ||
| if variants.iter().any(|v| &v.name == variant && v.fields.is_none())) |
There was a problem hiding this comment.
v.fields.is_none() treats only a missing fields key as a unit variant. Some IDL generators emit "fields": [] for unit variants. That deserializes to Some(Named/Tuple(vec![])), so the by-name path would silently not apply and the value would fall through to json_to_txtx_value as a plain string. Consider also accepting an empty field list.
| than config.oracle_max_age seconds; a health check also reads the prices of the other banks | ||
| the account uses, and once the override plays every loaded Pyth Core Compatible price stays | ||
| published too. A bank priced by another oracle is not: raise its oracle_max_age with | ||
| project0-bank-oracle. Read the bank first: when its |
There was a problem hiding this comment.
The line breaks here are uneven: project0-bank-oracle. Read the bank first: when its is followed by a short flags include... line. It reads fine to an LLM, but reflowing this paragraph would match the rest of the file.
| known as marginfi v2 (`MFv2hWf31Z9kbCa1snEPYctwafyhdvnV7FZnsebVacA`, v0.1.12), so a scenario can | ||
| put a bank or a user account into whatever state you need before your code runs against it. | ||
|
|
||
| The bundled IDL is the program's published 0.1.12 IDL with two changes. `WrappedI80F48.value` is |
There was a problem hiding this comment.
The README says the bundled IDL has "two changes" from the published one, but the PR description says "one change". Please align them; the README is the more accurate of the two, since it also mentions address.
There was a problem hiding this comment.
Summary
This PR adds Project 0 (marginfi v2): a bundled 0.1.12 IDL, with WrappedI80F48.value retyped to i128, and eight Bank and MarginfiAccount templates. It also includes:
- a collection-level
llm_contextthat is placed before each template's own context - a conversion that lets a unit enum variant be passed by name
- a shared
diff_indicestest helper - live tests and LiteSVM tests that run the deployed program
I recommend merging after #33.
What I checked
- Rust changes: they are small and correct. The new enum match arm only applies to unit variants of a
Definedenum type and comes before the struct-object arm.Option<Enum>still works through the existingOptionrecursion. Thellm_contextmerge handles all four cases (shared and/or own context present or missing), and there is a test for it. - Example values: I recomputed the fixed-point numbers in the YAML and README and they are all correct:
- weights: 0.3, 0.8, 0.9, 1.1 and 1.25 × 2^48
- prices: $0.95 and $20 × 2^48
- shares: 10 SOL, 8 SOL and 500 USDC
- liability share value: 1.375 × 2^48
- interest-rate u32 encodings
- the base58
"4"for an oracle setup byte of 3
- Bank lookup filters: the offsets agree with the Bank layout (group at 41, mint at 8). The live tests check them against mainnet.
Minor points
- IDL changes: the project README says the bundled IDL differs from the published one in two ways (
i128and the filled-inaddress). The PR body says one. Worth making them match. - Prompt size: the collection
llm_contextis about 30 lines and is now copied into all eight templates. Any client that lists every template (for exampleget_override_templates) gets it eight times. That's acceptable, but if the MCP layer can show it once per protocol, that would save tokens. - Redeploy pin:
DEPLOYED_SLOTpins the behavioural tests to one program deployment. This is intentional and the failure message is clear, but every Project 0 upgrade will fail the integration suite until someone re-verifies it (this has already happened once, in 67f8a12). Consider documenting that step in the project README.
| .collect::<SurfpoolResult<Vec<_>>>()?; | ||
| Ok(Value::Array(Box::new(converted))) | ||
| } | ||
| // The encoder takes a unit variant as `{"Variant": null}`; editors and clients send its name. |
There was a problem hiding this comment.
Nit: when the string names a variant that has fields (e.g. "Limited" in the test), or is in the wrong case, this arm is skipped. The value then falls through to json_to_txtx_value as a plain Value::String, and the encoder's error won't mention enums. Consider adding an arm for (Defined enum, String) that returns an error listing the valid unit variant names. That would give editors and LLMs something they can act on.
| .idl_account_name | ||
| .unwrap_or_else(|| default_account_type.clone()); | ||
| let llm_context = match (&self.llm_context, entry.llm_context) { | ||
| (Some(shared), Some(own)) => Some(format!("{}\n\n{own}", shared.trim_end())), |
There was a problem hiding this comment.
This prepends the whole shared context to every template, so for Project 0 the lookup recipe appears 8 times in an all-templates listing. That works. If token size matters for MCP clients, another option is to expose the shared context once on the collection or protocol.
There was a problem hiding this comment.
PR #34: Project 0 (marginfi v2) state templates
This PR adds 8 Bank and MarginfiAccount templates on the bundled 0.1.12 IDL. It also makes three smaller changes:
- Enum names as strings: a unit enum variant can be written by its name (e.g.
"ReduceOnly") injson_to_txtx_value_for_idl_type. - Shared
llm_context: a collection can carry anllm_contextthat goes in front of every template's own context. - Shared test helper:
diff_indicesmoves into the test helpers instead of being copied per suite.
Overall: the PR is well structured and ready to merge after #33. The tests are strong: live round-trips, a per-property byte-isolation check, and LiteSVM runs of the deployed program.
What I checked
- Fixed-point examples: every one in
overrides.yamlis right.- Weights: 0.8, 0.9, 1.1, 1.25 and 0.3.
- Prices: $0.95 and $20.
- Shares: 10 SOL and 8 SOL.
- The 1.1x liability share value is right too.
- Rate and utilisation encodings: correct.
- Base58 enum byte:
"4"is the base58 encoding of byte value 3, so the oracle-setup filter is right. - Enum match arm: safely limited to unit variants. An unknown name, or a variant that carries fields, still falls through and fails, and the new test covers both cases.
Minor points / suggestions (details in the inline comments)
- Protocol key: the key is
"Project 0", with a space and capitals. The newer Kamino collections use kebab-case keys (kamino-lend, …). - Repeated context: the collection-level
llm_contextis about 30 lines and is copied into all 8 templates. Anything that sends several templates to an LLM repeats it each time. That's fine for now, but keep it short. - Hardcoded offsets: the lookup offsets (8/41/609/610) are fixed in the prompt text. The live lookup tests will catch drift, but the next layout change has to update the YAML and the tests together.
- Deploy-slot pin:
project0_elfpins the deploy slot, so every redeploy breaks all the behavioural tests. The git history shows this has already happened once.
| self.load_protocol_overrides( | ||
| PROJECT0_V1_IDL_CONTENT, | ||
| PROJECT0_V1_OVERRIDES_CONTENT, | ||
| "Project 0", |
There was a problem hiding this comment.
Nit: the protocol key "Project 0" (space, capitals) doesn't match the kebab-case keys the other recent collections use (kamino-lend, kamino-scope, …). Callers and Studio filters then have to use by_protocol("Project 0"). Consider project0 as the key and keep "Project 0" as a display name only.
| .idl_account_name | ||
| .unwrap_or_else(|| default_account_type.clone()); | ||
| let llm_context = match (&self.llm_context, entry.llm_context) { | ||
| (Some(shared), Some(own)) => Some(format!("{}\n\n{own}", shared.trim_end())), |
There was a problem hiding this comment.
Minor: the two branches don't treat whitespace the same way. When both contexts exist, the shared one is trim_end()ed. When only the shared one exists, it is returned untouched. Separately, putting the shared block in front of every template copies Project 0's ~30-line lookup recipe 8 times into anything that sends several templates to an LLM. It may be worth exposing it once per collection instead, or at least keeping it short.
| let program_data = fetch(&[PROGRAM_DATA]).await.remove(0).data; | ||
| let deployed_slot = u64::from_le_bytes(program_data[4..12].try_into().unwrap()); | ||
| assert_eq!( | ||
| deployed_slot, DEPLOYED_SLOT, |
There was a problem hiding this comment.
Pinning DEPLOYED_SLOT makes every behavioural test fail on any program redeploy, even one that doesn't touch the layout. The history already shows one such bump. Checking the account layout (Bank size and discriminator) or an IDL hash might be less brittle, if the goal is to catch changes that actually matter. Otherwise, document in the README that this is expected maintenance.
| {"encoding":"base58","dataSlice":{"offset":610,"length":32},"filters":[ | ||
| {"memcmp":{"offset":0,"bytes":"QnTef4UXSzF"}}, | ||
| {"memcmp":{"offset":41,"bytes":"4qp6Fx6tnZkY5Wropq9wUYgtFxXKwE6viZxFHg3rdAG8"}}, | ||
| {"memcmp":{"offset":8,"bytes":"<mint>"}},{"memcmp":{"offset":609,"bytes":"4"}}]}. |
There was a problem hiding this comment.
The offsets 8, 41, 609 and 610 are fixed in the prompt text. The live llm_context_bank_lookup_* tests will catch drift, but consider adding a comment that ties these offsets to the test constants (ORACLE_SLICE etc.), so a future IDL bump updates both places.
Adds Project 0 (formerly marginfi v2,
MFv2hWf31Z9kbCa1snEPYctwafyhdvnV7FZnsebVacA) state prep.Eight templates on the bundled 0.1.12 IDL: bank risk weights, limits, operational state, oracle guards, interest rate curve and share accounting, plus account position and authority. The IDL is the one the program publishes, with two changes:
WrappedI80F48.valueis typedi128, so fixed-point fields take decimal strings instead of 16-byte arrays, andaddress, empty in the published IDL, holds the program's address. The README records the sha256 of both files.pyth-core-compatible-price-feed-v2; merge feat(scenarios): add the Pyth Core Compatible receiver price feed template #33 first.pyth-core-compatible-price-feed-v2override withkeepAlive: trueplays, keep-alive (feat(scenarios): add the Pyth Core Compatible receiver price feed template #33 on feat(scenarios): keep-alive for every account of a kind, and per-override stop and restart #53) keeps every Pyth Core Compatible price account the surfnet has loaded published, so a health check or liquidation finds every bank's price fresh. A bank priced by another oracle still ages; raise itsoracle_max_agewithproject0-bank-oracle. To keep one price stale while the others stay fresh,project0-bank-oracle'sllm_contextand the README's "Prices on a fork" section give the recipe: one price override withkeepAlive: falseand one withkeepAlive: true.flagsinclude bit 11) that rejects borrow, withdraw and liquidation once the price, net of 2.12 x its confidence, moves the first tier from its reference: 50 bps for USDC, about a 0.7% move at a typical confidence.project0-bank-oracleexposes the two reference prices, so a depeg scenario can move them with the price."ReduceOnly"(a small change in the IDL value conversion, with a generic test).llm_contextand thediff_indicestest helper. The MCPparamsdescription comes with feat(scenarios): add the Pyth Core Compatible receiver price feed template #33.Tests: live tests round-trip real mainnet banks and accounts, check that every template property changes only its own bytes, and run the
llm_contextbank lookup against mainnet. Behavioural tests run the deployed program in LiteSVM: the weight and price levers make a SOL-backed USDC borrow liquidatable, and a 5% USDC move trips the circuit breaker until the reference prices follow it. Verified in Studio with real transactions through the editor, AI prompts and the chip.Studio: LimeChain/surfpool-web-ui#28