diff --git a/.github/workflows/reusable-tests.yaml b/.github/workflows/reusable-tests.yaml index 5819267891..3cc8718eea 100644 --- a/.github/workflows/reusable-tests.yaml +++ b/.github/workflows/reusable-tests.yaml @@ -472,6 +472,8 @@ jobs: path: tests/bench - cmd: cd tests/idl && ./test.sh path: tests/idl + - cmd: cd tests/interface-account && anchor test + path: tests/interface-account - cmd: cd tests/lazy-account && anchor test --skip-lint path: tests/lazy-account - cmd: cd tests/test-instruction-validation && ./test.sh diff --git a/lang/src/accounts/interface_account.rs b/lang/src/accounts/interface_account.rs index d12f8cd537..4665c36de8 100644 --- a/lang/src/accounts/interface_account.rs +++ b/lang/src/accounts/interface_account.rs @@ -1,14 +1,14 @@ //! Account container that checks ownership on deserialization. use crate::accounts::account::Account; -use crate::error::{Error, ErrorCode}; +use crate::error::ErrorCode; use crate::solana_program::account_info::AccountInfo; use crate::solana_program::instruction::AccountMeta; use crate::solana_program::pubkey::Pubkey; use crate::solana_program::system_program; use crate::{ AccountDeserialize, AccountSerialize, Accounts, AccountsClose, AccountsExit, CheckOwner, Key, - Owners, Result, ToAccountInfo, ToAccountInfos, ToAccountMetas, + Owners, Result, ToAccountInfos, ToAccountMetas, }; use std::collections::BTreeSet; use std::fmt; @@ -182,32 +182,6 @@ impl<'a, T: AccountSerialize + AccountDeserialize + Clone> InterfaceAccount<'a, } } - /// Reloads the account from storage. This is useful, for example, when - /// observing side effects after CPI. - /// - /// No Anchor discriminator is checked during reload. Instead, this method enforces - /// owner stability by verifying that `info.owner == self.owner` (i.e., the pubkey - /// validated at construction) to avoid TOCTOU issues, and then re-deserializes `T` - /// from the updated bytes. - /// - /// If you need discriminator validation on reload, use `Account` with an Anchor - /// #[account] type. - pub fn reload(&mut self) -> Result<()> { - let info = self.account.to_account_info(); - - // Enforce owner stability: must match the one validated at construction. - if info.owner != &self.owner { - return Err(Error::from(ErrorCode::AccountOwnedByWrongProgram) - .with_pubkeys((*info.owner, self.owner))); - } - - // Re-deserialize fresh data into the inner account. - let mut data: &[u8] = &info.try_borrow_data()?; - let new_val = T::try_deserialize_unchecked(&mut data)?; - self.account.set_inner(new_val); - Ok(()) - } - pub fn into_inner(self) -> T { self.account.into_inner() } @@ -234,28 +208,19 @@ impl<'a, T: AccountSerialize + AccountDeserialize + Clone> InterfaceAccount<'a, } impl<'a, T: AccountSerialize + AccountDeserialize + CheckOwner + Clone> InterfaceAccount<'a, T> { - /// Deserializes the given `info` into a `InterfaceAccount`. - /// - /// This **does not** check an Anchor discriminator. It first validates - /// program ownership via `T::check_owner`, then deserializes using - /// `AccountDeserialize::try_deserialize_unchecked`. + /// Deserializes the given `info` into an `InterfaceAccount`. #[inline(never)] pub fn try_from(info: &'a AccountInfo<'a>) -> Result { - // `InterfaceAccount` targets foreign program accounts (e.g., SPL Token - // accounts) that do not have Anchor discriminators. Because of that, we - // intentionally skip the Anchor discriminator check here and instead: - // - // 1) Validate program ownership via `T::check_owner(info.owner)?` - // 2) Deserialize without a discriminator by delegating to - // `T::try_deserialize_unchecked` - Self::try_from_unchecked(info) + if info.owner == &system_program::ID && info.lamports() == 0 { + return Err(ErrorCode::AccountNotInitialized.into()); + } + T::check_owner(info.owner)?; + let mut data: &[u8] = &info.try_borrow_data()?; + Ok(Self::new(info, T::try_deserialize(&mut data)?)) } - /// Deserializes the given `info` into a `InterfaceAccount` **without** checking - /// the account discriminator. This is intended for foreign program accounts. - /// Prefer `Self::try_from` when you also want the ownership check, but note - /// that both skip Anchor discriminator checks, and `try_from` additionally - /// enforces ownership. + /// Deserializes the given `info` into an `InterfaceAccount` without checking the account + /// discriminator. Be careful when using this and avoid it if possible. #[inline(never)] pub fn try_from_unchecked(info: &'a AccountInfo<'a>) -> Result { if info.owner == &system_program::ID && info.lamports() == 0 { @@ -265,6 +230,22 @@ impl<'a, T: AccountSerialize + AccountDeserialize + CheckOwner + Clone> Interfac let mut data: &[u8] = &info.try_borrow_data()?; Ok(Self::new(info, T::try_deserialize_unchecked(&mut data)?)) } + + /// Reloads the account from storage. This is useful, for example, when observing side effects + /// after CPI. + /// + /// This method also validates that the account is owned by one of the expected programs. + pub fn reload(&mut self) -> Result<()> { + let info: &AccountInfo = self.account.as_ref(); + T::check_owner(info.owner)?; + + // Re-deserialize fresh data into the inner account. + self.account.set_inner({ + let mut data: &[u8] = &info.try_borrow_data()?; + T::try_deserialize(&mut data)? + }); + Ok(()) + } } impl<'info, B, T: AccountSerialize + AccountDeserialize + CheckOwner + Clone> Accounts<'info, B> diff --git a/tests/interface-account/Anchor.toml b/tests/interface-account/Anchor.toml new file mode 100644 index 0000000000..619f4cc271 --- /dev/null +++ b/tests/interface-account/Anchor.toml @@ -0,0 +1,11 @@ +[programs.localnet] +interface_account = "interfaceAccount111111111111111111111111111" +new = "New1111111111111111111111111111111111111111" +old = "oLd1111111111111111111111111111111111111111" + +[provider] +cluster = "localnet" +wallet = "~/.config/solana/id.json" + +[scripts] +test = "yarn run ts-mocha -p ./tsconfig.json -t 1000000 tests/**/*.ts" diff --git a/tests/interface-account/Cargo.toml b/tests/interface-account/Cargo.toml new file mode 100644 index 0000000000..f397704811 --- /dev/null +++ b/tests/interface-account/Cargo.toml @@ -0,0 +1,14 @@ +[workspace] +members = [ + "programs/*" +] +resolver = "2" + +[profile.release] +overflow-checks = true +lto = "fat" +codegen-units = 1 +[profile.release.build-override] +opt-level = 3 +incremental = false +codegen-units = 1 diff --git a/tests/interface-account/package.json b/tests/interface-account/package.json new file mode 100644 index 0000000000..5bb13482cb --- /dev/null +++ b/tests/interface-account/package.json @@ -0,0 +1,16 @@ +{ + "name": "interface-account", + "version": "0.32.1", + "license": "(MIT OR Apache-2.0)", + "homepage": "https://github.com/coral-xyz/anchor#readme", + "bugs": { + "url": "https://github.com/coral-xyz/anchor/issues" + }, + "repository": { + "type": "git", + "url": "https://github.com/coral-xyz/anchor.git" + }, + "engines": { + "node": ">=17" + } +} diff --git a/tests/interface-account/programs/interface-account/Cargo.toml b/tests/interface-account/programs/interface-account/Cargo.toml new file mode 100644 index 0000000000..5a777958a5 --- /dev/null +++ b/tests/interface-account/programs/interface-account/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "interface-account" +version = "0.1.0" +description = "Created with Anchor" +edition = "2021" + +[lib] +crate-type = ["cdylib", "lib"] +name = "interface_account" + +[features] +default = [] +cpi = ["no-entrypoint"] +no-entrypoint = [] +no-idl = [] +idl-build = ["anchor-lang/idl-build"] + +[dependencies] +anchor-lang = { path = "../../../../lang" } +new = { path = "../new", features = ["cpi"] } +old = { path = "../old", features = ["cpi"] } diff --git a/tests/interface-account/programs/interface-account/src/lib.rs b/tests/interface-account/programs/interface-account/src/lib.rs new file mode 100644 index 0000000000..a8cef9cd54 --- /dev/null +++ b/tests/interface-account/programs/interface-account/src/lib.rs @@ -0,0 +1,52 @@ +#![allow(warnings)] + +use anchor_lang::prelude::*; + +declare_id!("interfaceAccount111111111111111111111111111"); + +#[program] +pub mod interface_account { + use super::*; + + pub fn test(ctx: Context) -> Result<()> { + Ok(()) + } +} + +#[derive(Accounts)] +pub struct Test<'info> { + pub expected_account: InterfaceAccount<'info, interface::ExpectedAccount>, +} + +mod interface { + #[derive(Clone)] + pub struct ExpectedAccount(new::ExpectedAccount); + + impl anchor_lang::AccountDeserialize for ExpectedAccount { + fn try_deserialize(buf: &mut &[u8]) -> anchor_lang::Result { + new::ExpectedAccount::try_deserialize(buf).map(Self) + } + + fn try_deserialize_unchecked(buf: &mut &[u8]) -> anchor_lang::Result { + new::ExpectedAccount::try_deserialize_unchecked(buf).map(Self) + } + } + + impl anchor_lang::AccountSerialize for ExpectedAccount {} + + impl anchor_lang::Owners for ExpectedAccount { + fn owners() -> &'static [anchor_lang::prelude::Pubkey] { + &[old::ID_CONST, new::ID_CONST] + } + } + + #[cfg(feature = "idl-build")] + mod idl_impls { + use super::ExpectedAccount; + + impl anchor_lang::IdlBuild for ExpectedAccount {} + impl anchor_lang::Discriminator for ExpectedAccount { + const DISCRIMINATOR: &'static [u8] = &[]; + } + } +} diff --git a/tests/interface-account/programs/new/Cargo.toml b/tests/interface-account/programs/new/Cargo.toml new file mode 100644 index 0000000000..9d007ca48f --- /dev/null +++ b/tests/interface-account/programs/new/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "new" +version = "0.1.0" +description = "Created with Anchor" +edition = "2021" + +[lib] +crate-type = ["cdylib", "lib"] + +[features] +default = [] +cpi = ["no-entrypoint"] +no-entrypoint = [] +no-idl = [] +idl-build = ["anchor-lang/idl-build"] + +[dependencies] +anchor-lang = { path = "../../../../lang" } diff --git a/tests/interface-account/programs/new/src/lib.rs b/tests/interface-account/programs/new/src/lib.rs new file mode 100644 index 0000000000..7b8c28fe0d --- /dev/null +++ b/tests/interface-account/programs/new/src/lib.rs @@ -0,0 +1,47 @@ +#![allow(warnings)] + +use anchor_lang::prelude::*; + +declare_id!("New1111111111111111111111111111111111111111"); + +#[program] +pub mod new { + use super::*; + + pub fn init(ctx: Context) -> Result<()> { + Ok(()) + } + + pub fn init_another(ctx: Context) -> Result<()> { + Ok(()) + } +} + +#[derive(Accounts)] +pub struct Init<'info> { + #[account(mut)] + pub authority: Signer<'info>, + #[account(init, payer = authority, space = 40)] + pub expected_account: Account<'info, ExpectedAccount>, + pub system_program: Program<'info, System>, +} + +#[derive(Accounts)] +pub struct InitAnother<'info> { + #[account(mut)] + pub authority: Signer<'info>, + #[account(init, payer = authority, space = 72)] + pub another_account: Account<'info, AnotherAccount>, + pub system_program: Program<'info, System>, +} + +#[account] +pub struct ExpectedAccount { + pub data: Pubkey, +} + +#[account] +pub struct AnotherAccount { + pub a: Pubkey, + pub b: Pubkey, +} diff --git a/tests/interface-account/programs/old/Cargo.toml b/tests/interface-account/programs/old/Cargo.toml new file mode 100644 index 0000000000..43381ffb6e --- /dev/null +++ b/tests/interface-account/programs/old/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "old" +version = "0.1.0" +description = "Created with Anchor" +edition = "2021" + +[lib] +crate-type = ["cdylib", "lib"] + +[features] +default = [] +cpi = ["no-entrypoint"] +no-entrypoint = [] +no-idl = [] +idl-build = ["anchor-lang/idl-build"] + +[dependencies] +anchor-lang = { path = "../../../../lang" } diff --git a/tests/interface-account/programs/old/src/lib.rs b/tests/interface-account/programs/old/src/lib.rs new file mode 100644 index 0000000000..1fded4cf19 --- /dev/null +++ b/tests/interface-account/programs/old/src/lib.rs @@ -0,0 +1,28 @@ +#![allow(warnings)] + +use anchor_lang::prelude::*; + +declare_id!("oLd1111111111111111111111111111111111111111"); + +#[program] +pub mod old { + use super::*; + + pub fn init(ctx: Context) -> Result<()> { + Ok(()) + } +} + +#[derive(Accounts)] +pub struct Init<'info> { + #[account(mut)] + pub authority: Signer<'info>, + #[account(init, payer = authority, space = 40)] + pub expected_account: Account<'info, ExpectedAccount>, + pub system_program: Program<'info, System>, +} + +#[account] +pub struct ExpectedAccount { + pub data: Pubkey, +} diff --git a/tests/interface-account/tests/interface-account.ts b/tests/interface-account/tests/interface-account.ts new file mode 100644 index 0000000000..25e8c8406b --- /dev/null +++ b/tests/interface-account/tests/interface-account.ts @@ -0,0 +1,90 @@ +import * as anchor from "@coral-xyz/anchor"; +import assert from "assert"; + +import type { InterfaceAccount } from "../target/types/interface_account"; +import type { New } from "../target/types/new"; +import type { Old } from "../target/types/old"; + +describe("interface-account", () => { + anchor.setProvider(anchor.AnchorProvider.env()); + const program: anchor.Program = + anchor.workspace.interfaceAccount; + const oldProgram: anchor.Program = anchor.workspace.old; + const newProgram: anchor.Program = anchor.workspace.new; + + const oldExpectedAccount = anchor.web3.Keypair.generate(); + const newExpectedAccount = anchor.web3.Keypair.generate(); + const anotherAccount = anchor.web3.Keypair.generate(); + + // Initialize accounts + before(async () => { + // Initialize the expected account of the old program + await oldProgram.methods + .init() + .accounts({ expectedAccount: oldExpectedAccount.publicKey }) + .signers([oldExpectedAccount]) + .rpc(); + + // Initialize the expected account of the new program + await newProgram.methods + .init() + .accounts({ expectedAccount: newExpectedAccount.publicKey }) + .signers([newExpectedAccount]) + .rpc(); + + // Initialize another account of the new program + await newProgram.methods + .initAnother() + .accounts({ anotherAccount: anotherAccount.publicKey }) + .signers([anotherAccount]) + .rpc(); + }); + + it("Allows old exptected accounts", async () => { + await program.methods + .test() + .accounts({ expectedAccount: oldExpectedAccount.publicKey }) + .rpc(); + }); + + it("Allows new exptected accounts", async () => { + await program.methods + .test() + .accounts({ expectedAccount: newExpectedAccount.publicKey }) + .rpc(); + }); + + it("Doesn't allow accounts owned by other programs", async () => { + try { + await program.methods + .test() + .accounts({ expectedAccount: program.provider.wallet!.publicKey }) + .rpc(); + + assert.fail("Allowed unexpected account substitution!"); + } catch (e) { + assert(e instanceof anchor.AnchorError); + assert.strictEqual( + e.error.errorCode.number, + anchor.LangErrorCode.AccountOwnedByWrongProgram + ); + } + }); + + it("Doesn't allow unexpected accounts owned by the expected programs", async () => { + try { + await program.methods + .test() + .accounts({ expectedAccount: anotherAccount.publicKey }) + .rpc(); + + assert.fail("Allowed unexpected account substitution!"); + } catch (e) { + assert(e instanceof anchor.AnchorError); + assert.strictEqual( + e.error.errorCode.number, + anchor.LangErrorCode.AccountDiscriminatorMismatch + ); + } + }); +}); diff --git a/tests/interface-account/tsconfig.json b/tests/interface-account/tsconfig.json new file mode 100644 index 0000000000..adfaf3bb84 --- /dev/null +++ b/tests/interface-account/tsconfig.json @@ -0,0 +1,10 @@ +{ + "compilerOptions": { + "lib": ["es2020"], + "module": "commonjs", + "target": "es6", + "esModuleInterop": true, + "strict": true, + "skipLibCheck": true + } +} diff --git a/tests/package.json b/tests/package.json index 7571a236b2..f5428d6cc2 100644 --- a/tests/package.json +++ b/tests/package.json @@ -25,7 +25,7 @@ "floats", "idl", "ido-pool", - "interface", + "interface-account", "lazy-account", "lockup", "misc",