search

anza-xyz / wallet-adapter

Modular TypeScript wallet adapters and components for Solana applications.
https://anza-xyz.github.io/wallet-adapter/
Apache License 2.0
1.59k stars 960 forks source link

Walletconnect adapter implementation is likely to lead to sig failure #806

Open Arrowana opened 1 year ago

Arrowana commented 1 year ago

Describe the bug WalletConnect adapter is not implemented in a way in which it can guarantee signing the message of the transaction that will be returned to the dApp.

the legacyTransaction is spread and sent. The wallet connect adapter sends deserialized transaction fields, which are then serialized at the other side into a message to produce a signature. The adapter has no idea what message was signed and adds the signature in hope. https://github.com/jnwng/walletconnect-solana/blob/87df0e74e5195f36c21883d60d198243eea369df/packages/walletconnect-solana/src/adapter.ts#L143-L158

"attempting to reserialize a transaction that was deserialized from a message is conceptually invalid." The current design forces the interface to exactly do this, which is unreliable. This will work only when serialization is identical out of luck. https://github.com/solana-labs/solana/pull/23720

Opening this issue on the wallet adapter as this is the top level and it is using a dependency that technically can never function properly, so the whole chain has to be updated.

The other part, where the WalletConnect glue lives https://github.com/jnwng/walletconnect-solana/

To Reproduce Our chain is the following: rust api =(serialized tx) => jup.ag = Transaction object => WalletConnect adapter = (deserialized TransactionInstruction[] + blockhash + feePayer) = > remote wallet

Steps to reproduce the behavior: Pick a wallet that definitely doesn't use the more stable interface while still not perfect interface, so not ottr or use https://github.com/WalletConnect/web-examples/tree/main/wallets/react-wallet-v2 with a Solana throwaway wallet

  1. Go to jup.ag, disable ver tx after wallet connect connection, pick a swap, click swap EDIT: This won't work anymore as we are doing best effort to avoid the bug (rebuild tx from ixs with js Transaction before calling adapter to sign)
  2. Sign it in the wallet
  3. Signature verification fails

Expected behavior The expected behaviour is to send the original serialized transaction so that the wallet integrator can sign the same message. Technically it should only return the whole serialized transaction through the interface so that wallets are capable of mutation (allows compute budget prefixing,...).

The PR to allow versioned transaction is a stride in the right direction as the tx is serialized and sent, however that doesn't fix the current return interface. This might be "fair" if the message is deemed immutable.

Additional context This is not an issue caused by the latest change to allow versioned transaction "support" through WalletConnect

jordaaash commented 1 year ago

cc @jnwng