Skip to content

Add hand-written Anchor-compatible IDL - #65

Open
kaze-cow wants to merge 24 commits into
mainfrom
idl
Open

Add hand-written Anchor-compatible IDL#65
kaze-cow wants to merge 24 commits into
mainfrom
idl

Conversation

@kaze-cow

@kaze-cow kaze-cow commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

Generate a Solana IDL using AI, and validate its baseline correctness using smoke tests.

Summary

  • Adds an AI-generated Anchor-compatible IDL (programs/settlement/idl/cow_settlement.json) describing the settlement program's instructions/accounts/types, for IDL-driven tooling (e.g. Solscan). This program is native/Pinocchio, not Anchor, so there's no generated IDL to start from.
  • A few spots can't be fully expressed in the IDL grammar and are documented inline via docs fields instead:
    • BeginSettle's dynamically-shaped tail (order count / bumps / transfer counts / pull amounts) has no Borsh-expressible layout (no length prefixes, and a trailing array whose length is the sum of an earlier array).
    • order_pda's PDA seed is sha256(intent_bytes) — a hash of the whole instruction argument, not a plain field/account reference the PDA-seed grammar can point at.
    • create_buffer's account list only includes the first buffer account, since its not possible to specify more than one account as an array specified. Additional buffer accounts must be specified manually.
  • add some smoke tests to validate that the . In particular, the tests enforce that many changes in instructions, account data, or errors will come with IDL updates.

How to review this PR

The IDL file is quite long. To minimize the amount of excess effort needed, after only a quick review/skim of the IDL file itself, check out the tests and see what properties are checked/validated.

Summary of coverage

The tests are primarily focused on identifying drift rather than fundamental correctness.

For fundamental correctness, it is expected that further tests will be included in #73 , as its much easier to test the functional outputs of the IDL (for example, the TS library) for successful encoding behavior.

Summary of what is covered and what is not:

Covered
File-level

  • Valid JSON; canonically pretty-formatted with trailing newline
  • Conforms to the bundled IDL spec schema, and the schema's $id version matches metadata.spec
  • address == declare_id!; metadata.version == CARGO_PKG_VERSION

Cross-checked against Rust source (via syn)

  • Instructions and Accounts: every rust enum variant present with matching single-byte discriminator and doc paragraphs, and no superfluous entries defined
  • Struct types: OrderIntent, OrderAccount, StateAccount -> SettlementState — docs, field name/order/type
  • Enum types: OrderKind, Role — docs, variant names in order, and any Rust variant pinning = N must agree with its index (the only thing the spec can express about a variant's wire value)
  • Instruction args: all 8 instructions — 4 compared field-by-field against their builder structs (name, type, order), create_order handled explicitly, 3 asserted arg-free. New instructions will need to have the appropriate check added manually here.
  • Errors: every SettlementError variant with matching code/msg, plus no extra entries
  • PDA seeds: every const seed is a real interface::pda constant; SETTLEMENT_SEED and BUFFER_SEED each appear

Not covered

  • Account names, order, signer/writable — Rust has no named account list. Flags only come from running the builders (positional, nameless); the names live inside parse_body's slice destructuring, and three instructions have variable-length tails. Doable, but half-covering at best.
  • Wire-format round-trip — This is better to do as integration tests with actual tests on the generated libs, such as in generate js client with codama and add a basic test to confirm works #73
  • Field/arg docs — the IDL has none for fields (0 of 17), and args document themselves inconsistently.
  • Instruction docs are subset-only — deliberate; some additional things (such as compatibility notes) should be included as additional documentation/comments.
  • metadata.name — cow_settlement is the [lib] name, which cargo doesn't expose to the test. Any check just hardcodes the string twice.

How to test

The smoke tests are run alongside the existing tests crate suite, so you can run just test to verify the tests.

Manually inspect the IDL file itself, especially the instructions and how they were translated. Comment on anything unusual or bad comments.

Check out #73 , which this PR is stacked upon, to see the IDL being used to generate a Javascript library. This Javascript library has its own tests verifying that the program can be interacted with on a LiteSVM instance!

New Dependencies!

jsonschema = "0.30" # Popular package with around a million weekly downloads
serde_json = { version = "1", features = ["preserve_order"] } # Part of the ubiquitous serde suite; one of the most downloaded crates, with something like 5 million weekly downloads
syn = { version = "2", features = ["full"] } # Second most downloaded crate on crates.io

Stacked on by #73

fixes kaze/sc-255-write-idl-and-generate-corresponding-libraries-for

🤖 Generated with Claude Code

Base automatically changed from discriminators to le-encoding-fixes July 16, 2026 05:52
Base automatically changed from le-encoding-fixes to main July 16, 2026 11:17
@socket-security

socket-security Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedjsonschema@​0.30.0100100100100100

View full report


/// Canonical order intent. Also the exact bytes hashed (SHA-256) to produce the order UID used in the order PDA's seeds,
/// and the exact wire format of create_order's `intent` argument. Field order and encoding here are load-bearing: they
/// must match this program's Rust definition exactly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this comment was added because it probably should have existed in the first place, and not having it triggers an error in the IDL tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fine to add a comment, but this one is wrong, this is a struct, it doesn't store bytes, and its (Rust) encoding totally isn't the bytes hashed to produce the order UID. If anything, this is EncodedOrderIntent.

@kaze-cow
kaze-cow marked this pull request as ready for review July 29, 2026 15:01
@kaze-cow
kaze-cow requested a review from a team as a code owner July 29, 2026 15:01
@kaze-cow

kaze-cow commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@linear-code

linear-code Bot commented Aug 4, 2026

Copy link
Copy Markdown

SC-255

@fedgiac fedgiac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One overarching comment: I was never sure what exactly these tests check and what not. Maybe we can add docs to the start of the test file that state clearly what's being covered for each IDL field? This helps us in the future to understand what's needed to improve on the current tests and avoiding duplicated work. It also makes it obvious what a follow-up PR does in the code diff.

Something like this.
Top level:
- address ✔️ 
- metadata: partial
- docs: ❌ 
- instructions: partial
- accounts: partial
- events: ✔️ (no events) # and we should actually test this!
- errors: ✔️
- types: ...
- constants: ...

instructions:
- name ✔️ 
- docs ❌ 
- discriminator ✔️
- accounts ❌ 
- args ❌ 
- return ❌ 

...

Overall the design makes a lot of sense. It was too complex for the time allotted so I'll need to continue at a later point, but there are quite a bit of comments already.


/// Canonical order intent. Also the exact bytes hashed (SHA-256) to produce the order UID used in the order PDA's seeds,
/// and the exact wire format of create_order's `intent` argument. Field order and encoding here are load-bearing: they
/// must match this program's Rust definition exactly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fine to add a comment, but this one is wrong, this is a struct, it doesn't store bytes, and its (Rust) encoding totally isn't the bytes hashed to produce the order UID. If anything, this is EncodedOrderIntent.

Comment thread programs/settlement/idl/schema/idl-spec-v0.1.0.json Outdated
Comment thread programs/settlement/Cargo.toml
Comment thread programs/settlement/tests/idl.rs Outdated
Comment thread programs/settlement/tests/idl.rs Outdated
Comment thread programs/settlement/tests/idl.rs Outdated
Comment thread programs/settlement/tests/idl.rs Outdated
fn idl_matches_instruction_discriminators() {
let idl = idl();
for byte in 0u8..=255 {
if let Ok(ix) = SettlementInstruction::try_from(byte) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably at this point it wouldn't be that bad to create a function similar to parse_instruction where we populate each builder with placeholder data. This is super helpful because then we can check everything in an instruction automatically (number of accounts, order, whether it's signer/writable), the discriminator comes for free.
Also, we're going to remember to add a new function because we need to add a new variant to compile.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

didn't this actually get added by the backend team? not sure.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Right, this is exactly fn build(instruction: SettlementInstruction) -> Instruction, we can use that for testing.

Comment thread programs/settlement/tests/idl.rs Outdated
Comment on lines +180 to +182
/// Translates a Rust field type into the IDL spec's type grammar, so field
/// types can be compared as JSON. Panics on anything the program's data types
/// don't currently use.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be much nicer if the Rust fields were converted to a struct with all relevant content, the same for the fields in the JSON, and then the two fields were compared with each other. This should give a clearer diff and overall be more flexible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yea ok that makes sense, but how does that have to do with the specific segment of code you highlighted? are you suggesting we should focus on stringifying here instead of constructing a json! type?

Comment thread programs/settlement/tests/idl.rs Outdated
Comment on lines +201 to +208
let syn::Expr::Lit(syn::ExprLit {
lit: syn::Lit::Int(len),
..
}) = &array.len
else {
panic!("{context}: array length must be an integer literal");
};
let len: u64 = len.base10_parse().expect("array length must be a u64");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was dark magic to me. I'd suggest isolating this into a function get_array_length or something.

Comment thread programs/settlement/tests/idl.rs Outdated
kaze-cow and others added 9 commits August 20, 2026 16:47
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
* add comments for settlement instruction and validate match
* simplify superfluous comments in the IDL in general
* switch to using `LazyLock` and update call sites
Comment thread programs/settlement/tests/idl.rs Outdated
Comment thread programs/settlement/tests/idl.rs Outdated
Comment thread programs/settlement/tests/idl.rs Outdated
fn idl_matches_instruction_discriminators() {
let idl = idl();
for byte in 0u8..=255 {
if let Ok(ix) = SettlementInstruction::try_from(byte) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Right, this is exactly fn build(instruction: SettlementInstruction) -> Instruction, we can use that for testing.

Comment thread programs/settlement/Cargo.toml
Comment thread interface/src/lib.rs
Comment on lines +20 to +41
/// Pulls funds for a batch of orders. Must be paired in the same
/// transaction with a `FinalizeSettle` at `finalize_ix_index`.
BeginSettle = 0,
/// Validates that a `BeginSettle` at `begin_ix_index` exists and points
/// back at this instruction. Must not be called via CPI.
FinalizeSettle = 1,
/// Allocates a per-order PDA and writes the initial `OrderAccount` body.
CreateOrder = 2,
/// Creates the singleton settlement state PDA. Succeeds only once.
Initialize = 3,
/// Creates one or more per-token buffer PDAs (SPL token accounts) in a
/// single instruction.
///
/// Each buffer_pda_i must be the canonical PDA for seeds
/// [SETTLEMENT_SEED, mint_i, "buffer"].
CreateBuffer = 4,
/// Closes an expired order PDA and returns its rent lamports to the
/// created_by account recorded in the order body. The instruction may only
/// be executed after the order's valid_to timestamp has elapsed.
///
/// No signature requirement: anyone may reclaim an expired order on behalf
/// of its reclaim_recipient.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: I don't think any of these descriptions says something that someone reading this for the first time should be reading.

What I'd expect:

  • Begin/Finalize: they process user orders, one takes funds from the user, the other sends funds to the user, respecting limit prices.
  • CreateOrder: lets an owner create an order for the protocol.
  • Initialize: determines the initial parameters of the protocol, like the authorities.
  • CreateBuffer: ok but the second line is waaay too specific compared to everything else.
  • ReclaimOrder: fine-ish but I wouldn't use variable names, rather an actual name describing what happens, like "after order expiration."

Comment on lines +55 to +56
/// Since each IDL section follows the same pattern where each section is an array of objects which contain a field `name`, this function
/// is useful for finding just about any item we need in the IDL file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very nitty but line length is weird. The comment itself is very helpful.

Comment on lines +123 to +127
if let Some(Value::Array(bytes)) = map.get("value") {
let decoded: Vec<u8> = bytes
.iter()
.map(|b| b.as_u64().expect("seed byte must be a number") as u8)
.collect();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Repeating code from discriminator. In particular, this code introduces a bug by using as unlike the code from the other function. Maybe we should just introduce decode_byte_array and use that in both functions. (Maybe discriminator isn't needed anymore then.)

Comment thread programs/settlement/tests/idl/main.rs Outdated
out
}

fn confirm_idl_match(idl_section: Section, idl_name: &str, discriminator_byte: u8) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: a comment should specify that this is only for accounts and instructions, it sounds too generic given its name. Also the match part is too generic, there are quite a few fields not being checked. None of this is really a problem per se, it's just that the name of the function doesn't show it.

Comment thread programs/settlement/tests/idl/main.rs Outdated
}

#[test]
fn idl_matches_account_discriminators() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested extra test: this test is exhaustive, that is, there are no accounts with a discriminator of more than 1 byte. Probably the same applies to instructions.

it increases the amount of code overall, but it puts us in the right
trajectory to be effectively generating parts of the IDL from rust.

@fedgiac fedgiac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A lot of things are going on and it's overall complex logic to review; I think there are a few places where a redesign could help.
However, there's a strong pressure to have the IDL ready and this is progress.
I only have a shallow understanding of the current code but since it doesn't affect the rest of the program, I think we can merge.
None of my comments prevent merging.

Comment on lines +4 to +8
//! [`assert_superset`] holds the checked-in file to everything
//! [`crate::generate`] could derive, and to nothing more: an IDL key the
//! generated document doesn't mention is a fact the Rust source can't state, so
//! it's left for the schema and the human reviewer. That asymmetry is the whole
//! design — the generated side never has to be a complete IDL, only a true one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: the comment is useful but written weirdly imho. "hold" here makes sense somewhat but takes me unnecessary effort to parse. This is true for the entire sentence.

Maybe:

assert_superset checks that the checked-in IDL is consistent with the JSON generated by crate::generate. The checked-in version may contain more data than what has been generated, but the available information cannot be different.

Comment on lines +179 to +180
/// [...]}`, with the fields in declaration order, which is the order they're
/// laid out on the wire.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I never intentionally sorted structs based on that, is this really true?

Comment on lines +194 to +198
/// An enum as an IDL `types[]` entry's `type`: `{"kind": "enum", "variants":
/// [...]}`, with the variants in declaration order, which is the order the wire
/// discriminant counts in. The spec's `IdlEnumVariant` carries a name and
/// nothing else — no discriminant, since a variant's index is its wire byte,
/// and nowhere to put docs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't really understand this comment.

Comment on lines +221 to +223
/// The display text and the remainder past `[display](target)`, when `text`
/// starts with a link whose two halves nest no brackets of their own.
/// Anything else isn't a link and is left exactly as written.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I spent about 1 minute reading this and I didn't understand this. 😅

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Conversely, understanding the code took me 5 seconds.

}
}
}
out.push_str(rest);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Links aren't stripped, they are just pushed to the end, right?
So: "There are two ixs, Begin and Finalize" becomes "There are two ixs, Begin and Finalize::begin::finalize" right? I'd drop that.
I didn't see this happen when skimming the IDL by the way, I suppose there are no links there?

Comment on lines +168 to +169
/// Checks the IDL documents everything the Rust source does, in the same order.
/// The IDL may contain more docs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comparing the generated and checked-in IDL I see this:

      "name": "create_buffer",
      "docs": [
+        "Creates one or more per-token buffer PDAs (SPL token accounts) in a single instruction.",
+        "IDL LIMITATION: the real instruction accepts an unbounded number of (buffer_pda, mint) pairs as remaining accounts, one pair per buffer, with at least one pair required (CreateBuffer rejects zero buffers). IDL grammar has no 'repeated group' construct, so this file only declares the guaranteed index-0 template (buffer_pda_0/mint_0).",
+        "Each buffer_pda_i must be the canonical PDA for seeds [SETTLEMENT_SEED, mint_i, \"buffer\"]."
-        "Creates one or more per-token buffer PDAs (SPL token accounts) in a single instruction.",
-        "Each buffer_pda_i must be the canonical PDA for seeds [SETTLEMENT_SEED, mint_i, \"buffer\"]."
      ],

It looks like we only compare the first line of the docs and all the remaining lines in the generated IDL are ignored.

Comment on lines +11 to +14
//! The tables below are the one place a name has to be written twice. They
//! exist because nothing in the Rust source says which file holds an
//! instruction's parsed input, which struct backs a `types[]` entry, or which
//! of an instruction's accounts the IDL derives as a PDA.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is a table? I don't see any table and I don't see any duplication.

Comment on lines +45 to +52
/// A per-token buffer PDA. The IDL can only declare the guaranteed index-0
/// buffer of the unbounded run an instruction actually accepts, so the mint it
/// derives from is `mint_0`.
const BUFFER_PDA_0: &[Seed] = &[
Seed::Const(SETTLEMENT_SEED),
Seed::Account("mint_0"),
Seed::Const(BUFFER_SEED),
];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Where is the string mint_0 coming from? If we use a string referenced from the IDL, the seeds cannot be a constant.

Comment on lines +59 to +64
/// The file declaring `<variant>Input`, the struct [`args`] reads.
input: &'static Source,
/// The accounts the IDL declares a `pda` for, and the seeds that PDA is
/// derived from. Accounts without one aren't listed: nothing in the Rust
/// source pins the name the IDL gives them.
pda_accounts: &'static [(&'static str, &'static [Seed])],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

&'static? Why? Is there any reason not to own data?

Instruction {
variant: SettlementInstruction::ReclaimBuffer,
input: &parse_rust::RECLAIM_BUFFER_RS,
pda_accounts: &[("state_pda", STATE_PDA), ("buffer_pda_0", BUFFER_PDA_0)],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see now why the constant was hardcoded, the current design forced this choice. Fine enough, not worth a refactor.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants