Browse Source

lang: Require `zero` accounts to be unique (#3409)

acheron 10 months ago
parent
commit
955e7eaf61

+ 2 - 1
CHANGELOG.md

@@ -61,7 +61,7 @@ The minor version will be incremented upon a breaking change and the patch versi
 - cli: Add optional `package-manager` flag in `init` command to set package manager field in Anchor.toml ([#3328](https://github.com/coral-xyz/anchor/pull/3328)).
 - cli: Add test template for [Mollusk](https://github.com/buffalojoec/mollusk) ([#3352](https://github.com/coral-xyz/anchor/pull/3352)).
 - idl: Disallow account discriminators that can conflict with the `zero` constraint ([#3365](https://github.com/coral-xyz/anchor/pull/3365)).
-- cli: Include recommended solana args by default and add new --max-retries ([#3354](https://github.com/coral-xyz/anchor/pull/3354)).
+- cli: Include recommended solana args by default and add new `--max-retries` option to the `deploy` command ([#3354](https://github.com/coral-xyz/anchor/pull/3354)).
 
 ### Fixes
 
@@ -100,6 +100,7 @@ The minor version will be incremented upon a breaking change and the patch versi
 - lang: Fix `cpi` feature instructions not accounting for discriminator overrides ([#3376](https://github.com/coral-xyz/anchor/pull/3376)).
 - idl: Ignore compiler warnings during builds ([#3396](https://github.com/coral-xyz/anchor/pull/3396)).
 - cli: Avoid extra IDL generation during `verify` ([#3398](https://github.com/coral-xyz/anchor/pull/3398)).
+- lang: Require `zero` accounts to be unique ([#3409](https://github.com/coral-xyz/anchor/pull/3409)).
 
 ### Breaking
 

+ 47 - 2
lang/syn/src/codegen/accounts/constraints.rs

@@ -144,7 +144,7 @@ fn generate_constraint(
 ) -> proc_macro2::TokenStream {
     match c {
         Constraint::Init(c) => generate_constraint_init(f, c, accs),
-        Constraint::Zeroed(c) => generate_constraint_zeroed(f, c),
+        Constraint::Zeroed(c) => generate_constraint_zeroed(f, c, accs),
         Constraint::Mut(c) => generate_constraint_mut(f, c),
         Constraint::HasOne(c) => generate_constraint_has_one(f, c, accs),
         Constraint::Signer(c) => generate_constraint_signer(f, c),
@@ -197,7 +197,11 @@ pub fn generate_constraint_init(
     generate_constraint_init_group(f, c, accs)
 }
 
-pub fn generate_constraint_zeroed(f: &Field, _c: &ConstraintZeroed) -> proc_macro2::TokenStream {
+pub fn generate_constraint_zeroed(
+    f: &Field,
+    _c: &ConstraintZeroed,
+    accs: &AccountsStruct,
+) -> proc_macro2::TokenStream {
     let account_ty = f.account_ty();
     let discriminator = quote! { #account_ty::DISCRIMINATOR };
 
@@ -205,6 +209,46 @@ pub fn generate_constraint_zeroed(f: &Field, _c: &ConstraintZeroed) -> proc_macr
     let name_str = field.to_string();
     let ty_decl = f.ty_decl(true);
     let from_account_info = f.from_account_info(None, false);
+
+    // Require `zero` constraint accounts to be unique by:
+    //
+    // 1. Getting the names of all accounts that have the `zero` constraint and are declared before
+    //    the current field (in order to avoid checking the same field).
+    // 2. Comparing the key of the current field with all the previous fields' keys.
+    // 3. Returning an error if a match is found.
+    let unique_account_checks = accs
+        .fields
+        .iter()
+        .filter_map(|af| match af {
+            AccountField::Field(field) => Some(field),
+            _ => None,
+        })
+        .take_while(|field| field.ident != f.ident)
+        .filter(|field| field.constraints.is_zeroed())
+        .map(|other_field| {
+            let other = &other_field.ident;
+            let err = quote! {
+                Err(
+                    anchor_lang::error::Error::from(
+                        anchor_lang::error::ErrorCode::ConstraintZero
+                    ).with_account_name(#name_str)
+                )
+            };
+            if other_field.is_optional {
+                quote! {
+                    if #other.is_some() && #field.key == &#other.as_ref().unwrap().key() {
+                        return #err;
+                    }
+                }
+            } else {
+                quote! {
+                    if #field.key == &#other.key() {
+                        return #err;
+                    }
+                }
+            }
+        });
+
     quote! {
         let #field: #ty_decl = {
             let mut __data: &[u8] = &#field.try_borrow_data()?;
@@ -213,6 +257,7 @@ pub fn generate_constraint_zeroed(f: &Field, _c: &ConstraintZeroed) -> proc_macr
             if __has_disc {
                 return Err(anchor_lang::error::Error::from(anchor_lang::error::ErrorCode::ConstraintZero).with_account_name(#name_str));
             }
+            #(#unique_account_checks)*
             #from_account_info
         };
     }

+ 8 - 0
tests/misc/programs/misc-optional/src/context.rs

@@ -744,3 +744,11 @@ pub struct InitManyAssociatedTokenAccounts<'info> {
     pub token_program: Program<'info, Token>,
     pub associated_token_program: Program<'info, AssociatedToken>,
 }
+
+#[derive(Accounts)]
+pub struct TestMultipleZeroConstraint<'info> {
+    #[account(zero)]
+    pub one: Option<Account<'info, Data>>,
+    #[account(zero)]
+    pub two: Option<Account<'info, Data>>,
+}

+ 4 - 0
tests/misc/programs/misc-optional/src/lib.rs

@@ -402,4 +402,8 @@ pub mod misc_optional {
     ) -> Result<()> {
         Ok(())
     }
+
+    pub fn test_multiple_zero_constraint(_ctx: Context<TestMultipleZeroConstraint>) -> Result<()> {
+        Ok(())
+    }
 }

+ 8 - 0
tests/misc/programs/misc/src/context.rs

@@ -816,3 +816,11 @@ pub struct TestBoxedOwnerConstraint<'info> {
 #[cfg(feature = "my-feature")]
 #[derive(Accounts)]
 pub struct Empty {}
+
+#[derive(Accounts)]
+pub struct TestMultipleZeroConstraint<'info> {
+    #[account(zero)]
+    pub one: Account<'info, Data>,
+    #[account(zero)]
+    pub two: Account<'info, Data>,
+}

+ 4 - 0
tests/misc/programs/misc/src/lib.rs

@@ -401,4 +401,8 @@ pub mod misc {
     pub fn only_my_feature(_ctx: Context<Empty>) -> Result<()> {
         Ok(())
     }
+
+    pub fn test_multiple_zero_constraint(_ctx: Context<TestMultipleZeroConstraint>) -> Result<()> {
+        Ok(())
+    }
 }

+ 43 - 0
tests/misc/tests/misc/misc.ts

@@ -3240,6 +3240,49 @@ const miscTest = (
         assert.isDefined(thisTx);
       });
     });
+
+    describe("Multiple `zero` constraint", () => {
+      it("Passing different accounts works", async () => {
+        const oneKp = anchor.web3.Keypair.generate();
+        const twoKp = anchor.web3.Keypair.generate();
+        await program.methods
+          .testMultipleZeroConstraint()
+          .preInstructions(
+            await Promise.all([
+              program.account.data.createInstruction(oneKp),
+              program.account.data.createInstruction(twoKp),
+            ])
+          )
+          .accounts({ one: oneKp.publicKey, two: twoKp.publicKey })
+          .signers([oneKp, twoKp])
+          .rpc();
+      });
+
+      it("Passing the same account throws", async () => {
+        const oneKp = anchor.web3.Keypair.generate();
+        try {
+          await program.methods
+            .testMultipleZeroConstraint()
+            .preInstructions([
+              await program.account.data.createInstruction(oneKp),
+            ])
+            .accounts({
+              one: oneKp.publicKey,
+              two: oneKp.publicKey,
+            })
+            .signers([oneKp])
+            .rpc();
+          throw new Error("Transaction did not fail!");
+        } catch (e) {
+          assert(e instanceof AnchorError);
+          const err: AnchorError = e;
+          assert.strictEqual(
+            err.error.errorCode.number,
+            anchor.LangErrorCode.ConstraintZero
+          );
+        }
+      });
+    });
   };
 };