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

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

Open
Arrowana opened this issue Jul 16, 2023 · 1 comment
Open

Comments

@Arrowana
Copy link

Arrowana commented Jul 16, 2023

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.
solana-labs/solana#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
Copy link
Collaborator

cc @jnwng

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants