IDL generation through compilation

[kklas]

I've implemented an IDL generation method which instead of parsing the program source files generates the idl generation code by hooking into various anchor derive and attribute macros. This IDL generation code is then executed by running cargo test on the program crate. The benefit of this approach is that we're now able to evaluate expressions within the program source which allows for embedding additional information into the IDL.

The scope of this PR is to cover the same features that the current (parse) IDL generation has (I haven't implemented any of the eval stuff from #1927), with the addition of supporting external types (discussed #1972).

The change is fairly big so I've seperated it into multiple stand-alone commits to make the review easier. I strongly suggest to merge this via a rebase vs squash for the purposes of git history.

In summary, here is the diff between the IDLs generated by the parse and gen methods: https://www.diffchecker.com/UzVPnS0o
(for the following test program here)

Also, generics on accounts and AnchorSerialize structs are now supported. Here's the IDL it generates https://gist.github.com/kklas/1617d2b636e8d24e6789f3cf99519b1f generated for this program.

Notes

1. I have put the new IDL generation behind the anchor idl build command. The parsing method is still default when running anchor build so this PR doesn't actually introduce any breaking changes.
2. The changes to anchor macros have been put behind a idl-gen feature flag so there's no idl generation code generated when it's not enabled.
3. The new IDL references defined types via their "full_path" field vs "name". This means the new IDL cannot be used with old clients as they will search the types in the "types" field by their name. We can fix the clients to support both IDLs by e.g. searching for the name and then full_path (or vice versa).
4. SOLVED The new IDL generation method currently doesn't support generics (i.e. if you have account, event, etc. structs which are generics, the generation will fail with a compile error). There's a way to go around this which I believe would work support for generics in compile based IDL generation #2013
5. Since it's not possible to know which events are being referenced in the program instructions, the idl generation for events is done slightly differently than accounts (there's a separate test method which prints the IDL for each event). This means that external events (event structs imported from different crates) will not be included. I really like the idl import macro solution for these edge cases Imported types should be added to IDL #1972). With the compilation approach it should be fairly straightforward to implement.
6. DEPRECATED I didn't implement "state" stuff because it's going to get deprecated
7. PDA parsing is not covered here - we can add it later with eval
8. relations derivations is supported
9. in order to support generics without creating any breaking changes to the current IDL generation method, I had to introduce a few new IDL types (lang/syn/src/idl/types.rs)
10. idl-gen feature needs to be added to the [features] section of Cargo.toml of programs, including all external crates that we import types to be included in the IDL, e.g.:

[features]
idl-gen = [
    "anchor-lang/idl-gen",
    "some-external-program/idl-gen",
]

Implementation Details

The basis of the IDL generation is the hook in the AnchorSerialize derive macro (gated behind 'idl-gen' feature). It generates the following methods for each type that derives AnchorSerialize:

• __anchor_private_full_path() - calling this will return the type's full path (implemented using the std::module_path! macro)
• __anchor_private_gen_idl_type() - this will return the IdlTypeDefinition for the type when called
• __anchor_private_insert_idl_defined(defined_types: &mut HashMap<String, IdlTypeDefinition>) - a helper method which will insert any defined types in the types fields into the provided defined_types map.

#[zero_copy] does the same as above and #[account] and #[account(zero_copy)] are covered due to the fact that these attribute macros already add #[zero_copy] and #[derive(AnchorSerialize)] respectively.

#[derive(Accounts)] adds a __anchor_private_gen_idl_accounts method which will return Vec<IdlAccountItem> when called. It also adds any types referenced within Account<...> or AccountLoader<...> into the provided defined_types hash map.

#[program] generates a __anchor_private_print_idl_program function which is marked as a #[test]. When executed it will collect all the accounts from ctx by calling __anchor_private_gen_idl_accounts on the ctx type, and __anchor_private_gen_idl_type on each ix arg. This is how all the defined types are collected recursively. It will print out the Idl json.

#[event], #[error_code], and #[constant] work a bit different in that they also generate an __anchor_private_print... function marked as a test to print them out. This is because it's not possible to know which events, etc. are being referenced and we can't collect them recursively in #[program] as mentioned in the note 5).

Then the anchor idl build command runs cargo test __anchor_private_print_idl --features idl-gen -- --show-output --quiet on the crate and collects the Idl jsons from the stdout printed by the generated test methods. The events, errors, and constants are printed separately and are combined by the command into the final IDL json which is then outputted to stdout.
When running anchor idl build on the workspace with multiple programs, it will collect all the IDLs and print them in form of an array: [<idl1>, <idl2>]

[vercel]

@kklas is attempting to deploy a commit to the 200ms Team on Vercel.

A member of the Team first needs to authorize it.

[callensm]

What implications (if any) does adding prefixes to type definition names like idl::Bar and extern_program::some_module::Foo have on type inference during IDL parsing in the TS package?

[kklas]

What implications (if any) does adding prefixes to type definition names like idl::Bar and extern_program::some_module::Foo have on type inference during IDL parsing in the TS package?

I touched upon this in note 3). It means that current clients wont be able to parse the new IDL successfully but we can make the new clients work with both IDLs easily.
Alternatively we could have the "name" be default and use "full_path" only when referring to an external type.
The reason I did it this way is because it was simpler to implement and we can have this discussion later when we actually make the compile method the default.

Right now the generated IDL won't work with the current TS client until we change it to search for full_name.

[callensm]

ahh must have missed that part. so the introduction of the path segmented naming via full_path was added for name conflicted resolution (correct me if I'm wrong). would it not make more sense for backwards compatibility to do ExternProgramBaz as the name (converting the crate name to title case as the prefix) instead of a new full_path and doing extern_program::...::Baz that does the resolution?

this is a great PR, just trying to think of ways to make it less breaking for existing clients.

[kklas]

so the introduction of the path segmented naming via full_path was added for name conflicted resolution (correct me if I'm wrong).

Exactly. Name conflicts don't happen super often but when they do it's nasty so it's a good idea to guarantee unique names.

would it not make more sense for backwards compatibility to do ExternProgramBaz as the name (converting the crate name to title case as the prefix) instead of a new full_path and doing extern_program::...::Baz that does the resolution?

If I understood your suggestion correctly, this doesn't properly solve name conflicts because in theory you could have extern_program::some_module::Baz and extern_program::Baz and they would both resolve to ExternProgramBaz.

There probably isn't a way to introduce unique names and have this be fully backward compatible, but I think it's ok to have a breaking change here since anchor is v0 anyways. I'd rather have it be a breaking change rather than some frankenstein solution, but let's hear what Armani says.

Anyways since there are multiple ways to do it my idea was to do this in a separate PR not to complicate this PR more than it is.

[Henry-E]

Seems sad to let this much work go to waste but i don't really know if it would be possible to merge. The IDL stuff isn't something I'm greatly familiar with yet

[kklas]

I will take a look into this again when I finish what I'm currently working on and try to push it through. I think there might be a cleaner solution using schemars which seemingly already implements the functionality of this PR and also supports generics.

[Henry-E]

Wooh! Everyone I've talked to really likes this approach over the current IDL approach. So I really hope to be able to see this PR reviewed and merged into anchor

[kklas]

Yes, I also think this is the right approach. My only concern with it previously was the generics support, but now with this implemented it should be very solid. It needs to be tested on more programs but I wouldn't be surprised it's already more reliable than the current method, esp. w.r.t. generics.

There's still a bit of work to be done before it can replace the current IDL approach though, and it will require breaking changes to the IDL. I think that should be done in a separate PR, probably along with other breaking changes to the IDL (I feel like some things need to be consolidated in the IDL format).
Also the PDA support needs to be added here. It will probably work differently than the current PDA seed parsing method (and thus be a breaking change), but will be much more reliable (currently the PDA seed generation is hidden behind a feature and is not super reliable in my experience). I already made a POC for this in #1927.

Even so, this PR will already be useful for some people as it introduces:

• support for including external types into the IDL (including from other crates)
• proper support for generics

What I think is the right way forward:

1. merge this as a separate cli command anchor idl build so that people can test it out on their programs and edge cases are discovered if any
2. in the meantime, the PDA support is worked out, IDL format is consolidated (introducing breaking changes if needed), and support for parsing the new IDL is added to the clients
3. after the above is done and stable, this method is made the default one and the old one is marked deprecated

W.r.t. the review, yea it will be a though review, but I'm happy to hop on a call with whoever will be reviewing this to go through the changes and provide context. But if there's only one commit to review it's this one 5da2b51 which gates the changes to the derive and attribute macros behind the idl-gen flag. So if this is OK, there are no actual changes to the generated BPF programs. There are also multiple pure refactoring commits which should be a straightforward review.

[acheroncrypto]

First of all, thank you so much for this PR, it has been open for a very long time and I'd like to merge it as soon as possible.

I have a few questions and have some changes that I'd like to get your opinion on.

Questions

• How should the external wrapper types like TokenAccount work? Currently it doesn't seem to build the IDL with an SPL account. Should we skip the known types?

• How should a generic accounts struct work?

#[derive(Accounts)]
pub struct GenericsTest<'info, T>
where
    T: AccountSerialize + AccountDeserialize + Owner + Clone,
{
    pub generic: Account<'info, T>,
}

Changes

• Add InterfaceAccount to the account types(we are checking for Account and AccountLoader).
• Remove full_path and use name instead because clients can derive the name from full_path anyway.
• Use idl-build instead of idl-gen to make it more clear and also parse command is anchor idl parse and the build command is anchor idl build.
• Convert type_args to camelCase in IDL.

I don't want to change too much, this seems to work mostly great as is and I think this could replace the parsing method after we implement the remaining parts. Please let me know if you are OK with these changes and I can continue what you've started.

[kklas]

@acheroncrypto

How should the external wrapper types like TokenAccount work? Currently it doesn't seem to build the IDL with an SPL account. Should we skip the known types?

Do you mean that the IDL build fails with the compilation error or they're just not included? I think for this PR they should be skipped to keep it minimal and this functionality added later on.
The general problem with external wrappers is that they each have their own specific encoding / decoding method and this info needs to be included in the IDL also so the clients know how to enc/dec those accounts. I think that wrapper types can implement their own __anchor_private_gen_idl_type method manually to pass this info in.

How should a generic accounts struct work?

This is an entirely different problem far out of scope of this work. The question you should be asking is whether you want to implement this to make derive(Accounts) structs reusable (avoid copy paste on similar structs) or you want to enable generic CPI calls. If it's the former then in my opinion this shouldn't be allowed because it results in very complex code and is a foot-gun. If it's the latter then this is a very different problem to solve (and a tough one). It's not fully clear to me how one would do that and what security implications it has. IIRC the Solana team is working on adding type definitions to the SBF perhaps to cover this kind of functionality, so might be something to look into.

- Add InterfaceAccount to the account types(we are checking for Account and AccountLoader).
- Remove full_path and use name instead because clients can derive the name from full_path anyway.
- Use idl-build instead of idl-gen to make it more clear and also parse command is anchor idl parse and the build command is anchor idl build.
- Convert type_args to camelCase in IDL.

LGTM, just be careful with old IDL / new IDL parsing compatibility for the clients and how you want to roll this out. People parsing the new IDL with the old client could run into issues.

[acheroncrypto]

Do you mean that the IDL build fails with the compilation error or they're just not included?

The former, IDL build fails because those types don't have __anchor_private_gen_idl_type method. As you know, they implement AnchorSerialize trait but the trait itself doesn't have the IDL print functionality, derive macro version does.

I think that wrapper types can implement their own __anchor_private_gen_idl_type method manually to pass this info in.

Sounds good.

This is an entirely different problem far out of scope of this work. The question you should be asking is whether you want to implement this to make derive(Accounts) structs reusable (avoid copy paste on similar structs) or you want to enable generic CPI calls. If it's the former then in my opinion this shouldn't be allowed because it results in very complex code and is a foot-gun. If it's the latter then this is a very different problem to solve (and a tough one). It's not fully clear to me how one would do that and what security implications it has. IIRC the Solana team is working on adding type definitions to the SBF perhaps to cover this kind of functionality, so might be something to look into.

Sorry I wasn't clear enough, I've asked this because of the tests here

anchor/lang/tests/generics_test.rs

Lines 11 to 23 in 9a93a2e

	#[derive(Accounts)]
	pub struct GenericsTest<'info, T, U, const N: usize>
	where
	T: AccountSerialize + AccountDeserialize + Owner + Clone,
	U: BorshSerialize + BorshDeserialize + Default + Clone,
	{
	pub non_generic: AccountInfo<'info>,
	pub generic: Account<'info, T>,
	pub const_generic: AccountLoader<'info, FooAccount<N>>,
	pub const_generic_loader: AccountLoader<'info, FooAccount<N>>,
	pub associated: Account<'info, Associated<U>>,
	}

I don't think it's being used at all and the current IDL doesn't have a support for it either. I also share your sentiment about the security aspect of it and we should probably remove it.

LGTM, just be careful with old IDL / new IDL parsing compatibility for the clients and how you want to roll this out. People parsing the new IDL with the old client could run into issues.

Thanks, it seems to me that the old clients won't be able to properly use the new IDL regardless of what we do and since people upload their IDLs on-chain, I think we should get rid of any extra fields that can be derived from client side.

[kklas]

The former, IDL build fails because those types don't have __anchor_private_gen_idl_type method. As you know, they implement AnchorSerialize trait but the trait itself doesn't have the IDL print functionality, derive macro version does.

Oh I'm a bit fuzzy on the details here since this was a while ago, but I thought these would be skipped here https://github.com/coral-xyz/anchor/pull/2011/files#diff-b2688db258a9b7f16103c6bd084af5bc13cfd095a651f723a66d29d27acd7f24R656-R658 for derive(Accounts) (code for inserting IDL types generated only for Account and AccountLoader). Now that I think of it, these can in theory also be fields in #[account] structs which is something that I haven't considered. Nice catch!

Maybe there's a way to add a default implementation of __anchor_private_gen_idl_type for those types by introducing a special trait for them or have a derive / attribute macro to avoid the compile error.

Sorry I wasn't clear enough, I've asked this because of the tests here

Yea, not only is it not supported by the current IDL, but I've had even anchor build failing in some cases.

[AnderUstarroz]

wow, great work @kklas ! You put a serious amount of work on this PR. Do you think this could be a potential fix for the issue mentioned here?

[kklas]

@AnderUstarroz yes, it will fix it

[acheroncrypto]

@kklas I've fixed the conflicts and included some changes/fixes. Some parts are still rough around the edges such as external types, duplication of accounts/types, larger IDL size and client compatibility. I'll work on these issues in subsequent PRs.

Is there anything you'd like to change before we merge this?

[acheroncrypto]

Finally able to merge this, thank you for making this incredible contribution.

[kklas]

Thanks for continuing the effort!