signAndSendTransaction + signAndSendAllTransactions API #889
Conversation
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@solana/[email protected], npm/@solana/[email protected], npm/@solana/[email protected] |
packages/core/base/src/adapter.ts
Outdated
| } | ||
|
|
||
| /** Mode for signing and sending transactions. */ | ||
| export type SolanaSignAndSendTransactionMode = 'parallel' | 'serial'; |
There was a problem hiding this comment.
I would prefer to re-export this from the @solana/wallet-standard-features package.
packages/core/base/src/adapter.ts
Outdated
| @@ -61,7 +60,7 @@ export interface WalletAdapterProps<Name extends string = string> { | |||
| transactions: TransactionOrVersionedTransaction<this['supportedTransactionVersions']>[], | |||
| connection: Connection, | |||
| options?: SignAndSendTransactionOptions | |||
| ): Promise<(TransactionSignature | SignAndSendAllTransactionsError)[]>; | |||
| ): Promise<(TransactionSignature | undefined)[]>; | |||
There was a problem hiding this comment.
Returning undefined is generally not ideal because the code may early return or simply return implicitly. Returning null is better because it's explicit.
Unfortunately returning either undefined or null isn't great, because it gives the caller no information about the failure.
What is an app supposed to do with this result? I don't think we can kick the can on having an error of some kind.
packages/core/base/src/adapter.ts
Outdated
| code: result.reason.code, | ||
| message: result.reason.message, | ||
| }; | ||
| return result.status === 'fulfilled' ? result.value : undefined; |
| ); | ||
| sendOptions.preflightCommitment = sendOptions.preflightCommitment || connection.commitment; | ||
|
|
||
| const { signatures } = await wallet.signAndSendAllTransactions(transactions, sendOptions); |
There was a problem hiding this comment.
I don't think we can call this without feature detection because there's no guarantee of what version of the wallet the user has that supports this method. If it's older, it'll just null reference with nothing for the app to do.
We should consider having/calling a super method for this that will do signAllTransactions and send them after, in the same way we have sendTransaction in the base class if only signTransaction is supported.
There was a problem hiding this comment.
I suppose we don't even need to add an implementation for signAndSendAll in individual wallet adapters (other than the unsafe burner, where we won't have to do feature detection)?
continued from #841