Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: prepare release #941

Merged
merged 4 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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
  • Loading branch information
tmjssz authored Jul 19, 2024
commit 0f22ff1015caaeb43fafd4aef14bafc061ed2347
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the comment on the right-hand side still correct? You could use the viem#parseEther and remove the comment altogether.

// 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)
})
})