Validate Transfer function should return success even if the token transfer was not performed in the last instruction

[AnishDe12020]

Currently the validateTransfer function in the SDK only returns (does not throw) when the token transfer was done on the last instruction. However many wallets (for e.g. Glow) add an instruction at the end of every Solana Pay transaction (they use it to tag accounts and send messages) and hence the validateTransfer function throws.

It would be better if we loop through all the instructions, check if it is a token transfer instruction (jn case it is an spl token) transfer or try to decode it (continue upon failure) so that we can check all the instructions

Here is how I have implemented it

export async function validateTransfer(
  connection: Connection,
  signature: TransactionSignature,
  { recipient, amount, splToken }: ValidateTransferFields,
  options?: { commitment?: Finality }
): Promise<TransactionResponse> {
  const response = await connection.getTransaction(signature, options);
  if (!response) throw new ValidateTransferError("not found");

  const { message, signatures } = response.transaction;
  const meta = response.meta;
  if (!meta) throw new ValidateTransferError("missing meta");
  if (meta.err) throw meta.err;

  // Deserialize the transaction and make a copy of the instructions we're going to mutate it.
  const transaction = Transaction.populate(message, signatures);
  const instructions = transaction.instructions.slice();

  let transferred: boolean = false;

  console.log("before for loop");

  for (const instruction of instructions) {
    if (splToken) {
      const recipientATA = await getAssociatedTokenAddress(splToken, recipient);
      const accountIndex = message.accountKeys.findIndex((pubkey) =>
        pubkey.equals(recipientATA)
      );

      console.log("accountIndex: " + accountIndex);

      const decodedInstruction = decodeInstruction(instruction);
      if (
        isTransferCheckedInstruction(decodedInstruction) ||
        isTransferInstruction(decodedInstruction)
      ) {
        if (accountIndex === -1)
          throw new ValidateTransferError("recipient not found");

        const preBalance = new BigNumber(
          meta.preTokenBalances?.find((x) => x.accountIndex === accountIndex)
            ?.uiTokenAmount.uiAmountString || 0
        );
        const postBalance = new BigNumber(
          meta.postTokenBalances?.find((x) => x.accountIndex === accountIndex)
            ?.uiTokenAmount.uiAmountString || 0
        );

        if (postBalance.minus(preBalance).lt(amount))
          throw new ValidateTransferError("amount not transferred");

        transferred = true;
      }
    } else {
      const accountIndex = message.accountKeys.findIndex((pubkey) =>
        pubkey.equals(recipient)
      );
      if (accountIndex === -1)
        throw new ValidateTransferError("recipient not found");

      // Check that the instruction is a system transfer instruction.
      try {
        SystemInstruction.decodeTransfer(instruction);
      } catch {
        continue;
      }

      const preBalance = new BigNumber(meta.preBalances[accountIndex] || 0).div(
        LAMPORTS_PER_SOL
      );
      const postBalance = new BigNumber(
        meta.postBalances[accountIndex] || 0
      ).div(LAMPORTS_PER_SOL);

      if (postBalance.minus(preBalance).lt(amount))
        throw new ValidateTransferError("amount not transferred");

      transferred = true;
    }
  }

  if (!transferred) {
    throw new ValidateTransferError("transfer not found");
  }

  return response;
}

[jordaaash]

However many wallets (for e.g. Glow) add an instruction at the end of every Solana Pay transaction (they use it to tag accounts and send messages)

Can you provide an example of such a transaction for us to look at?

The relevant part of the spec says

the wallet must include a TokenProgram.Transfer or TokenProgram.TransferChecked instruction as the last instruction of the transaction. [...] the wallet must include a SystemProgram.Transfer instruction as the last instruction of the transaction instead.

If Glow, or any wallet, is not doing this, it's not following the spec, so we need to figure out why and if we can change that.

[AnishDe12020]

Can you provide an example of such a transaction for us to look at?

https://explorer.solana.com/tx/47NmtJWhxamL2an67KySKEz5KbsiQTLHfQcyRGMU3o5XxhwgtBV3hpTXL7TuJM5Rkic5kTP2jwrpnKqyTRgpuKqs

the last program is a note program

I had reached out to Glow on twitter and this was their response -

[amilz]

I had the same issue just running the example here with a native SOL transfer.

I was consistently getting Error: invalid instruction; programId is not SystemProgram when running validateTransfer.

https://github.com/solana-labs/solana-pay/blob/9ab0cf9c6addccc986787153024a6427002d513c/core/src/validateTransfer.ts#LL76-L77C44

For a quick fix, just changed instructions.pop() to instructions[0]and the instruction was validated. Agree with OP that some type of iteration or filter might be more appropriate that the pop. Also, I could imagine an optional program parameter for this function so you could validate custom instructions.

Example: https://explorer.solana.com/tx/3jutrJBNxkduxkhQaMe6ks44XRSPEyCT5nMyQM9PnnAMkYyyaBRTmm5vvfrqrGP1VcYEEmEdmcLbiv8Xv9UuS9wZ?cluster=devnet

[jordaaash]

The issue here is that the spec says the transfer ix must be last, immediately preceded by the memo ix. Glow is adding its own note ix at the end. I'm told that this issue is fixed in Glow now, I believe by inserting the note ix first. Can you check?

[amilz]

told that this issue is fixed in

yeah that works. thx for quick reply. sry about that.