Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solana web3.js library incorrectly encodes AccountMeta if there are multiple accounts with the same pubkey #23641

Closed
bji opened this issue Mar 14, 2022 · 3 comments

Comments

@bji
Copy link
Contributor

bji commented Mar 14, 2022

Problem

This code in web3.js transaction.ts file:

    const instructions: CompiledInstruction[] = this.instructions.map(
      instruction => {
        const {data, programId} = instruction;
        return {
          programIdIndex: accountKeys.indexOf(programId.toString()),
          accounts: instruction.keys.map(meta =>
            accountKeys.indexOf(meta.pubkey.toString()),
          ),
          data: bs58.encode(data),
        };
      },
    );
(currently line 345 of transacton.ts)

is incorrect. If the transaction has the same pubkey listed twice in the message header:

  • One as signer, read-write
  • Once as signer, read-only

Then the above code will always select the index of the signer, read-write account reference, even if the AccountMeta in the instruction itself (i.e. the account as listed in the instruction accounts list) is signer, read-only. This is because the function passed to map just looks up the accountKeys.indexOf(meta.pubkey.toString()) and does not consider the flags on the AccountMeta.

I have an on-chain program that inspects the account metas in the instruction params and enforces that the signer and writable flags are set to exactly the expected values. But the transaction is encoded incorrectly by web3.js as I just described, and my program fails with an error because it is being given a signer, read-write account meta, not a signer, read-only as it should be, because the signer, read-write was found "first" by the map code when the instruction was encoded.

(in other words, the keys[] I am giving to the instruction include an AcountMeta that is signer, read-only; but the transaction is being encoded with a reference to an index from the message header accounts list that is the signer, read-write account, which is not the account that I specified at that position in the instruction accountmetas list)

Proposed Solution

I am not that familiar with javascript so I can't offer specific code examples. But I can say that the code I quoted needs to be modified to, instead of using account.indexOf(), do a more thorough search for the account that both has the correct pubkey and the correct flags (signer and/or writable, whatever matches what's in the meta).

@t-nelson
Copy link
Contributor

Account metas are encoded transaction-wide and each account receives the highest promoted position. Consider the MessageHeader, which declares ranges in the account_keys vector

@jordaaash
Copy link
Contributor

Duplicate of #23631

@jordaaash jordaaash marked this as a duplicate of #23631 Mar 17, 2022
@bji bji reopened this Mar 17, 2022
@bji bji closed this as completed Mar 17, 2022
@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any activity in past 7 days after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants