From 5fbd0fa38d836d393a49cc89c977e3ffadcee670 Mon Sep 17 00:00:00 2001 From: blockiosaurus Date: Mon, 29 Apr 2024 08:57:55 -0400 Subject: [PATCH 1/2] Adding stricter allowlist checks. --- clients/js/test/burn.test.ts | 15 ++++++++ .../js/test/plugins/asset/royalties.test.ts | 35 +++++-------------- programs/mpl-core/src/plugins/royalties.rs | 2 +- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/clients/js/test/burn.test.ts b/clients/js/test/burn.test.ts index 4ddad4d9..d4bbbd8f 100644 --- a/clients/js/test/burn.test.ts +++ b/clients/js/test/burn.test.ts @@ -240,3 +240,18 @@ test('it cannot use an invalid noop program for collections', async (t) => { await t.throwsAsync(result, { name: 'InvalidLogWrapperProgram' }); }); + +test('it cannot burn an asset with the wrong collection specified', async (t) => { + // Given a Umi instance and a new signer. + const umi = await createUmi(); + + const asset = await createAsset(umi); + const wrongCollection = await createCollection(umi); + + const result = burnV1(umi, { + asset: asset.publicKey, + collection: wrongCollection.publicKey, + }).sendAndConfirm(umi); + + await t.throwsAsync(result, { name: 'InvalidCollection' }); +}); \ No newline at end of file diff --git a/clients/js/test/plugins/asset/royalties.test.ts b/clients/js/test/plugins/asset/royalties.test.ts index 1b7cb0b7..7cdbd1bf 100644 --- a/clients/js/test/plugins/asset/royalties.test.ts +++ b/clients/js/test/plugins/asset/royalties.test.ts @@ -137,7 +137,7 @@ test('it can transfer an asset with royalties to an allowlisted program address' data: { basisPoints: 5, creators: [{ address: umi.identity.publicKey, percentage: 100 }], - ruleSet: ruleSet('ProgramAllowList', [[MPL_CORE_PROGRAM_ID]]), + ruleSet: ruleSet('ProgramAllowList', [[SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID]]), }, }), ], @@ -158,7 +158,7 @@ test('it can transfer an asset with royalties to an allowlisted program address' }, basisPoints: 5, creators: [{ address: umi.identity.publicKey, percentage: 100 }], - ruleSet: ruleSet('ProgramAllowList', [[MPL_CORE_PROGRAM_ID]]), + ruleSet: ruleSet('ProgramAllowList', [[SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID]]), }, }); @@ -186,7 +186,7 @@ test('it can transfer an asset with collection royalties to an allowlisted progr data: { basisPoints: 5, creators: [{ address: umi.identity.publicKey, percentage: 100 }], - ruleSet: ruleSet('ProgramAllowList', [[MPL_CORE_PROGRAM_ID]]), + ruleSet: ruleSet('ProgramAllowList', [[SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID]]), }, }), ], @@ -213,7 +213,7 @@ test('it can transfer an asset with collection royalties to an allowlisted progr }, basisPoints: 5, creators: [{ address: umi.identity.publicKey, percentage: 100 }], - ruleSet: ruleSet('ProgramAllowList', [[MPL_CORE_PROGRAM_ID]]), + ruleSet: ruleSet('ProgramAllowList', [[SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID]]), }, }); @@ -239,9 +239,6 @@ test('it cannot transfer an asset with royalties to a program address not on the owner: programOwner.publicKey, }); - // Create a second one because allowlist needs both to be off the allowlist. - const programOwned2 = await createAsset(umi); - // Creating a new asset to transfer. const asset = await createAsset(umi, { plugins: [ @@ -271,18 +268,13 @@ test('it cannot transfer an asset with royalties to a program address not on the }, }); - await transferV1(umi, { + const result = transferV1(umi, { asset: asset.publicKey, newOwner: programOwned.publicKey, }).sendAndConfirm(umi); - const result = transferV1(umi, { - asset: asset.publicKey, - newOwner: programOwned2.publicKey, - authority: programOwner, - }).sendAndConfirm(umi); - await t.throwsAsync(result, { name: 'NoApprovals' }); + await t.throwsAsync(result, { name: 'InvalidAuthority' }); }); test('it cannot transfer an asset with collection royalties to a program address not on allowlist', async (t) => { @@ -310,8 +302,6 @@ test('it cannot transfer an asset with collection royalties to a program address // Here we're creating a new owner that's program owned, so we're just going to use another asset. const programOwned = await createAsset(umi); - // Create a second one because allowlist needs both to be off the allowlist. - const programOwned2 = await createAsset(umi); // Then an account was created with the correct data. await assertAsset(t, umi, { @@ -334,25 +324,18 @@ test('it cannot transfer an asset with collection royalties to a program address }, }); - await transferV1(umi, { - asset: asset.publicKey, - collection: collection.publicKey, - newOwner: programOwned.publicKey, - authority: programOwner, - }).sendAndConfirm(umi); - const result = transferV1(umi, { asset: asset.publicKey, collection: collection.publicKey, - newOwner: programOwned2.publicKey, + newOwner: programOwned.publicKey, authority: programOwner, }).sendAndConfirm(umi); - await t.throwsAsync(result, { name: 'NoApprovals' }); + await t.throwsAsync(result, { name: 'InvalidAuthority' }); await assertAsset(t, umi, { asset: asset.publicKey, - owner: programOwned.publicKey, + owner: programOwner.publicKey, }); }); diff --git a/programs/mpl-core/src/plugins/royalties.rs b/programs/mpl-core/src/plugins/royalties.rs index ccd6cd1d..c79e65cb 100644 --- a/programs/mpl-core/src/plugins/royalties.rs +++ b/programs/mpl-core/src/plugins/royalties.rs @@ -81,7 +81,7 @@ impl PluginValidation for Royalties { RuleSet::None => Ok(ValidationResult::Pass), RuleSet::ProgramAllowList(allow_list) => { if allow_list.contains(ctx.authority_info.owner) - || allow_list.contains(new_owner.owner) + && allow_list.contains(new_owner.owner) { Ok(ValidationResult::Pass) } else { From 8a5564d3294000c637c025b9ec937faa4b05f087 Mon Sep 17 00:00:00 2001 From: blockiosaurus Date: Mon, 29 Apr 2024 20:27:37 -0400 Subject: [PATCH 2/2] Fixing formatting. --- clients/js/test/burn.test.ts | 2 +- clients/js/test/plugins/asset/royalties.test.ts | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/clients/js/test/burn.test.ts b/clients/js/test/burn.test.ts index d4bbbd8f..f32cd6f0 100644 --- a/clients/js/test/burn.test.ts +++ b/clients/js/test/burn.test.ts @@ -254,4 +254,4 @@ test('it cannot burn an asset with the wrong collection specified', async (t) => }).sendAndConfirm(umi); await t.throwsAsync(result, { name: 'InvalidCollection' }); -}); \ No newline at end of file +}); diff --git a/clients/js/test/plugins/asset/royalties.test.ts b/clients/js/test/plugins/asset/royalties.test.ts index 7cdbd1bf..58f1dfd6 100644 --- a/clients/js/test/plugins/asset/royalties.test.ts +++ b/clients/js/test/plugins/asset/royalties.test.ts @@ -137,7 +137,9 @@ test('it can transfer an asset with royalties to an allowlisted program address' data: { basisPoints: 5, creators: [{ address: umi.identity.publicKey, percentage: 100 }], - ruleSet: ruleSet('ProgramAllowList', [[SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID]]), + ruleSet: ruleSet('ProgramAllowList', [ + [SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID], + ]), }, }), ], @@ -158,7 +160,9 @@ test('it can transfer an asset with royalties to an allowlisted program address' }, basisPoints: 5, creators: [{ address: umi.identity.publicKey, percentage: 100 }], - ruleSet: ruleSet('ProgramAllowList', [[SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID]]), + ruleSet: ruleSet('ProgramAllowList', [ + [SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID], + ]), }, }); @@ -186,7 +190,9 @@ test('it can transfer an asset with collection royalties to an allowlisted progr data: { basisPoints: 5, creators: [{ address: umi.identity.publicKey, percentage: 100 }], - ruleSet: ruleSet('ProgramAllowList', [[SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID]]), + ruleSet: ruleSet('ProgramAllowList', [ + [SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID], + ]), }, }), ], @@ -213,7 +219,9 @@ test('it can transfer an asset with collection royalties to an allowlisted progr }, basisPoints: 5, creators: [{ address: umi.identity.publicKey, percentage: 100 }], - ruleSet: ruleSet('ProgramAllowList', [[SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID]]), + ruleSet: ruleSet('ProgramAllowList', [ + [SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID], + ]), }, }); @@ -273,7 +281,6 @@ test('it cannot transfer an asset with royalties to a program address not on the newOwner: programOwned.publicKey, }).sendAndConfirm(umi); - await t.throwsAsync(result, { name: 'InvalidAuthority' }); });