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

cli: idl close to directly an idl address #2760

Merged
merged 7 commits into from
Jan 16, 2024

Conversation

ochaloup
Copy link
Contributor

@ochaloup ochaloup commented Jan 3, 2024

On upgrading the IDL account

anchor --provider.cluster devnet idl \
  --provider.wallet <fee-payer> upgrade <program-id> -f ./target/idl/program.json

It creates the IDL buffer and then it writes the buffer to the program IDL account.
The IDL buffer is still opened. It's not possible to use anchor idl close with specific IDL address (e.g., address of the buffer) to close it and get back the rent for the account.
Adding the optional argument of providing the IDL address directly makes possible to close the buffer account as well.

Copy link

vercel bot commented Jan 3, 2024

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

A member of the Team first needs to authorize it.

@acheroncrypto acheroncrypto added cli idl related to the IDL, either program or client side labels Jan 4, 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.

I wonder why upgrade command doesn't close the buffer account by default, perhaps to version the IDLs? If you don't have any objections, I think we should close the buffer account similar to how program upgrades work and historical data should be left to RPC providers.

cli/src/lib.rs Outdated Show resolved Hide resolved
@ochaloup
Copy link
Contributor Author

ochaloup commented Jan 8, 2024

I wonder why upgrade command doesn't close the buffer account by default, perhaps to version the IDLs? ...

To me the suggestion makes perfect sense. I would expect it should work this way to follow how the solana program cli does.
Do you thing adding the idl_close call at id_upgrade makes this working as you proposed?
https://github.com/coral-xyz/anchor/pull/2760/files#diff-c1f8f7498da827a634bddc8a7559198bc99b296e9d9e8b91a70b503662995b8cR2264

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.

Do you thing adding the idl_close call at id_upgrade makes this working as you proposed?

Yes, it works great but the upgrade output is a bit confusing:

Idl buffer created: CK5yrU9hjfgPRjEN5PKTLK7UD6niW1C7RvBXstrwjJfm
Idl account closed: CK5yrU9hjfgPRjEN5PKTLK7UD6niW1C7RvBXstrwjJfm

Maybe we should log the upgrade was successful and the IDL account key?

Also, could you note both of these changes in the CHANGELOG? I think closing the buffer account is a breaking change since it's possible some people are using the buffer account.

@ochaloup
Copy link
Contributor Author

@acheroncrypto Sure. So it's a new update here in PR. Let's see.

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.

Somehow the previous change that removed short was overridden but I restored it. Thanks!

@acheroncrypto acheroncrypto merged commit bcd7e17 into coral-xyz:master Jan 16, 2024
48 of 49 checks passed
@ochaloup
Copy link
Contributor Author

Somehow the previous change that removed short was overridden but I restored it. Thanks!

Ah, that was probably my wrong, sorry.

Thanks for looking into this, for fixing and merging 😉

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

Successfully merging this pull request may close these issues.

2 participants