solana-labs / solana-web3.js

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

Make getting PDAs a little easier #1446

Closed mikemaccana closed 1 year ago

mikemaccana commented 1 year ago

Motivation

Heya! I've been working with web3.js for about a year, and recently started with Solana Foundation working on some new projects involving Solana education. I also have a background doing training and writing for technical concepts (mainly around Linux and node.js).

I have some small suggestion for findProgramAddressSync() and would like to propose a slightly simpler implementation.

Details

Add a new function to get PDAs with the following name and signature:

static getProgramDerivedAddress(
  programId: PublicKey,
  seeds: Array<Buffer | Uint8Array>,
): { address: PublicKey, nonce: number} {
  ...
}

Add

* @deprecated Use {@link getProgramDerivedAddress} instead

to findProgramAddressSync().

Optionally

getProgramDerivedAddress() is two characters longer than findProgramAddressSync(), and Solana users often simply say 'PDA' rather than 'Program Derived Address' if we wanted to we could also export getProgramDerivedAddress() a second time as getPDA(). Maybe we don't want to do this as we may prefer having one obvious way to do something, What do you think?

Example use case

will deterministically derive a PDA from a programId and seeds (collection of bytes)

image

steveluscher commented 1 year ago

Love love love these API suggestions. I was just working on this yesterday so your timing could not be better.

cc/ @2501babe

steveluscher commented 1 year ago

How do we feel about getProgramDerivedAddress() taking in only string[] as the seeds argument. Does anyone actually appreciate having to supply seeds as byte buffers, or are you mostly only ever doing this?

const seeds = [
  Buffer.from('hello'),
  Buffer.from('world'),
];

…when you could more easily do this:

const seeds = ['hello', 'world'];

I guess this doesn't really work because, for instance, there's no utf-8 string that would produce the bytes [255].

mikemaccana commented 1 year ago

While I wouldn't want to limit seeds to only be strings, I think the general idea of that code - turning seeds into buffers inside getProgramDerivedAddress() - is a good idea as it would allow developers to replace this:

const seeds = [userKeypair.publicKey.toBuffer(), new TextEncoder().encode(movie.title)]

with

const seeds = [userKeypair.publicKey, movie.title]

Which is far clearer.

Slightly off topic: what's preferred between Buffer.from() vs TextEncoder().encode()?

I guess this doesn't really work because, for instance, there's no utf-8 string that would produce the bytes [255].

Could you please explain this more? Why would a user want specific byte values? 🤔 Thanks!

mcintyre94 commented 1 year ago

Could you please explain this more? Why would a user want specific byte values?

Since seeds don't have to be strings, you need to be able to represent any set of seeds that might have been used to create the PDA - and that's any list of u8 arrays.

One example I've used, which is a bit weird but makes the seed really clean on the Anchor side, is a PDA for co-ordinates like:

[Buffer.from("pixel"), Buffer.from([posX, posY])]

There posX and posY are a u8 in rust, and could be any number 0-255, ie any byte. You just pass them in as numbers on the JS side, but they're really setting specific byte values. And anything to do with strings wouldn't make any sense there.

mikemaccana commented 1 year ago

Ah got it - thanks @mcintyre94. Yep my feeling is - don't restrict seed types to strings for this reasons, do allow strings to be in the array and encode them as necessary for user convenience.

steveluscher commented 1 year ago

Slightly off topic: what's preferred between Buffer.from() vs TextEncoder().encode()?

If you want max pain, use Buffer. Buffer is a Node-only primitive, and is not available in browsers or React Native.

The only reason folks have been using Buffer (imo) is because it gives you a handy little .toString('hex') API. Definitely not worth all of the hell its put us through: #1100.

steveluscher commented 1 year ago

The sync part seems to a holdover from the async findProgramAddress() which is deprecated per https://github.com/solana-labs/solana/issues/23184. It's understandable to want to distinguish between the two, but since one is deprecated it can eventually be removed, and there will be no need to distinguish between the current implementation and the now-removed one.

Fun fact: we're going async again, permanently. Read more.

mikemaccana commented 1 year ago

Tagging #1453

github-actions[bot] commented 1 year ago

Because there has been no activity on this issue for 7 days since it was closed, it has been automatically locked. Please open a new issue if it requires a follow up.