Improve Oracle account deserialization error handling (#115)

* Remap oracle account deserialization errors Also fix extra space on last error message * Regenerate IDL and clients * Add tests and more error handling to Oracle borrowing and slicing * Change test name
metaplex-foundation · May 9, 2024 · 022d677 · 022d677
1 parent 64bc55b
commit 022d677
Show file tree

Hide file tree

Showing 6 changed files with 188 additions and 8 deletions.
diff --git a/clients/js/src/generated/errors/mplCore.ts b/clients/js/src/generated/errors/mplCore.ts
@@ -526,15 +526,15 @@ export class RequiresLifecycleCheckError extends ProgramError {
 codeToErrorMap.set(0x24, RequiresLifecycleCheckError);
 nameToErrorMap.set('RequiresLifecycleCheck', RequiresLifecycleCheckError);
 
-/** DuplicateLifecycleChecks: Duplicate lifecycle checks were provided for external plugin  */
+/** DuplicateLifecycleChecks: Duplicate lifecycle checks were provided for external plugin */
 export class DuplicateLifecycleChecksError extends ProgramError {
   override readonly name: string = 'DuplicateLifecycleChecks';
 
   readonly code: number = 0x25; // 37
 
   constructor(program: Program, cause?: Error) {
     super(
-      'Duplicate lifecycle checks were provided for external plugin ',
+      'Duplicate lifecycle checks were provided for external plugin',
       program,
       cause
     );
@@ -543,6 +543,19 @@ export class DuplicateLifecycleChecksError extends ProgramError {
 codeToErrorMap.set(0x25, DuplicateLifecycleChecksError);
 nameToErrorMap.set('DuplicateLifecycleChecks', DuplicateLifecycleChecksError);
 
+/** InvalidOracleAccountData: Could not read from oracle account */
+export class InvalidOracleAccountDataError extends ProgramError {
+  override readonly name: string = 'InvalidOracleAccountData';
+
+  readonly code: number = 0x26; // 38
+
+  constructor(program: Program, cause?: Error) {
+    super('Could not read from oracle account', program, cause);
+  }
+}
+codeToErrorMap.set(0x26, InvalidOracleAccountDataError);
+nameToErrorMap.set('InvalidOracleAccountData', InvalidOracleAccountDataError);
+
 /**
  * Attempts to resolve a custom program error from the provided error code.
  * @category Errors

diff --git a/clients/js/test/externalPlugins/oracle.test.ts b/clients/js/test/externalPlugins/oracle.test.ts
@@ -21,7 +21,7 @@ import {
   preconfiguredAssetPdaCustomOffsetSet,
   close,
 } from '@metaplex-foundation/mpl-core-oracle-example';
-import { generateSigner } from '@metaplex-foundation/umi';
+import { generateSigner, sol } from '@metaplex-foundation/umi';
 import { ExternalValidationResult } from '@metaplex-foundation/mpl-core-oracle-example/dist/src/hooked';
 import { generateSignerWithSol } from '@metaplex-foundation/umi-bundle-tests';
 import {
@@ -45,6 +45,7 @@ import {
   updatePlugin,
   fetchAssetV1,
 } from '../../src';
+import { createAccount } from '@metaplex-foundation/mpl-toolbox';
 
 const createUmi = async () =>
   (await baseCreateUmi()).use(mplCoreOracleExample());
@@ -2388,3 +2389,140 @@ test('it can update oracle to different size external plugin', async (t) => {
     ],
   });
 });
+
+test('create fails but does not panic when oracle account does not exist', async (t) => {
+  const umi = await createUmi();
+  const oracleSigner = generateSigner(umi);
+
+  const asset = generateSigner(umi);
+  const result = create(umi, {
+    asset,
+    name: 'Test name',
+    uri: 'https://example.com',
+    plugins: [
+      {
+        type: 'Oracle',
+        resultsOffset: {
+          type: 'Anchor',
+        },
+        lifecycleChecks: {
+          create: [CheckResult.CAN_REJECT],
+          update: [CheckResult.CAN_REJECT],
+          transfer: [CheckResult.CAN_REJECT],
+          burn: [CheckResult.CAN_REJECT],
+        },
+        baseAddress: oracleSigner.publicKey,
+      },
+    ],
+  }).sendAndConfirm(umi);
+
+  await t.throwsAsync(result, { name: 'InvalidOracleAccountData' });
+});
+
+test('transfer fails but does not panic when oracle account does not exist', async (t) => {
+  const umi = await createUmi();
+  const oracleSigner = generateSigner(umi);
+
+  const asset = await createAsset(umi, {
+    plugins: [
+      {
+        type: 'Oracle',
+        resultsOffset: {
+          type: 'Anchor',
+        },
+        lifecycleChecks: {
+          transfer: [CheckResult.CAN_REJECT],
+        },
+        baseAddress: oracleSigner.publicKey,
+      },
+    ],
+  });
+
+  await assertAsset(t, umi, {
+    ...DEFAULT_ASSET,
+    asset: asset.publicKey,
+    owner: umi.identity.publicKey,
+    oracles: [
+      {
+        type: 'Oracle',
+        resultsOffset: {
+          type: 'Anchor',
+        },
+        authority: {
+          type: 'UpdateAuthority',
+        },
+        baseAddress: oracleSigner.publicKey,
+        lifecycleChecks: {
+          transfer: [CheckResult.CAN_REJECT],
+        },
+        pda: undefined,
+      },
+    ],
+  });
+
+  const newOwner = generateSigner(umi);
+  const result = transfer(umi, {
+    asset,
+    newOwner: newOwner.publicKey,
+  }).sendAndConfirm(umi);
+
+  await t.throwsAsync(result, { name: 'InvalidOracleAccountData' });
+});
+
+test('transfer fails but does not panic when oracle account is too small', async (t) => {
+  const umi = await createUmi();
+  const newAccount = generateSigner(umi);
+
+  // Create an invalid oracle account that is an account with 3 bytes.
+  await createAccount(umi, {
+    newAccount,
+    lamports: sol(0.1),
+    space: 3,
+    programId: umi.programs.get('mplCore').publicKey,
+  }).sendAndConfirm(umi);
+
+  const asset = await createAsset(umi, {
+    plugins: [
+      {
+        type: 'Oracle',
+        resultsOffset: {
+          type: 'NoOffset',
+        },
+        lifecycleChecks: {
+          transfer: [CheckResult.CAN_REJECT],
+        },
+        baseAddress: newAccount.publicKey,
+      },
+    ],
+  });
+
+  await assertAsset(t, umi, {
+    ...DEFAULT_ASSET,
+    asset: asset.publicKey,
+    owner: umi.identity.publicKey,
+    oracles: [
+      {
+        type: 'Oracle',
+        resultsOffset: {
+          type: 'NoOffset',
+        },
+        authority: {
+          type: 'UpdateAuthority',
+        },
+        baseAddress: newAccount.publicKey,
+        lifecycleChecks: {
+          transfer: [CheckResult.CAN_REJECT],
+        },
+        pda: undefined,
+      },
+    ],
+  });
+
+  const newOwner = generateSigner(umi);
+  const result = transfer(umi, {
+    asset,
+    newOwner: newOwner.publicKey,
+  }).sendAndConfirm(umi);
+
+  await t.throwsAsync(result, { name: 'InvalidOracleAccountData' });
+});
diff --git a/clients/rust/src/generated/errors/mpl_core.rs b/clients/rust/src/generated/errors/mpl_core.rs
@@ -122,8 +122,11 @@ pub enum MplCoreError {
     #[error("External plugin must have at least one lifecycle check")]
     RequiresLifecycleCheck,
     /// 37 (0x25) - Duplicate lifecycle checks were provided for external plugin
-    #[error("Duplicate lifecycle checks were provided for external plugin ")]
+    #[error("Duplicate lifecycle checks were provided for external plugin")]
     DuplicateLifecycleChecks,
+    /// 38 (0x26) - Could not read from oracle account
+    #[error("Could not read from oracle account")]
+    InvalidOracleAccountData,
 }
 
 impl solana_program::program_error::PrintProgramError for MplCoreError {

diff --git a/idls/mpl_core.json b/idls/mpl_core.json
@@ -4209,7 +4209,12 @@
     {
       "code": 37,
       "name": "DuplicateLifecycleChecks",
-      "msg": "Duplicate lifecycle checks were provided for external plugin "
+      "msg": "Duplicate lifecycle checks were provided for external plugin"
+    },
+    {
+      "code": 38,
+      "name": "InvalidOracleAccountData",
+      "msg": "Could not read from oracle account"
     }
   ],
   "metadata": {

diff --git a/programs/mpl-core/src/error.rs b/programs/mpl-core/src/error.rs
@@ -158,8 +158,12 @@ pub enum MplCoreError {
     RequiresLifecycleCheck,
 
     /// 37 - Duplicate lifecycle checks were provided for external plugin
-    #[error("Duplicate lifecycle checks were provided for external plugin ")]
+    #[error("Duplicate lifecycle checks were provided for external plugin")]
     DuplicateLifecycleChecks,
+
+    /// 38 - Could not read from oracle account
+    #[error("Could not read from oracle account")]
+    InvalidOracleAccountData,
 }
 
 impl PrintProgramError for MplCoreError {

diff --git a/programs/mpl-core/src/plugins/oracle.rs b/programs/mpl-core/src/plugins/oracle.rs
@@ -89,8 +89,18 @@ impl Oracle {
             .ok_or(MplCoreError::MissingExternalAccount)?;
 
         let offset = self.results_offset.to_offset_usize();
-        let validation_result =
-            OracleValidation::deserialize(&mut &(*oracle_account.data).borrow()[offset..])?;
+
+        let oracle_data = (*oracle_account.data).borrow();
+        let mut oracle_data_slice = oracle_data
+            .get(offset..)
+            .ok_or(MplCoreError::InvalidOracleAccountData)?;
+
+        if oracle_data_slice.len() < OracleValidation::serialized_size() {
+            return Err(MplCoreError::InvalidOracleAccountData.into());
+        }
+
+        let validation_result = OracleValidation::deserialize(&mut oracle_data_slice)
+            .map_err(|_| MplCoreError::InvalidOracleAccountData)?;
 
         match validation_result {
             OracleValidation::V1 {
@@ -187,3 +197,10 @@ pub enum OracleValidation {
         update: ExternalValidationResult,
     },
 }
+
+impl OracleValidation {
+    /// Borsh- and Anchor-serialized size of the `OracleValidation` struct.
+    pub fn serialized_size() -> usize {
+        5
+    }
+}