Skip to content

Add hand-written Anchor-compatible IDL - #65

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

Add hand-written Anchor-compatible IDL#65
kaze-cow wants to merge 20 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.

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.

@@ -0,0 +1,780 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"$id": "https://github.com/solana-idl/idl-spec/schema/v0.1.0.json",

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.

Not found? Image

I independently found this: https://github.com/solana-foundation/idl-spec/blob/main/schema/v0.1.0.json
It's exactly the same, the url doesn't change. 😅
Fine to keep then if this is the official one. In general it would be nice in the "how to review" step of the PR description to have something like diff <(curl ...) ./.../idl-spec-v0.1.0.json.

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.

omg good eye

I put up a PR request upstream 😆 solana-foundation/idl-spec#3 and will fix it here since it appears to be a trivial error/issue.

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.

My suggestion would be to create a new crate with just the IDL, so that people can import a dependency-free crate if they want to.
If you don't like this, I'd still suggest to move this to the interface crate (or client? unsure) since there's no released package for the settlement.

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.

how about we add this as an NPM package instead?

The problem is that its not normal to push up non-rust files as a cargo crate. This pattern is a thing on NPM and it makes sense since the largest consumers of our IDL will be using codama, which is a (at least partially) node.js application installed with npm install codama or so.

Also, we don't actually publish the crate for the settlement program itself (nor do I think we will be doing so in the future) so I think it makes sense to put it here alongside the actual program code itself.

I don't feel too strongly on this, so lmk again if you think we should still switch.

Comment thread programs/settlement/tests/idl.rs Outdated
Comment thread programs/settlement/tests/idl.rs Outdated
Comment on lines +33 to +38
fn find_item_in_idl<'a>(idl: &'a Value, type_name: &str, name: &str) -> Option<&'a Value> {
idl[type_name]
.as_array()?
.iter()
.find(|ix| ix["name"] == name)
}

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.

If you use the previous suggestion, then you can drop the idl input since it costs nothing to use it here directly. I was confused at first since idl doesn't need to be the actual IDL, I wondered if it was expected to be a subfield.

In general this function is confusing: it doesn't find recursively (as I would have expected), it looks at a specific fields and, assuming it's an array, it searches an object entry with a specific "name".

Not sure how to change, but maybe we can drop it, do the type_name field inline (so something like idl["accounts"].as_array().expect()) and only keep a function "find_object_with_name"?

@kaze-cow kaze-cow Aug 20, 2026

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.

the reason it was set up this way is because in the IDL, instructions, accounts, errors, and types ALL are arrays containing objects that must contain a name field, so this is a really convenient/shorthand way of gaining access to the relevant data in a large number of tests.

here is my idea: lets use an actual enum here, and then the type_name will become much more clear as to what it actually signifies. And we can add a comment explaining what I just wrote above.

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.

Comment thread programs/settlement/tests/idl.rs Outdated
);
}
}
}

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.

Missing check: the length in the IDL is exactly the number of processed instructions.

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.

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
/// `idl_name` is passed separately because the two don't always agree:
/// `StateAccount` is called `SettlementState` in the IDL, matching the
/// `SettlementAccount` discriminator variant that names the account.
fn confirm_idl_types_entry(idl: &Value, rust_file_name: &str, rust_type_name: &str, idl_type_name: &str) {

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.

Stopping my review here, it's getting late. At this point I was unsure where this is going, I think we could benefit from reorganizing the structure of the file a bit, though I'm not sure how. Probably the main difficulty is that I'm not use to work with ASTs and so I need more time to think about this.

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