lang: Always execute constraints for init_if_needed (otter-sec#1096)

Author: paul-schaaf

CHANGELOG.md

@@ -14,17 +14,22 @@ incremented for features.
 ### Fixes
 
 * lang: Add `deprecated` attribute to `ProgramAccount` ([#1014](https://github.com/project-serum/anchor/pull/1014)).
-* cli: Add version number from programs `Cargo.toml` into extracted IDL
-* lang: Add `deprecated` attribute to `Loader`([#1078](https://github.com/project-serum/anchor/pull/1078))
+* cli: Add version number from programs `Cargo.toml` into extracted IDL ([#1061](https://github.com/project-serum/anchor/pull/1061)).
+* lang: Add `deprecated` attribute to `Loader`([#1078](https://github.com/project-serum/anchor/pull/1078)).
+* lang: the `init_if_needed` attribute now checks that given attributes (e.g. space, owner, token::authority etc.) are validated even when init is not needed ([#1096](https://github.com/project-serum/anchor/pull/1096)).
 
 ### Features
 
-* lang: Add `ErrorCode::AccountNotInitialized` error to separate the situation when the account has the wrong owner from when it does not exist (#[1024](https://github.com/project-serum/anchor/pull/1024))
-* lang: Called instructions now log their name by default. This can be turned off with the `no-log-ix-name` flag ([#1057](https://github.com/project-serum/anchor/pull/1057))
-* ts: Add `getAccountInfo` helper method to account namespace/client ([#1084](https://github.com/project-serum/anchor/pull/1084))
-* lang: `ProgramData` and `UpgradableLoaderState` can now be passed into `Account` as generics. see [UpgradeableLoaderState](https://docs.rs/solana-program/latest/solana_program/bpf_loader_upgradeable/enum.UpgradeableLoaderState.html). `UpgradableLoaderState` can also be matched on to get `ProgramData`, but when `ProgramData` is used instead, anchor does the serialization and checking that it is actually program data for you  ([#1095](https://github.com/project-serum/anchor/pull/1095))
-* ts: Add better error msgs in the ts client if something wrong (i.e. not a pubkey or a string) is passed in as an account in an instruction accounts object ([#1098](https://github.com/project-serum/anchor/pull/1098))
+* lang: Add `ErrorCode::AccountNotInitialized` error to separate the situation when the account has the wrong owner from when it does not exist (#[1024](https://github.com/project-serum/anchor/pull/1024)).
+* lang: Called instructions now log their name by default. This can be turned off with the `no-log-ix-name` flag ([#1057](https://github.com/project-serum/anchor/pull/1057)).
+* lang: `ProgramData` and `UpgradableLoaderState` can now be passed into `Account` as generics. see [UpgradeableLoaderState](https://docs.rs/solana-program/latest/solana_program/bpf_loader_upgradeable/enum.UpgradeableLoaderState.html). `UpgradableLoaderState` can also be matched on to get `ProgramData`, but when `ProgramData` is used instead, anchor does the serialization and checking that it is actually program data for you  ([#1095](https://github.com/project-serum/anchor/pull/1095)).
+* ts: Add better error msgs in the ts client if something wrong (i.e. not a pubkey or a string) is passed in as an account in an instruction accounts object ([#1098](https://github.com/project-serum/anchor/pull/1098)).
+* ts: Add inputs `postInstructions` and `preInstructions` as a replacement for (the now deprecated) `instructions` ([#1007](https://github.com/project-serum/anchor/pull/1007)).
+* ts: Add `getAccountInfo` helper method to account namespace/client ([#1084](https://github.com/project-serum/anchor/pull/1084)).
 
+### Breaking
+
+* lang, ts: Error codes have been mapped to new numbers to allow for more errors per namespace ([#1096](https://github.com/project-serum/anchor/pull/1096)).
 
 ## [0.18.2] - 2021-11-14
 
@@ -33,7 +38,6 @@ incremented for features.
 ### Features
 
 * lang: Add `SystemAccount<'info>` account type for generic wallet addresses or accounts owned by the system program ([#954](https://github.com/project-serum/anchor/pull/954))
-* ts: Add inputs `postInstructions` and `preInstructions` as a replacement for (the now deprecated) `instructions`
 
 ### Fixes
 

client/example/Cargo.toml

@@ -15,5 +15,5 @@ events = { path = "../../tests/events/programs/events", features = ["no-entrypoi
 shellexpand = "2.1.0"
 anyhow = "1.0.32"
 rand = "0.7.3"
-clap = "3.0.0-beta.5"
+clap = { version = "3.0.0-rc.0", features = ["derive"] }
 solana-sdk = "1.7.11"

client/example/src/main.rs

@@ -16,8 +16,8 @@ use events::MyEvent;
 use basic_4::accounts as basic_4_accounts;
 use basic_4::basic_4::Counter as CounterState;
 use basic_4::instruction as basic_4_instruction;
-// The `accounts` and `instructions` modules are generated by the framework.
 use clap::Parser;
+// The `accounts` and `instructions` modules are generated by the framework.
 use composite::accounts::{Bar, CompositeUpdate, Foo, Initialize};
 use composite::instruction as composite_instruction;
 use composite::{DummyA, DummyB};

lang/src/error.rs

@@ -15,13 +15,13 @@ pub enum ErrorCode {
 
     // IDL instructions.
     #[msg("The program was compiled without idl instructions")]
-    IdlInstructionStub = 120,
+    IdlInstructionStub = 1000,
     #[msg("Invalid program given to the IDL instruction")]
     IdlInstructionInvalidProgram,
 
     // Constraints.
     #[msg("A mut constraint was violated")]
-    ConstraintMut = 140,
+    ConstraintMut = 2000,
     #[msg("A has one constraint was violated")]
     ConstraintHasOne,
     #[msg("A signer constraint as violated")]
@@ -48,10 +48,23 @@ pub enum ErrorCode {
     ConstraintAddress,
     #[msg("Expected zero account discriminant")]
     ConstraintZero,
+    #[msg("A token mint constraint was violated")]
+    ConstraintTokenMint,
+    #[msg("A token owner constraint was violated")]
+    ConstraintTokenOwner,
+    // The mint mint is intentional -> a mint authority for the mint.
+    #[msg("A mint mint authority constraint was violated")]
+    ConstraintMintMintAuthority,
+    #[msg("A mint freeze authority constraint was violated")]
+    ConstraintMintFreezeAuthority,
+    #[msg("A mint decimals constraint was violated")]
+    ConstraintMintDecimals,
+    #[msg("A space constraint was violated")]
+    ConstraintSpace,
 
     // Accounts.
     #[msg("The account discriminator was already set on this account")]
-    AccountDiscriminatorAlreadySet = 160,
+    AccountDiscriminatorAlreadySet = 3000,
     #[msg("No 8 byte discriminator was found on the account")]
     AccountDiscriminatorNotFound,
     #[msg("8 byte discriminator did not match what was expected")]
@@ -81,9 +94,9 @@ pub enum ErrorCode {
 
     // State.
     #[msg("The given state account does not have the correct address")]
-    StateInvalidAddress = 180,
+    StateInvalidAddress = 4000,
 
     // Used for APIs that shouldn't be used anymore.
     #[msg("The API being used is deprecated and should no longer be used")]
-    Deprecated = 299,
+    Deprecated = 5000,
 }

lang/src/lib.rs

@@ -183,8 +183,8 @@ pub trait AccountDeserialize: Sized {
     /// uninitialized accounts, where the bytes are zeroed. Implementations
     /// should be unique to a particular account type so that one can never
     /// successfully deserialize the data of one account type into another.
-    /// For example, if the SPL token program where to implement this trait,
-    /// it should impossible to deserialize a `Mint` account into a token
+    /// For example, if the SPL token program were to implement this trait,
+    /// it should be impossible to deserialize a `Mint` account into a token
     /// `Account`.
     fn try_deserialize(buf: &mut &[u8]) -> Result<Self, ProgramError>;
 
@@ -303,7 +303,7 @@ pub mod __private {
     }
 
     // The starting point for user defined error codes.
-    pub const ERROR_CODE_OFFSET: u32 = 300;
+    pub const ERROR_CODE_OFFSET: u32 = 6000;
 
     // Calculates the size of an account, which may be larger than the deserialized
     // data in it. This trait is currently only used for `#[state]` accounts.

lang/syn/src/codegen/accounts/constraints.rs

@@ -449,6 +449,14 @@ pub fn generate_init(
                     }
 
                     let pa: #ty_decl = #from_account_info;
+                    if !(!#if_needed || #field.to_account_info().owner == &anchor_lang::solana_program::system_program::ID) {
+                        if pa.mint != #mint.key() {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintTokenMint.into());
+                        }
+                        if pa.owner != #owner.key() {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintTokenOwner.into());
+                        }
+                    }
                     pa
                 };
             }
@@ -473,6 +481,14 @@ pub fn generate_init(
                         anchor_spl::associated_token::create(cpi_ctx)?;
                     }
                     let pa: #ty_decl = #from_account_info;
+                    if !(!#if_needed || #field.to_account_info().owner == &anchor_lang::solana_program::system_program::ID) {
+                        if pa.mint != #mint.key() {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintTokenMint.into());
+                        }
+                        if pa.owner != #owner.key() {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintTokenOwner.into());
+                        }
+                    }
                     pa
                 };
             }
@@ -489,8 +505,8 @@ pub fn generate_init(
                 seeds_with_nonce,
             );
             let freeze_authority = match freeze_authority {
-                Some(fa) => quote! { Some(&#fa.key()) },
-                None => quote! { None },
+                Some(fa) => quote! { Option::<&anchor_lang::prelude::Pubkey>::Some(&#fa.key()) },
+                None => quote! { Option::<&anchor_lang::prelude::Pubkey>::None },
             };
             quote! {
                 let #field: #ty_decl = {
@@ -508,9 +524,23 @@ pub fn generate_init(
                             rent: rent.to_account_info(),
                         };
                         let cpi_ctx = CpiContext::new(cpi_program, accounts);
-                        anchor_spl::token::initialize_mint(cpi_ctx, #decimals, &#owner.to_account_info().key, #freeze_authority)?;
+                        anchor_spl::token::initialize_mint(cpi_ctx, #decimals, &#owner.key(), #freeze_authority)?;
                     }
                     let pa: #ty_decl = #from_account_info;
+                    if !(!#if_needed || #field.to_account_info().owner == &anchor_lang::solana_program::system_program::ID) {
+                        if pa.mint_authority != anchor_lang::solana_program::program_option::COption::Some(#owner.key()) {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintMintMintAuthority.into());
+                        }
+                        if pa.freeze_authority
+                            .as_ref()
+                            .map(|fa| #freeze_authority.as_ref().map(|expected_fa| fa != *expected_fa).unwrap_or(true))
+                            .unwrap_or(#freeze_authority.is_some()) {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintMintFreezeAuthority.into());
+                        }
+                        if pa.decimals != #decimals {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintMintDecimals.into());
+                        }
+                    }
                     pa
                 };
             }
@@ -550,16 +580,42 @@ pub fn generate_init(
                     &#o
                 },
             };
+            let pda_check = if !seeds_with_nonce.is_empty() {
+                quote! {
+                    let expected_key = anchor_lang::prelude::Pubkey::create_program_address(
+                        #seeds_with_nonce,
+                        #owner
+                    ).map_err(|_| anchor_lang::__private::ErrorCode::ConstraintSeeds)?;
+                    if expected_key != #field.key() {
+                        return Err(anchor_lang::__private::ErrorCode::ConstraintSeeds.into());
+                    }
+                }
+            } else {
+                quote! {}
+            };
             let create_account =
-                generate_create_account(field, quote! {space}, owner, seeds_with_nonce);
+                generate_create_account(field, quote! {space}, owner.clone(), seeds_with_nonce);
             quote! {
                 let #field = {
-                    if !#if_needed || #field.to_account_info().owner == &anchor_lang::solana_program::system_program::ID {
-                        #space
+                    let actual_field = #field.to_account_info();
+                    let actual_owner = actual_field.owner;
+                    #space
+                    if !#if_needed || actual_owner == &anchor_lang::solana_program::system_program::ID {
                         #payer
                         #create_account
                     }
                     let pa: #ty_decl = #from_account_info;
+                    if !(!#if_needed || actual_owner == &anchor_lang::solana_program::system_program::ID) {
+                        if space != actual_field.data_len() {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintSpace.into());
+                        }
+
+                        if actual_owner != #owner {
+                            return Err(anchor_lang::__private::ErrorCode::ConstraintOwner.into());
+                        }
+
+                        #pda_check
+                    }
                     pa
                 };
             }

lang/syn/src/idl/file.rs

@@ -11,7 +11,7 @@ use std::path::Path;
 
 const DERIVE_NAME: &str = "Accounts";
 // TODO: sharee this with `anchor_lang` crate.
-const ERROR_CODE_OFFSET: u32 = 300;
+const ERROR_CODE_OFFSET: u32 = 6000;
 
 // Parse an entire interface file.
 pub fn parse(filename: impl AsRef<Path>, version: String) -> Result<Option<Idl>> {

lang/syn/src/parser/accounts/constraints.rs

@@ -432,7 +432,7 @@ impl<'ty> ConstraintGroupBuilder<'ty> {
             if self.token_mint.is_none() {
                 return Err(ParseError::new(
                     token_authority.span(),
-                    "token authority must be provided if token mint is",
+                    "token mint must be provided if token authority is",
                 ));
             }
         }

tests/bpf-upgradeable-state/tests/bpf-upgradable-state.ts

@@ -53,7 +53,7 @@ describe('bpf_upgradeable_state', () => {
       });
       assert.ok(false);
     } catch (err) {
-      assert.equal(err.code, 143);
+      assert.equal(err.code, 2003);
       assert.equal(err.msg, "A raw constraint was violated");
     }
   });
@@ -73,7 +73,7 @@ describe('bpf_upgradeable_state', () => {
       });
       assert.ok(false);
     } catch (err) {
-      assert.equal(err.code, 173);
+      assert.equal(err.code, 3013);
       assert.equal(err.msg, "The given account is not a program data account");
     }
   });
@@ -93,7 +93,7 @@ describe('bpf_upgradeable_state', () => {
       });
       assert.ok(false);
     } catch (err) {
-      assert.equal(err.code, 167);
+      assert.equal(err.code, 3007);
       assert.equal(err.msg, "The given account is not owned by the executing program");
     }
   });
@@ -119,7 +119,7 @@ describe('bpf_upgradeable_state', () => {
       });
       assert.ok(false);
     } catch (err) {
-      assert.equal(err.code, 300);
+      assert.equal(err.code, 6000);
     }
   });
 });

tests/errors/tests/errors.js

@@ -17,7 +17,7 @@ describe("errors", () => {
         "This is an error message clients will automatically display";
       assert.equal(err.toString(), errMsg);
       assert.equal(err.msg, errMsg);
-      assert.equal(err.code, 300);
+      assert.equal(err.code, 6000);
     }
   });
 
@@ -29,7 +29,7 @@ describe("errors", () => {
       const errMsg = "HelloNoMsg";
       assert.equal(err.toString(), errMsg);
       assert.equal(err.msg, errMsg);
-      assert.equal(err.code, 300 + 123);
+      assert.equal(err.code, 6000 + 123);
     }
   });
 
@@ -41,7 +41,7 @@ describe("errors", () => {
       const errMsg = "HelloNext";
       assert.equal(err.toString(), errMsg);
       assert.equal(err.msg, errMsg);
-      assert.equal(err.code, 300 + 124);
+      assert.equal(err.code, 6000 + 124);
     }
   });
 
@@ -57,7 +57,7 @@ describe("errors", () => {
       const errMsg = "A mut constraint was violated";
       assert.equal(err.toString(), errMsg);
       assert.equal(err.msg, errMsg);
-      assert.equal(err.code, 140);
+      assert.equal(err.code, 2000);
     }
   });
 
@@ -80,7 +80,7 @@ describe("errors", () => {
       const errMsg = "A has_one constraint was violated";
       assert.equal(err.toString(), errMsg);
       assert.equal(err.msg, errMsg);
-      assert.equal(err.code, 141);
+      assert.equal(err.code, 2001);
     }
   });
 
@@ -108,7 +108,7 @@ describe("errors", () => {
       assert.ok(false);
     } catch (err) {
       const errMsg =
-        "Error: failed to send transaction: Transaction simulation failed: Error processing Instruction 0: custom program error: 0x8e";
+        "Error: failed to send transaction: Transaction simulation failed: Error processing Instruction 0: custom program error: 0x7d2";
       assert.equal(err.toString(), errMsg);
     }
   });
@@ -125,7 +125,7 @@ describe("errors", () => {
       const errMsg = "HelloCustom";
       assert.equal(err.toString(), errMsg);
       assert.equal(err.msg, errMsg);
-      assert.equal(err.code, 300 + 125);
+      assert.equal(err.code, 6000 + 125);
     }
   });