solana-labs / solana-web3.js

Solana JavaScript SDK
https://solana-labs.github.io/solana-web3.js
MIT License
1.97k stars 792 forks source link

refactor(experimental): assert single sending signer inside signAndSend function #2852

Closed lorisleiva closed 1 week ago

lorisleiva commented 1 week ago

Fixes https://github.com/solana-labs/solana-web3.js/issues/2818

This PR calls the assertIsTransactionMessageWithSingleSendingSigner inside the signAndSendTransactionMessageWithSigners so the end-user doesn't need to do it explicitly.

This is partly to improve the developer experience and partly because TS doesn't fail properly when the transaction message hasn't been asserted on prior to calling the sign and send function.

Note that I did not remove the assertIsTransactionMessageWithSingleSendingSigner and the isTransactionMessageWithSingleSendingSigner from the package's exports since they may still be useful for custom use-cases such as the one described in the documentation:

let transactionSignature: SignatureBytes;
if (isTransactionMessageWithSingleSendingSigner(transaction)) {
    transactionSignature = await signAndSendTransactionMessageWithSigners(transaction);
} else {
    const signedTransaction = await signTransactionMessageWithSigners(transaction);
    const encodedTransaction = getBase64EncodedWireTransaction(signedTransaction);
    transactionSignature = await rpc.sendTransaction(encodedTransaction).send();
}
changeset-bot[bot] commented 1 week ago

🦋 Changeset detected

Latest commit: 6ea686ef2f7dcfa15df9d6d005e48e77001b8ac4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 37 packages | Name | Type | | --------------------------------------------- | ----- | | @solana/signers | Patch | | @solana/web3.js-experimental | Patch | | @solana/react | Patch | | @solana/accounts | Patch | | @solana/addresses | Patch | | @solana/assertions | Patch | | @solana/codecs-core | Patch | | @solana/codecs-data-structures | Patch | | @solana/codecs-numbers | Patch | | @solana/codecs-strings | Patch | | @solana/codecs | Patch | | @solana/compat | Patch | | @solana/errors | Patch | | @solana/fast-stable-stringify | Patch | | @solana/functional | Patch | | @solana/instructions | Patch | | @solana/keys | Patch | | @solana/options | Patch | | @solana/programs | Patch | | @solana/rpc-api | Patch | | @solana/rpc-graphql | Patch | | @solana/rpc-parsed-types | Patch | | @solana/rpc-spec-types | Patch | | @solana/rpc-spec | Patch | | @solana/rpc-subscriptions-api | Patch | | @solana/rpc-subscriptions-spec | Patch | | @solana/rpc-subscriptions-transport-websocket | Patch | | @solana/rpc-subscriptions | Patch | | @solana/rpc-transformers | Patch | | @solana/rpc-transport-http | Patch | | @solana/rpc-types | Patch | | @solana/rpc | Patch | | @solana/sysvars | Patch | | @solana/transaction-confirmation | Patch | | @solana/transaction-messages | Patch | | @solana/transactions | Patch | | @solana/webcrypto-ed25519-polyfill | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

lorisleiva commented 1 week ago

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @lorisleiva and the rest of your teammates on Graphite Graphite

lorisleiva commented 1 week ago

I think that since there's some value in exporting the helper functions for single sending signer transaction messages, it may be cleaner to keep the type as well so their implementation is consistent to some of the other assertions we provided that use an opaque type.

I don't feel too strongly about it though, so I can kill the type if you feel that's a better move.

steveluscher commented 1 week ago

Totally up to you. It's just literally usable for nothing, until someone marks an input to a new function as requiring ITransactionMessageWithSingleSendingSigner, right?

lorisleiva commented 1 week ago

I'm leaning towards leaving it in. Otherwise the assertIs* and is* helper methods no longer narrow down the type which makes them less valuable — even though we don't make use of that type internally.

lorisleiva commented 1 week ago

Merge activity