Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: recursive idl gen #2946

Merged
merged 4 commits into from
May 7, 2024

Conversation

guibescos
Copy link
Contributor

@guibescos guibescos commented May 3, 2024

There's a bug in the logic for adding external types from other crates into the idl.

The way it works is a macro that :

  • first checks use statements to see which crate the type is imported from'
  • looks for the Cargo.lock of the project
  • then finds the name and version of the crate in Cargo.lock
  • then looks for the type definition in the cargo registry for that crate locally

The bug I'm trying to fix appears in a recursive situation where a program A imports a type from crate B which uses a type from crate C.
When the macro gets expanded in crate B, it is incapable of finding the Cargo.lock of the project and panics (this is because at that point the working directory is the ~/.cargo/registry and not the anchor workspace ).

I propose to update the code so Cargo.lock is searched from the program path.

Here's a reproducible example:
https://github.com/diyahir/minimal-pyth-version-error

  • Program is tic_tac_toe, it imports the Anchor type PriceUpdateV2 from Crate B
  • Crate B is pyth-solana-receiver-sdk, it imports the Anchor typePriceFeedMessage from Crate C.
  • Crate C is pythnet-sdk

Copy link

vercel bot commented May 3, 2024

@guibescos is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@guibescos guibescos marked this pull request as ready for review May 3, 2024 16:06
@acheroncrypto acheroncrypto added bug Something isn't working idl related to the IDL, either program or client side labels May 3, 2024
Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

External type resolution by looking at the source code is not even necessary for external Anchor types, but unfortunately, we don't have a direct way of deciding if the type is an Anchor type or not.

Before we merge, could you note this fix in the CHANGELOG?

lang/syn/src/idl/external.rs Outdated Show resolved Hide resolved
@guibescos
Copy link
Contributor Author

Addressed the comments

idl/src/build.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working idl related to the IDL, either program or client side
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants