Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make getting PDAs a little easier #1446

Closed
mikemaccana opened this issue Jul 27, 2023 · 9 comments
Closed

Make getting PDAs a little easier #1446

mikemaccana opened this issue Jul 27, 2023 · 9 comments
Labels
enhancement New feature or request

Comments

@mikemaccana
Copy link
Contributor

mikemaccana commented Jul 27, 2023

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.

  • The name findProgramAddressSync() uses a different terminology - 'program address' - than the Solana documentation which generally uses 'program derived address' or just 'PDA'. The inconsistency here could lead to confusion, ie give a web3.js user the perception that that findProgramAddressSync() isn't the function someone is looking for to create a PDA.

  • The name findProgramAddressSync() isn't discoverable. Ideally someone using web3.js should be able to find how to get a PDA without looking at the documentation.

  • The sync part seems to a holdover from the async findProgramAddress() which is deprecated per issue #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.

  • The order of the arguments doesn't match how the program is described in Solana Foundation docs or by developers - docs normally mention mention we start with the program address, and use the seeds to find the PDA. However the function findProgramAddressSync() signature is seeds first, program address second.

  • I know PDAs are found based on the program address and seeds, however I'm not sure that justifies the find in the name. Many software projects use get for deterministically found values, get doesn't imply generation.

  • The findProgramAddressSync() output is an array, where the 0-indexed value is the address, and the 1-indexed value is the nonce. The rest of web3.js generally uses object keys rather than array indexes to distinguish between different output values, as this allows the programmer to inspect the output and understand the values inside.

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

  • A web3.js user wishes to get a program derived address. They know what PDAs are, and how a PDA starts with a program address and then uses the seeds to generate a PDA. They type getProgramDerivedAddress blindly and their editor automatically adds the import because there's a function with that name.

  • A user searches the web for get program derived address and finds a function called getProgramDerivedAddress(). They are a hundred percent confident that this function does what they expect. Compared to getProgramAddressSync() which doesn't quite match what they searched for - is a 'program address' and a 'program derived address' the same?

  • A user sees some code with getProgramDerivedAddress() in it. They understand the result is a program derived address. They can search for 'program derived address' and find an exact match for this concept, versus 'program address' which would return unrelated concepts.

  • A user understands PDAs, and wants to get a PDA. They type the on-curve address they want to start from, and then an array of seeds, matching the description and diagrams in the Solana docs.

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

image

@mikemaccana mikemaccana added the enhancement New feature or request label Jul 27, 2023
@steveluscher
Copy link
Collaborator

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

cc/ @2501babe

@steveluscher
Copy link
Collaborator

steveluscher commented Jul 27, 2023

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
Copy link
Contributor Author

mikemaccana commented Jul 28, 2023

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
Copy link
Collaborator

mcintyre94 commented Jul 28, 2023

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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Collaborator

The sync part seems to a holdover from the async findProgramAddress() which is deprecated per solana-labs/solana#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
Copy link
Contributor Author

Tagging #1453

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants