refactor(experimental): implement getBase58EncodedAddressForSeed #1563

2501babe · 2023-09-02T20:30:09Z

doing this now because its a blocker for me. straightforward reimpl of createWithSeed:

  static async createWithSeed(
    fromPublicKey: PublicKey,
    seed: string,
    programId: PublicKey,
  ): Promise<PublicKey> {
    const buffer = Buffer.concat([
      fromPublicKey.toBuffer(),
      Buffer.from(seed),
      programId.toBuffer(),
    ]);
    const publicKeyBytes = sha256(buffer);
    return new PublicKey(publicKeyBytes);
  }

not ready to merge as-is, pls let me know:

how to run lint:fix, theres no command for it so i have no idea
if you want this in a new file
any style complaints, i tried to copy existing code as much as possible

Closes #999.

buffalojoec · 2023-09-03T20:43:23Z

how to run lint:fix, theres no command for it so i have no idea

This works from root:

pnpm eslint --fix packages/addresses/src/__tests__/base58-test.ts

Curious what you think of createBase58AddressWithSeed? To match original

2501babe · 2023-09-04T12:54:02Z

This works from root

this doesnt run the other linter... it turns out the command was pnpm prettier -w src/base58.ts. if theres a way to run both linters on all files in write mode, that would be a worthwhile command to add to toplevel package.json

Curious what you think of createBase58AddressWithSeed? To match original

sounds good!

buffalojoec

This looks good and definitely matches the legacy library's implementation, however I wonder why the legacy lib doesn't look more like the Rust side of things.

https://github.com/solana-labs/solana/blob/adee97fe38b175ae185d34a7aa71b2e7bfd570e8/sdk/program/src/pubkey.rs#L206-L224

Why doesn't it have the checks that solana_program does (MAX_SEED_LENGTH, PDA_MARKER), any idea?

I think we should probably add these checks even though they weren't originally there, unless we have good reason not to.

steveluscher · 2023-09-04T15:22:02Z

Hang on one sec before you ship this. I’ll make some suggestions tomorrow.

steveluscher

…I wonder why the legacy lib doesn't look more like the Rust side of things.

Yeah, 100%. I remember seeing that code and being like ‘hmm, we should roll that in.’ Add to this implementation:

A check for max seed length like we did for program-derived-address (add tests)
The Rust-style check for a forged PDA marker. What you're trying to protect against is this super weird edge case: create(baseAddress, seed, '4vJ9JU1bJJE96FbKdjWme2JfVK1knU936FHTDZV7AC2'). The last 21 bytes of 4vJ9JU1bJJE96FbKdjWme2JfVK1knU936FHTDZV7AC2 just happen to decode to the ASCII string ProgramDerivedAddress. (add tests)

Just one other thought: I think this is one of those APIs where a struct arg would be better. See createProgramDerivedAddress(pdaInput: PdaInput).

steveluscher · 2023-09-05T17:18:07Z

packages/addresses/src/base58.ts

+    programAddress: Base58EncodedAddress
+): Promise<Base58EncodedAddress> {
+    const { serialize, deserialize } = getBase58EncodedAddressCodec();
+    const seedBytes = new TextEncoder().encode(seed);


I was going to point out that TextEncoder isn't available in React Native, but:

We also use it in program-derived-address.ts

We should presume that, in React Native, we need to call out to an async API to do this encoding, but you've already made this method async so… carry on.

steveluscher · 2023-09-05T17:19:54Z

packages/addresses/src/__tests__/base58-test.ts

@@ -132,4 +133,16 @@ describe('base58', () => {
            ]);
        });
    });
+    describe('createBase58AddressWithSeed', () => {
+        it('works', async () => {


For the benefit of the next reader who doesn't know what this is supposed to do in the first place.

Suggested change

it('works', async () => {

it('returns an address that is the SHA-256 hash of the concatenated base address, seed, and program address', async () => {

steveluscher · 2023-09-05T17:20:18Z

packages/addresses/src/base58.ts

@@ -52,3 +52,18 @@ export function getBase58EncodedAddressComparator(): (x: string, y: string) => n
        usage: 'sort',
    }).compare;
 }
+
+export async function createBase58AddressWithSeed(


For pedantry:

Suggested change

export async function createBase58AddressWithSeed(

export async function createBase58EncodedAddressWithSeed(

Akshually; I just renamed everything to *Address* didn't I? So createAddressWithSeed is the thing that would match everything else now.

2501babe · 2023-09-07T18:41:04Z

ok i

renamed the function to createAddressWithSeed
gave it an args struct
renamed program-derived-address*.ts to computed-address*.ts and moved the functionality there since they both require the same constants and have a certain harmony
added guardrails as in rust and tests for them

2501babe · 2023-09-07T19:56:25Z

note ci failure is due to a bug in node nodejs/node#49497

steveluscher

github-actions · 2023-09-12T11:13:36Z

🎉 This PR is included in version 1.78.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions · 2023-09-27T08:03:15Z

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

2501babe requested a review from steveluscher September 2, 2023 20:30

2501babe force-pushed the 20230902_seed branch from 5500277 to 0f8fe6c Compare September 4, 2023 12:56

2501babe marked this pull request as ready for review September 4, 2023 12:56

2501babe requested a review from buffalojoec September 4, 2023 12:56

2501babe force-pushed the 20230902_seed branch 3 times, most recently from ab0535c to bdda9c5 Compare September 4, 2023 13:22

buffalojoec reviewed Sep 4, 2023

View reviewed changes

steveluscher requested changes Sep 5, 2023

View reviewed changes

refactor(experimental): implement createAddressWithSeed

3d76d55

2501babe force-pushed the 20230902_seed branch from bdda9c5 to 3d76d55 Compare September 7, 2023 18:38

2501babe requested a review from steveluscher September 7, 2023 18:42

2501babe mentioned this pull request Sep 7, 2023

refactor(experimental): add lint:fix #1568

Merged

nit: add comment to test about malicious PDA marker

1acdcce

steveluscher approved these changes Sep 7, 2023

View reviewed changes

steveluscher merged commit 5030000 into solana-labs:master Sep 7, 2023
3 of 6 checks passed

github-actions bot added the released label Sep 12, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(experimental): implement getBase58EncodedAddressForSeed #1563

refactor(experimental): implement getBase58EncodedAddressForSeed #1563

2501babe commented Sep 2, 2023 •

edited by steveluscher

Loading

buffalojoec commented Sep 3, 2023

2501babe commented Sep 4, 2023 •

edited

Loading

buffalojoec left a comment •

edited

Loading

steveluscher commented Sep 4, 2023

steveluscher left a comment •

edited

Loading

steveluscher Sep 5, 2023

steveluscher Sep 5, 2023

steveluscher Sep 5, 2023

steveluscher Sep 6, 2023

2501babe commented Sep 7, 2023 •

edited

Loading

2501babe commented Sep 7, 2023

steveluscher left a comment

github-actions bot commented Sep 12, 2023

github-actions bot commented Sep 27, 2023

	it('works', async () => {
	it('returns an address that is the SHA-256 hash of the concatenated base address, seed, and program address', async () => {

	export async function createBase58AddressWithSeed(
	export async function createBase58EncodedAddressWithSeed(

refactor(experimental): implement getBase58EncodedAddressForSeed #1563

refactor(experimental): implement getBase58EncodedAddressForSeed #1563

Conversation

2501babe commented Sep 2, 2023 • edited by steveluscher Loading

buffalojoec commented Sep 3, 2023

2501babe commented Sep 4, 2023 • edited Loading

buffalojoec left a comment • edited Loading

Choose a reason for hiding this comment

steveluscher commented Sep 4, 2023

steveluscher left a comment • edited Loading

Choose a reason for hiding this comment

steveluscher Sep 5, 2023

Choose a reason for hiding this comment

steveluscher Sep 5, 2023

Choose a reason for hiding this comment

steveluscher Sep 5, 2023

Choose a reason for hiding this comment

steveluscher Sep 6, 2023

Choose a reason for hiding this comment

2501babe commented Sep 7, 2023 • edited Loading

2501babe commented Sep 7, 2023

steveluscher left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 12, 2023

github-actions bot commented Sep 27, 2023

2501babe commented Sep 2, 2023 •

edited by steveluscher

Loading

2501babe commented Sep 4, 2023 •

edited

Loading

buffalojoec left a comment •

edited

Loading

steveluscher left a comment •

edited

Loading

2501babe commented Sep 7, 2023 •

edited

Loading