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

Moving ME checks to the plugin. #222

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
language: "en-US"
early_access: false
reviews:
profile: "chill"
request_changes_workflow: false
high_level_summary: true
poem: true
review_status: true
collapse_walkthrough: false
path_instructions:
[
{
path: "programs/mpl-core/src/processor/*",
instructions: "Make sure the owner is checked on all accounts somehow",
},
{
path: "programs/mpl-core/*",
instructions: "Carefully consider performance implications and make sure there are no performance inefficiencies",
},
{
path: "**/*.rs",
instructions: "Make duplicated logic between the rust client and program are consistent",
},
]
auto_review:
enabled: true
drafts: false
chat:
auto_reply: true
13 changes: 13 additions & 0 deletions clients/js/src/generated/errors/mplCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,19 @@ export class InvalidExecutePdaError extends ProgramError {
codeToErrorMap.set(0x31, InvalidExecutePdaError);
nameToErrorMap.set('InvalidExecutePda', InvalidExecutePdaError);

/** PluginNotAllowedOnAsset: Plugin is not allowed to be added to an Asset */
export class PluginNotAllowedOnAssetError extends ProgramError {
override readonly name: string = 'PluginNotAllowedOnAsset';

readonly code: number = 0x32; // 50

constructor(program: Program, cause?: Error) {
super('Plugin is not allowed to be added to an Asset', program, cause);
}
}
codeToErrorMap.set(0x32, PluginNotAllowedOnAssetError);
nameToErrorMap.set('PluginNotAllowedOnAsset', PluginNotAllowedOnAssetError);

/**
* Attempts to resolve a custom program error from the provided error code.
* @category Errors
Expand Down
4 changes: 2 additions & 2 deletions clients/js/test/plugins/collection/masterEdition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ test('it cannot add masterEdition to asset', async (t) => {
}).sendAndConfirm(umi);

await t.throwsAsync(result, {
name: 'InvalidPlugin',
name: 'PluginNotAllowedOnAsset',
});
});

Expand All @@ -150,6 +150,6 @@ test('it cannot create asset with masterEdition', async (t) => {
});

await t.throwsAsync(result, {
name: 'InvalidPlugin',
name: 'PluginNotAllowedOnAsset',
});
});
3 changes: 3 additions & 0 deletions clients/rust/src/generated/errors/mpl_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ pub enum MplCoreError {
/// 49 (0x31) - Invalid Signing PDA for Asset or Collection Execute
#[error("Invalid Signing PDA for Asset or Collection Execute")]
InvalidExecutePda,
/// 50 (0x32) - Plugin is not allowed to be added to an Asset
#[error("Plugin is not allowed to be added to an Asset")]
PluginNotAllowedOnAsset,
}

impl solana_program::program_error::PrintProgramError for MplCoreError {
Expand Down
5 changes: 5 additions & 0 deletions idls/mpl_core.json
Original file line number Diff line number Diff line change
Expand Up @@ -4949,6 +4949,11 @@
"code": 49,
"name": "InvalidExecutePda",
"msg": "Invalid Signing PDA for Asset or Collection Execute"
},
{
"code": 50,
"name": "PluginNotAllowedOnAsset",
"msg": "Plugin is not allowed to be added to an Asset"
}
],
"metadata": {
Expand Down
4 changes: 4 additions & 0 deletions programs/mpl-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,10 @@ pub enum MplCoreError {
/// 49 - Invalid Signing PDA for Asset or Collection Execute
#[error("Invalid Signing PDA for Asset or Collection Execute")]
InvalidExecutePda,

/// 50 - Plugin is not allowed to be added to an Asset
#[error("Plugin is not allowed to be added to an Asset")]
PluginNotAllowedOnAsset,
}

impl PrintProgramError for MplCoreError {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
use borsh::{BorshDeserialize, BorshSerialize};
use solana_program::program_error::ProgramError;

use crate::{plugins::PluginValidation, state::DataBlob};
use crate::{
error::MplCoreError,
plugins::{abstain, PluginType, PluginValidation, PluginValidationContext, ValidationResult},
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.
Expand All @@ -21,7 +26,40 @@ impl MasterEdition {
+ 1; // The uri option
}

impl PluginValidation for MasterEdition {}
impl PluginValidation for MasterEdition {
fn validate_create(
&self,
ctx: &PluginValidationContext,
) -> Result<ValidationResult, ProgramError> {
// Target plugin doesn't need to be populated for create, so we check if it exists, otherwise we pass.
if let Some(target_plugin) = ctx.target_plugin {
// You can't create the master edition plugin on an asset.
if PluginType::from(target_plugin) == PluginType::MasterEdition
&& ctx.asset_info.is_some()
{
return Err(MplCoreError::PluginNotAllowedOnAsset.into());
}
}

abstain!()
}

fn validate_add_plugin(
&self,
ctx: &PluginValidationContext,
) -> Result<ValidationResult, ProgramError> {
// Target plugin must be populated for add_plugin.
let target_plugin = ctx.target_plugin.ok_or(MplCoreError::InvalidPlugin)?;

// You can't add the master edition plugin to an asset.
if PluginType::from(target_plugin) == PluginType::MasterEdition && ctx.asset_info.is_some()
{
Err(MplCoreError::PluginNotAllowedOnAsset.into())
} else {
Ok(ValidationResult::Pass)
}
}
}

impl DataBlob for MasterEdition {
fn len(&self) -> usize {
Expand Down
2 changes: 2 additions & 0 deletions programs/mpl-core/src/plugins/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ impl PluginType {
PluginType::Edition => CheckResult::CanReject,
PluginType::Autograph => CheckResult::CanReject,
PluginType::VerifiedCreators => CheckResult::CanReject,
PluginType::MasterEdition => CheckResult::CanReject,
_ => CheckResult::None,
}
}
Expand Down Expand Up @@ -142,6 +143,7 @@ impl PluginType {
PluginType::UpdateDelegate => CheckResult::CanApprove,
PluginType::Autograph => CheckResult::CanReject,
PluginType::VerifiedCreators => CheckResult::CanReject,
PluginType::MasterEdition => CheckResult::CanReject,
_ => CheckResult::None,
}
}
Expand Down
6 changes: 0 additions & 6 deletions programs/mpl-core/src/processor/add_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,6 @@ pub(crate) fn add_plugin<'a>(
return Err(MplCoreError::NotAvailable.into());
}

// TODO move into plugin validation when asset/collection is part of validation context
let plugin_type = PluginType::from(&args.plugin);
if plugin_type == PluginType::MasterEdition {
return Err(MplCoreError::InvalidPlugin.into());
}

let target_plugin_authority = args.init_authority.unwrap_or(args.plugin.manager());

// TODO: Seed with Rejected
Expand Down
7 changes: 1 addition & 6 deletions programs/mpl-core/src/processor/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,6 @@ pub(crate) fn process_create<'a>(
ctx.accounts.system_program,
)?;
for plugin in &plugins {
// TODO move into plugin validation when asset/collection is part of validation context
let plugin_type = PluginType::from(&plugin.plugin);
if plugin_type == PluginType::MasterEdition {
return Err(MplCoreError::InvalidPlugin.into());
}
if PluginType::check_create(&PluginType::from(&plugin.plugin))
!= CheckResult::None
{
Expand All @@ -201,7 +196,7 @@ pub(crate) fn process_create<'a>(
new_owner: None,
new_asset_authority: None,
new_collection_authority: None,
target_plugin: None,
target_plugin: Some(&plugin.plugin),
target_plugin_authority: None,
target_external_plugin: None,
target_external_plugin_authority: None,
Expand Down