[Detail Bug] Solana: wallet method wrappers can route transactions/signatures to the wrong wallet after switching

[detail-app]

Detail Bug Report

https://app.detail.dev/org_ea7bf3e3-a2f4-4402-9351-baa0e1eaa1f5/bugs/bug_767b3eaa-f84e-47ff-9ba0-914548fba7c0

Summary

• Context: SolanaAdapter tracks wallet events by wrapping methods like sendTransaction and signMessage on Solana wallet adapters.
• Bug: The SolanaAdapter instance uses shared properties (e.g., originalAdapterSendTransaction) to store references to the original methods of the wrapped adapter. These properties are overwritten whenever a new wallet is set or the wallet context is rebound.
• Actual vs. expected: When a previously wrapped method of an old wallet is called after switching to a new wallet, the wrapper invokes the original method of the newly active wallet instead of the old one.
• Impact: Wallet interactions (transactions and signatures) can be routed to the wrong wallet adapter if the application or a library holds a reference to a wrapped method, leading to incorrect analytics and potentially failed or misdirected transactions.

Code with Bug

// src/solana/SolanaAdapter.ts

// Shared properties on the SolanaAdapter instance <-- BUG 🔴
private originalAdapterSendTransaction?: ISolanaAdapter["sendTransaction"];
private originalAdapterSignMessage?: ISolanaAdapter["signMessage"];
private originalAdapterSignTransaction?: ISolanaAdapter["signTransaction"];

// ...

private wrapAdapterMethods(adapter: ISolanaAdapter): void {
  // ...
  if (adapter.sendTransaction) {
    // Overwrites the shared property with the current adapter's method <-- BUG 🔴 (overwritten on wallet switch)
    this.originalAdapterSendTransaction = adapter.sendTransaction.bind(adapter);
    this.boundWrappedSendTransaction = this.wrappedSendTransaction.bind(this);
    adapter.sendTransaction = this.boundWrappedSendTransaction;
  }
  // ...
}

private async wrappedSendTransaction(
  transaction: SolanaTransaction,
  connection: SolanaConnection,
  options?: SendTransactionOptions
): Promise<TransactionSignature> {
  this.checkAndRebindContextAdapter();

  if (!this.originalAdapterSendTransaction) {
    throw new Error("sendTransaction not available");
  }

  // ...

  try {
    // Calls whatever is currently in the shared property,
    // which might belong to a different adapter! <-- BUG 🔴 (can call new wallet's original method)
    const signature = await this.originalAdapterSendTransaction(
      transaction,
      connection,
      options
    );
    // ...
  }
}

Explanation

• wrapAdapterMethods() stores the “original” adapter methods on the SolanaAdapter instance (this.originalAdapterSendTransaction, etc.) and replaces the adapter methods with wrappers.
• When the wallet is switched via setWallet(...) (or context is rebound), wrapAdapterMethods() runs again and overwrites these shared instance properties with the new adapter’s bound methods.
• If any code retains a reference to a previously wrapped method (e.g., const oldSend = oldAdapter.sendTransaction) and calls it after a wallet switch, that wrapper executes this.originalAdapterSendTransaction, which now points to the new wallet’s method—routing the call to the wrong adapter.
• Repro evidence: a test switching from a mocked “Phantom” adapter to “Solflare” shows calling Phantom’s previously captured wrapped sendTransaction returns “Solflare” (assertion failure).

Recommended Fix

The wrapper functions should capture the original method in a local closure or store it in a way that is uniquely associated with the specific adapter instance (e.g., using a WeakMap or a Symbol on the adapter itself), rather than using shared properties on the SolanaAdapter instance.

History

This bug was introduced in commit ec2108d. This commit added the Solana wallet integration and established the pattern of using shared instance properties to store original wallet methods, which incorrectly causes method calls to be routed to the active wallet even if the method was captured from a previous one.