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

Changes to new Update instruction #1058

Merged
merged 45 commits into from
May 1, 2023
Merged

Changes to new Update instruction #1058

merged 45 commits into from
May 1, 2023

Conversation

danenbm
Copy link
Contributor

@danenbm danenbm commented Mar 30, 2023 •
edited
Loading

New change

  • Binary space savings changes
    • Remove InternalUpdateArgs struct and From trait implementation.
    • Deconstruct UpdateArgs where used.
    • Remove redundant authority type and metadata delegate role checks from Metadata::update_v1 because validate_update already checks that the authority is allowed to perform an update, and the UpdateArgs enforces that the authority is only changing fields for which it is authorized.

Notes

  • Add Additional Metadata Delegates
    • Add support in Delegate and Revoke for AuthorityItem, Data, DataItem, CollectionItem, and ProgrammableConfigItem Metadata delegates.
    • Authority Metadata delegate is replaced by AuthorityItem, and Update Metadata delegate is replaced by Data. This is OK because @samuelvanderwaal ran a script and saw no current usage of those delegates (those delegates cannot currently do anything anyways).
  • Changes to Update to support new and changed delegates
    • Modify authority check to separate out item and collection-level delegates.
    • Add several new V2 UpdateArgs variants for update authority and each metadata delegate type, which makes it clear to users what metadata fields can be updated by each authority type. It also makes it trivial to enforce this in the handler.
    • Modify Update handler to update metadata areas based on update authority or delegated authority.

Cargo lock change

During test development Cargo.lock file was updated, had to run cargo update -p mpl-token-metadata@1.10.0 to get the compiler to select a newer version of mpl-utils and succeed overall.

Docs

Here is the related docs PR: metaplex-foundation/docs#425

Enum Compatibility

  • For UpdateArgs I added all new variants to the end of the enum.
  • For MetadataDelegateRole enum I changed the name of Authority to AuthorityItem, Update to Data, and added the new delegates to the end of the enum. This is OK because @samuelvanderwaal ran a script and saw no current usage of those delegates (those delegates cannot currently do anything anyways).
  • For DelegateArgs and RevokeArgs enums I changed the name of UpdateV1 to DataV1, and added new delegates to the end of the enum.

Delegate expiration

  • This is an idea that was explored as part of this PR, but decided to keep this PR focused on new delegates and the Update instruction.
  • Current idea for delegate expiration can be seen in: Adding expiration for delegates #1063

Tests

  • Change unverify test on collection delegate behavior change
  • Added tests for:
    • new delegates passing and failing to update
    • persistent delegate failing to update
    • updating token standard
    • collection-level Collection delegate can update collection for items in the collection.
    • collection-level Collection Item delegate cannot update items in the collection.
    • collection-level Programmable Config delegate cannot update collection for items in the collection by using wrong UpdateArgs.
    • collection-level Programmable Config delegate can update programmable config for items in the collection, using both the V2 UpdateArgs meant for Programmable Config delegate, as well as the V1 UpdateArgs.
      • This was to maintain backwards compatibility since Programmable Config delegate was in place before this PR.
    • collection-level Programmable Config delegate cannot update collection for items in the collection by using collection field in V1 UpdateArgs.
    • collection cannot be updated to verified.
    • old version of lib can be used to delegate and update with new deployed program (even though DelegateArgs enum was modified).

Unrelated change

  • Fix delegate test based on auth rules update.

@danenbm danenbm changed the title New unified Update instruction Update instruction Mar 30, 2023
@danenbm danenbm changed the title Update instruction Changes to new Update instruction Mar 30, 2023
@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/4561889528.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023

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/4598802660.

@danenbm
Additional Metadata Delegates
5f7b7aa 
* Add support in Delegate and Revoke for Authority, Data,
CollectionItem, and ProgrammableConfigItem Metadata
delegates.
* Remove Update Metadata delegate.
@danenbm
Changes to Update to support new and changed delegates
12ce7d7 
* Modify authority check to separate out item and
collection-level delegates.
* Add V2 Update args struct to allow user to specify token
standard.
* Check that new delegates are only changing metadata
for which they are meant to have access.
* Modify Update handler to update metadata fields based
on the delegate type.
@danenbm
Update tests to support new version of UpdateArgs
8d3e5f4 
* Also add macro to help destructure UpdateArgs fields.
@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

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/4610937013.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

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/4611076067.

@danenbm
Put enums back in original order
ce19636 
* Update is still changed to Data but order is preserved.
* Also remove unnecessary Option for token_record in Update.
* Add some comments clarifying authority types in Unverify.
@danenbm
Change unverify test on collection delegate behavior change
5789cfb
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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/4627158045.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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/4627358794.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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/4627569868.

@danenbm
Modify DA test util object to return delegate or token record
71db91a 
* Return the value derived in the delegate method.
* Also add a test for Authority delegate.
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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/4627813314.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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/4627827526.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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/4628014650.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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/4628068920.

@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/4801203150.

@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/4803365090.

@danenbm danenbm mentioned this pull request Apr 26, 2023
Merged
@danenbm
Space savings
b284445 
* Remove InternalUpdateArgs struct and `From` trait
implementation.
* Deconstruct `UpdateArgs` where used.
* Remove redundant authority type and metadata delegate
role checks from `Metadata::update_v1` because `validate_update`
already checks that the authority is allowed to perform an update,
and the `UpdateArgs` enforces that the authority is only changing
fields for which it is authorized.
@danenbm
Allow passing in current token standard
540c093 
Also make match statements in validate_update
more concise.
@danenbm
Clarify comments and reorder account validations
8cf70af
@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/4823533102.

@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/4823555763.

@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/4823697261.

@danenbm
Add more BPF tests
e586ea0 
* Holder authority test.
* Pass with same token standard being passed in.
* Token doesn't match mint test.
* Metadata doesn't match mint test.
* Wrong edition test.
* Some failure cases around collection-level delegates.
lorisleiva
lorisleiva approved these changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@lorisleiva lorisleiva left a comment

Choose a reason for hiding this comment

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

Thanks Michael! My approval vouches only for the client/JS side but I do really like the API that came out of this PR.

I left a little comment out of curiosity.

token-metadata/program/Cargo.toml Show resolved Hide resolved
febo
febo approved these changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@febo febo left a comment

Choose a reason for hiding this comment

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

LGTM

@danenbm danenbm merged commit 587ad02 into master May 1, 2023
@danenbm danenbm deleted the danenbm/new-update branch May 1, 2023 20:02
@danenbm danenbm mentioned this pull request May 1, 2023
Closed
@danenbm danenbm restored the danenbm/new-update branch May 3, 2023 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@febo febo febo approved these changes

@samuelvanderwaal samuelvanderwaal samuelvanderwaal approved these changes

@lorisleiva lorisleiva lorisleiva approved these changes

@blockiosaurus blockiosaurus blockiosaurus approved these changes

Assignees

@febo febo

@samuelvanderwaal samuelvanderwaal

@lorisleiva lorisleiva

@blockiosaurus blockiosaurus

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@danenbm @febo @samuelvanderwaal @lorisleiva @blockiosaurus