coral-xyz / anchor

⚓ Solana Sealevel Framework
https://anchor-lang.com
Apache License 2.0
3.36k stars 1.25k forks source link

Anchor AccountClient#fetchMultiple throw error when account on given Pubkeys have incorrect discriminator #2961

Open FredoNook opened 1 month ago

FredoNook commented 1 month ago

Greetings

When in AccountClient#fetchMultiple method passed Pubkeys with account not matching to AccountClient's account discriminator, BorschAccountDecoder throw error and that error not catched, so in this case any wrong Pubkey break AccountClient#fetchMultiple instead of return null for wrong Pubkey account as discribed in JSDoc to method AccountClient#fetchMultiple.

Method: AccountClient#fetchMultipleAndContext

  /**
   * Returns multiple deserialized accounts.
   * Accounts not found or with wrong discriminator are returned as null.
   *
   * @param addresses The addresses of the accounts to fetch.
   */
  async fetchMultipleAndContext(
    addresses: Address[],
    commitment?: Commitment
  ): Promise<({ data: T; context: Context } | null)[]> {
    const accounts = await rpcUtil.getMultipleAccountsAndContext(
      this._provider.connection,
      addresses.map((address) => translateAddress(address)),
      commitment
    );

    // Decode accounts where discriminator is correct, null otherwise
    return accounts.map((result) => {
      if (result == null) {
        return null;
      }
      const { account, context } = result;
      return {
        data: this._coder.accounts.decode(this._idlAccount.name, account.data),
        context,
      };
    });
  }

In line data: this._coder.accounts.decode(this._idlAccount.name, account.data), called BorschAccountDecoder#decode which throw error if discriminator.compare(data.slice(0, DISCRIMINATOR_SIZE)):

  public decode<T = any>(accountName: A, data: Buffer): T {
    // Assert the account discriminator is correct.
    const discriminator = this.accountDiscriminator(accountName);
    if (discriminator.compare(data.slice(0, DISCRIMINATOR_SIZE))) {
      throw new Error("Invalid account discriminator");
    }
    return this.decodeUnchecked(accountName, data);
  }

And this error not catched in AccountClient#fetchMultipleAndContext, so in this case this account not mapped as null but break execution with error.

acheroncrypto commented 1 month ago

To me, the behavior is expected, but the comment "Accounts not found or with wrong discriminator are returned as null." is half correct. Having different discriminator is an error in all cases, and null is only returned if the account doesn't exist.

FredoNook commented 1 month ago

What is benefit to get error in this case? Why not exist account less critical than account with different discriminator? Says we have program instruction that take bunch of same type accounts as input and that accounts isn't PDA so in this use case we have to fetch one by one to check that account exists and have expected type right? Why not to check them in one RPC request utilizing #fetchMultiple?