From efcf6704aa47833a202e40e6081c4fd37a703d1e Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 12 Jun 2023 09:57:41 +0200 Subject: [PATCH 1/4] refactor: evm and time cond to inherit from rpc --- src/conditions/base/evm.ts | 32 +++++++++++++++----------------- src/conditions/base/rpc.ts | 22 ++++++++++++---------- src/conditions/base/time.ts | 26 ++++++++++---------------- 3 files changed, 37 insertions(+), 43 deletions(-) diff --git a/src/conditions/base/evm.ts b/src/conditions/base/evm.ts index 604817948..86c269df7 100644 --- a/src/conditions/base/evm.ts +++ b/src/conditions/base/evm.ts @@ -1,26 +1,24 @@ import Joi from 'joi'; -import { ETH_ADDRESS_REGEXP, SUPPORTED_CHAINS } from '../const'; +import { ETH_ADDRESS_REGEXP } from '../const'; -import { Condition } from './condition'; -import { returnValueTestSchema } from './schema'; +import { RpcCondition, rpcConditionSchema } from './rpc'; export const STANDARD_CONTRACT_TYPES = ['ERC20', 'ERC721']; -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: Joi.object().optional(), - method: Joi.string().required(), - parameters: Joi.array().required(), - returnValueTest: returnValueTestSchema, - }) +const evmMethodSchemas: Record = { + ...rpcConditionSchema, + contractAddress: Joi.string().pattern(ETH_ADDRESS_REGEXP).required(), + standardContractType: Joi.string() + .valid(...STANDARD_CONTRACT_TYPES) + .optional(), + method: Joi.string().required(), + functionAbi: Joi.object().optional(), + parameters: Joi.array().required(), +}; + +export class EvmCondition extends RpcCondition { + public readonly schema = Joi.object(evmMethodSchemas) // At most one of these keys needs to be present .xor('standardContractType', 'functionAbi'); } diff --git a/src/conditions/base/rpc.ts b/src/conditions/base/rpc.ts index 5144c251f..4a088817d 100644 --- a/src/conditions/base/rpc.ts +++ b/src/conditions/base/rpc.ts @@ -18,15 +18,17 @@ const makeParameters = () => })), }); +export const rpcConditionSchema = { + chain: Joi.number() + .valid(...SUPPORTED_CHAINS) + .required(), + method: Joi.string() + .valid(...Object.keys(rpcMethodSchemas)) + .required(), + parameters: makeParameters(), + returnValueTest: returnValueTestSchema.required(), +}; + export class RpcCondition extends Condition { - public readonly schema = Joi.object({ - chain: Joi.number() - .valid(...SUPPORTED_CHAINS) - .required(), - method: Joi.string() - .valid(...Object.keys(rpcMethodSchemas)) - .required(), - parameters: makeParameters(), - returnValueTest: returnValueTestSchema.required(), - }); + public readonly schema = Joi.object(rpcConditionSchema); } diff --git a/src/conditions/base/time.ts b/src/conditions/base/time.ts index 78fa28795..9bf3a7cf5 100644 --- a/src/conditions/base/time.ts +++ b/src/conditions/base/time.ts @@ -1,24 +1,18 @@ import Joi from 'joi'; -import { SUPPORTED_CHAINS } from '../const'; +import { RpcCondition, rpcConditionSchema } from './rpc'; -import { Condition } from './condition'; -import { returnValueTestSchema } from './schema'; +const BLOCKTIME_METHOD = 'blocktime'; -export class TimeCondition extends Condition { - // TODO: This is the only condition that uses defaults, and also the only condition that uses `method` in order - // to determine the schema. I.e. the only method that used `METHOD = 'blocktime'` in `nucypher/nucypher`. - // TODO: Consider introducing a different field for this, e.g. `conditionType` or `type`. Use this field in a - // condition factory. +const timeConditionSchema = { + ...rpcConditionSchema, + method: Joi.string().valid(BLOCKTIME_METHOD).required(), +}; + +export class TimeCondition extends RpcCondition { public readonly defaults: Record = { - method: 'blocktime', + method: BLOCKTIME_METHOD, }; - public readonly schema = Joi.object({ - method: Joi.string().valid(this.defaults.method).required(), - returnValueTest: returnValueTestSchema.required(), - chain: Joi.number() - .valid(...SUPPORTED_CHAINS) - .required(), - }); + public readonly schema = Joi.object(timeConditionSchema); } From 77278d6cccacca87be176a0b807426a5a5b5c11f Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 12 Jun 2023 16:05:48 +0200 Subject: [PATCH 2/4] feat!: rename EvmCondition to ContractCondition --- src/conditions/base/{evm.ts => contract.ts} | 6 +-- src/conditions/base/index.ts | 2 +- src/conditions/predefined/erc721.ts | 6 +-- test/docs/cbd.test.ts | 4 +- test/unit/conditions/base/condition.test.ts | 20 ++++---- test/unit/conditions/base/evm.test.ts | 56 ++++++++++----------- test/unit/conditions/context.test.ts | 28 +++++------ test/unit/testVariables.ts | 2 +- 8 files changed, 62 insertions(+), 62 deletions(-) rename src/conditions/base/{evm.ts => contract.ts} (77%) diff --git a/src/conditions/base/evm.ts b/src/conditions/base/contract.ts similarity index 77% rename from src/conditions/base/evm.ts rename to src/conditions/base/contract.ts index 86c269df7..8485227bc 100644 --- a/src/conditions/base/evm.ts +++ b/src/conditions/base/contract.ts @@ -6,7 +6,7 @@ import { RpcCondition, rpcConditionSchema } from './rpc'; export const STANDARD_CONTRACT_TYPES = ['ERC20', 'ERC721']; -const evmMethodSchemas: Record = { +const contractMethodSchemas: Record = { ...rpcConditionSchema, contractAddress: Joi.string().pattern(ETH_ADDRESS_REGEXP).required(), standardContractType: Joi.string() @@ -17,8 +17,8 @@ const evmMethodSchemas: Record = { parameters: Joi.array().required(), }; -export class EvmCondition extends RpcCondition { - public readonly schema = Joi.object(evmMethodSchemas) +export class ContractCondition extends RpcCondition { + public readonly schema = Joi.object(contractMethodSchemas) // At most one of these keys needs to be present .xor('standardContractType', 'functionAbi'); } diff --git a/src/conditions/base/index.ts b/src/conditions/base/index.ts index dc2fcf522..805717e01 100644 --- a/src/conditions/base/index.ts +++ b/src/conditions/base/index.ts @@ -1,4 +1,4 @@ export { Condition } from './condition'; -export { EvmCondition } from './evm'; +export { ContractCondition } from './contract'; export { RpcCondition } from './rpc'; export { TimeCondition } from './time'; diff --git a/src/conditions/predefined/erc721.ts b/src/conditions/predefined/erc721.ts index ef9f880f3..beed9a5ce 100644 --- a/src/conditions/predefined/erc721.ts +++ b/src/conditions/predefined/erc721.ts @@ -1,7 +1,7 @@ -import { EvmCondition } from '../base'; +import { ContractCondition } from '../base'; import { USER_ADDRESS_PARAM } from '../const'; -export class ERC721Ownership extends EvmCondition { +export class ERC721Ownership extends ContractCondition { public readonly defaults = { method: 'ownerOf', parameters: [], @@ -14,7 +14,7 @@ export class ERC721Ownership extends EvmCondition { }; } -export class ERC721Balance extends EvmCondition { +export class ERC721Balance extends ContractCondition { public readonly defaults = { method: 'balanceOf', parameters: [USER_ADDRESS_PARAM], diff --git a/test/docs/cbd.test.ts b/test/docs/cbd.test.ts index 86be24fe6..5d4a4702a 100644 --- a/test/docs/cbd.test.ts +++ b/test/docs/cbd.test.ts @@ -18,7 +18,7 @@ import { const { predefined: { ERC721Ownership }, - base: { EvmCondition }, + base: { ContractCondition }, ConditionSet, } = conditions; @@ -100,7 +100,7 @@ describe('Get Started (CBD PoC)', () => { value: 3, }, }; - const NFTBalance = new EvmCondition(NFTBalanceConfig); + const NFTBalance = new ContractCondition(NFTBalanceConfig); const encrypter = newDeployed.encrypter; diff --git a/test/unit/conditions/base/condition.test.ts b/test/unit/conditions/base/condition.test.ts index a38798744..dc9b310ab 100644 --- a/test/unit/conditions/base/condition.test.ts +++ b/test/unit/conditions/base/condition.test.ts @@ -1,4 +1,4 @@ -import { EvmCondition } from '../../../../src/conditions/base'; +import { ContractCondition } from '../../../../src/conditions/base'; import { ERC721Balance, ERC721Ownership, @@ -7,7 +7,7 @@ import { TEST_CHAIN_ID, TEST_CONTRACT_ADDR, TEST_CONTRACT_ADDR_2, - testEvmConditionObj, + testContractConditionObj, } from '../../testVariables'; describe('validation', () => { @@ -47,18 +47,18 @@ describe('validation', () => { describe('serialization', () => { it('serializes to a plain object', () => { - const evm = new EvmCondition(testEvmConditionObj); - expect(evm.toObj()).toEqual({ - ...testEvmConditionObj, - _class: 'EvmCondition', + const contract = new ContractCondition(testContractConditionObj); + expect(contract.toObj()).toEqual({ + ...testContractConditionObj, + _class: 'ContractCondition', }); }); it('serializes predefined conditions', () => { - const evm = new ERC721Ownership(testEvmConditionObj); - expect(evm.toObj()).toEqual({ - ...evm.defaults, - ...testEvmConditionObj, + const contract = new ERC721Ownership(testContractConditionObj); + expect(contract.toObj()).toEqual({ + ...contract.defaults, + ...testContractConditionObj, _class: 'ERC721Ownership', }); }); diff --git a/test/unit/conditions/base/evm.test.ts b/test/unit/conditions/base/evm.test.ts index 5a35efa93..404d2d90b 100644 --- a/test/unit/conditions/base/evm.test.ts +++ b/test/unit/conditions/base/evm.test.ts @@ -1,27 +1,27 @@ -import { EvmCondition } from '../../../../src/conditions/base'; -import { testEvmConditionObj } from '../../testVariables'; +import { ContractCondition } from '../../../../src/conditions/base'; +import { testContractConditionObj } from '../../testVariables'; describe('validation', () => { it('accepts on a valid schema', () => { - const evm = new EvmCondition(testEvmConditionObj); - expect(evm.toObj()).toEqual({ - ...testEvmConditionObj, - _class: 'EvmCondition', + const contract = new ContractCondition(testContractConditionObj); + expect(contract.toObj()).toEqual({ + ...testContractConditionObj, + _class: 'ContractCondition', }); }); it('rejects an invalid schema', () => { - const badEvmCondition = { - ...testEvmConditionObj, + const badContractCondition = { + ...testContractConditionObj, // Intentionally removing `contractAddress` contractAddress: undefined, }; - const badEvm = new EvmCondition(badEvmCondition); + const badEvm = new ContractCondition(badContractCondition); expect(() => badEvm.toObj()).toThrow( 'Invalid condition: "contractAddress" is required' ); - const { error } = badEvm.validate(badEvmCondition); + const { error } = badEvm.validate(badContractCondition); expect(error?.message).toEqual('"contractAddress" is required'); }); }); @@ -32,50 +32,50 @@ describe('accepts either standardContractType or functionAbi but not both or non it('accepts standardContractType', () => { const conditionObj = { - ...testEvmConditionObj, + ...testContractConditionObj, standardContractType, functionAbi: undefined, }; - const evmCondition = new EvmCondition(conditionObj); - expect(evmCondition.toObj()).toEqual({ + const contractCondition = new ContractCondition(conditionObj); + expect(contractCondition.toObj()).toEqual({ ...conditionObj, - _class: 'EvmCondition', + _class: 'ContractCondition', }); }); it('accepts functionAbi', () => { const conditionObj = { - ...testEvmConditionObj, + ...testContractConditionObj, functionAbi, standardContractType: undefined, }; - const evmCondition = new EvmCondition(conditionObj); - expect(evmCondition.toObj()).toEqual({ + const contractCondition = new ContractCondition(conditionObj); + expect(contractCondition.toObj()).toEqual({ ...conditionObj, - _class: 'EvmCondition', + _class: 'ContractCondition', }); }); it('rejects both', () => { const conditionObj = { - ...testEvmConditionObj, + ...testContractConditionObj, standardContractType, functionAbi, }; - const evmCondition = new EvmCondition(conditionObj); - expect(() => evmCondition.toObj()).toThrow( + const contractCondition = new ContractCondition(conditionObj); + expect(() => contractCondition.toObj()).toThrow( '"value" contains a conflict between exclusive peers [standardContractType, functionAbi]' ); }); it('rejects none', () => { const conditionObj = { - ...testEvmConditionObj, + ...testContractConditionObj, standardContractType: undefined, functionAbi: undefined, }; - const evmCondition = new EvmCondition(conditionObj); - expect(() => evmCondition.toObj()).toThrow( + const contractCondition = new ContractCondition(conditionObj); + expect(() => contractCondition.toObj()).toThrow( '"value" must contain at least one of [standardContractType, functionAbi]' ); }); @@ -108,8 +108,8 @@ describe('accepts either standardContractType or functionAbi but not both or non // }, // ], // }; -// const evmConditionObj = { -// ...testEvmConditionObj, +// const contractConditionObj = { +// ...testContractConditionObj, // functionAbi: fakeFunctionAbi, // method: 'myFunction', // parameters: [USER_ADDRESS_PARAM, ':customParam'], @@ -119,9 +119,9 @@ describe('accepts either standardContractType or functionAbi but not both or non // value: USER_ADDRESS_PARAM, // }, // }; -// const evmCondition = new EvmCondition(evmConditionObj); +// const contractCondition = new ContractCondition(contractConditionObj); // const web3Provider = fakeWeb3Provider(SecretKey.random().toBEBytes()); -// const conditionSet = new ConditionSet([evmCondition]); +// const conditionSet = new ConditionSet([contractCondition]); // const conditionContext = new ConditionContext( // conditionSet.toWASMConditions(), // web3Provider diff --git a/test/unit/conditions/context.test.ts b/test/unit/conditions/context.test.ts index 9a1746919..79d048157 100644 --- a/test/unit/conditions/context.test.ts +++ b/test/unit/conditions/context.test.ts @@ -2,12 +2,12 @@ import { SecretKey } from '@nucypher/nucypher-core'; import { CustomContextParam } from '../../../src'; import { ConditionSet } from '../../../src/conditions'; -import { EvmCondition, RpcCondition } from '../../../src/conditions/base'; +import { ContractCondition, RpcCondition } from '../../../src/conditions/base'; import { USER_ADDRESS_PARAM } from '../../../src/conditions/const'; import { RESERVED_CONTEXT_PARAMS } from '../../../src/conditions/context/context'; import { fakeWeb3Provider } from '../../utils'; import { - testEvmConditionObj, + testContractConditionObj, testFunctionAbi, testReturnValueTest, testRpcConditionObj, @@ -40,15 +40,15 @@ describe('context parameters', () => { const customParams: Record = {}; customParams[customParamKey] = 1234; - const evmConditionObj = { - ...testEvmConditionObj, + const contractConditionObj = { + ...testContractConditionObj, returnValueTest: { ...testReturnValueTest, value: customParamKey, }, }; - const evmCondition = new EvmCondition(evmConditionObj); - const conditionSet = new ConditionSet([evmCondition]); + const contractCondition = new ContractCondition(contractConditionObj); + const conditionSet = new ConditionSet([contractCondition]); const conditionContext = conditionSet.buildContext(web3Provider); describe('return value test', () => { @@ -78,8 +78,8 @@ describe('context parameters', () => { }); describe('custom method parameters', () => { - const evmConditionObj = { - ...testEvmConditionObj, + const contractConditionObj = { + ...testContractConditionObj, standardContractType: undefined, // We're going to use a custom function ABI functionAbi: testFunctionAbi, parameters: [USER_ADDRESS_PARAM, customParamKey], // We're going to use a custom parameter @@ -89,12 +89,12 @@ describe('context parameters', () => { }; it('rejects on a missing parameter ', async () => { - const customEvmCondition = new EvmCondition({ - ...evmConditionObj, + const customContractCondition = new ContractCondition({ + ...contractConditionObj, parameters: [USER_ADDRESS_PARAM, customParamKey], }); const conditionContext = new ConditionSet([ - customEvmCondition, + customContractCondition, ]).buildContext(web3Provider); await expect(async () => conditionContext.toObj()).rejects.toThrow( @@ -103,12 +103,12 @@ describe('context parameters', () => { }); it('accepts on a hard-coded parameter', async () => { - const customEvmCondition = new EvmCondition({ - ...evmConditionObj, + const customContractCondition = new ContractCondition({ + ...contractConditionObj, parameters: [USER_ADDRESS_PARAM, 100], }); const conditionContext = new ConditionSet([ - customEvmCondition, + customContractCondition, ]).buildContext(web3Provider); const asObj = await conditionContext.toObj(); diff --git a/test/unit/testVariables.ts b/test/unit/testVariables.ts index 99c219be0..6ece2f977 100644 --- a/test/unit/testVariables.ts +++ b/test/unit/testVariables.ts @@ -26,7 +26,7 @@ export const testRpcConditionObj = { returnValueTest: testReturnValueTest, }; -export const testEvmConditionObj = { +export const testContractConditionObj = { contractAddress: '0x0000000000000000000000000000000000000000', chain: 5, standardContractType: 'ERC20', From 662b5f538556bb85084ad03be6d9d410260f6435 Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 12 Jun 2023 16:31:29 +0200 Subject: [PATCH 3/4] timelock doesn't take parameters --- src/conditions/base/time.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conditions/base/time.ts b/src/conditions/base/time.ts index 9bf3a7cf5..ad4093d48 100644 --- a/src/conditions/base/time.ts +++ b/src/conditions/base/time.ts @@ -7,6 +7,7 @@ const BLOCKTIME_METHOD = 'blocktime'; const timeConditionSchema = { ...rpcConditionSchema, method: Joi.string().valid(BLOCKTIME_METHOD).required(), + parameters: undefined, // TimeCondition does not accept parameters }; export class TimeCondition extends RpcCondition { From 96a92d58294cb5cf327930df47f7bd2bb31abde3 Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Tue, 13 Jun 2023 10:02:30 +0200 Subject: [PATCH 4/4] fix leaking rpc condition parmeters to time condition schema --- src/conditions/base/time.ts | 6 ++++-- src/utils.ts | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/conditions/base/time.ts b/src/conditions/base/time.ts index ad4093d48..8b136e4a7 100644 --- a/src/conditions/base/time.ts +++ b/src/conditions/base/time.ts @@ -1,13 +1,15 @@ import Joi from 'joi'; +import { omit } from '../../utils'; + import { RpcCondition, rpcConditionSchema } from './rpc'; const BLOCKTIME_METHOD = 'blocktime'; const timeConditionSchema = { - ...rpcConditionSchema, + // TimeCondition is an RpcCondition with the method set to 'blocktime' and no parameters + ...omit(rpcConditionSchema, ['parameters']), method: Joi.string().valid(BLOCKTIME_METHOD).required(), - parameters: undefined, // TimeCondition does not accept parameters }; export class TimeCondition extends RpcCondition { diff --git a/src/utils.ts b/src/utils.ts index 8b80d5463..722e98488 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -67,3 +67,9 @@ export const bytesEquals = (first: Uint8Array, second: Uint8Array): boolean => export const objectEquals = (a: unknown, b: unknown, strict = true): boolean => deepEqual(a, b, { strict }); + +export const omit = (obj: Record, keys: string[]) => { + const copy = { ...obj }; + keys.forEach((key) => delete copy[key]); + return copy; +};