From d7db27a90f8a27448568430a0654e6721813416e Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 12 Jun 2023 12:34:26 +0200 Subject: [PATCH 1/2] feat: validate function abi params and method --- src/conditions/base/evm.ts | 59 +++++++++++ test/unit/conditions/base/evm.test.ts | 139 +++++++++++++++----------- test/unit/conditions/context.test.ts | 1 + test/unit/testVariables.ts | 3 + 4 files changed, 141 insertions(+), 61 deletions(-) create mode 100644 src/conditions/base/evm.ts diff --git a/src/conditions/base/evm.ts b/src/conditions/base/evm.ts new file mode 100644 index 000000000..bfbd44200 --- /dev/null +++ b/src/conditions/base/evm.ts @@ -0,0 +1,59 @@ +import Joi from 'joi'; + +import { ETH_ADDRESS_REGEXP, SUPPORTED_CHAINS } from '../const'; + +import { Condition } from './condition'; +import { returnValueTestSchema } from './schema'; + +export const STANDARD_CONTRACT_TYPES = ['ERC20', 'ERC721']; + +const functionAbiVariable = Joi.object({ + internalType: Joi.string().required(), + name: Joi.string().required(), + type: Joi.string().required(), +}); + +const functionAbiSchema = Joi.object({ + name: Joi.string().required(), + type: Joi.string().valid('function').required(), + inputs: Joi.array().items(functionAbiVariable), + outputs: Joi.array().items(functionAbiVariable), + // TODO: Should we restrict this to 'view'? + // stateMutability: Joi.string().valid('view').required(), +}).custom((functionAbi, helper) => { + // Validate method name + const method = helper.state.ancestors[0].method; + if (functionAbi.name !== method) { + return helper.message({ + custom: '"method" must be the same as "functionAbi.name"', + }); + } + + // Validate nr of parameters + const parameters = helper.state.ancestors[0].parameters; + if (functionAbi.inputs?.length !== parameters.length) { + return helper.message({ + custom: '"parameters" must have the same length as "functionAbi.inputs"', + }); + } + + return functionAbi; +}); + +export class EvmCondition extends Condition { + public readonly schema = Joi.object({ + contractAddress: Joi.string().pattern(ETH_ADDRESS_REGEXP).required(), + chain: Joi.number() + .valid(...SUPPORTED_CHAINS) + .required(), + standardContractType: Joi.string() + .valid(...STANDARD_CONTRACT_TYPES) + .optional(), + functionAbi: functionAbiSchema.optional(), + method: Joi.string().required(), + parameters: Joi.array().required(), + returnValueTest: returnValueTestSchema, + }) + // At most one of these keys needs to be present + .xor('standardContractType', 'functionAbi'); +} diff --git a/test/unit/conditions/base/evm.test.ts b/test/unit/conditions/base/evm.test.ts index 404d2d90b..c696f4925 100644 --- a/test/unit/conditions/base/evm.test.ts +++ b/test/unit/conditions/base/evm.test.ts @@ -1,5 +1,11 @@ +import { SecretKey } from '@nucypher/nucypher-core'; + +import { CustomContextParam } from '../../../../build/main/src'; +import { ConditionContext, ConditionSet } from '../../../../src/conditions'; import { ContractCondition } from '../../../../src/conditions/base'; -import { testContractConditionObj } from '../../testVariables'; +import { USER_ADDRESS_PARAM } from '../../../../src/conditions/const'; +import { fakeWeb3Provider } from '../../../utils'; +import { testContractConditionObj, testFunctionAbi } from '../../testVariables'; describe('validation', () => { it('accepts on a valid schema', () => { @@ -28,7 +34,6 @@ describe('validation', () => { describe('accepts either standardContractType or functionAbi but not both or none', () => { const standardContractType = 'ERC20'; - const functionAbi = { fake_function_abi: true }; it('accepts standardContractType', () => { const conditionObj = { @@ -46,7 +51,9 @@ describe('accepts either standardContractType or functionAbi but not both or non it('accepts functionAbi', () => { const conditionObj = { ...testContractConditionObj, - functionAbi, + functionAbi: testFunctionAbi, + method: testFunctionAbi.name, + parameters: ['0x1234', 1234], standardContractType: undefined, }; const contractCondition = new ContractCondition(conditionObj); @@ -60,7 +67,9 @@ describe('accepts either standardContractType or functionAbi but not both or non const conditionObj = { ...testContractConditionObj, standardContractType, - functionAbi, + parameters: ['0x1234', 1234], + functionAbi: testFunctionAbi, + method: testFunctionAbi.name, }; const contractCondition = new ContractCondition(conditionObj); expect(() => contractCondition.toObj()).toThrow( @@ -81,61 +90,69 @@ describe('accepts either standardContractType or functionAbi but not both or non }); }); -// TODO(#124) -// it('accepts custom parameters in function abi methods', async () => { -// throw new Error('Not implemented'); -// }); +describe('supports custom function abi', () => { + const evmConditionObj = { + ...testEvmConditionObj, + standardContractType: undefined, + functionAbi: testFunctionAbi, + method: 'myFunction', + parameters: [USER_ADDRESS_PARAM, ':customParam'], + returnValueTest: { + index: 0, + comparator: '==', + value: USER_ADDRESS_PARAM, + }, + }; + const evmCondition = new EvmCondition(evmConditionObj); + const web3Provider = fakeWeb3Provider(SecretKey.random().toBEBytes()); + const conditionSet = new ConditionSet([evmCondition]); + const conditionContext = new ConditionContext( + conditionSet.toWASMConditions(), + web3Provider + ); + const myCustomParam = ':customParam'; + const customParams: Record = {}; + customParams[myCustomParam] = 1234; + + it('accepts a custom param in function abi method params', async () => { + const asJson = await conditionContext + .withCustomParams(customParams) + .toJson(); + expect(asJson).toBeDefined(); + expect(asJson).toContain(USER_ADDRESS_PARAM); + expect(asJson).toContain(myCustomParam); + }); + + it('rejects on functionAbi mismatch', async () => { + const badEvmConditionObj = { + ...evmConditionObj, + functionAbi: { bad_function_abi: 'bad_function_abi' }, + }; + const evmCondition = new EvmCondition(badEvmConditionObj); + expect(() => evmCondition.toObj()).toThrow( + 'Invalid condition: "functionAbi.name" is required' + ); + }); -// TODO(#124) -// describe('supports custom function abi', () => { -// const fakeFunctionAbi = { -// name: 'myFunction', -// type: 'function', -// inputs: [ -// { -// name: 'account', -// type: 'address', -// }, -// { -// name: 'myCustomParam', -// type: 'uint256', -// }, -// ], -// outputs: [ -// { -// name: 'someValue', -// type: 'uint256', -// }, -// ], -// }; -// const contractConditionObj = { -// ...testContractConditionObj, -// functionAbi: fakeFunctionAbi, -// method: 'myFunction', -// parameters: [USER_ADDRESS_PARAM, ':customParam'], -// returnValueTest: { -// index: 0, -// comparator: '==', -// value: USER_ADDRESS_PARAM, -// }, -// }; -// const contractCondition = new ContractCondition(contractConditionObj); -// const web3Provider = fakeWeb3Provider(SecretKey.random().toBEBytes()); -// const conditionSet = new ConditionSet([contractCondition]); -// const conditionContext = new ConditionContext( -// conditionSet.toWASMConditions(), -// web3Provider -// ); -// const myCustomParam = ':customParam'; -// const customParams: Record = {}; -// customParams[myCustomParam] = 1234; -// -// it('accepts custom function abi', async () => { -// const asJson = await conditionContext -// .withCustomParams(customParams) -// .toJson(); -// expect(asJson).toBeDefined(); -// expect(asJson).toContain(USER_ADDRESS_PARAM); -// expect(asJson).toContain(myCustomParam); -// }); -// }); + it('rejects on functionAbi mismatch', async () => { + const badEvmConditionObj = { + ...evmConditionObj, + method: 'badMethod', + }; + const evmCondition = new EvmCondition(badEvmConditionObj); + expect(() => evmCondition.toObj()).toThrow( + 'Invalid condition: "method" must be the same as "functionAbi.name"' + ); + }); + + it('rejects on functionAbi mismatch', async () => { + const badEvmConditionObj = { + ...evmConditionObj, + parameters: [], + }; + const evmCondition = new EvmCondition(badEvmConditionObj); + expect(() => evmCondition.toObj()).toThrow( + 'Invalid condition: "parameters" must have the same length as "functionAbi.inputs"' + ); + }); +}); diff --git a/test/unit/conditions/context.test.ts b/test/unit/conditions/context.test.ts index 79d048157..b86fb0d0c 100644 --- a/test/unit/conditions/context.test.ts +++ b/test/unit/conditions/context.test.ts @@ -82,6 +82,7 @@ describe('context parameters', () => { ...testContractConditionObj, standardContractType: undefined, // We're going to use a custom function ABI functionAbi: testFunctionAbi, + method: testFunctionAbi.name, parameters: [USER_ADDRESS_PARAM, customParamKey], // We're going to use a custom parameter returnValueTest: { ...testReturnValueTest, diff --git a/test/unit/testVariables.ts b/test/unit/testVariables.ts index 6ece2f977..c199a3043 100644 --- a/test/unit/testVariables.ts +++ b/test/unit/testVariables.ts @@ -40,16 +40,19 @@ export const testFunctionAbi = { type: 'function', inputs: [ { + internalType: 'address', name: 'account', type: 'address', }, { + internalType: 'uint256', name: 'myCustomParam', type: 'uint256', }, ], outputs: [ { + internalType: 'uint256', name: 'someValue', type: 'uint256', }, From 4f324113fcddc18698d87b3e45d9acbddfc9fa26 Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Wed, 14 Jun 2023 15:26:48 +0200 Subject: [PATCH 2/2] fix after rebase --- src/conditions/base/contract.ts | 35 +++++- src/conditions/base/evm.ts | 59 ---------- test/unit/conditions/base/evm.test.ts | 158 -------------------------- 3 files changed, 34 insertions(+), 218 deletions(-) delete mode 100644 src/conditions/base/evm.ts delete mode 100644 test/unit/conditions/base/evm.test.ts diff --git a/src/conditions/base/contract.ts b/src/conditions/base/contract.ts index 8485227bc..4f4b5dd3c 100644 --- a/src/conditions/base/contract.ts +++ b/src/conditions/base/contract.ts @@ -6,6 +6,39 @@ import { RpcCondition, rpcConditionSchema } from './rpc'; export const STANDARD_CONTRACT_TYPES = ['ERC20', 'ERC721']; +const functionAbiVariable = Joi.object({ + internalType: Joi.string().required(), + name: Joi.string().required(), + type: Joi.string().required(), +}); + +const functionAbiSchema = Joi.object({ + name: Joi.string().required(), + type: Joi.string().valid('function').required(), + inputs: Joi.array().items(functionAbiVariable), + outputs: Joi.array().items(functionAbiVariable), + // TODO: Should we restrict this to 'view'? + // stateMutability: Joi.string().valid('view').required(), +}).custom((functionAbi, helper) => { + // Validate method name + const method = helper.state.ancestors[0].method; + if (functionAbi.name !== method) { + return helper.message({ + custom: '"method" must be the same as "functionAbi.name"', + }); + } + + // Validate nr of parameters + const parameters = helper.state.ancestors[0].parameters; + if (functionAbi.inputs?.length !== parameters.length) { + return helper.message({ + custom: '"parameters" must have the same length as "functionAbi.inputs"', + }); + } + + return functionAbi; +}); + const contractMethodSchemas: Record = { ...rpcConditionSchema, contractAddress: Joi.string().pattern(ETH_ADDRESS_REGEXP).required(), @@ -13,7 +46,7 @@ const contractMethodSchemas: Record = { .valid(...STANDARD_CONTRACT_TYPES) .optional(), method: Joi.string().required(), - functionAbi: Joi.object().optional(), + functionAbi: functionAbiSchema.optional(), parameters: Joi.array().required(), }; diff --git a/src/conditions/base/evm.ts b/src/conditions/base/evm.ts deleted file mode 100644 index bfbd44200..000000000 --- a/src/conditions/base/evm.ts +++ /dev/null @@ -1,59 +0,0 @@ -import Joi from 'joi'; - -import { ETH_ADDRESS_REGEXP, SUPPORTED_CHAINS } from '../const'; - -import { Condition } from './condition'; -import { returnValueTestSchema } from './schema'; - -export const STANDARD_CONTRACT_TYPES = ['ERC20', 'ERC721']; - -const functionAbiVariable = Joi.object({ - internalType: Joi.string().required(), - name: Joi.string().required(), - type: Joi.string().required(), -}); - -const functionAbiSchema = Joi.object({ - name: Joi.string().required(), - type: Joi.string().valid('function').required(), - inputs: Joi.array().items(functionAbiVariable), - outputs: Joi.array().items(functionAbiVariable), - // TODO: Should we restrict this to 'view'? - // stateMutability: Joi.string().valid('view').required(), -}).custom((functionAbi, helper) => { - // Validate method name - const method = helper.state.ancestors[0].method; - if (functionAbi.name !== method) { - return helper.message({ - custom: '"method" must be the same as "functionAbi.name"', - }); - } - - // Validate nr of parameters - const parameters = helper.state.ancestors[0].parameters; - if (functionAbi.inputs?.length !== parameters.length) { - return helper.message({ - custom: '"parameters" must have the same length as "functionAbi.inputs"', - }); - } - - return functionAbi; -}); - -export class EvmCondition extends Condition { - public readonly schema = Joi.object({ - contractAddress: Joi.string().pattern(ETH_ADDRESS_REGEXP).required(), - chain: Joi.number() - .valid(...SUPPORTED_CHAINS) - .required(), - standardContractType: Joi.string() - .valid(...STANDARD_CONTRACT_TYPES) - .optional(), - functionAbi: functionAbiSchema.optional(), - method: Joi.string().required(), - parameters: Joi.array().required(), - returnValueTest: returnValueTestSchema, - }) - // At most one of these keys needs to be present - .xor('standardContractType', 'functionAbi'); -} diff --git a/test/unit/conditions/base/evm.test.ts b/test/unit/conditions/base/evm.test.ts deleted file mode 100644 index c696f4925..000000000 --- a/test/unit/conditions/base/evm.test.ts +++ /dev/null @@ -1,158 +0,0 @@ -import { SecretKey } from '@nucypher/nucypher-core'; - -import { CustomContextParam } from '../../../../build/main/src'; -import { ConditionContext, ConditionSet } from '../../../../src/conditions'; -import { ContractCondition } from '../../../../src/conditions/base'; -import { USER_ADDRESS_PARAM } from '../../../../src/conditions/const'; -import { fakeWeb3Provider } from '../../../utils'; -import { testContractConditionObj, testFunctionAbi } from '../../testVariables'; - -describe('validation', () => { - it('accepts on a valid schema', () => { - const contract = new ContractCondition(testContractConditionObj); - expect(contract.toObj()).toEqual({ - ...testContractConditionObj, - _class: 'ContractCondition', - }); - }); - - it('rejects an invalid schema', () => { - const badContractCondition = { - ...testContractConditionObj, - // Intentionally removing `contractAddress` - contractAddress: undefined, - }; - const badEvm = new ContractCondition(badContractCondition); - expect(() => badEvm.toObj()).toThrow( - 'Invalid condition: "contractAddress" is required' - ); - - const { error } = badEvm.validate(badContractCondition); - expect(error?.message).toEqual('"contractAddress" is required'); - }); -}); - -describe('accepts either standardContractType or functionAbi but not both or none', () => { - const standardContractType = 'ERC20'; - - it('accepts standardContractType', () => { - const conditionObj = { - ...testContractConditionObj, - standardContractType, - functionAbi: undefined, - }; - const contractCondition = new ContractCondition(conditionObj); - expect(contractCondition.toObj()).toEqual({ - ...conditionObj, - _class: 'ContractCondition', - }); - }); - - it('accepts functionAbi', () => { - const conditionObj = { - ...testContractConditionObj, - functionAbi: testFunctionAbi, - method: testFunctionAbi.name, - parameters: ['0x1234', 1234], - standardContractType: undefined, - }; - const contractCondition = new ContractCondition(conditionObj); - expect(contractCondition.toObj()).toEqual({ - ...conditionObj, - _class: 'ContractCondition', - }); - }); - - it('rejects both', () => { - const conditionObj = { - ...testContractConditionObj, - standardContractType, - parameters: ['0x1234', 1234], - functionAbi: testFunctionAbi, - method: testFunctionAbi.name, - }; - const contractCondition = new ContractCondition(conditionObj); - expect(() => contractCondition.toObj()).toThrow( - '"value" contains a conflict between exclusive peers [standardContractType, functionAbi]' - ); - }); - - it('rejects none', () => { - const conditionObj = { - ...testContractConditionObj, - standardContractType: undefined, - functionAbi: undefined, - }; - const contractCondition = new ContractCondition(conditionObj); - expect(() => contractCondition.toObj()).toThrow( - '"value" must contain at least one of [standardContractType, functionAbi]' - ); - }); -}); - -describe('supports custom function abi', () => { - const evmConditionObj = { - ...testEvmConditionObj, - standardContractType: undefined, - functionAbi: testFunctionAbi, - method: 'myFunction', - parameters: [USER_ADDRESS_PARAM, ':customParam'], - returnValueTest: { - index: 0, - comparator: '==', - value: USER_ADDRESS_PARAM, - }, - }; - const evmCondition = new EvmCondition(evmConditionObj); - const web3Provider = fakeWeb3Provider(SecretKey.random().toBEBytes()); - const conditionSet = new ConditionSet([evmCondition]); - const conditionContext = new ConditionContext( - conditionSet.toWASMConditions(), - web3Provider - ); - const myCustomParam = ':customParam'; - const customParams: Record = {}; - customParams[myCustomParam] = 1234; - - it('accepts a custom param in function abi method params', async () => { - const asJson = await conditionContext - .withCustomParams(customParams) - .toJson(); - expect(asJson).toBeDefined(); - expect(asJson).toContain(USER_ADDRESS_PARAM); - expect(asJson).toContain(myCustomParam); - }); - - it('rejects on functionAbi mismatch', async () => { - const badEvmConditionObj = { - ...evmConditionObj, - functionAbi: { bad_function_abi: 'bad_function_abi' }, - }; - const evmCondition = new EvmCondition(badEvmConditionObj); - expect(() => evmCondition.toObj()).toThrow( - 'Invalid condition: "functionAbi.name" is required' - ); - }); - - it('rejects on functionAbi mismatch', async () => { - const badEvmConditionObj = { - ...evmConditionObj, - method: 'badMethod', - }; - const evmCondition = new EvmCondition(badEvmConditionObj); - expect(() => evmCondition.toObj()).toThrow( - 'Invalid condition: "method" must be the same as "functionAbi.name"' - ); - }); - - it('rejects on functionAbi mismatch', async () => { - const badEvmConditionObj = { - ...evmConditionObj, - parameters: [], - }; - const evmCondition = new EvmCondition(badEvmConditionObj); - expect(() => evmCondition.toObj()).toThrow( - 'Invalid condition: "parameters" must have the same length as "functionAbi.inputs"' - ); - }); -});