Skip to content

Commit

Permalink
fix(api-kit): Fix e2e tests for multisig transactions (safe-global#919)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
tmjssz authored Jul 19, 2024
1 parent ea0d501 commit 0f22ff1
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 22 deletions.
49 changes: 39 additions & 10 deletions packages/api-kit/tests/e2e/confirmTransaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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] })
Expand Down Expand Up @@ -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'
}
])
})
})
12 changes: 6 additions & 6 deletions packages/api-kit/tests/e2e/getMultisigTransactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
12 changes: 6 additions & 6 deletions packages/api-kit/tests/e2e/getPendingTransactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})

0 comments on commit 0f22ff1

Please sign in to comment.