lang: Fix using non-instruction composite accounts multiple times with declare_program! (solana-foundation#4113)

Author: acheroncrypto

CHANGELOG.md

@@ -17,6 +17,7 @@ The minor version will be incremented upon a breaking change and the patch versi
 ### Fixes
 
 - idl: Fix defined types with unsupported fields not producing an error ([#4088](https://github.com/solana-foundation/anchor/pull/4088)).
+- lang: Fix using non-instruction composite accounts multiple times with `declare_program!` ([#4113](https://github.com/solana-foundation/anchor/pull/4113)).
 
 ### Breaking
 

lang/attribute/program/src/declare_program/mods/internal.rs

@@ -109,6 +109,9 @@ fn gen_internal_accounts_common(
 ) -> proc_macro2::TokenStream {
     // It's possible to declare an accounts struct and not use it as an instruction, see
     // https://github.com/coral-xyz/anchor/issues/3274
+    //
+    // NOTE: Returned accounts will not be unique if non-instruction composite accounts have been
+    // used multiple times https://github.com/solana-foundation/anchor/issues/3349
     fn get_non_instruction_composite_accounts<'a>(
         accs: &'a [IdlInstructionAccountItem],
         idl: &'a Idl,
@@ -121,35 +124,53 @@ fn gen_internal_accounts_common(
                         .iter()
                         .any(|ix| ix.accounts == accs.accounts) =>
                 {
-                    let mut non_ix_composite_accs =
-                        get_non_instruction_composite_accounts(&accs.accounts, idl);
-                    if !non_ix_composite_accs.contains(&accs) {
-                        non_ix_composite_accs.push(accs);
-                    }
-                    non_ix_composite_accs
+                    let mut nica = get_non_instruction_composite_accounts(&accs.accounts, idl);
+                    nica.push(accs);
+                    nica
                 }
                 _ => Default::default(),
             })
             .collect()
     }
 
+    // Combine regular instructions with non-instruction composite accounts
     let ix_accs = idl
         .instructions
         .iter()
         .flat_map(|ix| ix.accounts.to_owned())
         .collect::<Vec<_>>();
     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(),
+        .fold(Vec::<IdlInstruction>::default(), |mut ixs, accs| {
+            // Make sure they are unique
+            if ixs.iter().all(|ix| ix.accounts != accs.accounts) {
+                // 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.
+                let name = if ixs.iter().all(|ix| ix.name != accs.name) {
+                    accs.name.to_owned()
+                } else {
+                    // Append numbers to the field name until we find a unique name
+                    (2..)
+                        .find_map(|i| {
+                            let name = format!("{}{i}", accs.name);
+                            ixs.iter().all(|ix| ix.name != name).then_some(name)
+                        })
+                        .expect("Should always find a valid name")
+                };
+
+                ixs.push(IdlInstruction {
+                    name,
+                    accounts: accs.accounts.to_owned(),
+                    args: Default::default(),
+                    discriminator: Default::default(),
+                    docs: Default::default(),
+                    returns: Default::default(),
+                })
+            }
+
+            ixs
         })
+        .into_iter()
         .chain(idl.instructions.iter().cloned())
         .collect::<Vec<_>>();
 

tests/declare-program/idls/external.json

@@ -258,6 +258,61 @@
           "type": "u32"
         }
       ]
+    },
+    {
+      "name": "update_non_instruction_composite2",
+      "discriminator": [
+        218,
+        106,
+        203,
+        167,
+        112,
+        177,
+        145,
+        22
+      ],
+      "accounts": [
+        {
+          "name": "non_instruction_update",
+          "accounts": [
+            {
+              "name": "program",
+              "address": "Externa111111111111111111111111111111111111"
+            }
+          ]
+        },
+        {
+          "name": "non_instruction_update_with_different_ident",
+          "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"
+        }
+      ]
     }
   ],
   "accounts": [

tests/declare-program/programs/declare-program/src/lib.rs

@@ -64,7 +64,26 @@ pub mod declare_program {
                 },
             },
         );
-        external::cpi::update_non_instruction_composite(cpi_ctx, value)?;
+        external::cpi::update_non_instruction_composite(cpi_ctx, 10)?;
+        cpi_my_account.reload()?;
+        require_eq!(cpi_my_account.field, 10);
+
+        // Composite accounts but not an actual instruction (intentionally checking multiple times)
+        let cpi_ctx = CpiContext::new(
+            ctx.accounts.external_program.key(),
+            external::cpi::accounts::UpdateNonInstructionComposite2 {
+                non_instruction_update: external::cpi::accounts::NonInstructionUpdate2 {
+                    program: ctx.accounts.external_program.to_account_info(),
+                },
+                non_instruction_update_with_different_ident:
+                    external::cpi::accounts::NonInstructionUpdate {
+                        authority: ctx.accounts.authority.to_account_info(),
+                        my_account: cpi_my_account.to_account_info(),
+                        program: ctx.accounts.external_program.to_account_info(),
+                    },
+            },
+        );
+        external::cpi::update_non_instruction_composite2(cpi_ctx, value)?;
         cpi_my_account.reload()?;
         require_eq!(cpi_my_account.field, value);
 

tests/declare-program/programs/external/src/lib.rs

@@ -35,6 +35,18 @@ pub mod external {
         Ok(())
     }
 
+    // Test the issue described in https://github.com/coral-xyz/anchor/issues/3349
+    pub fn update_non_instruction_composite2(
+        ctx: Context<UpdateNonInstructionComposite2>,
+        value: u32,
+    ) -> Result<()> {
+        ctx.accounts
+            .non_instruction_update_with_different_ident
+            .my_account
+            .field = value;
+        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>,
@@ -92,6 +104,24 @@ pub struct Update<'info> {
     pub my_account: Account<'info, MyAccount>,
 }
 
+#[derive(Accounts)]
+pub struct UpdateComposite<'info> {
+    pub update: Update<'info>,
+}
+
+#[derive(Accounts)]
+pub struct UpdateNonInstructionComposite<'info> {
+    pub non_instruction_update: NonInstructionUpdate<'info>,
+}
+
+#[derive(Accounts)]
+pub struct UpdateNonInstructionComposite2<'info> {
+    // Intenionally using different composite account with the same identifier
+    // https://github.com/solana-foundation/anchor/pull/3350#pullrequestreview-2425405970
+    pub non_instruction_update: NonInstructionUpdate2<'info>,
+    pub non_instruction_update_with_different_ident: NonInstructionUpdate<'info>,
+}
+
 #[derive(Accounts)]
 pub struct NonInstructionUpdate<'info> {
     pub authority: Signer<'info>,
@@ -101,13 +131,8 @@ pub struct NonInstructionUpdate<'info> {
 }
 
 #[derive(Accounts)]
-pub struct UpdateComposite<'info> {
-    pub update: Update<'info>,
-}
-
-#[derive(Accounts)]
-pub struct UpdateNonInstructionComposite<'info> {
-    pub non_instruction_update: NonInstructionUpdate<'info>,
+pub struct NonInstructionUpdate2<'info> {
+    pub program: Program<'info, program::External>,
 }
 
 #[account]