From 0f22ff1015caaeb43fafd4aef14bafc061ed2347 Mon Sep 17 00:00:00 2001 From: Tim <4171783+tmjssz@users.noreply.github.com> Date: Fri, 19 Jul 2024 17:25:25 +0200 Subject: [PATCH] fix(api-kit): Fix e2e tests for multisig transactions (#919) * Fix + improve the confirm transaction e2e test The test was incorrect because it used the same transaction hash repeatedly, leading to assertions being made on an already confirmed transaction instead of a newly proposed one. By adding a timestamp to the transaction data, we ensure that each transaction hash is unique. Additionally, the test incorrectly called the `isValidSignature` function with a transaction hash. This function is intended only for messages, causing the contract call to fail consistently. However, the error was not properly caught; the function merely returns `false` when an error occurs. With the migration to viem, the `isValidSignature` contract call sometimes takes too long to fail, resulting in occasional test timeouts. By removing the incorrect call, we prevent those timeout errors. * Fix tests by using another Safe address that is not used by the confirmTransaction test --- .../tests/e2e/confirmTransaction.test.ts | 49 +++++++++++++++---- .../tests/e2e/getMultisigTransactions.test.ts | 12 ++--- .../tests/e2e/getPendingTransactions.test.ts | 12 ++--- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/packages/api-kit/tests/e2e/confirmTransaction.test.ts b/packages/api-kit/tests/e2e/confirmTransaction.test.ts index c2878e7f5..7da0f4abe 100644 --- a/packages/api-kit/tests/e2e/confirmTransaction.test.ts +++ b/packages/api-kit/tests/e2e/confirmTransaction.test.ts @@ -4,16 +4,20 @@ import Safe, { SigningMethod, buildContractSignature } from '@safe-global/protocol-kit' -import { SafeTransactionDataPartial } from '@safe-global/safe-core-sdk-types' +import { + SafeMultisigConfirmationResponse, + SafeTransactionDataPartial +} from '@safe-global/safe-core-sdk-types' import SafeApiKit from '@safe-global/api-kit/index' import chai from 'chai' import chaiAsPromised from 'chai-as-promised' +import { toBytes, toHex } from 'viem' import { getKits } from '../utils/setupKits' chai.use(chaiAsPromised) -const PRIVATE_KEY_1 = '0x83a415ca62e11f5fa5567e98450d0f82ae19ff36ef876c10a8d448c788a53676' -const PRIVATE_KEY_2 = '0xb88ad5789871315d0dab6fc5961d6714f24f35a6393f13a6f426dfecfc00ab44' +const PRIVATE_KEY_1 = '0x83a415ca62e11f5fa5567e98450d0f82ae19ff36ef876c10a8d448c788a53676' // Address: 0x56e2C102c664De6DfD7315d12c0178b61D16F171 +const PRIVATE_KEY_2 = '0xb88ad5789871315d0dab6fc5961d6714f24f35a6393f13a6f426dfecfc00ab44' // Address: 0x9ccbde03edd71074ea9c49e413fa9cdff16d263b let safeApiKit: SafeApiKit let protocolKit: Safe @@ -32,8 +36,9 @@ describe('proposeTransaction', () => { it('should allow to create and confirm transactions signature using a Safe signer', async () => { const safeTransactionData: SafeTransactionDataPartial = { to: safeAddress, - value: '100000000000000000', // 0.01 ETH - data: '0x' + value: '10000000000000000', // 0.01 ETH + // We generate unique data from the current timestamp to receive a different tx hash each time + data: toHex(toBytes(Date.now())) } let tx = await protocolKit.createTransaction({ transactions: [safeTransactionData] }) @@ -90,14 +95,38 @@ describe('proposeTransaction', () => { safeAddress }) - const isValidSignature = await protocolKit.isValidSignature(txHash, [ethSig, signerSafeSig]) - console.log('- isValidSignature(txHash, signature) = ', isValidSignature) - // chai.expect(isValidSignature).to.be.true - const contractSig = buildSignatureBytes([signerSafeSig]) + await chai.expect(safeApiKit.confirmTransaction(txHash, contractSig)).to.be.fulfilled const confirmedMessage = await safeApiKit.getTransaction(txHash) - chai.expect(confirmedMessage?.confirmations?.length).to.eq(2) + + chai.expect(confirmedMessage.confirmations?.length).to.eq(2) + + const [confirmation1, confirmation2] = confirmedMessage!.confirmations as [ + a: SafeMultisigConfirmationResponse, + b: SafeMultisigConfirmationResponse + ] + + // Check that the submission date is within the last minute + chai.expect(Date.now() - new Date(confirmation1.submissionDate).valueOf()).lte(60000) + chai.expect(Date.now() - new Date(confirmation2.submissionDate).valueOf()).lte(60000) + + chai.expect(confirmedMessage.confirmations).to.deep.eq([ + { + owner: signerAddress, + submissionDate: confirmation1.submissionDate, + transactionHash: null, + signature: ethSig.data, + signatureType: 'EOA' + }, + { + owner: signerSafeAddress, + submissionDate: confirmation2.submissionDate, + transactionHash: null, + signature: contractSig.toLowerCase(), + signatureType: 'CONTRACT_SIGNATURE' + } + ]) }) }) diff --git a/packages/api-kit/tests/e2e/getMultisigTransactions.test.ts b/packages/api-kit/tests/e2e/getMultisigTransactions.test.ts index d617831a0..630f4a803 100644 --- a/packages/api-kit/tests/e2e/getMultisigTransactions.test.ts +++ b/packages/api-kit/tests/e2e/getMultisigTransactions.test.ts @@ -36,23 +36,23 @@ describe('getMultisigTransactions', () => { }) it('should return the list of multisig transactions', async () => { - const safeAddress = '0xF8ef84392f7542576F6b9d1b140334144930Ac78' // Safe with multisig transactions + const safeAddress = '0xCa2f5A815b642c79FC530B60BC15Aee4eF6252b3' // Safe with multisig transactions const safeMultisigTransactionListResponse = await safeApiKit.getMultisigTransactions(safeAddress) - chai.expect(safeMultisigTransactionListResponse.count).to.be.equal(22) - chai.expect(safeMultisigTransactionListResponse.results.length).to.be.equal(22) + chai.expect(safeMultisigTransactionListResponse.count).to.be.equal(10) + chai.expect(safeMultisigTransactionListResponse.results.length).to.be.equal(10) safeMultisigTransactionListResponse.results.map((transaction) => { chai.expect(transaction.safe).to.be.equal(safeAddress) }) }) it('should return the list of multisig transactions EIP-3770', async () => { - const safeAddress = '0xF8ef84392f7542576F6b9d1b140334144930Ac78' // Safe with multisig transactions + const safeAddress = '0xCa2f5A815b642c79FC530B60BC15Aee4eF6252b3' // Safe with multisig transactions const eip3770SafeAddress = `${config.EIP_3770_PREFIX}:${safeAddress}` const safeMultisigTransactionListResponse = await safeApiKit.getMultisigTransactions(eip3770SafeAddress) - chai.expect(safeMultisigTransactionListResponse.count).to.be.equal(22) - chai.expect(safeMultisigTransactionListResponse.results.length).to.be.equal(22) + chai.expect(safeMultisigTransactionListResponse.count).to.be.equal(10) + chai.expect(safeMultisigTransactionListResponse.results.length).to.be.equal(10) safeMultisigTransactionListResponse.results.map((transaction) => { chai.expect(transaction.safe).to.be.equal(safeAddress) }) diff --git a/packages/api-kit/tests/e2e/getPendingTransactions.test.ts b/packages/api-kit/tests/e2e/getPendingTransactions.test.ts index e8d4e3938..e136ae8b4 100644 --- a/packages/api-kit/tests/e2e/getPendingTransactions.test.ts +++ b/packages/api-kit/tests/e2e/getPendingTransactions.test.ts @@ -35,17 +35,17 @@ describe('getPendingTransactions', () => { }) it('should return the the transaction list', async () => { - const safeAddress = '0xF8ef84392f7542576F6b9d1b140334144930Ac78' // Safe with pending transaction + const safeAddress = '0xCa2f5A815b642c79FC530B60BC15Aee4eF6252b3' // Safe with pending transaction const transactionList = await safeApiKit.getPendingTransactions(safeAddress) - chai.expect(transactionList.count).to.be.equal(3) - chai.expect(transactionList.results.length).to.be.equal(3) + chai.expect(transactionList.count).to.be.equal(10) + chai.expect(transactionList.results.length).to.be.equal(10) }) it('should return the the transaction list EIP-3770', async () => { - const safeAddress = '0xF8ef84392f7542576F6b9d1b140334144930Ac78' // Safe with pending transaction + const safeAddress = '0xCa2f5A815b642c79FC530B60BC15Aee4eF6252b3' // Safe with pending transaction const eip3770SafeAddress = `${config.EIP_3770_PREFIX}:${safeAddress}` const transactionList = await safeApiKit.getPendingTransactions(eip3770SafeAddress) - chai.expect(transactionList.count).to.be.equal(3) - chai.expect(transactionList.results.length).to.be.equal(3) + chai.expect(transactionList.count).to.be.equal(10) + chai.expect(transactionList.results.length).to.be.equal(10) }) })