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

lang: allow token::... and mint::... to be used as checks without init #1505

Merged
merged 11 commits into from
Apr 11, 2022
Merged

lang: allow token::... and mint::... to be used as checks without init #1505

merged 11 commits into from
Apr 11, 2022

Conversation

0xquantech
Copy link
Contributor

@0xquantech 0xquantech commented Feb 22, 2022

  • I added token_account and mint in ConstraintGroup.
pub struct ConstraintGroup {
    init: Option<ConstraintInitGroup>,
    zeroed: Option<ConstraintZeroed>,
... ... ...
    associated_token: Option<ConstraintAssociatedToken>,
// newly added
    token_account: Option<ConstraintTokenAccountGroup>,
    mint: Option<ConstraintTokenMintGroup>,
}
#[derive(Debug, Clone)]
pub struct ConstraintTokenAccountGroup {
    pub mint: Option<Expr>,
    pub authority: Option<Expr>,
}

#[derive(Debug, Clone)]
pub struct ConstraintTokenMintGroup {
    pub decimals: Option<Expr>,
    pub mint_authority: Option<Expr>,
    pub freeze_authority: Option<Expr>,
}
  • Made some unit tests for test of token constraints in misc
    i.e. for mint::decimals and mint::freeze_authority test
#[derive(Accounts)]
#[instruction(_decimals: u8)]
pub struct TestMintMissMintAuthConstraint<'info> {
    #[account(
        mint::decimals = _decimals,
        mint::freeze_authority = freeze_authority,
    )]
    pub mint: Account<'info, Mint>,
    #[account(mut)]
    pub payer: Signer<'info>,
    pub rent: Sysvar<'info, Rent>,
    pub system_program: AccountInfo<'info>,
    pub token_program: AccountInfo<'info>,
    pub freeze_authority: AccountInfo<'info>,
}

@0xquantech 0xquantech changed the title Add TokenConstraint feature and Constraint unit test lang: allow token::.. and mint::... to be used as checks without init Feb 27, 2022
@0xquantech 0xquantech changed the title lang: allow token::.. and mint::... to be used as checks without init lang: allow token::.. and mint::... to be used as checks without init Feb 27, 2022
@0xquantech 0xquantech changed the title lang: allow token::.. and mint::... to be used as checks without init lang: allow token::.. and mint::... to be used as checks without init Feb 27, 2022
@0xquantech 0xquantech changed the title lang: allow token::.. and mint::... to be used as checks without init lang: allow token::.. and mint::... to be used as checks without init Feb 27, 2022
@0xquantech 0xquantech changed the title lang: allow token::.. and mint::... to be used as checks without init lang: allow *token::..* and mint::... to be used as checks without init Feb 27, 2022
@0xquantech 0xquantech changed the title lang: allow *token::..* and mint::... to be used as checks without init lang: allow token::.. and mint::... to be used as checks without init Feb 27, 2022
@0xquantech 0xquantech changed the title lang: allow token::.. and mint::... to be used as checks without init lang: allow token::.. and mint::... to be used as checks without init Feb 27, 2022
@0xquantech 0xquantech changed the title lang: allow token::.. and mint::... to be used as checks without init lang: allow token::.. and mint::... to be used as checks without init Feb 27, 2022
@0xquantech 0xquantech changed the title lang: allow token::.. and mint::... to be used as checks without init lang: allow token::... and mint::... to be used as checks without init Feb 27, 2022
@0xquantech
Copy link
Contributor Author

Hi, @paul-schaaf, Can you review this PR?

@0xquantech
Copy link
Contributor Author

0xquantech commented Mar 5, 2022

Hi, @fanatid, Can you review my PR?
I want to jump to #issue 695, but I am stuck here.

tests/misc/programs/misc/src/context.rs Outdated Show resolved Hide resolved
lang/syn/src/lib.rs Outdated Show resolved Hide resolved
lang/syn/src/lib.rs Outdated Show resolved Hide resolved
lang/syn/src/parser/accounts/constraints.rs Outdated Show resolved Hide resolved
lang/syn/src/parser/accounts/constraints.rs Show resolved Hide resolved
tests/misc/tests/misc.js Outdated Show resolved Hide resolved
tests/misc/tests/misc.js Outdated Show resolved Hide resolved
tests/misc/tests/misc.js Outdated Show resolved Hide resolved
lang/syn/src/parser/accounts/constraints.rs Outdated Show resolved Hide resolved
lang/syn/src/parser/accounts/constraints.rs Show resolved Hide resolved
@paul-schaaf
Copy link
Contributor

the review dismissal was a missclick. pls do have a look at my comments :)

@paul-schaaf
Copy link
Contributor

@blockchainlover2019 do you have time to finish this or do you want me to take over?

@0xquantech
Copy link
Contributor Author

Sorry, @paul-schaaf , I have been very busy for MVP launch.
I will finish modification this week.

lang/syn/src/codegen/accounts/constraints.rs Outdated Show resolved Hide resolved
lang/syn/src/parser/accounts/constraints.rs Show resolved Hide resolved
tests/misc/tests/misc.ts Outdated Show resolved Hide resolved
lang/syn/src/parser/accounts/constraints.rs Show resolved Hide resolved
@0xquantech
Copy link
Contributor Author

@paul-schaaf, I've finished modifying codes. I want to know your opinions. Please don't hesitate to tell me about the modification and other improvements. I feel good at working with you

signers: [token],
});
try {
await program.rpc.testTokenConstraint({
Copy link
Member

Choose a reason for hiding this comment

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

we should stop using this api with new tests

Copy link
Member

Choose a reason for hiding this comment

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

though for now let's not worry about it and just get this in

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, got it.
I think there should be a unit test framework to manage tests on Anchor framework.
misc.ts is so long in its codes.

Copy link
Contributor

Choose a reason for hiding this comment

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

yea we have issues to split up the misc tests and generally rework the testing for anchor itself

@paul-schaaf
Copy link
Contributor

hey @blockchainlover2019 we want to release today and would like to include this PR so Im going to finish it now

@0xquantech
Copy link
Contributor Author

0xquantech commented Apr 11, 2022

hey @blockchainlover2019 we want to release today and would like to include this PR so Im going to finish it now

Okay!

Comment on lines +615 to +617
freeze_authority: mint_freeze_authority
.as_ref()
.map(|a| a.clone().into_inner().mint_freeze_auth),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
freeze_authority: mint_freeze_authority
.as_ref()
.map(|a| a.clone().into_inner().mint_freeze_auth),
freeze_authority: mint_freeze_authority.cloned().map(|a| a.into_inner().mint_freeze_auth),

Can be simpler 🙂
But current also looks good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

Copy link
Contributor

@paul-schaaf paul-schaaf Apr 11, 2022

Choose a reason for hiding this comment

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

that doesnt actually work because cloned only exists on Option<&T> (and Option<&mut T>) but mint_freeze_authority is an Option<T>. could do clone().map() but I like my solution more than that

@paul-schaaf paul-schaaf merged commit f5dffe6 into coral-xyz:master Apr 11, 2022
@paul-schaaf
Copy link
Contributor

thanks for the contribution @blockchainlover2019 !

@0xquantech
Copy link
Contributor Author

0xquantech commented Apr 12, 2022

I feel happy to contribute, and in the opposite, I feel bad about not finishing work quickly and wonderfully.
I started learning solana from your blog and learning a lot from you guys - @paul-schaaf, @armaniferrante, @fanatid.
Thank you guys!

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