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

Feature - grouping delegate #52

Merged
merged 4 commits into from
Apr 10, 2024
Merged

Conversation

joefitter
Copy link
Collaborator

This PR adds an optional delegate field to a grouping extension. The delegate is authorised to mint new assets into the group.

The main use case for this change is a program controlling either a mint (candy machine) or a conversion from another asset type.

  • Added delegate field to grouping extension
  • Updated group processor to check grouping delegate if present
  • Updated create instruction to accept an optional grouping_delegate signer

Todo: Build changes into interface program - this isn't rebuilt when running programs:build, so the generation never runs and tests fail

Also perhaps worth considering not reinitialising the data when the max_size is written as there are likely cases where this may need to be updated post creation.

This PR adds an optional delegate field to a grouping extension. The delegate is authorised to mint new assets into the group.

The main use case for this change is a program controlling either a mint (candy machine) or a conversion from another asset type.

* Added delegate field to grouping extension
* Updated group processor to check grouping delegate if present
* Updated create instruction to accept an optional grouping_delegate signer

Todo: Build changes into interface program - this isn't rebuilt when running programs:build

Also perhaps worth considering not reinitialising the data when the max_size is written as there are likely cases where this may need to be updated post creation.
@joefitter joefitter mentioned this pull request Apr 9, 2024
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.

Good stuff! There are small little things on the logic for using the delegate. We should also do the same logic on the ungroup instruction, i.e., allow the grouping delegate to ungroup an asset from a group.

@febo
Copy link
Contributor

febo commented Apr 10, 2024

You can also run pnpm format:fix on ./clients/js/asset folder to fix the lint error.

…authorise changes to the collection is there is a delegate present.

* Replicated logic in ungroup processor to ensure delegate can also ungroup assets
@joefitter
Copy link
Collaborator Author

Thanks for your comments! All addressed pending rebuilding the interface program

@joefitter joefitter changed the title WIP - Feature - grouping delegate Feature - grouping delegate Apr 10, 2024
@joefitter joefitter requested a review from febo April 10, 2024 10:58
@febo
Copy link
Contributor

febo commented Apr 10, 2024

Thanks for your comments! All addressed pending rebuilding the interface program

Amazing! I will rebuild the interface on a separate PR after we merge this one.

Can you please run the lint on the JS client as well?

pnpm lint:fix

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.

Looks great, thank you!

CI is failing because the interface is not updated – I will do it on a separate PR.

@febo febo merged commit ec4a3dd into nifty-oss:main Apr 10, 2024
10 of 16 checks passed
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.

2 participants