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

feat: add anchor bindings anchor-lang #3440

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mgild
Copy link
Contributor

@mgild mgild commented Dec 19, 2024

This change adds wrapper bindings for the AddressLookupTable program and AddressLookupTable Account.

Usage:

#[derive(Accounts)]
pub struct Action<'info> {
    pub lut_program: Program<'info, AddressLookupTable>,
    pub lut_signer: Signer<'info>,
    pub lookup_table: Account<'info, AddressLookupTableAccount<'info>>,
}

Tests:

cd lang
cargo build

Copy link

vercel bot commented Dec 19, 2024

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

A member of the Team first needs to authorize it.

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.

This has been requested before, so thank you for adding this!

It would be very helpful to have some tests for these types. They don't need to be very complicated; just some tests to check compilation and basic functionality. Compilation tests are especially important given how easy it is to miss the idl-build feature impl.

There are also a couple more issues that I mentioned in the comments, but it looks good otherwise.

lang/Cargo.toml Outdated Show resolved Hide resolved
lang/src/address_lookup_table_program.rs Outdated Show resolved Hide resolved
lang/src/address_lookup_table_program.rs Outdated Show resolved Hide resolved
lang/src/address_lookup_table_program.rs Show resolved Hide resolved
lang/src/lib.rs Outdated Show resolved Hide resolved
lang/src/lib.rs 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.

Thanks. We also need to add the tests to the workflow file to make them run in CI.

The test folder looks like you may have uncommitted changes because there are unused keypairs and definitions (the test itself also doesn't work). I left some comments for those issues.

Lastly, this feature surely deserves a mention in the CHANGELOG — could you add an entry?

Comment on lines +44 to +48
impl<'info> DerefMut for AddressLookupTableAccount<'info> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It wouldn't hurt to have this, but does it make sense to have it when the account is always going to be immutable in our usage?

@@ -0,0 +1,16 @@
{
"name": "address-lookup-table",
"version": "0.24.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: other tests use the current version of the repo (0.30.1) here, so it would be nice to keep things consistent when possible.

Suggested change
"version": "0.24.0",
"version": "0.30.1",

@@ -0,0 +1,30 @@
use anchor_lang::prelude::*;

use crate::program::AddressLookupTableTest;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't look like this is being used anywhere. If so, could you remove it?

Suggested change
use crate::program::AddressLookupTableTest;

Comment on lines +18 to +22
#[error_code]
pub enum CustomError {
InvalidProgramDataAddress,
AccountNotProgram,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Errors also don't seem to be used. Please remove if that's the case.

Suggested change
#[error_code]
pub enum CustomError {
InvalidProgramDataAddress,
AccountNotProgram,
}

Comment on lines +22 to +29
const tx = await program.rpc.test({
accounts: {
authority: provider.wallet.publicKey,
lutProgram: lutProgramAddress,
table: lutProgramAddress, // Just a dummy value. fix
},
signers: [settings],
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This syntax has been deprecated for a long time, and we plan to remove it soon — could you use the newer syntax?

Suggested change
const tx = await program.rpc.test({
accounts: {
authority: provider.wallet.publicKey,
lutProgram: lutProgramAddress,
table: lutProgramAddress, // Just a dummy value. fix
},
signers: [settings],
});
const tx = await program.methods
.test()
.accounts({ table: lutProgramAddress })
.rpc();

Also, settings don't seem to be defined anywhere.

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

Successfully merging this pull request may close these issues.

2 participants