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.

Sign up for GitHub

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

Jump to bottom

Adding verify_collection, unverify_collection, and set_and_verify_collection to Bubblegum #687

Merged
merged 12 commits into from
Aug 30, 2022
Merged

Adding verify_collection, unverify_collection, and set_and_verify_collection to Bubblegum #687

merged 12 commits into from
Aug 30, 2022

Conversation

danenbm
Copy link
Contributor

@danenbm danenbm commented Aug 23, 2022 •
edited
Loading

Notes

  • Similar functionality as token-metadata versions of these functions.
  • Uses some existing assertion functions from token-metadata.
  • Adding new BubblegumError variants.
  • Add code to prevent minting in Bubblegum with collection verified set to true.
  • Regenerating APIs.
  • Make use of same collection authority delegate system as used in token-metadata. In fact it calls assert_has_collection_authority() from token-metadata to verify.
  • For set_and_verify_collection() require that either the tree authority signed this transaction, or the tree authority is the collection update authority.
  • Add payer to both creator and collection verification instructions.

Discussion items

  1. Discuss if we want to allow minting with collection verified.
    a. At first I thought we did but it looks like this is currently not allowed in existing token-metadata program.
    b. Also it will require adding additional accounts to mint_v1() which does not seem like what we want right now.
    c. A solution that might work well is in a follow-on PR add a separate mint_to_collection_v1() instruction that takes all the required collection accounts.
    d. Alternatively we could just say for minting to a collection you just have to do mint_v1() followed by verify_collection(). To be honest in this case we don't even need set_and_verify_collection() because we would just set it as unverified in mint_v1().
  2. This one is more of an FYI on a variance from token-metadata. For set_and_verify_collection(), this PR requires that either the tree authority signed this transaction, or the tree authority is the collection update authority which means the leaf update is approved via proxy, when we later call assert_has_collection_authority().
    a. This is similar to logic in token-metadata for set_and_verify_collection() except this logic also allows the tree authority (which we are treating as the leaf metadata authority) to be different than the collection authority (actual or delegated). The token-metadata program required them to be the same.

@danenbm danenbm changed the title Danenbm/bubblegum verify collection Adding verify_collection and unverify_collection to Bubblegum Aug 23, 2022
@danenbm danenbm marked this pull request as ready for review August 23, 2022 17:04
@danenbm danenbm requested a review from a team as a code owner August 23, 2022 17:04
austbot
austbot suggested changes Aug 23, 2022
View reviewed changes
bubblegum/program/src/lib.rs Show resolved Hide resolved
bubblegum/program/src/lib.rs Outdated Show resolved Hide resolved
bubblegum/program/src/lib.rs Outdated Show resolved Hide resolved
bubblegum/program/src/lib.rs Outdated Show resolved Hide resolved
bubblegum/program/src/lib.rs Outdated Show resolved Hide resolved
@danenbm danenbm force-pushed the danenbm/bubblegum-verify-collection branch 2 times, most recently from caff926 to 80a2a90 Compare August 24, 2022 15:27
@github-actions
Copy link
Contributor

Workflow Verify package library found differences when running yarn api:gen in the JS lib for bubblegum. Please see the job for more details: https://github.com/metaplex-foundation/metaplex-program-library/actions/runs/2921325909.

@danenbm danenbm marked this pull request as draft August 25, 2022 08:07
danenbm added 6 commits August 25, 2022 12:51
@danenbm
Adding verify_collection and unverify_collection to Bubblegum
438e34b 
- Same functionality as token-metadata versions of these functions.
- Uses some existing assertion functions from token-metadata.
- Adding new `BubblegumError` variants.
- Add code to prevent minting with collection `verified` set to true.
@danenbm
API update
edafb81
@danenbm
Check incoming metadata hash in creator and collector verification fu…
681acca 
…nctions.

- Verify that user-provided metadata results in same hash
as user-provided `data_hash`.
- This is done to ensure that creator and collection
verification functions only modify the verified flag in the
metadata and do not pick up other extraneous changes.
- This was already being done with the creator hash in
creator verification.
- Also made change to use helper functions to do creator
and data hashing in one place.
@danenbm
Add support for sized collections in Bubblegum collection verification
3995229 
- Add instruction to token-metadata that allows for a Bubblegum PDA
to modify the size of a sized collection.
- Add `bubblegum_program_authority` PDA in Bubblegum
`CollectionVerification` struct to be used as a signing PDA.
- Add CPI call in Bubblegum to modify size of a sized collection.
- Modify `assert_collection_verify_is_valid` in token-metadata
to make it more usable with Bubblegum and simplify usage in Bubblegum.
@danenbm
Regenerating API for token-metadata
ef6582d
@danenbm
Regenerating API for Bubblegum
634f233
@danenbm danenbm force-pushed the danenbm/bubblegum-verify-collection branch from ac72a35 to 634f233 Compare August 26, 2022 17:51
@danenbm danenbm mentioned this pull request Aug 26, 2022
Closed
@danenbm danenbm marked this pull request as ready for review August 26, 2022 17:55
@danenbm
Copy link
Contributor Author

danenbm commented Aug 26, 2022

Workflow Verify package library found differences when running yarn api:gen in the JS lib for bubblegum. Please see the job for more details: https://github.com/metaplex-foundation/metaplex-program-library/actions/runs/2921325909.

Reran API gen for token-metadata and bubblegum and picked up the changes.

@danenbm danenbm requested a review from austbot August 26, 2022 18:32
@github-actions
Copy link
Contributor

Workflow Verify package library found differences when running yarn api:gen in the JS lib for bubblegum. Please see the job for more details: https://github.com/metaplex-foundation/metaplex-program-library/actions/runs/2936607673.

@danenbm danenbm changed the title Adding verify_collection and unverify_collection to Bubblegum Adding verify_collection, unverify_collection, and set_and_verify_collection to Bubblegum Aug 27, 2022
@danenbm
Adding set_and_verify_collection instruction to Bubblegum
8d0ac5b 
- Require that either the tree authority signed this transaction,
or the tree authority is the collection update authority.
- Add `payer` to both creator and collection verification
instructions.
- Regenerating API.
- Renaming `bubblegum_program_authority` to `bubblegum_signer`
and adding a PDA seed.
- Some additional minor changes to existing logic.
@danenbm danenbm force-pushed the danenbm/bubblegum-verify-collection branch from 16a6df4 to 8d0ac5b Compare August 29, 2022 18:11
samuelvanderwaal
samuelvanderwaal suggested changes Aug 29, 2022
View reviewed changes
bubblegum/program/Cargo.toml Outdated Show resolved Hide resolved
token-metadata/program/src/instruction.rs Outdated Show resolved Hide resolved
token-metadata/program/src/processor.rs Outdated Show resolved Hide resolved
@danenbm danenbm force-pushed the danenbm/bubblegum-verify-collection branch from 00825fc to 55238e8 Compare August 29, 2022 22:10
@danenbm
Move BubblegumSetCollectionSize to end of instruction vector
f251483 
- Also fix BubblegumSetCollectionSize instruction processor to call
`bubblegum_set_collection_size()`.
- Also regenerate API.
@danenbm danenbm force-pushed the danenbm/bubblegum-verify-collection branch from 55238e8 to f251483 Compare August 30, 2022 17:10
@danenbm danenbm force-pushed the danenbm/bubblegum-verify-collection branch from 5bbb35d to da22b07 Compare August 30, 2022 19:59
@github-actions
Copy link
Contributor

Workflow Verify package library found differences when running yarn api:gen in the JS lib for bubblegum. Please see the job for more details: https://github.com/metaplex-foundation/metaplex-program-library/actions/runs/2959146367.

@github-actions
Copy link
Contributor

Workflow Verify package library found differences when running yarn api:gen in the JS lib for token-metadata. Please see the job for more details: https://github.com/metaplex-foundation/metaplex-program-library/actions/runs/2959164987.

@danenbm
Copy link
Contributor Author

danenbm commented Aug 30, 2022

Workflow Verify package library found differences when running yarn api:gen in the JS lib for token-metadata. Please see the job for more details: https://github.com/metaplex-foundation/metaplex-program-library/actions/runs/2959164987.

I have regenerated api for token-metadata

@danenbm
Copy link
Contributor Author

danenbm commented Aug 30, 2022

Workflow Verify package library found differences when running yarn api:gen in the JS lib for bubblegum. Please see the job for more details: https://github.com/metaplex-foundation/metaplex-program-library/actions/runs/2959146367.

I have regenerated api for bubblegum

austbot
austbot approved these changes Aug 30, 2022
View reviewed changes
bubblegum/program/src/lib.rs Show resolved Hide resolved
bubblegum/program/src/lib.rs Show resolved Hide resolved
@austbot austbot merged commit fedbdea into master Aug 30, 2022
@stegaBOB stegaBOB deleted the danenbm/bubblegum-verify-collection branch September 12, 2022 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austbot austbot austbot approved these changes

@samuelvanderwaal samuelvanderwaal samuelvanderwaal approved these changes

@alexandruag alexandruag Awaiting requested review from alexandruag

Assignees

@samuelvanderwaal samuelvanderwaal

@austbot austbot

@alexandruag alexandruag

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danenbm @samuelvanderwaal @austbot @alexandruag