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

Implementing get_size for everything #197

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,27 @@ use borsh::{BorshDeserialize, BorshSerialize};
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq, Default)]
pub struct Attribute {
/// The Key of the attribute.
pub key: String, // 4
pub key: String, // 4 + len
/// The Value of the attribute.
pub value: String, // 4
pub value: String, // 4 + len
}

impl DataBlob for Attribute {
fn get_initial_size() -> usize {
4 + 4
}

fn get_size(&self) -> usize {
4 + self.key.len() + 4 + self.value.len()
}
}

/// The Attributes plugin allows the authority to add arbitrary Key-Value pairs to the asset.
#[repr(C)]
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq, Default)]
pub struct Attributes {
/// A vector of Key-Value pairs.
pub attribute_list: Vec<Attribute>, // 4
pub attribute_list: Vec<Attribute>, // 4 + len * Attribute
}

impl Attributes {
Expand All @@ -32,7 +42,10 @@ impl DataBlob for Attributes {
}

fn get_size(&self) -> usize {
4 // TODO: Implement this.
4 + self
.attribute_list
.iter()
.fold(0, |acc, attr| acc + attr.get_size())
Copy link
Contributor

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.:

        self
        .attribute_list
        .map(|attr| attr.get_size())
        .sum::<usize>()

Copy link
Contributor Author

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.

}
}

Expand Down
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
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 4 + len * Object

}

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be 7, or 2 + 4 + RuleSet::get_initial_size()?

}

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> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::plugins::{
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq)]
pub struct UpdateDelegate {
/// Additional update delegates. Not currently available to be used.
pub additional_delegates: Vec<Pubkey>, // 4
pub additional_delegates: Vec<Pubkey>, // 4 + len * 32
}

impl UpdateDelegate {
Expand All @@ -39,11 +39,11 @@ impl Default for UpdateDelegate {

impl DataBlob for UpdateDelegate {
fn get_initial_size() -> usize {
0
4
}

fn get_size(&self) -> usize {
0
4 + self.additional_delegates.len() * 32
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The 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 get_size() here rhather than get_initial_size().

}
}

struct SignatureChangeIndices {
Expand Down
30 changes: 27 additions & 3 deletions programs/mpl-core/src/plugins/internal/owner_managed/autograph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,31 @@ use crate::{
plugins::{
abstain, approve, Plugin, PluginValidation, PluginValidationContext, ValidationResult,
},
state::DataBlob,
};

/// The creator on an asset and whether or not they are verified.
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq, Hash)]
pub struct AutographSignature {
address: Pubkey,
message: String,
address: Pubkey, // 32
message: String, // 4 + len
}

impl DataBlob for AutographSignature {
fn get_initial_size() -> usize {
32 + 4
}

fn get_size(&self) -> usize {
32 + 4 + self.message.len()
}
}

/// Structure for an autograph book, often used in conjunction with the Royalties plugin for verified creators
#[derive(Clone, BorshSerialize, BorshDeserialize, Debug, Eq, PartialEq)]
pub struct Autograph {
/// A list of signatures with option message
signatures: Vec<AutographSignature>,
signatures: Vec<AutographSignature>, // 4 + len * Autograph len
}

fn validate_autograph(
Expand Down Expand Up @@ -122,3 +133,16 @@ impl PluginValidation for Autograph {
}
}
}

impl DataBlob for Autograph {
fn get_initial_size() -> usize {
4
}

fn get_size(&self) -> usize {
4 + self
.signatures
.iter()
.fold(0, |acc, sig| acc + sig.get_size())
}
}
17 changes: 15 additions & 2 deletions programs/mpl-core/src/plugins/internal/permanent/edition.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use borsh::{BorshDeserialize, BorshSerialize};
use solana_program::program_error::ProgramError;

use crate::plugins::{
abstain, reject, Plugin, PluginValidation, PluginValidationContext, ValidationResult,
use crate::{
plugins::{
abstain, reject, Plugin, PluginValidation, PluginValidationContext, ValidationResult,
},
state::DataBlob,
};

/// The edition plugin allows the creator to set an edition number on the asset
Expand Down Expand Up @@ -43,3 +46,13 @@ impl PluginValidation for Edition {
}
}
}

impl DataBlob for Edition {
fn get_initial_size() -> usize {
4
}

fn get_size(&self) -> usize {
4
}
}
2 changes: 1 addition & 1 deletion programs/mpl-core/src/plugins/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
state::{Authority, Key, UpdateAuthority},
};

use crate::plugins::{
use super::{
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I thought in another PR you switched all the super to crate?

ExternalPluginAdapter, ExternalPluginAdapterKey, ExternalRegistryRecord, Plugin, PluginType,
RegistryRecord,
};
Expand Down
42 changes: 40 additions & 2 deletions programs/mpl-core/src/plugins/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,38 @@ impl Plugin {

impl Compressible for Plugin {}

impl DataBlob for Plugin {
fn get_initial_size() -> usize {
1
Copy link
Contributor

Choose a reason for hiding this comment

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

is this really 1 or since there's no None, is it each plugin's initial size?

1 + match self {
    Plugin::Royalties(royalties) => royalties.get_initial_size()
    ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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(
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we normally do this with #[derive(ToPrimitive)]

10 changes: 10 additions & 0 deletions programs/mpl-core/src/plugins/plugin_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

PluginType should be 1? But actually isn't it better to use PluginType::get_initial_size() and get_size()?

Even for Authority could use self.authority.get_initial_size() as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)]
Expand Down
Loading