solana-labs / solana-web3.js

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

Suggestion: add a helper function just for sender -> recipient SOL transfers #1696

Open mikemaccana opened 9 months ago

mikemaccana commented 9 months ago

Motivation

A lot of the time users just want to send some SOL from a single account to a single recipient. Creating transactions for these should be simpler.

I can understand concern about wanting to present the reusable low level functionality of web3.js, but we can always document the helper function, then show the equivalent in the low-level tool to help people's understanding.

Example use case

A user wishes to send some SOL to another person, possibly adding a note.

const signature = await sendSOL({ 
  from: CryptoKeyPair, 
  to: address, 
  lamports: bigInt or number, 
  note?: string
})

This would:

Details

Ideally we'd be able to handle any SPL token:

const signature = await sendTokens({ 
  from: CryptoKeyPair, 
  to: address, 
  amountInMinorUnits: bigInt or number, 
  tokenMint: address,
  note?: string
})

Which would:

But my understanding is that the Token and AssociatedTokenAddress will be supported in a separate library built by kinobi.

Still a person can dream ☺️

mikemaccana commented 8 months ago

Linking @mcintyre94's comment at https://github.com/solana-labs/solana-web3.js/issues/1694#issuecomment-1750789156 re: shipping a general purpose high level library.

lorisleiva commented 8 months ago

IMO we should autogenerate these and create a good API around the generated instructions so they can be easily composed together. Here's a rough example of what this could look like.

await pipe(
  createTransaction(),
  transferSol(context, { from, to, lamports }),
  addMemo(context, { memo }),
  tx => sendAndConfirm(tx)
)

And by passing a context offered by a framework like Umi, this could even become.

await transferSol(umi, { from, to, lamports })
  .add(addMemo(umi, { memo }))
  .sendAndConfirm(umi)

We can always tweak the end-user API to make it more friendly. If people feel comfortable using them directly, then we've done a good job. If we start to feel the need of adding lots of helper functions to abstract the end-user API, it's an indication that we should revisit its design.

mikemaccana commented 8 months ago

The main thing there is that piping and method chaining are not popular among JS developers, compared to using plain JS objects for configuration. https://github.com/solana-labs/solana-web3.js/issues/1693 has a lot of detail on this, @Woody4618 may have some thoughts too.

lorisleiva commented 8 months ago

That proposal doesn't really help with signing, sending and confirming transactions though. Which mean devs will end up using 3-4 const variables or piping anyway. However, notice the second code snippet which uses a TransactionBuilder object provided by Umi. That's just an example showing that, in the end, we can design whichever API we want even from the same autogenerated code. Right now, the new Web3.js doesn't offer an TransactionBuilder object to optimise tree-shakability. But we could offer an optional TransactionBuilder for users that want to have a different (more fluent) experience.

I also agree that piping and functional programming in general is not super popular amongst JS devs but it is the optimal way to offer slim libraries. If we provide a good function programming experience and educate devs about it then there's a way this can work even without a TransactionBuilder or similar.

Woody4618 commented 8 months ago

I dont really have a strong oppinion on this. I didnt know the pipe operator until yesterday. Looks like another thing ppl need to learn. How would this example look like with a instruction to an anchor program? And can i also just get the instruction from the sendSol to build my own transaction? How do i sign? How can i add another feepayer or partial sign? I havnt really tried the new package yet.

mikemaccana commented 7 months ago

Pardon the wait, it's been a long month with Breakpoint

That proposal doesn't really help with signing, sending and confirming transactions though.

That's correct. The intention is to create the transaction with a plain JS object per other common JS SDKs.

Which mean devs will end up using 3-4 const variables or piping anyway.

That's not correct - the intention of the proposal is require a single const for the entire transaction and its instructions and other internal components.

I've added more detail to https://github.com/solana-labs/solana-web3.js/issues/1693#issuecomment-1823460118 to explain the thinking here.

lorisleiva commented 7 months ago

Oh, you want mutable transactions. The entire new web3.js is based on immutable data though. It is even defined as the first principle of the rewrite.

In order to improve the developer experience around this, @steveluscher created an "API for transaction builder" issue allowing us to offer and discuss potential solutions. From this, the pipe operator was chosen because it is the one that offers the most tree-shakability whilst having a decent developer experience.

I think going back on the data immutability principle would not be wise but I do agree that the pipe operator is not an ideal first introduction to the library for developers. As you can see on the issue above though, other alternatives also have their own drawbacks.


CleanShot 2023-11-23 at 11 18 30@2x

mcintyre94 commented 7 months ago

I think this is somewhere where the immutability has clear benefits that IMO outweigh the cost of an additional concept to learn.

The most important limitation here comes from TypeScript. Given an object that has eg feePayer?: Address, we have no way of writing a function that only accepts an instance of that object when feePayer is set. We're able to do that with the current approach because we can funnel you through a function that returns ITransactionWithFeePayer and track that type through the entire lifetime.

Because of this, if we allowed transactions to be mutable and for you to eg. just set the feePayer field, then all checks would become runtime checks, and all of our ability to detect mistakes at compile time would go away. I think the tradeoff here for maintaining apps is too great.