Merge pull request #93 from metaplex-foundation/fix/stricter-allowlist

Adding stricter allowlist checks
metaplex-foundation · Apr 30, 2024 · fa3c7f7 · fa3c7f7
2 parents b02f850 + 8a5564d
commit fa3c7f7
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 28 deletions.
diff --git 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' });
+});
diff --git 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', [[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', [[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', [[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', [[MPL_CORE_PROGRAM_ID]]),
+      ruleSet: ruleSet('ProgramAllowList', [
+        [SPL_SYSTEM_PROGRAM_ID, MPL_CORE_PROGRAM_ID],
+      ]),
     },
   });
 
@@ -239,9 +247,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 +276,12 @@ test('it cannot transfer an asset with royalties to a program address not on the
     },
   });
 
-  await transferV1(umi, {
-    asset: asset.publicKey,
-    newOwner: programOwned.publicKey,
-  }).sendAndConfirm(umi);
-
   const result = transferV1(umi, {
     asset: asset.publicKey,
-    newOwner: programOwned2.publicKey,
-    authority: programOwner,
+    newOwner: programOwned.publicKey,
   }).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 +309,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 +331,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
@@ -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 {