From 9db4f0fc00fac7c7db4d61737cc37ef5f3929ff6 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Wed, 14 Feb 2024 02:46:19 -0600 Subject: [PATCH] Fix handling of nameless parameters, add more testing (#26) * Nit * Add sinon as an optional peer dependency too use the stubs * Fix friendlyToArray for empty name params * Add testing for utils and use `describe` to format output * Add changeset --- .changeset/kind-news-admire.md | 5 + packages/evm-client-viem/package.json | 6 + packages/evm-client/package.json | 10 +- .../evm-client/src/base/testing/IERC20.ts | 20 +-- .../factories/CachedReadContract.test.ts | 152 ++++++++-------- .../contract/stubs/ReadContractStub.test.ts | 162 +++++++++--------- .../stubs/ReadWriteContractStub.test.ts | 26 +-- .../contract/utils/arrayToFriendly.test.ts | 67 ++++++++ .../contract/utils/friendlyToArray.test.ts | 67 ++++++++ .../src/contract/utils/friendlyToArray.ts | 6 +- 10 files changed, 340 insertions(+), 181 deletions(-) create mode 100644 .changeset/kind-news-admire.md create mode 100644 packages/evm-client/src/contract/utils/arrayToFriendly.test.ts create mode 100644 packages/evm-client/src/contract/utils/friendlyToArray.test.ts diff --git a/.changeset/kind-news-admire.md b/.changeset/kind-news-admire.md new file mode 100644 index 00000000..4de7c6b8 --- /dev/null +++ b/.changeset/kind-news-admire.md @@ -0,0 +1,5 @@ +--- +"@delvtech/evm-client": patch +--- + +Fix argument handling for parameters that have empty strings as names diff --git a/packages/evm-client-viem/package.json b/packages/evm-client-viem/package.json index a93ee647..f3cc1e98 100644 --- a/packages/evm-client-viem/package.json +++ b/packages/evm-client-viem/package.json @@ -30,8 +30,14 @@ "watch": "tsup --watch" }, "peerDependencies": { + "sinon": "^17.0.1", "viem": "^2" }, + "peerDependenciesMeta": { + "sinon": { + "optional": true + } + }, "dependencies": { "@delvtech/evm-client": "0.0.6" }, diff --git a/packages/evm-client/package.json b/packages/evm-client/package.json index 38e96285..9f070549 100644 --- a/packages/evm-client/package.json +++ b/packages/evm-client/package.json @@ -52,11 +52,19 @@ "scripts": { "build": "tsup", "lint": "eslint .", - "test:watch": "vitest", + "test:watch": "vitest --reporter=verbose", "test": "vitest run", "typecheck": "tsc --noEmit", "watch": "tsup --watch" }, + "peerDependencies": { + "sinon": "^17.0.1" + }, + "peerDependenciesMeta": { + "sinon": { + "optional": true + } + }, "dependencies": { "lru-cache": "^10.0.1" }, diff --git a/packages/evm-client/src/base/testing/IERC20.ts b/packages/evm-client/src/base/testing/IERC20.ts index 0363d599..2aad6b84 100644 --- a/packages/evm-client/src/base/testing/IERC20.ts +++ b/packages/evm-client/src/base/testing/IERC20.ts @@ -18,11 +18,11 @@ export const IERC20 = { constant: false, inputs: [ { - name: '_spender', + name: 'spender', type: 'address', }, { - name: '_value', + name: 'value', type: 'uint256', }, ], @@ -55,15 +55,15 @@ export const IERC20 = { constant: false, inputs: [ { - name: '_from', + name: 'from', type: 'address', }, { - name: '_to', + name: 'to', type: 'address', }, { - name: '_value', + name: 'value', type: 'uint256', }, ], @@ -96,7 +96,7 @@ export const IERC20 = { constant: true, inputs: [ { - name: '_owner', + name: 'owner', type: 'address', }, ], @@ -129,11 +129,11 @@ export const IERC20 = { constant: false, inputs: [ { - name: '_to', + name: 'to', type: 'address', }, { - name: '_value', + name: 'value', type: 'uint256', }, ], @@ -152,11 +152,11 @@ export const IERC20 = { constant: true, inputs: [ { - name: '_owner', + name: 'owner', type: 'address', }, { - name: '_spender', + name: 'spender', type: 'address', }, ], diff --git a/packages/evm-client/src/contract/factories/CachedReadContract.test.ts b/packages/evm-client/src/contract/factories/CachedReadContract.test.ts index eb54646a..a4d6ac91 100644 --- a/packages/evm-client/src/contract/factories/CachedReadContract.test.ts +++ b/packages/evm-client/src/contract/factories/CachedReadContract.test.ts @@ -2,100 +2,102 @@ import { IERC20 } from 'src/base/testing/IERC20'; import { ALICE, BOB } from 'src/base/testing/accounts'; import { ReadContractStub } from 'src/contract/stubs/ReadContractStub'; import { Event } from 'src/contract/types/Event'; -import { expect, test } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { createCachedReadContract } from './createCachedReadContract'; const ERC20ABI = IERC20.abi; -test('It caches the read function', async () => { - const contract = new ReadContractStub(ERC20ABI); - const cachedContract = createCachedReadContract({ contract }); +describe('createCachedReadContract', () => { + it('caches the read function', async () => { + const contract = new ReadContractStub(ERC20ABI); + const cachedContract = createCachedReadContract({ contract }); - const stubbedValue = '0x123abc'; - contract.stubRead({ - functionName: 'name', - value: stubbedValue, - }); + const stubbedValue = '0x123abc'; + contract.stubRead({ + functionName: 'name', + value: stubbedValue, + }); - const value = await cachedContract.read('name'); - expect(value).toBe(stubbedValue); + const value = await cachedContract.read('name'); + expect(value).toBe(stubbedValue); - const value2 = await cachedContract.read('name'); - expect(value2).toBe(stubbedValue); + const value2 = await cachedContract.read('name'); + expect(value2).toBe(stubbedValue); - const stub = contract.getReadStub('name'); - expect(stub?.callCount).toBe(1); -}); + const stub = contract.getReadStub('name'); + expect(stub?.callCount).toBe(1); + }); -test('It caches the getEvents function', async () => { - const contract = new ReadContractStub(ERC20ABI); - const cachedContract = createCachedReadContract({ contract }); - - const stubbedEvents: Event[] = [ - { - eventName: 'Transfer', - args: { - from: ALICE, - to: BOB, - value: 100n, + it('caches the getEvents function', async () => { + const contract = new ReadContractStub(ERC20ABI); + const cachedContract = createCachedReadContract({ contract }); + + const stubbedEvents: Event[] = [ + { + eventName: 'Transfer', + args: { + from: ALICE, + to: BOB, + value: 100n, + }, + blockNumber: 1n, + data: '0x123abc', + transactionHash: '0x123abc', }, - blockNumber: 1n, - data: '0x123abc', - transactionHash: '0x123abc', - }, - ]; - contract.stubEvents('Transfer', stubbedEvents); + ]; + contract.stubEvents('Transfer', stubbedEvents); - const events = await cachedContract.getEvents('Transfer'); - expect(events).toBe(stubbedEvents); - - const events2 = await cachedContract.getEvents('Transfer'); - expect(events2).toBe(stubbedEvents); - - const stub = contract.getEventsStub('Transfer'); - expect(stub?.callCount).toBe(1); -}); + const events = await cachedContract.getEvents('Transfer'); + expect(events).toBe(stubbedEvents); -test('The deleteRead function deletes the cached read value', async () => { - const contract = new ReadContractStub(ERC20ABI); - const cachedContract = createCachedReadContract({ contract }); + const events2 = await cachedContract.getEvents('Transfer'); + expect(events2).toBe(stubbedEvents); - const stubbedValue = 100n; - contract.stubRead({ functionName: 'balanceOf', value: stubbedValue }); + const stub = contract.getEventsStub('Transfer'); + expect(stub?.callCount).toBe(1); + }); - const value = await cachedContract.read('balanceOf', '0x123abc'); - expect(value).toBe(stubbedValue); + it('deletes cached reads', async () => { + const contract = new ReadContractStub(ERC20ABI); + const cachedContract = createCachedReadContract({ contract }); - cachedContract.deleteRead('balanceOf', '0x123abc'); + const stubbedValue = 100n; + contract.stubRead({ functionName: 'balanceOf', value: stubbedValue }); - const value2 = await cachedContract.read('balanceOf', '0x123abc'); - expect(value2).toBe(stubbedValue); + const value = await cachedContract.read('balanceOf', '0x123abc'); + expect(value).toBe(stubbedValue); - const stub = contract.getReadStub('balanceOf'); - expect(stub?.callCount).toBe(2); -}); + cachedContract.deleteRead('balanceOf', '0x123abc'); -test('It clears the cache', async () => { - const contract = new ReadContractStub(ERC20ABI); - const cachedContract = createCachedReadContract({ contract }); + const value2 = await cachedContract.read('balanceOf', '0x123abc'); + expect(value2).toBe(stubbedValue); - contract.stubRead({ functionName: 'balanceOf', value: 100n }); - const stubbedValue = '0x123abc'; - contract.stubRead({ - functionName: 'name', - value: stubbedValue, + const stub = contract.getReadStub('balanceOf'); + expect(stub?.callCount).toBe(2); }); - 2; - await cachedContract.read('balanceOf', '0x123abc'); - await cachedContract.read('name'); - - cachedContract.clearCache(); - await cachedContract.read('balanceOf', '0x123abc'); - await cachedContract.read('name'); - - const stubA = contract.getReadStub('balanceOf'); - const stubB = contract.getReadStub('name'); - expect(stubA?.callCount).toBe(2); - expect(stubB?.callCount).toBe(2); + it('clears the cache', async () => { + const contract = new ReadContractStub(ERC20ABI); + const cachedContract = createCachedReadContract({ contract }); + + contract.stubRead({ functionName: 'balanceOf', value: 100n }); + const stubbedValue = '0x123abc'; + contract.stubRead({ + functionName: 'name', + value: stubbedValue, + }); + 2; + await cachedContract.read('balanceOf', '0x123abc'); + await cachedContract.read('name'); + + cachedContract.clearCache(); + + await cachedContract.read('balanceOf', '0x123abc'); + await cachedContract.read('name'); + + const stubA = contract.getReadStub('balanceOf'); + const stubB = contract.getReadStub('name'); + expect(stubA?.callCount).toBe(2); + expect(stubB?.callCount).toBe(2); + }); }); diff --git a/packages/evm-client/src/contract/stubs/ReadContractStub.test.ts b/packages/evm-client/src/contract/stubs/ReadContractStub.test.ts index 4e08e8e0..49ea9612 100644 --- a/packages/evm-client/src/contract/stubs/ReadContractStub.test.ts +++ b/packages/evm-client/src/contract/stubs/ReadContractStub.test.ts @@ -2,98 +2,100 @@ import { IERC20 } from 'src/base/testing/IERC20'; import { ALICE, BOB, NANCY } from 'src/base/testing/accounts'; import { ReadContractStub } from 'src/contract/stubs/ReadContractStub'; import { Event } from 'src/contract/types/Event'; -import { expect, test } from 'vitest'; +import { describe, expect, it } from 'vitest'; const ERC20ABI = IERC20.abi; -test('It stubs the read function', async () => { - const contract = new ReadContractStub(IERC20.abi); - - expect(contract.read('balanceOf', NANCY)).rejects.toThrowError(); - - // Stub bob and alice's balances first - const bobValue = 10n; - contract.stubRead({ - functionName: 'balanceOf', - args: BOB, - value: bobValue, +describe('ReadContractStub', () => { + it('stubs the read function', async () => { + const contract = new ReadContractStub(IERC20.abi); + + expect(contract.read('balanceOf', NANCY)).rejects.toThrowError(); + + // Stub bob and alice's balances first + const bobValue = 10n; + contract.stubRead({ + functionName: 'balanceOf', + args: BOB, + value: bobValue, + }); + + const aliceValue = 20n; + contract.stubRead({ + functionName: 'balanceOf', + args: ALICE, + value: aliceValue, + }); + + // Now try and read them based on their args + const bobResult = await contract.read('balanceOf', BOB); + const aliceResult = await contract.read('balanceOf', ALICE); + expect(bobResult).toBe(bobValue); + expect(aliceResult).toBe(aliceValue); + + // Now stub w/out any args and see if we get the default value back + const defaultValue = 30n; + contract.stubRead({ + functionName: 'balanceOf', + value: defaultValue, + }); + const defaultResult = await contract.read('balanceOf', NANCY); + expect(defaultResult).toBe(defaultValue); + + const stub = contract.getReadStub('balanceOf'); + expect(stub?.callCount).toBe(3); }); - const aliceValue = 20n; - contract.stubRead({ - functionName: 'balanceOf', - args: ALICE, - value: aliceValue, - }); + it('stubs the simulateWrite function', async () => { + const contract = new ReadContractStub(ERC20ABI); - // Now try and read them based on their args - const bobResult = await contract.read('balanceOf', BOB); - const aliceResult = await contract.read('balanceOf', ALICE); - expect(bobResult).toBe(bobValue); - expect(aliceResult).toBe(aliceValue); - - // Now stub w/out any args and see if we get the default value back - const defaultValue = 30n; - contract.stubRead({ - functionName: 'balanceOf', - value: defaultValue, - }); - const defaultResult = await contract.read('balanceOf', NANCY); - expect(defaultResult).toBe(defaultValue); - - const stub = contract.getReadStub('balanceOf'); - expect(stub?.callCount).toBe(3); -}); + expect( + contract.simulateWrite('transferFrom', { + from: ALICE, + to: BOB, + value: 100n, + }), + ).rejects.toThrowError(); -test('It stubs the simulateWrite function', async () => { - const contract = new ReadContractStub(ERC20ABI); + const stubbedResult = true; + contract.stubSimulateWrite('transferFrom', stubbedResult); - expect( - contract.simulateWrite('transferFrom', { - _from: ALICE, - _to: BOB, - _value: 100n, - }), - ).rejects.toThrowError(); + const result = await contract.simulateWrite('transferFrom', { + from: ALICE, + to: BOB, + value: 100n, + }); - const stubbedResult = true; - contract.stubSimulateWrite('transferFrom', stubbedResult); + expect(result).toStrictEqual(stubbedResult); - const result = await contract.simulateWrite('transferFrom', { - _from: ALICE, - _to: BOB, - _value: 100n, + const stub = contract.getSimulateWriteStub('transferFrom'); + expect(stub?.callCount).toBe(1); }); - expect(result).toStrictEqual(stubbedResult); - - const stub = contract.getSimulateWriteStub('transferFrom'); - expect(stub?.callCount).toBe(1); -}); - -test('It stubs the getEvents function', async () => { - const contract = new ReadContractStub(ERC20ABI); + it('stubs the getEvents function', async () => { + const contract = new ReadContractStub(ERC20ABI); + + expect(contract.getEvents('Transfer')).rejects.toThrowError(); + + const stubbedEvents: Event[] = [ + { + eventName: 'Transfer', + args: { + to: ALICE, + from: BOB, + value: 100n, + }, + blockNumber: 1n, + data: '0x123abc', + transactionHash: '0x123abc', + }, + ]; + contract.stubEvents('Transfer', stubbedEvents); - expect(contract.getEvents('Transfer')).rejects.toThrowError(); + const events = await contract.getEvents('Transfer'); + expect(events).toBe(stubbedEvents); - const stubbedEvents: Event[] = [ - { - eventName: 'Transfer', - args: { - to: ALICE, - from: BOB, - value: 100n, - }, - blockNumber: 1n, - data: '0x123abc', - transactionHash: '0x123abc', - }, - ]; - contract.stubEvents('Transfer', stubbedEvents); - - const events = await contract.getEvents('Transfer'); - expect(events).toBe(stubbedEvents); - - const stub = contract.getEventsStub('Transfer'); - expect(stub?.callCount).toBe(1); + const stub = contract.getEventsStub('Transfer'); + expect(stub?.callCount).toBe(1); + }); }); diff --git a/packages/evm-client/src/contract/stubs/ReadWriteContractStub.test.ts b/packages/evm-client/src/contract/stubs/ReadWriteContractStub.test.ts index d7c02db7..295cef3f 100644 --- a/packages/evm-client/src/contract/stubs/ReadWriteContractStub.test.ts +++ b/packages/evm-client/src/contract/stubs/ReadWriteContractStub.test.ts @@ -1,21 +1,23 @@ import { IERC20 } from 'src/base/testing/IERC20'; import { ReadWriteContractStub } from 'src/contract/stubs/ReadWriteContractStub'; -import { expect, test } from 'vitest'; +import { describe, expect, it } from 'vitest'; const ERC20ABI = IERC20.abi; -test('It stubs the write function', async () => { - const contract = new ReadWriteContractStub(ERC20ABI); +describe('ReadWriteContractStub', () => { + it('stubs the write function', async () => { + const contract = new ReadWriteContractStub(ERC20ABI); - const stubbedValue = '0x01234'; - contract.stubWrite('transfer', stubbedValue); + const stubbedValue = '0x01234'; + contract.stubWrite('transfer', stubbedValue); - const value = await contract.write('transfer', { - _to: '0x123abc', - _value: 100n, - }); - expect(value).toBe(stubbedValue); + const value = await contract.write('transfer', { + to: '0x123abc', + value: 100n, + }); + expect(value).toBe(stubbedValue); - const stub = contract.getWriteStub('transfer'); - expect(stub?.callCount).toBe(1); + const stub = contract.getWriteStub('transfer'); + expect(stub?.callCount).toBe(1); + }); }); diff --git a/packages/evm-client/src/contract/utils/arrayToFriendly.test.ts b/packages/evm-client/src/contract/utils/arrayToFriendly.test.ts new file mode 100644 index 00000000..52ac1873 --- /dev/null +++ b/packages/evm-client/src/contract/utils/arrayToFriendly.test.ts @@ -0,0 +1,67 @@ +import { IERC20 } from 'src/base/testing/IERC20'; +import { arrayToFriendly } from 'src/contract/utils/arrayToFriendly'; +import { describe, expect, it } from 'vitest'; + +describe('arrayToFriendly', () => { + it('correctly converts arrays with multiple items into objects', async () => { + const transferArgsObject = arrayToFriendly({ + abi: IERC20.abi, + type: 'function', + name: 'transfer', + kind: 'inputs', + values: ['0x123', 123n], + }); + expect(transferArgsObject).toEqual({ + to: '0x123', + value: 123n, + }); + + // empty parameter names (index keys) + const votesArgsObject = arrayToFriendly({ + abi: exampleAbi, + type: 'function', + name: 'votes', + kind: 'inputs', + values: ['0x123', 0n], + }); + expect(votesArgsObject).toEqual({ + '0': '0x123', + '1': 0n, + }); + }); + + it('returns the item from arrays with a single item', async () => { + const balanceInput = arrayToFriendly({ + abi: IERC20.abi, + type: 'function', + name: 'balanceOf', + kind: 'inputs', + values: ['0x123'], + }); + expect(balanceInput).toEqual('0x123'); + }); + + it('Converts an empty arrays into undefined', async () => { + const notDefined = arrayToFriendly({ + abi: IERC20.abi, + type: 'function', + name: 'symbol', + kind: 'inputs', + values: [], + }); + expect(notDefined).toBeUndefined(); + }); +}); + +export const exampleAbi = [ + { + inputs: [ + { name: '', type: 'address' }, + { name: '', type: 'uint256' }, + ], + name: 'votes', + outputs: [], + stateMutability: 'view', + type: 'function', + }, +] as const; diff --git a/packages/evm-client/src/contract/utils/friendlyToArray.test.ts b/packages/evm-client/src/contract/utils/friendlyToArray.test.ts new file mode 100644 index 00000000..31ca1bc9 --- /dev/null +++ b/packages/evm-client/src/contract/utils/friendlyToArray.test.ts @@ -0,0 +1,67 @@ +import { IERC20 } from 'src/base/testing/IERC20'; +import { friendlyToArray } from 'src/contract/utils/friendlyToArray'; +import { describe, expect, it } from 'vitest'; + +describe('friendlyToArray', () => { + it('correctly converts object into arrays', async () => { + const transferArgsArray = friendlyToArray({ + abi: IERC20.abi, + type: 'function', + name: 'transfer', + kind: 'inputs', + value: { + to: '0x123', + value: 123n, + }, + }); + expect(transferArgsArray).toEqual(['0x123', 123n]); + + // empty parameter names (index keys) + const votesArgsArray = friendlyToArray({ + abi: exampleAbi, + type: 'function', + name: 'votes', + kind: 'inputs', + value: { + '0': '0x123', + '1': 0n, + }, + }); + expect(votesArgsArray).toEqual(['0x123', 0n]); + }); + + it('correctly converts single values into arrays', async () => { + const balanceInputArray = friendlyToArray({ + abi: IERC20.abi, + type: 'function', + name: 'balanceOf', + kind: 'inputs', + value: '0x123', + }); + expect(balanceInputArray).toEqual(['0x123']); + }); + + it('converts undefined into an empty array', async () => { + const emptyArray = friendlyToArray({ + abi: IERC20.abi, + type: 'function', + name: 'symbol', + kind: 'inputs', + value: undefined, + }); + expect(emptyArray).toEqual([]); + }); +}); + +export const exampleAbi = [ + { + inputs: [ + { name: '', type: 'address' }, + { name: '', type: 'uint256' }, + ], + name: 'votes', + outputs: [], + stateMutability: 'view', + type: 'function', + }, +] as const; diff --git a/packages/evm-client/src/contract/utils/friendlyToArray.ts b/packages/evm-client/src/contract/utils/friendlyToArray.ts index efc2db71..e90d34bb 100644 --- a/packages/evm-client/src/contract/utils/friendlyToArray.ts +++ b/packages/evm-client/src/contract/utils/friendlyToArray.ts @@ -81,17 +81,17 @@ export function friendlyToArray< return [] as AbiArrayType; } - // Single parameters + // Single parameter if (parameters.length === 1) { return [value] as AbiArrayType; } const valueObject: Record = - !!value && typeof value === 'object' ? value : {}; + value && typeof value === 'object' ? value : {}; const array: unknown[] = []; parameters.forEach(({ name }, i) => { - array.push(valueObject[name ?? i]); + array.push(valueObject[name || i]); }); return array as AbiArrayType;