Add supporting Versioned Transaction signing #9
Conversation
…d for VersionedTransaction
| params: { ...legacyTransaction, transaction: rawTransaction }, | ||
| }, |
There was a problem hiding this comment.
Hmm, why are the transaction properties passed as params? I realize this isn't a change from how it's currently being done, but the transaction bytes are all that should be needed, right?
There was a problem hiding this comment.
@jordansexton It was old way of doing this implemented by WalletConnect. So we are keeping for backward compatibility and introducing a new way that could support both legacy and v0 transactions.
| request: { | ||
| method: WalletConnectRPCMethods.signTransaction, | ||
| params: { transaction: rawTransaction }, | ||
| }, |
There was a problem hiding this comment.
this whole block looks identical to the same request for the legacy transaction, i'd suggest combining the two blocks and just create the parameters differently depending on the incoming transaction
| topic: this._session.topic, | ||
| request: { | ||
| method: WalletConnectRPCMethods.signTransaction, | ||
| params: { ...transaction, transaction: rawTransaction }, |
There was a problem hiding this comment.
the previous version did not require an explicit serialization of the transaction, is there a particular reason why that's needed for the existing non-versioned transactions?
There was a problem hiding this comment.
@jnwng Versioned Transaction and legacy Transaction have completely different set of properties. Passing non-serialized transaction will require to create different methods for the future changes of Transaction class.
Also it makes its much harder to consume deconstructed transactions on the wallet side.
To make API future proof and easy to use we believe that moving to serialized transaction is a better approach, which is similar to approach of Solana Pay.
| request: { | ||
| method: WalletConnectRPCMethods.signTransaction, | ||
| params: { | ||
| ...legacyTransaction, // Legacy sign transaction request format |
There was a problem hiding this comment.
@bergusman make a comment here. That it is deprecated and in the future versions only transaction will be sent.
| if (transaction.version === 'legacy') { | ||
| // Build Transaction for legacy sign transaction request format | ||
| legacyTransaction = Transaction.from(transaction.serialize()); | ||
| } else { |
There was a problem hiding this comment.
@bergusman @jnwng This "else" block I think should be completely removed.
We can't create legacy transaction from non-legacy one. So legacyTransaction should be undefined here.
Add field
transactionforsolana_signTransactionmethod with serialized transaction.For 0 and next version of transaction used only
transactionparameter.For
Transactionand legacyVersionedTransactionput old WalletConnect specification parameters and newtransactionparameter.Updated @solana/web3.js to 1.63.0 version to support
addSignatureforVersionedTransaction.