-
Notifications
You must be signed in to change notification settings - Fork 32
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
chore(enum helpers): use kinobi generated enum helpers #3
Conversation
__kind: 'Freeze', | ||
fields: [{ frozen: false }], | ||
}, | ||
plugin: plugin('Freeze', [{ frozen: false }]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a way to make this more like this?
plugin('Freeze', { frozen: false })
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plugin()
is a Kinobi-generated data enum helper. Currently, it's used like that. Loris has said, that this task is on his todo list and it will look like you describe in newer versions of Kinobi. For now, I can only suggest custom helpers.
offset: BigInt(117), | ||
authorities: [ | ||
ownerAuthority(), | ||
pubkeyAuthority(delegateAddress.publicKey), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are nice for when you need to send an instruction however see the comment regarding deserialization
plugins: [ | ||
{ | ||
formPluginWithAuthorities({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so i think the bigger issue is on the deserialization side. The internal structure of an Asset
should not have all this weird { __kind: } stuff in it.
when someone inspects an asset, they should ideally be able to easily see the properties of an asset.
I can imagine the deserialized structure to be something like:
{
... top level asset fields like name/uri etc.
pluginHeader: {...}
plugins: {
Freeze: {
authorities: [{
type: 'Owner'
}, {
type: 'Pubkey',
address: <pubkey>
}],
offset: BigInt,
data: {
frozen: false
}
}
}
// delete plugin registry
}
maybe we don't need the data
container if it is guaranteed that a plugin's fields will never overlapped with the reserved fields of authorities and offset.
i'm curious to hear what @blockiosaurus @danenbm (and probably Loris too) think about this too since there could be a counter argument that these structures should more closely mirror their on chain format.
IIRC token metadata did some transformation into a DigitalAsset
struct from the token accounts + metadata + master edition. It looks hand rolled rather than generated though =/
fields: [{ frozen: false }], | ||
}, | ||
authorities: [authority('Owner')], | ||
plugin: plugin('Freeze', [{ frozen: false }]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we able to make it plugin('Freeze', { frozen: false })
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plugin() is a Kinobi-generated data enum helper. Currently, it's used like that. Loris has said, that this task is on his todo list and it will look like you describe in newer versions of Kinobi. For now, I can only suggest custom helpers.
…th-check Security Fix: Update Delegate and Default update_plugin approvals
No description provided.