Prevent delegate to change the update authority (#44)

* Deprecate new update authority field * Return error on update by delegate * Add update authority by delegate test * Add bpf test * Fix formatting * Consolidate update authority test
metaplex-foundation · Sep 1, 2023 · 753f663 · 753f663
1 parent bcd3106
commit 753f663
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 14 deletions.
diff --git a/clients/js/src/generated/errors/mplTokenMetadata.ts b/clients/js/src/generated/errors/mplTokenMetadata.ts
@@ -2837,6 +2837,22 @@ export class InvalidMetadataFlagsError extends ProgramError {
 codeToErrorMap.set(0xc0, InvalidMetadataFlagsError);
 nameToErrorMap.set('InvalidMetadataFlags', InvalidMetadataFlagsError);
 
+/** CannotChangeUpdateAuthorityWithDelegate: Cannot change the update authority with a delegate */
+export class CannotChangeUpdateAuthorityWithDelegateError extends ProgramError {
+  readonly name: string = 'CannotChangeUpdateAuthorityWithDelegate';
+
+  readonly code: number = 0xc1; // 193
+
+  constructor(program: Program, cause?: Error) {
+    super('Cannot change the update authority with a delegate', program, cause);
+  }
+}
+codeToErrorMap.set(0xc1, CannotChangeUpdateAuthorityWithDelegateError);
+nameToErrorMap.set(
+  'CannotChangeUpdateAuthorityWithDelegate',
+  CannotChangeUpdateAuthorityWithDelegateError
+);
+
 /**
  * Attempts to resolve a custom program error from the provided error code.
  * @category Errors

diff --git a/clients/js/test/updateAsAuthorityItemDelegateV2.test.ts b/clients/js/test/updateAsAuthorityItemDelegateV2.test.ts
@@ -29,16 +29,44 @@ NON_EDITION_TOKEN_STANDARDS.forEach((tokenStandard) => {
       tokenStandard: TokenStandard[tokenStandard],
     }).sendAndConfirm(umi);
 
-    // When the delegate updates the update authority of the asset.
-    const newUpdateAuthority = generateSigner(umi).publicKey;
+    // When the delegate updates the asset to be immutable.
     await updateAsAuthorityItemDelegateV2(umi, {
       mint,
       authority: authorityItemDelegate,
-      newUpdateAuthority: some(newUpdateAuthority),
+      isMutable: false,
     }).sendAndConfirm(umi);
 
     // Then the account data was updated.
     const updatedMetadata = await fetchMetadataFromSeeds(umi, { mint });
-    t.like(updatedMetadata, <Metadata>{ updateAuthority: newUpdateAuthority });
+    t.like(updatedMetadata, <Metadata>{ isMutable: false });
+  });
+
+  test(`it cannot change the update authority of a ${tokenStandard} asset`, async (t) => {
+    // Given an existing asset.
+    const umi = await createUmi();
+    const { publicKey: mint } = await createDigitalAsset(umi, {
+      tokenStandard: TokenStandard[tokenStandard],
+    });
+
+    // And an authority item delegate approved on the asset.
+    const authorityItemDelegate = generateSigner(umi);
+    await delegateAuthorityItemV1(umi, {
+      mint,
+      delegate: authorityItemDelegate.publicKey,
+      tokenStandard: TokenStandard[tokenStandard],
+    }).sendAndConfirm(umi);
+
+    // When the delegate updates the update authority of the asset.
+    const newUpdateAuthority = generateSigner(umi).publicKey;
+    const promise = updateAsAuthorityItemDelegateV2(umi, {
+      mint,
+      authority: authorityItemDelegate,
+      newUpdateAuthority: some(newUpdateAuthority),
+    }).sendAndConfirm(umi);
+
+    // Then we expect a program error.
+    await t.throwsAsync(promise, {
+      name: 'CannotChangeUpdateAuthorityWithDelegate',
+    });
   });
 });
diff --git a/clients/rust/src/generated/errors/mpl_token_metadata.rs b/clients/rust/src/generated/errors/mpl_token_metadata.rs
@@ -588,4 +588,7 @@ pub enum MplTokenMetadataError {
     /// 0xC0 -
     #[error("")]
     InvalidMetadataFlags,
+    /// 0xC1 - Cannot change the update authority with a delegate
+    #[error("Cannot change the update authority with a delegate")]
+    CannotChangeUpdateAuthorityWithDelegate,
 }
diff --git a/idls/token_metadata.json b/idls/token_metadata.json
@@ -7721,6 +7721,11 @@
       "code": 192,
       "name": "InvalidMetadataFlags",
       "msg": ""
+    },
+    {
+      "code": 193,
+      "name": "CannotChangeUpdateAuthorityWithDelegate",
+      "msg": "Cannot change the update authority with a delegate"
     }
   ],
   "metadata": {

diff --git a/programs/token-metadata/program/src/error.rs b/programs/token-metadata/program/src/error.rs
@@ -759,6 +759,10 @@ pub enum MetadataError {
     /// 192
     #[error("")]
     InvalidMetadataFlags,
+
+    /// 193
+    #[error("Cannot change the update authority with a delegate")]
+    CannotChangeUpdateAuthorityWithDelegate,
 }
 
 impl PrintProgramError for MetadataError {

diff --git a/programs/token-metadata/program/src/instruction/metadata.rs b/programs/token-metadata/program/src/instruction/metadata.rs
@@ -127,6 +127,10 @@ pub enum UpdateArgs {
     },
     AsAuthorityItemDelegateV2 {
         /// The new update authority.
+        #[deprecated(
+            since = "1.13.3",
+            note = "A delegate cannot change the update authority. This field will be removed in a future release."
+        )]
         new_update_authority: Option<Pubkey>,
         /// Indicates whether the primary sale has happened or not (once set to `true`, it cannot be
         /// changed back).

diff --git a/programs/token-metadata/program/src/state/metadata.rs b/programs/token-metadata/program/src/state/metadata.rs
@@ -111,15 +111,21 @@ impl Metadata {
         // Only the Update Authority can update this section.
         match &args {
             UpdateArgs::V1 {
+                new_update_authority,
                 uses,
                 collection_details,
                 ..
             }
             | UpdateArgs::AsUpdateAuthorityV2 {
+                new_update_authority,
                 uses,
                 collection_details,
                 ..
             } => {
+                if let Some(authority) = new_update_authority {
+                    self.update_authority = *authority;
+                }
+
                 if uses.is_some() {
                     let uses_option = uses.clone().to_option();
                     // If already None leave it as None.
@@ -168,27 +174,20 @@ impl Metadata {
         // Update Authority or Authority Item Delegate can update this section.
         match &args {
             UpdateArgs::V1 {
-                new_update_authority,
                 primary_sale_happened,
                 is_mutable,
                 ..
             }
             | UpdateArgs::AsUpdateAuthorityV2 {
-                new_update_authority,
                 primary_sale_happened,
                 is_mutable,
                 ..
             }
             | UpdateArgs::AsAuthorityItemDelegateV2 {
-                new_update_authority,
                 primary_sale_happened,
                 is_mutable,
                 ..
             } => {
-                if let Some(authority) = new_update_authority {
-                    self.update_authority = *authority;
-                }
-
                 if let Some(primary_sale) = primary_sale_happened {
                     // If received primary_sale is true, flip to true.
                     if *primary_sale || !self.primary_sale_happened {
@@ -210,6 +209,15 @@ impl Metadata {
             _ => (),
         }
 
+        // Update authority by Authority Item Delegate is deprecated.
+        if let UpdateArgs::AsAuthorityItemDelegateV2 {
+            new_update_authority: Some(_authority),
+            ..
+        } = &args
+        {
+            return Err(MetadataError::CannotChangeUpdateAuthorityWithDelegate.into());
+        };
+
         // Update Authority or Collection Delegates can update this section.
         match &args {
             UpdateArgs::V1 { collection, .. }

diff --git a/programs/token-metadata/program/tests/update.rs b/programs/token-metadata/program/tests/update.rs
@@ -140,12 +140,10 @@ mod update {
 
         match &mut args {
             UpdateArgs::AsAuthorityItemDelegateV2 {
-                new_update_authority,
                 primary_sale_happened,
                 is_mutable,
                 ..
             } => {
-                *new_update_authority = Some(delegate.pubkey());
                 *primary_sale_happened = Some(true);
                 *is_mutable = Some(false);
             }
@@ -177,12 +175,96 @@ mod update {
 
         // checks the created metadata values
         let metadata = da.get_metadata(context).await;
+        let original_update_authority = Keypair::from_bytes(&context.payer.to_bytes()).unwrap();
 
-        assert_eq!(metadata.update_authority, delegate.pubkey());
+        assert_eq!(
+            metadata.update_authority,
+            original_update_authority.pubkey()
+        );
         assert!(metadata.primary_sale_happened);
         assert!(!metadata.is_mutable);
     }
 
+    #[tokio::test]
+    #[allow(deprecated)]
+    async fn fail_change_update_authority_by_authority_item_delegate() {
+        let context = &mut program_test().start_with_context().await;
+
+        let update_authority = Keypair::from_bytes(&context.payer.to_bytes()).unwrap();
+
+        let mut da = DigitalAsset::new();
+        da.create(context, TokenStandard::NonFungible, None)
+            .await
+            .unwrap();
+
+        let metadata = da.get_metadata(context).await;
+        assert_eq!(metadata.update_authority, update_authority.pubkey());
+        assert!(!metadata.primary_sale_happened);
+        assert!(metadata.is_mutable);
+
+        // Create metadata delegate.
+        let delegate = Keypair::new();
+        delegate.airdrop(context, 1_000_000_000).await.unwrap();
+        let delegate_record = da
+            .delegate(
+                context,
+                update_authority,
+                delegate.pubkey(),
+                DelegateArgs::AuthorityItemV1 {
+                    authorization_data: None,
+                },
+            )
+            .await
+            .unwrap()
+            .unwrap();
+
+        // Change a few values that this delegate is allowed to change.
+        let mut args = UpdateArgs::default_as_authority_item_delegate();
+
+        match &mut args {
+            UpdateArgs::AsAuthorityItemDelegateV2 {
+                new_update_authority,
+                ..
+            } => {
+                *new_update_authority = Some(delegate.pubkey());
+            }
+            _ => panic!("Unexpected enum variant"),
+        }
+
+        let mut builder = UpdateBuilder::new();
+        builder
+            .authority(delegate.pubkey())
+            .delegate_record(delegate_record)
+            .metadata(da.metadata)
+            .mint(da.mint.pubkey())
+            .payer(delegate.pubkey());
+
+        if let Some(edition) = da.edition {
+            builder.edition(edition);
+        }
+
+        let update_ix = builder.build(args).unwrap().instruction();
+
+        let tx = Transaction::new_signed_with_payer(
+            &[update_ix],
+            Some(&delegate.pubkey()),
+            &[&delegate],
+            context.last_blockhash,
+        );
+
+        let error = context
+            .banks_client
+            .process_transaction(tx)
+            .await
+            .unwrap_err();
+
+        // there should be an error.
+        assert_custom_error!(
+            error,
+            MetadataError::CannotChangeUpdateAuthorityWithDelegate
+        );
+    }
+
     #[tokio::test]
     async fn success_update_by_items_collection_delegate() {
         let delegate_args = DelegateArgs::CollectionV1 {