From cd39bad7a96245b5091f34e7338ce622f409def6 Mon Sep 17 00:00:00 2001 From: Michael Danenberg <56533526+danenbm@users.noreply.github.com> Date: Wed, 8 May 2024 15:20:42 -0700 Subject: [PATCH 1/4] Remap oracle account deserialization errors Also fix extra space on last error message --- programs/mpl-core/src/error.rs | 6 +++++- programs/mpl-core/src/plugins/oracle.rs | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/programs/mpl-core/src/error.rs b/programs/mpl-core/src/error.rs index d39bd150..7b7003bb 100644 --- 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 index 28155a8a..1a27274b 100644 --- a/programs/mpl-core/src/plugins/oracle.rs +++ b/programs/mpl-core/src/plugins/oracle.rs @@ -90,7 +90,8 @@ impl Oracle { let offset = self.results_offset.to_offset_usize(); let validation_result = - OracleValidation::deserialize(&mut &(*oracle_account.data).borrow()[offset..])?; + OracleValidation::deserialize(&mut &(*oracle_account.data).borrow()[offset..]) + .map_err(|_| MplCoreError::InvalidOracleAccountData)?; match validation_result { OracleValidation::V1 { From c17a462149c48033eca213e89a56f1c77a369d43 Mon Sep 17 00:00:00 2001 From: Michael Danenberg <56533526+danenbm@users.noreply.github.com> Date: Wed, 8 May 2024 15:27:37 -0700 Subject: [PATCH 2/4] Regenerate IDL and clients --- clients/js/src/generated/errors/mplCore.ts | 17 +++++++++++++++-- clients/rust/src/generated/errors/mpl_core.rs | 5 ++++- idls/mpl_core.json | 7 ++++++- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/clients/js/src/generated/errors/mplCore.ts b/clients/js/src/generated/errors/mplCore.ts index 9d392424..724769a6 100644 --- a/clients/js/src/generated/errors/mplCore.ts +++ b/clients/js/src/generated/errors/mplCore.ts @@ -526,7 +526,7 @@ 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'; @@ -534,7 +534,7 @@ export class DuplicateLifecycleChecksError extends ProgramError { 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/rust/src/generated/errors/mpl_core.rs b/clients/rust/src/generated/errors/mpl_core.rs index c2db7449..81aaff8b 100644 --- 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 index a9619d18..a75de249 100644 --- 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": { From ec50b68add30ac0ea95ded3c4a3a4071d45a8445 Mon Sep 17 00:00:00 2001 From: Michael Danenberg <56533526+danenbm@users.noreply.github.com> Date: Wed, 8 May 2024 16:56:16 -0700 Subject: [PATCH 3/4] Add tests and more error handling to Oracle borrowing and slicing --- .../js/test/externalPlugins/oracle.test.ts | 140 +++++++++++++++++- programs/mpl-core/src/plugins/oracle.rs | 22 ++- 2 files changed, 158 insertions(+), 4 deletions(-) diff --git a/clients/js/test/externalPlugins/oracle.test.ts b/clients/js/test/externalPlugins/oracle.test.ts index 55a554af..a5e27a7e 100644 --- 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('transferring an asset 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/programs/mpl-core/src/plugins/oracle.rs b/programs/mpl-core/src/plugins/oracle.rs index 1a27274b..441dfedc 100644 --- a/programs/mpl-core/src/plugins/oracle.rs +++ b/programs/mpl-core/src/plugins/oracle.rs @@ -89,9 +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..]) - .map_err(|_| MplCoreError::InvalidOracleAccountData)?; + + 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 { @@ -188,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 + } +} From 1f60d162c4beca2ddb41e40f5a96fb21753b5e84 Mon Sep 17 00:00:00 2001 From: Michael Danenberg <56533526+danenbm@users.noreply.github.com> Date: Wed, 8 May 2024 17:48:09 -0700 Subject: [PATCH 4/4] Change test name --- clients/js/test/externalPlugins/oracle.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/js/test/externalPlugins/oracle.test.ts b/clients/js/test/externalPlugins/oracle.test.ts index a5e27a7e..be1fd6ed 100644 --- a/clients/js/test/externalPlugins/oracle.test.ts +++ b/clients/js/test/externalPlugins/oracle.test.ts @@ -2469,7 +2469,7 @@ test('transfer fails but does not panic when oracle account does not exist', asy await t.throwsAsync(result, { name: 'InvalidOracleAccountData' }); }); -test('transferring an asset fails but does not panic when oracle account is too small', async (t) => { +test('transfer fails but does not panic when oracle account is too small', async (t) => { const umi = await createUmi(); const newAccount = generateSigner(umi);