diff --git a/packages/api-kit/src/SafeApiKit.ts b/packages/api-kit/src/SafeApiKit.ts index 406e56318..60f857e95 100644 --- a/packages/api-kit/src/SafeApiKit.ts +++ b/packages/api-kit/src/SafeApiKit.ts @@ -785,13 +785,14 @@ class SafeApiKit { * @throws "Invalid Safe address {safeAddress}" * @throws "Module address must not be empty" * @throws "Invalid module address {moduleAddress}" - * @throws "SafeOperation is not signed by the given signer {signerAddress}" + * @throws "Signature must not be empty" */ async addSafeOperation({ + entryPoint, moduleAddress: moduleAddressProp, + options, safeAddress: safeAddressProp, - safeOperation, - signer + userOperation }: AddSafeOperationProps): Promise { let safeAddress: string, moduleAddress: string @@ -814,32 +815,29 @@ class SafeApiKit { throw new Error(`Invalid module address ${moduleAddressProp}`) } - const signerAddress = await signer.getAddress() - const signature = safeOperation.getSignature(signerAddress) - - if (!signature) { - throw new Error(`SafeOperation is not signed by the given signer ${signerAddress}`) + if (isEmptyData(userOperation.signature)) { + throw new Error('Signature must not be empty') } - const { data } = safeOperation - return sendRequest({ url: `${this.#txServiceBaseUrl}/v1/safes/${safeAddress}/safe-operations/`, method: HttpMethod.Post, body: { - nonce: Number(data.nonce), - initCode: isEmptyData(data.initCode) ? null : data.initCode, - callData: data.callData, - callDataGasLimit: data.callGasLimit.toString(), - verificationGasLimit: data.verificationGasLimit.toString(), - preVerificationGas: data.preVerificationGas.toString(), - maxFeePerGas: data.maxFeePerGas.toString(), - maxPriorityFeePerGas: data.maxPriorityFeePerGas.toString(), - paymasterAndData: isEmptyData(data.paymasterAndData) ? null : data.paymasterAndData, - entryPoint: data.entryPoint, - validAfter: !data.validAfter ? null : data.validAfter, - validUntil: !data.validUntil ? null : data.validUntil, - signature: signature.data, + nonce: Number(userOperation.nonce), + initCode: isEmptyData(userOperation.initCode) ? null : userOperation.initCode, + callData: userOperation.callData, + callDataGasLimit: userOperation.callGasLimit.toString(), + verificationGasLimit: userOperation.verificationGasLimit.toString(), + preVerificationGas: userOperation.preVerificationGas.toString(), + maxFeePerGas: userOperation.maxFeePerGas.toString(), + maxPriorityFeePerGas: userOperation.maxPriorityFeePerGas.toString(), + paymasterAndData: isEmptyData(userOperation.paymasterAndData) + ? null + : userOperation.paymasterAndData, + entryPoint, + validAfter: !options?.validAfter ? null : options?.validAfter, + validUntil: !options?.validUntil ? null : options?.validUntil, + signature: userOperation.signature, moduleAddress } }) diff --git a/packages/api-kit/src/types/safeTransactionServiceTypes.ts b/packages/api-kit/src/types/safeTransactionServiceTypes.ts index ed3b83523..755e4995c 100644 --- a/packages/api-kit/src/types/safeTransactionServiceTypes.ts +++ b/packages/api-kit/src/types/safeTransactionServiceTypes.ts @@ -1,8 +1,8 @@ import { Signer, TypedDataDomain, TypedDataField } from 'ethers' import { SafeMultisigTransactionResponse, - SafeOperation, - SafeTransactionData + SafeTransactionData, + UserOperation } from '@safe-global/safe-core-sdk-types' export type SafeServiceInfoResponse = { @@ -342,12 +342,19 @@ export type GetSafeOperationListResponse = { } export type AddSafeOperationProps = { + /** Address of the EntryPoint contract */ + entryPoint: string /** Address of the Safe4337Module contract */ moduleAddress: string /** Address of the Safe to add a SafeOperation for */ safeAddress: string - /** Signed SafeOperation object to add */ - safeOperation: SafeOperation - /** Signer instance */ - signer: Signer + /** UserOperation object to add */ + userOperation: UserOperation + /** Options object */ + options?: { + /** The UserOperation will remain valid until this block's timestamp */ + validUntil?: number + /** The UserOperation will be valid after this block's timestamp */ + validAfter?: number + } } diff --git a/packages/api-kit/tests/e2e/addSafeOperation.test.ts b/packages/api-kit/tests/e2e/addSafeOperation.test.ts index fdd19b271..0163ff825 100644 --- a/packages/api-kit/tests/e2e/addSafeOperation.test.ts +++ b/packages/api-kit/tests/e2e/addSafeOperation.test.ts @@ -3,7 +3,7 @@ import chaiAsPromised from 'chai-as-promised' import { Wallet, ethers } from 'ethers' import sinon from 'sinon' import sinonChai from 'sinon-chai' -import { EthAdapter } from '@safe-global/safe-core-sdk-types' +import { EthAdapter, SafeOperation } from '@safe-global/safe-core-sdk-types' import SafeApiKit from '@safe-global/api-kit' import { Safe4337Pack } from '@safe-global/relay-kit' import { generateTransferCallData } from '@safe-global/relay-kit/src/packs/safe-4337/testing-utils/helpers' @@ -49,6 +49,13 @@ describe('addSafeOperation', () => { .returns( Promise.resolve({ fast: { maxFeePerGas: '0x3b9aca00', maxPriorityFeePerGas: '0x3b9aca00' } }) ) + providerStub.withArgs(RPC_4337_CALLS.ESTIMATE_USER_OPERATION_GAS, sinon.match.any).returns( + Promise.resolve({ + preVerificationGas: BigInt(Date.now()), + callGasLimit: BigInt(Date.now()), + verificationGasLimit: BigInt(Date.now()) + }) + ) providerStub.callThrough() @@ -80,18 +87,32 @@ describe('addSafeOperation', () => { })?.networkAddresses[chainId] as string }) + const getAddSafeOperationProps = async (safeOperation: SafeOperation) => { + const userOperation = safeOperation.toUserOperation() + userOperation.signature = safeOperation.encodedSignatures() + return { + entryPoint: safeOperation.data.entryPoint, + moduleAddress, + safeAddress: SAFE_ADDRESS, + userOperation, + options: { + validAfter: safeOperation.data.validAfter, + validUntil: safeOperation.data.validUntil + } + } + } + describe('should fail', () => { it('if safeAddress is empty', async () => { const safeOperation = await safe4337Pack.createTransaction({ transactions: [transferUSDC] }) const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation) + const addSafeOperationProps = await getAddSafeOperationProps(signedSafeOperation) await chai .expect( safeApiKit.addSafeOperation({ - moduleAddress, - safeAddress: '', - safeOperation: signedSafeOperation, - signer + ...addSafeOperationProps, + safeAddress: '' }) ) .to.be.rejectedWith('Safe address must not be empty') @@ -100,14 +121,13 @@ describe('addSafeOperation', () => { it('if safeAddress is invalid', async () => { const safeOperation = await safe4337Pack.createTransaction({ transactions: [transferUSDC] }) const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation) + const addSafeOperationProps = await getAddSafeOperationProps(signedSafeOperation) await chai .expect( safeApiKit.addSafeOperation({ - moduleAddress, - safeAddress: '0x123', - safeOperation: signedSafeOperation, - signer + ...addSafeOperationProps, + safeAddress: '0x123' }) ) .to.be.rejectedWith('Invalid Safe address 0x123') @@ -116,14 +136,13 @@ describe('addSafeOperation', () => { it('if moduleAddress is empty', async () => { const safeOperation = await safe4337Pack.createTransaction({ transactions: [transferUSDC] }) const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation) + const addSafeOperationProps = await getAddSafeOperationProps(signedSafeOperation) await chai .expect( safeApiKit.addSafeOperation({ - moduleAddress: '', - safeAddress: SAFE_ADDRESS, - safeOperation: signedSafeOperation, - signer + ...addSafeOperationProps, + moduleAddress: '' }) ) .to.be.rejectedWith('Module address must not be empty') @@ -132,14 +151,13 @@ describe('addSafeOperation', () => { it('if moduleAddress is invalid', async () => { const safeOperation = await safe4337Pack.createTransaction({ transactions: [transferUSDC] }) const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation) + const addSafeOperationProps = await getAddSafeOperationProps(signedSafeOperation) await chai .expect( safeApiKit.addSafeOperation({ - moduleAddress: '0x234', - safeAddress: SAFE_ADDRESS, - safeOperation: signedSafeOperation, - signer + ...addSafeOperationProps, + moduleAddress: '0x234' }) ) .to.be.rejectedWith('Invalid module address 0x234') @@ -147,47 +165,26 @@ describe('addSafeOperation', () => { it('if the SafeOperation is not signed', async () => { const safeOperation = await safe4337Pack.createTransaction({ transactions: [transferUSDC] }) + const addSafeOperationProps = await getAddSafeOperationProps(safeOperation) await chai - .expect( - safeApiKit.addSafeOperation({ - moduleAddress, - safeAddress: SAFE_ADDRESS, - safeOperation, - signer - }) - ) - .to.be.rejectedWith( - 'SafeOperation is not signed by the given signer 0x56e2C102c664De6DfD7315d12c0178b61D16F171' - ) + .expect(safeApiKit.addSafeOperation(addSafeOperationProps)) + .to.be.rejectedWith('Signature must not be empty') }) }) it('should add a new SafeOperation', async () => { - providerStub.withArgs(RPC_4337_CALLS.ESTIMATE_USER_OPERATION_GAS, sinon.match.any).returns( - Promise.resolve({ - preVerificationGas: BigInt(Date.now()), - callGasLimit: BigInt(Date.now()), - verificationGasLimit: BigInt(Date.now()) - }) - ) - const safeOperation = await safe4337Pack.createTransaction({ transactions: [transferUSDC] }) const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation) + const addSafeOperationProps = await getAddSafeOperationProps(signedSafeOperation) + // Get the number of SafeOperations before adding a new one const safeOperationsBefore = await safeApiKit.getSafeOperationsByAddress({ safeAddress: SAFE_ADDRESS }) const initialNumSafeOperations = safeOperationsBefore.results.length - await chai.expect( - safeApiKit.addSafeOperation({ - moduleAddress, - safeAddress: SAFE_ADDRESS, - safeOperation: signedSafeOperation, - signer - }) - ).to.be.fulfilled + await chai.expect(safeApiKit.addSafeOperation(addSafeOperationProps)).to.be.fulfilled const safeOperationsAfter = await safeApiKit.getSafeOperationsByAddress({ safeAddress: SAFE_ADDRESS diff --git a/packages/api-kit/tests/endpoint/index.test.ts b/packages/api-kit/tests/endpoint/index.test.ts index 6259f6dd1..8357472de 100644 --- a/packages/api-kit/tests/endpoint/index.test.ts +++ b/packages/api-kit/tests/endpoint/index.test.ts @@ -7,7 +7,7 @@ import SafeApiKit, { } from '@safe-global/api-kit/index' import * as httpRequests from '@safe-global/api-kit/utils/httpRequests' import Safe from '@safe-global/protocol-kit' -import { EthAdapter, SafeOperation } from '@safe-global/safe-core-sdk-types' +import { EthAdapter, UserOperation } from '@safe-global/safe-core-sdk-types' import chai from 'chai' import chaiAsPromised from 'chai-as-promised' import sinon from 'sinon' @@ -669,50 +669,54 @@ describe('Endpoint tests', () => { }) it('addSafeOperation', async () => { - const signature = '0xsignature' const moduleAddress = '0xa581c4A4DB7175302464fF3C06380BC3270b4037' - const safeOperation = { - data: { - nonce: 42, - initCode: '0xfbc38024f74946d9ec31e0c8658dd65e335c6e57c14575250787ec5fb270c08a', - callData: - '0x7bb374280000000000000000000000001c7d4b196cb0c7b01d743fbc6116a902379c72380000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044a9059cbb00000000000000000000000060c4ab82d06fd7dfe9517e17736c2dcc77443ef000000000000000000000000000000000000000000000000000000000000186a000000000000000000000000000000000000000000000000000000000', - callGasLimit: '150799', - verificationGasLimit: '200691', - preVerificationGas: '50943', - maxFeePerGas: '1949282597', - maxPriorityFeePerGas: '1380000000', - paymasterAndData: '0xdff7fa1077bce740a6a212b3995990682c0ba66d', - entryPoint: '0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789', - validAfter: '2024-01-01T00:00:00Z', - validUntil: '2024-04-12T00:00:00Z' - }, - getSignature: () => ({ data: signature }) - } as unknown as SafeOperation + const userOperation: UserOperation = { + sender: safeAddress, + nonce: '42', + initCode: '0xfbc38024f74946d9ec31e0c8658dd65e335c6e57c14575250787ec5fb270c08a', + callData: + '0x7bb374280000000000000000000000001c7d4b196cb0c7b01d743fbc6116a902379c72380000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044a9059cbb00000000000000000000000060c4ab82d06fd7dfe9517e17736c2dcc77443ef000000000000000000000000000000000000000000000000000000000000186a000000000000000000000000000000000000000000000000000000000', + callGasLimit: 150799n, + verificationGasLimit: 200691n, + preVerificationGas: 50943n, + maxFeePerGas: 1949282597n, + maxPriorityFeePerGas: 1380000000n, + paymasterAndData: '0xdff7fa1077bce740a6a212b3995990682c0ba66d', + signature: '0xsignature' + } + + const entryPoint = '0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789' + const options = { validAfter: 123, validUntil: 234 } await chai .expect( safeApiKit.addSafeOperation({ + entryPoint, moduleAddress, + options, safeAddress, - safeOperation, - signer + userOperation }) ) .to.be.eventually.deep.equals({ data: { success: true } }) - // We need to rename the "callGasLimit" prop here because the api endpoint expects it as "callDataGasLimit" - const { callGasLimit: callDataGasLimit, ...safeOperationWithoutCallGasLimit } = - safeOperation.data - chai.expect(fetchData).to.have.been.calledWith({ url: `${txServiceBaseUrl}/v1/safes/${safeAddress}/safe-operations/`, method: 'post', body: { - ...safeOperationWithoutCallGasLimit, - callDataGasLimit, - signature, + nonce: Number(userOperation.nonce), + initCode: userOperation.initCode, + callData: userOperation.callData, + callDataGasLimit: userOperation.callGasLimit.toString(), + verificationGasLimit: userOperation.verificationGasLimit.toString(), + preVerificationGas: userOperation.preVerificationGas.toString(), + maxFeePerGas: userOperation.maxFeePerGas.toString(), + maxPriorityFeePerGas: userOperation.maxPriorityFeePerGas.toString(), + paymasterAndData: userOperation.paymasterAndData, + entryPoint, + ...options, + signature: userOperation.signature, moduleAddress } })