solana-mobile / mobile-wallet-adapter

Other
250 stars 104 forks source link

[Bug] Potential Chain/Cluster bug when signing with Phantom/Solflare #958

Open Michaelsulistio opened 2 months ago

Michaelsulistio commented 2 months ago

Describe the bug A discord user ran into an issue when trying to sign an Anchor transaction with Phantom and Solflare.

It works fine when trying to sign a message, only failing on transactions

To Reproduce Have not been able to repro locally myself, but this is the repo of the user with the issue:

https://github.com/AlexScotte/SolanaButtonDapp/tree/main

Expected behavior The wallet should successfully sign the transaction.

Screenshots image

Smartphone (please complete the following information):

Additional context

Funkatronics commented 2 months ago

https://github.com/solana-mobile/mobile-wallet-adapter/blob/f204f275a15e83a1aef5ff2727c41c1e3cb4dfed/js/packages/wallet-adapter-mobile/src/adapter.ts#L146

a provided cluster should get converted to a valid chain identifier using the latest code.

Funkatronics commented 2 months ago

offline discussion, Mike pointed out that the repo is actaully using mobile-wallet-adapter-protocol lib directly.

Still interesting tho, as the mobile-wallet-adapter-protocol library does have logic to convert between chain and cluster depending on the session version: https://github.com/solana-mobile/mobile-wallet-adapter/blob/f204f275a15e83a1aef5ff2727c41c1e3cb4dfed/js/packages/mobile-wallet-adapter-protocol/src/createMobileWalletProxy.ts#L77

Michaelsulistio commented 2 months ago

Cloned the user's repo and tested out locally on newest version of Phantom and Solflare off Play Store.

Phantom Phantom Version: 24.15.0 (23517) Outcome: Successfully signed the transaction on devnet, as expected.

Solflare Solflare Version: 1.46.3 (Android 13) Outcome: Running into the same Network mismatch error as the user, when trying to sign the transaction.

I tried changing the authorize call to use different parameters for chain/cluster to no avail:

// 1. original from user - error
wallet.authorize({
            cluster: 'devnet',
            identity: APP_IDENTITY,
          }));

 // 2. Using `chain` correctly, still errors
wallet.authorize({
            chain: 'solana:devnet',
            identity: APP_IDENTITY,
          }));

// 3. Using chain with cluster semantics, still errors
wallet.authorize({
            chain: 'devnet',
            identity: APP_IDENTITY,
}));

Leads me to suspect there could be an issue with Solflare implementation starting at a certain version.

Update:

Funkatronics commented 1 month ago

@Michaelsulistio given that regular message signing works fine with Solflare, and this is only happening on on a sign-transactions request (as opposed to within the authorize request where we actually pass the cluster param), I think this must be a bug in Solflare.

The sign_transactions request only contains the transaction bytes (base64 encoded). so if authorization is working, and the wallet throws an error on a transaction payload that has been verified to work with a different wallet, the affected wallet must have some bug that triggers the incorrect cluster error.

Funkatronics commented 1 month ago

I am struggling to reproduce this in with any of our testing environments, using the latest version of Phantom from Google Play. Really tough one to track down. Phantom is indeed using a legacy version of MWA but both the native and React libraries are correctly converting the chain to the legacy cluster param. Signing is working with both Solflare and Phantom. I am not sure how to help here unfortunately!!

ETA: tested with Solflare 1.47.3 and Phantom 24.20.0