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

lang: Fix declare_program! using non-instruction composite accounts multiple times #3350

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ The minor version will be incremented upon a breaking change and the patch versi
- cli: Use OS-agnostic paths ([#3307](https://github.com/coral-xyz/anchor/pull/3307)).
- avm: Use `rustc 1.79.0` when installing versions older than v0.31 ([#3315](https://github.com/coral-xyz/anchor/pull/3315)).
- cli: Fix priority fee calculation causing panic on localnet ([#3318](https://github.com/coral-xyz/anchor/pull/3318)).
- lang: Fix `declare_program!` using non-instruction composite accounts multiple times ([#3350](https://github.com/coral-xyz/anchor/pull/3350)).

### Breaking

Expand Down
28 changes: 19 additions & 9 deletions lang/attribute/program/src/declare_program/mods/internal.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::HashSet;

use anchor_lang_idl::types::{
Idl, IdlInstruction, IdlInstructionAccountItem, IdlInstructionAccounts,
};
Expand Down Expand Up @@ -138,17 +140,25 @@ fn gen_internal_accounts_common(
.iter()
.flat_map(|ix| ix.accounts.to_owned())
.collect::<Vec<_>>();
let mut account_names: HashSet<String> = std::collections::HashSet::new();
Copy link
Collaborator

Choose a reason for hiding this comment

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

The get_non_instruction_composite_accounts should ideally return a HashSet rather than a Vec, but the IDL types don't implement the necessary traits currently. I guess I thought this check would make them unique, but it doesn't cover the initial outer case.

I think it would be better for the get_non_instruction_composite_accounts to return unique accounts rather than sorting them out here.

let combined_ixs = get_non_instruction_composite_accounts(&ix_accs, idl)
.into_iter()
.map(|accs| IdlInstruction {
// The name is not guaranteed to be the same as the one used in the actual source code
// of the program because the IDL only stores the field names.
name: accs.name.to_owned(),
accounts: accs.accounts.to_owned(),
args: Default::default(),
discriminator: Default::default(),
docs: Default::default(),
returns: Default::default(),
.filter_map(|accs| {
let name = accs.name.to_owned();
if account_names.insert(name) {
Some(IdlInstruction {
// The name is not guaranteed to be the same as the one used in the actual source code
// of the program because the IDL only stores the field names.
name: accs.name.to_owned(),
accounts: accs.accounts.to_owned(),
args: Default::default(),
discriminator: Default::default(),
docs: Default::default(),
returns: Default::default(),
})
} else {
None
}
})
.chain(idl.instructions.iter().cloned())
.collect::<Vec<_>>();
Expand Down
46 changes: 46 additions & 0 deletions tests/declare-program/idls/external.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,52 @@
],
"args": []
},
{
"name": "second_use_of_non_instruction_composite",
"discriminator": [
103,
165,
187,
198,
36,
148,
158,
119
],
"accounts": [
{
"name": "non_instruction_update",
"accounts": [
{
"name": "authority",
"signer": true
},
{
"name": "my_account",
"writable": true,
"pda": {
"seeds": [
{
"kind": "account",
"path": "authority"
}
]
}
},
{
"name": "program",
"address": "Externa111111111111111111111111111111111111"
}
]
}
],
"args": [
{
"name": "value",
"type": "u32"
}
]
},
{
"name": "test_compilation_defined_type_param",
"discriminator": [
Expand Down
13 changes: 13 additions & 0 deletions tests/declare-program/programs/external/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ pub mod external {
Ok(())
}

// Test the issue described in https://github.com/coral-xyz/anchor/issues/3349
pub fn second_use_of_non_instruction_composite(
_ctx: Context<SecondUseOfNonInstructionComposite>,
_value: u32,
) -> Result<()> {
Ok(())
}

// Compilation test for whether a defined type (an account in this case) can be used in `cpi` client.
pub fn test_compilation_defined_type_param(
_ctx: Context<TestCompilation>,
Expand Down Expand Up @@ -92,6 +100,11 @@ pub struct UpdateNonInstructionComposite<'info> {
pub non_instruction_update: NonInstructionUpdate<'info>,
}

#[derive(Accounts)]
pub struct SecondUseOfNonInstructionComposite<'info> {
pub non_instruction_update: NonInstructionUpdate<'info>,
}

#[account]
pub struct MyAccount {
pub field: u32,
Expand Down
Loading