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

Add seeds::program constraint for PDAs. #1197

Merged
merged 19 commits into from
Jan 11, 2022

Conversation

cqfd
Copy link
Contributor

@cqfd cqfd commented Dec 27, 2021

This addresses #1131

@paul-schaaf I should have double-checked about the desired name for the attribute—happy to change from program_id to something else, just wanted to hurry up and get a PR started.

@@ -182,6 +182,15 @@ pub fn parse_token(stream: ParseStream) -> ParseResult<ConstraintToken> {
};
ConstraintToken::Bump(Context::new(ident.span(), ConstraintTokenBump { bump }))
}
"program_id" => {
Copy link
Member

Choose a reason for hiding this comment

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

I think the other name that the solana sdk uses for this is base. I don't have a preference though.

@armaniferrante
Copy link
Member

Build failing due to clippy warnings.

@paul-schaaf
Copy link
Contributor

paul-schaaf commented Dec 27, 2021

program_seed would be more user-friendly. something that indicates this is used exclusively with seeds

pls also add a line to the changelog

@@ -182,6 +182,15 @@ pub fn parse_token(stream: ParseStream) -> ParseResult<ConstraintToken> {
};
ConstraintToken::Bump(Context::new(ident.span(), ConstraintTokenBump { bump }))
}
"program_seed" => {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any place in the existing solana sdk that uses the term "program_seed"?

Copy link
Member

Choose a reason for hiding this comment

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

Tag @jstarry. Any ideas for an accurate keyword to refer to the base program id for PDAs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah tbh I also find that term a little odd

Copy link
Contributor

@paul-schaaf paul-schaaf Jan 2, 2022

Choose a reason for hiding this comment

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

other words mentioned in discord: seeds::program, program, derive_from, seed_program

Copy link
Member

Choose a reason for hiding this comment

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

Discussed offline. Let's do seeds::program.

CHANGELOG.md Outdated
#### Fixes
### Fixes

### Features

*lang: Improved error msgs when required programs are missing when using the `init` constraint([#1257](https://github.com/project-serum/anchor/pull/1257))
Copy link
Contributor

@paul-schaaf paul-schaaf Jan 7, 2022

Choose a reason for hiding this comment

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

used the opportunity to move this into feature section (wasnt really a bug fix)

@paul-schaaf paul-schaaf marked this pull request as draft January 7, 2022 16:25
@paul-schaaf paul-schaaf marked this pull request as ready for review January 7, 2022 21:57
@paul-schaaf paul-schaaf changed the title Add program_id constraint for PDAs. Add seeds::program constraint for PDAs. Jan 7, 2022
@paul-schaaf
Copy link
Contributor

took over with alan's permission

@paul-schaaf
Copy link
Contributor

closes #1131

/// #[account(
/// seeds = [b"other_seed],
/// bump = second_bump,
/// seeds::program = other_program.key()
Copy link
Member

@armaniferrante armaniferrante Jan 11, 2022

Choose a reason for hiding this comment

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

A bit ugly now that I look at this next to seeds, bump. We'll want to rethink this in the future. Potentially just solved by the constraint grouping work, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

yea once we group them it can just be program cause it will be clear which constraint it belongs to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants