-
Notifications
You must be signed in to change notification settings - Fork 28
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
Implementing get_size for everything #197
base: main
Are you sure you want to change the base?
Changes from 2 commits
2274735
33288ba
84758e9
5243ba0
d25710d
d8aae30
55f12f6
f35402b
146c593
1e9f3aa
8c3e9d9
e97959b
15b9054
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,32 @@ | ||
use borsh::{BorshDeserialize, BorshSerialize}; | ||
|
||
use crate::plugins::PluginValidation; | ||
use crate::{plugins::PluginValidation, state::DataBlob}; | ||
|
||
/// The master edition plugin allows the creator to specify details on the master edition including max supply, name, and uri. | ||
/// The default authority for this plugin is the creator. | ||
#[repr(C)] | ||
#[derive(Clone, BorshSerialize, BorshDeserialize, Default, Debug, PartialEq, Eq)] | ||
pub struct MasterEdition { | ||
/// The max supply of editions | ||
pub max_supply: Option<u32>, | ||
pub max_supply: Option<u32>, // 1 + optional 4 | ||
/// optional master edition name | ||
pub name: Option<String>, | ||
pub name: Option<String>, // 1 + optional 4 | ||
/// optional master edition uri | ||
pub uri: Option<String>, | ||
pub uri: Option<String>, // 1 + optional 4 | ||
} | ||
|
||
impl PluginValidation for MasterEdition {} | ||
|
||
impl DataBlob for MasterEdition { | ||
fn get_initial_size() -> usize { | ||
1 + 1 + 1 | ||
} | ||
|
||
fn get_size(&self) -> usize { | ||
1 + self.max_supply.map_or(0, |_| 4) | ||
+ 1 | ||
+ self.name.as_ref().map_or(0, |name| 4 + name.len()) | ||
+ 1 | ||
+ self.uri.as_ref().map_or(0, |uri| 4 + uri.len()) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,34 +8,71 @@ use crate::error::MplCoreError; | |
use crate::plugins::{ | ||
abstain, reject, Plugin, PluginValidation, PluginValidationContext, ValidationResult, | ||
}; | ||
use crate::state::DataBlob; | ||
|
||
/// The creator on an asset and whether or not they are verified. | ||
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq)] | ||
pub struct Creator { | ||
address: Pubkey, | ||
percentage: u8, | ||
address: Pubkey, // 32 | ||
percentage: u8, // 1 | ||
} | ||
|
||
impl DataBlob for Creator { | ||
fn get_initial_size() -> usize { | ||
33 | ||
} | ||
|
||
fn get_size(&self) -> usize { | ||
33 | ||
} | ||
} | ||
|
||
/// The rule set for an asset indicating where it is allowed to be transferred. | ||
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq)] | ||
pub enum RuleSet { | ||
/// No rules are enforced. | ||
None, | ||
None, // 1 | ||
/// Allow list of programs that are allowed to transfer, receive, or send the asset. | ||
ProgramAllowList(Vec<Pubkey>), | ||
ProgramAllowList(Vec<Pubkey>), // 4 | ||
/// Deny list of programs that are not allowed to transfer, receive, or send the asset. | ||
ProgramDenyList(Vec<Pubkey>), | ||
ProgramDenyList(Vec<Pubkey>), // 4 | ||
Comment on lines
+37
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: consistency, sometimes the commented length is the initial/default length of zero, other times it is listed as like |
||
} | ||
|
||
impl DataBlob for RuleSet { | ||
fn get_initial_size() -> usize { | ||
1 | ||
} | ||
|
||
fn get_size(&self) -> usize { | ||
1 + match self { | ||
RuleSet::ProgramAllowList(allow_list) => 4 + allow_list.len() * 32, | ||
RuleSet::ProgramDenyList(deny_list) => 4 + deny_list.len() * 32, | ||
RuleSet::None => 0, | ||
} | ||
} | ||
} | ||
|
||
/// Traditional royalties structure for an asset. | ||
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, Eq, PartialEq)] | ||
pub struct Royalties { | ||
/// The percentage of royalties to be paid to the creators. | ||
basis_points: u16, | ||
basis_points: u16, // 2 | ||
/// A list of creators to receive royalties. | ||
creators: Vec<Creator>, | ||
creators: Vec<Creator>, // 4 | ||
/// The rule set for the asset to enforce royalties. | ||
rule_set: RuleSet, | ||
rule_set: RuleSet, // 1 | ||
} | ||
|
||
impl DataBlob for Royalties { | ||
fn get_initial_size() -> usize { | ||
1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be 7, or 2 + 4 + |
||
} | ||
|
||
fn get_size(&self) -> usize { | ||
2 // basis_points | ||
+ (4 + self.creators.len() * Creator::get_initial_size()) // creators | ||
+ self.rule_set.get_size() // rule_set | ||
} | ||
} | ||
|
||
fn validate_royalties(royalties: &Royalties) -> Result<ValidationResult, ProgramError> { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,19 +8,40 @@ use crate::error::MplCoreError; | |
use crate::plugins::{ | ||
abstain, Plugin, PluginValidation, PluginValidationContext, ValidationResult, | ||
}; | ||
use crate::state::DataBlob; | ||
|
||
/// The creator on an asset and whether or not they are verified. | ||
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq, Hash)] | ||
pub struct VerifiedCreatorsSignature { | ||
address: Pubkey, | ||
verified: bool, | ||
address: Pubkey, // 32 | ||
verified: bool, // 1 | ||
} | ||
|
||
impl DataBlob for VerifiedCreatorsSignature { | ||
fn get_initial_size() -> usize { | ||
32 + 1 | ||
} | ||
|
||
fn get_size(&self) -> usize { | ||
32 + 1 | ||
} | ||
} | ||
|
||
/// Structure for storing verified creators, often used in conjunction with the Royalties plugin | ||
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, Eq, PartialEq)] | ||
pub struct VerifiedCreators { | ||
/// A list of signatures | ||
signatures: Vec<VerifiedCreatorsSignature>, | ||
signatures: Vec<VerifiedCreatorsSignature>, // 4 + len * VerifiedCreatorsSignature | ||
} | ||
|
||
impl DataBlob for VerifiedCreators { | ||
fn get_initial_size() -> usize { | ||
4 | ||
} | ||
|
||
fn get_size(&self) -> usize { | ||
4 + self.signatures.len() * VerifiedCreatorsSignature::get_initial_size() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: It happens to be fine since its fixed size but I think technically better to use |
||
} | ||
} | ||
|
||
struct SignatureChangeIndices { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ use crate::{ | |
state::{Authority, Key, UpdateAuthority}, | ||
}; | ||
|
||
use crate::plugins::{ | ||
use super::{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I thought in another PR you switched all the |
||
ExternalPluginAdapter, ExternalPluginAdapterKey, ExternalRegistryRecord, Plugin, PluginType, | ||
RegistryRecord, | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,6 +88,38 @@ impl Plugin { | |
|
||
impl Compressible for Plugin {} | ||
|
||
impl DataBlob for Plugin { | ||
fn get_initial_size() -> usize { | ||
1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this really 1 or since there's no
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's still one byte required for the plugin enum discriminator. |
||
} | ||
|
||
fn get_size(&self) -> usize { | ||
1 + match self { | ||
Plugin::Royalties(royalties) => royalties.get_size(), | ||
Plugin::FreezeDelegate(freeze_delegate) => freeze_delegate.get_size(), | ||
Plugin::BurnDelegate(burn_delegate) => burn_delegate.get_size(), | ||
Plugin::TransferDelegate(transfer_delegate) => transfer_delegate.get_size(), | ||
Plugin::UpdateDelegate(update_delegate) => update_delegate.get_size(), | ||
Plugin::PermanentFreezeDelegate(permanent_freeze_delegate) => { | ||
permanent_freeze_delegate.get_size() | ||
} | ||
Plugin::Attributes(attributes) => attributes.get_size(), | ||
Plugin::PermanentTransferDelegate(permanent_transfer_delegate) => { | ||
permanent_transfer_delegate.get_size() | ||
} | ||
Plugin::PermanentBurnDelegate(permanent_burn_delegate) => { | ||
permanent_burn_delegate.get_size() | ||
} | ||
Plugin::Edition(edition) => edition.get_size(), | ||
Plugin::MasterEdition(master_edition) => master_edition.get_size(), | ||
Plugin::AddBlocker(add_blocker) => add_blocker.get_size(), | ||
Plugin::ImmutableMetadata(immutable_metadata) => immutable_metadata.get_size(), | ||
Plugin::VerifiedCreators(verified_creators) => verified_creators.get_size(), | ||
Plugin::Autograph(autograph) => autograph.get_size(), | ||
} | ||
} | ||
} | ||
|
||
/// List of first party plugin types. | ||
#[repr(C)] | ||
#[derive( | ||
|
@@ -138,11 +170,11 @@ pub enum PluginType { | |
|
||
impl DataBlob for PluginType { | ||
fn get_initial_size() -> usize { | ||
2 | ||
1 | ||
} | ||
|
||
fn get_size(&self) -> usize { | ||
2 | ||
1 | ||
} | ||
} | ||
|
||
|
@@ -198,3 +230,9 @@ pub(crate) struct PluginAuthorityPair { | |
pub(crate) plugin: Plugin, | ||
pub(crate) authority: Option<Authority>, | ||
} | ||
|
||
impl From<PluginType> for usize { | ||
fn from(plugin_type: PluginType) -> Self { | ||
plugin_type as usize | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we normally do this with |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -146,6 +146,16 @@ impl RegistryRecord { | |
} | ||
} | ||
|
||
impl DataBlob for RegistryRecord { | ||
fn get_initial_size() -> usize { | ||
2 + 1 + 8 | ||
} | ||
|
||
fn get_size(&self) -> usize { | ||
2 + self.authority.get_size() + 8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Even for Authority could use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup you're right. |
||
} | ||
} | ||
|
||
/// A type to store the mapping of third party plugin type to third party plugin header and data. | ||
#[repr(C)] | ||
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, Eq, PartialEq)] | ||
|
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.
Nit: This is elegant but I think using map and sum could be more simple, i.e.:
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.
Nice! That's much better and same in CUs.