Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Rework docs for Pubkey::find_program_address and friends#21528

Merged
mergify[bot] merged 5 commits intosolana-labs:masterfrom
brson:docs-pubkey
Dec 6, 2021
Merged

Rework docs for Pubkey::find_program_address and friends#21528
mergify[bot] merged 5 commits intosolana-labs:masterfrom
brson:docs-pubkey

Conversation

@brson
Copy link
Copy Markdown
Contributor

@brson brson commented Dec 1, 2021

Problem

This is a crucial function to understand, but learning how to use it correctly took me a long time.

Summary of Changes

  • Make find_program_address the first listed of {find,try_find,create}_program_address, as it is the function most callers should use.
  • Add a comprehensive example to find_program_address
  • Rework find_program_address's docs to synthesize its prior docs, create_program_address's, and my own learnings
  • Strongly indicate that create_program_address should not be used.

This adds anyhow and solana-sdk as dev-dependencies to solana-program. The solana-sdk is a weird semi-circular dependency but it works because compiling for tests results in a technically different crate than normal compilation.

Please read these carefully for accuracy, particularly this passage:

As all account addresses accessed by an on-chain Solana program must be explicitly passed to the program, it is typical for the PDAs to be derived in off-chain client programs, avoiding the compute cost of generating the address on-chain. The address may or may not then be verified by re-deriving it on-chain, depending on the requirements of the program.

As well as any instances of language like "commonly", "typically", "conventionally", etc.

@mergify mergify Bot added the community Community contribution label Dec 1, 2021
@mergify mergify Bot requested a review from a team December 1, 2021 02:02
@brson
Copy link
Copy Markdown
Contributor Author

brson commented Dec 1, 2021

CI doesn't like the circular dependency. I'll remove it.

@brson
Copy link
Copy Markdown
Contributor Author

brson commented Dec 1, 2021

Circular dependency removed at the expense of ignoring an example.

@brson
Copy link
Copy Markdown
Contributor Author

brson commented Dec 1, 2021

CI failure looks unrelated

Copy link
Copy Markdown
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me, and your verbiage seems correct. I'll ask @jackcmay to take a pass also.

A few nits, most of which are on text predating your changes, if you would be so kind? (I know commas are the nittiest of nits, but I only included spots I actually got hung up)

Comment thread sdk/program/src/pubkey.rs Outdated
Comment thread sdk/program/src/pubkey.rs Outdated
Comment thread sdk/program/src/pubkey.rs Outdated
Comment thread sdk/program/Cargo.toml Outdated
jackcmay
jackcmay previously approved these changes Dec 1, 2021
Copy link
Copy Markdown
Contributor

@jackcmay jackcmay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm after @CriesofCarrots suggestions, thanks!

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 1, 2021

Codecov Report

Merging #21528 (ec3aba7) into master (393c765) will increase coverage by 0.1%.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           master   #21528     +/-   ##
=========================================
+ Coverage    81.4%    81.6%   +0.1%     
=========================================
  Files         540      505     -35     
  Lines      143840   141768   -2072     
  Branches      296        0    -296     
=========================================
- Hits       117162   115685   -1477     
+ Misses      26577    26083    -494     
+ Partials      101        0    -101     

@mergify mergify Bot dismissed jackcmay’s stale review December 5, 2021 20:01

Pull request has been modified.

@brson
Copy link
Copy Markdown
Contributor Author

brson commented Dec 5, 2021

@CriesofCarrots applied your suggestions, alphabetized the dev-dependencies, and fixed a bogus commit message.

@brson
Copy link
Copy Markdown
Contributor Author

brson commented Dec 5, 2021

CI failure looks unrelated.

Copy link
Copy Markdown
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@CriesofCarrots CriesofCarrots added v1.9 automerge Merge this Pull Request automatically once CI passes and removed v1.8 labels Dec 6, 2021
@mergify mergify Bot merged commit d1c101c into solana-labs:master Dec 6, 2021
mergify Bot pushed a commit that referenced this pull request Dec 6, 2021
* Rework docs for Pubkey::find_program_address and friends

* Remove circular dependency

* Minor tweaks

* Apply suggestions from code review

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>

* Sort solana-program dev-dependencies

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
(cherry picked from commit d1c101c)
mergify Bot added a commit that referenced this pull request Dec 6, 2021
…1637)

* Rework docs for Pubkey::find_program_address and friends

* Remove circular dependency

* Minor tweaks

* Apply suggestions from code review

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>

* Sort solana-program dev-dependencies

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
(cherry picked from commit d1c101c)

Co-authored-by: Brian Anderson <andersrb@gmail.com>
@askibin
Copy link
Copy Markdown
Contributor

askibin commented Dec 7, 2021

why it says this for create_program_address:
"it may unpredictably return an error and should not be used. It exists for backwards compatibility reasons" ?
this is not the case, create_program_address is used extensively to verify a PDA with given seeds and bump

@brson
Copy link
Copy Markdown
Contributor Author

brson commented Dec 8, 2021

why it says this for create_program_address: "it may unpredictably return an error and should not be used. It exists for backwards compatibility reasons" ? this is not the case, create_program_address is used extensively to verify a PDA with given seeds and bump

I was not aware of this use of create_program_address. Thanks for informing me. I'll produce a follow up patch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

automerge Merge this Pull Request automatically once CI passes community Community contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants