protocol-kit shouldn't be a dep of SafeOperation

packages/api-kit/tests/e2e/addSafeOperation.test.ts

@@ -180,12 +180,11 @@ describe('addSafeOperation', () => {
     chai.expect(safeOperationsAfter.count).to.equal(initialNumSafeOperations + 1)
   })
 
-  it('should add a new SafeOperation using a SafeOperation object from the relay-kit', async () => {
+  it.only('should add a new SafeOperation using a SafeOperation object from the relay-kit', async () => {
     const safeOperation = await safe4337Pack.createTransaction({
       transactions: [transferUSDC, transferUSDC]
     })
     const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation)
-
     // Get the number of SafeOperations before adding a new one
     const safeOperationsBefore = await safeApiKit.getSafeOperationsByAddress({
       safeAddress: SAFE_ADDRESS

packages/api-kit/tests/e2e/confirmSafeOperation.test.ts

@@ -1,7 +1,12 @@
 import chai from 'chai'
 import chaiAsPromised from 'chai-as-promised'
 import sinon from 'sinon'
-import { BundlerClient, Safe4337InitOptions, Safe4337Pack } from '@safe-global/relay-kit'
+import {
+  BundlerClient,
+  Safe4337InitOptions,
+  Safe4337Pack,
+  SafeOperationBase
+} from '@safe-global/relay-kit'
 import { generateTransferCallData } from '@safe-global/relay-kit/packs/safe-4337/testing-utils/helpers'
 import SafeApiKit from '@safe-global/api-kit/index'
 import { getAddSafeOperationProps } from '@safe-global/api-kit/utils/safeOperation'
@@ -16,8 +21,8 @@ import {
 
 chai.use(chaiAsPromised)
 
-const PRIVATE_KEY_1 = '0x83a415ca62e11f5fa5567e98450d0f82ae19ff36ef876c10a8d448c788a53676'
-const PRIVATE_KEY_2 = '0xb88ad5789871315d0dab6fc5961d6714f24f35a6393f13a6f426dfecfc00ab44'
+const PRIVATE_KEY_1 = '0x83a415ca62e11f5fa5567e98450d0f82ae19ff36ef876c10a8d448c788a53676' // 0x56e2C102c664De6DfD7315d12c0178b61D16F171
+const PRIVATE_KEY_2 = '0xb88ad5789871315d0dab6fc5961d6714f24f35a6393f13a6f426dfecfc00ab44' // 0x9cCBDE03eDd71074ea9c49e413FA9CDfF16D263B
 const SAFE_ADDRESS = '0x60C4Ab82D06Fd7dFE9517e17736C2Dcc77443EF0' // 4337 enabled 1/2 Safe (v1.4.1) owned by PRIVATE_KEY_1 + PRIVATE_KEY_2
 const TX_SERVICE_URL = 'https://safe-transaction-sepolia.staging.5afe.dev/api'
 const BUNDLER_URL = `https://bundler.url`
@@ -47,7 +52,9 @@ describe('confirmSafeOperation', () => {
 
   const createSignature = async (safeOperation: SafeOperation, signer: string) => {
     const safe4337Pack = await getSafe4337Pack({ signer })
-    const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation)
+    const signedSafeOperation = await safe4337Pack.signSafeOperation(
+      safeOperation as SafeOperationBase
+    )
     const signerAddress = await safe4337Pack.protocolKit.getSafeProvider().getSignerAddress()
     return signedSafeOperation.getSignature(signerAddress!)
   }

packages/relay-kit/src/packs/safe-4337/Safe4337Pack.ts

@@ -1,4 +1,5 @@
 import semverSatisfies from 'semver/functions/satisfies'
+import { toHex } from 'viem'
 import Safe, {
   EthSafeSignature,
   encodeMultiSendData,
@@ -13,6 +14,7 @@ import {
   OperationType,
   SafeOperationConfirmation,
   SafeOperationResponse,
+  SafeSignature,
   SigningMethod
 } from '@safe-global/types-kit'
 import {
@@ -483,18 +485,14 @@ export class Safe4337Pack extends RelayKitBasePack<{
       userOperation.callData += this.#onchainIdentifier
     }
 
-    const safeOperation = SafeOperationFactory.createSafeOperation(
-      userOperation,
-      this.protocolKit,
-      {
-        chainId: this.#chainId,
-        moduleAddress: this.#SAFE_4337_MODULE_ADDRESS,
-        entryPoint: this.#ENTRYPOINT_ADDRESS,
-        sharedSigner: this.#SAFE_WEBAUTHN_SHARED_SIGNER_ADDRESS,
-        validUntil,
-        validAfter
-      }
-    )
+    const safeOperation = SafeOperationFactory.createSafeOperation(userOperation, {
+      chainId: this.#chainId,
+      moduleAddress: this.#SAFE_4337_MODULE_ADDRESS,
+      entryPoint: this.#ENTRYPOINT_ADDRESS,
+      sharedSigner: this.#SAFE_WEBAUTHN_SHARED_SIGNER_ADDRESS,
+      validUntil,
+      validAfter
+    })
 
     return await this.getEstimateFee({
       safeOperation,
@@ -527,7 +525,6 @@ export class Safe4337Pack extends RelayKitBasePack<{
         paymasterAndData: concat([paymaster, paymasterData]),
         signature: safeOperationResponse.preparedSignature || '0x'
       },
-      this.protocolKit,
       {
         chainId: this.#chainId,
         moduleAddress: this.#SAFE_4337_MODULE_ADDRESS,
@@ -577,7 +574,78 @@ export class Safe4337Pack extends RelayKitBasePack<{
       safeOp = safeOperation
     }
 
-    await safeOp.sign(signingMethod)
+    const safeProvider = this.protocolKit.getSafeProvider()
+    const signerAddress = await safeProvider.getSignerAddress()
+    const isPasskeySigner = await safeProvider.isPasskeySigner()
+
+    if (!signerAddress) {
+      throw new Error('There is no signer address available to sign the SafeOperation')
+    }
+
+    const isOwner = await this.protocolKit.isOwner(signerAddress)
+    const isSafeDeployed = await this.protocolKit.isSafeDeployed()
+
+    if ((!isOwner && isSafeDeployed) || (!isSafeDeployed && !isPasskeySigner && !isOwner)) {
+      throw new Error('UserOperations can only be signed by Safe owners')
+    }
+
+    let safeSignature: SafeSignature
+
+    if (isPasskeySigner) {
+      const safeOpHash = safeOp.getHash()
+
+      if (!isSafeDeployed) {
+        const passkeySignature = await this.protocolKit.signHash(safeOpHash)
+        safeSignature = new EthSafeSignature(
+          this.#SAFE_WEBAUTHN_SHARED_SIGNER_ADDRESS,
+          passkeySignature.data,
+          true
+        )
+      } else {
+        safeSignature = await this.protocolKit.signHash(safeOpHash)
+      }
+    } else {
+      if (
+        [
+          SigningMethod.ETH_SIGN_TYPED_DATA_V4,
+          SigningMethod.ETH_SIGN_TYPED_DATA_V3,
+          SigningMethod.ETH_SIGN_TYPED_DATA
+        ].includes(signingMethod)
+      ) {
+        const signer = await safeProvider.getExternalSigner()
+
+        if (!signer) {
+          throw new Error('No signer found')
+        }
+
+        const signerAddress = signer.account.address
+        const safeOperation = safeOp.getSafeOperation()
+        const signature = await signer.signTypedData({
+          domain: {
+            chainId: Number(this.#chainId),
+            verifyingContract: this.#SAFE_4337_MODULE_ADDRESS
+          },
+          types: safeOp.getEIP712Type(),
+          message: {
+            ...safeOperation,
+            nonce: BigInt(safeOperation.nonce),
+            validAfter: toHex(safeOperation.validAfter),
+            validUntil: toHex(safeOperation.validUntil),
+            maxFeePerGas: toHex(safeOperation.maxFeePerGas),
+            maxPriorityFeePerGas: toHex(safeOperation.maxPriorityFeePerGas)
+          },
+          primaryType: 'SafeOp'
+        })
+
+        safeSignature = new EthSafeSignature(signerAddress, signature)
+      } else {
+        const safeOpHash = safeOp.getHash()
+
+        safeSignature = await this.protocolKit.signHash(safeOpHash)
+      }
+    }
+
+    safeOp.addSignature(safeSignature)
 
     return safeOp
   }

packages/relay-kit/src/packs/safe-4337/SafeOperation.test.ts

@@ -1,11 +1,11 @@
 import { Hex, encodePacked } from 'viem'
-import Safe, { EthSafeSignature } from '@safe-global/protocol-kit'
+import { EthSafeSignature } from '@safe-global/protocol-kit'
 import SafeOperationV06 from './SafeOperationV06'
 import * as fixtures from './testing-utils/fixtures'
 
 describe('SafeOperation', () => {
   it('should create a SafeOperation from an UserOperation', () => {
-    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, new Safe(), {
+    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, {
       chainId: BigInt(fixtures.CHAIN_ID),
       moduleAddress: fixtures.MODULE_ADDRESS,
       entryPoint: fixtures.ENTRYPOINTS[0],
@@ -32,7 +32,7 @@ describe('SafeOperation', () => {
   })
 
   it('should add and retrieve signatures', () => {
-    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, new Safe(), {
+    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, {
       chainId: BigInt(fixtures.CHAIN_ID),
       moduleAddress: fixtures.MODULE_ADDRESS,
       entryPoint: fixtures.ENTRYPOINTS[0],
@@ -50,7 +50,7 @@ describe('SafeOperation', () => {
   })
 
   it('should encode the signatures', () => {
-    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, new Safe(), {
+    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, {
       chainId: BigInt(fixtures.CHAIN_ID),
       moduleAddress: fixtures.MODULE_ADDRESS,
       entryPoint: fixtures.ENTRYPOINTS[0],
@@ -64,7 +64,7 @@ describe('SafeOperation', () => {
   })
 
   it('should add estimations', () => {
-    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, new Safe(), {
+    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, {
       chainId: BigInt(fixtures.CHAIN_ID),
       moduleAddress: fixtures.MODULE_ADDRESS,
       entryPoint: fixtures.ENTRYPOINTS[0],
@@ -95,7 +95,7 @@ describe('SafeOperation', () => {
   })
 
   it('should retrieve the UserOperation', () => {
-    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, new Safe(), {
+    const safeOperation = new SafeOperationV06(fixtures.USER_OPERATION, {
       chainId: BigInt(fixtures.CHAIN_ID),
       moduleAddress: fixtures.MODULE_ADDRESS,
       entryPoint: fixtures.ENTRYPOINTS[0],

packages/relay-kit/src/packs/safe-4337/SafeOperationBase.ts

@@ -1,106 +1,32 @@
-import { Hex, encodePacked, hashTypedData, toHex } from 'viem'
+import { Hex, encodePacked, hashTypedData } from 'viem'
 import {
   EstimateGasData,
   SafeOperation,
   SafeOperationOptions,
   SafeSignature,
   SafeUserOperation,
-  UserOperation,
-  SigningMethod
+  UserOperation
 } from '@safe-global/types-kit'
-import Safe, { buildSignatureBytes, EthSafeSignature } from '@safe-global/protocol-kit'
+import { buildSignatureBytes } from '@safe-global/protocol-kit'
 import {
   EIP712_SAFE_OPERATION_TYPE_V06,
   EIP712_SAFE_OPERATION_TYPE_V07
 } from '@safe-global/relay-kit/packs/safe-4337/constants'
 
 abstract class SafeOperationBase implements SafeOperation {
   userOperation: UserOperation
-  protocolKit: Safe
   options: SafeOperationOptions
   signatures: Map<string, SafeSignature> = new Map()
 
-  constructor(userOperation: UserOperation, protocolKit: Safe, options: SafeOperationOptions) {
+  constructor(userOperation: UserOperation, options: SafeOperationOptions) {
     this.userOperation = userOperation
-    this.protocolKit = protocolKit
     this.options = options
   }
 
   abstract addEstimations(estimations: EstimateGasData): void
 
   abstract getSafeOperation(): SafeUserOperation
 
-  async sign(signingMethod: SigningMethod = SigningMethod.ETH_SIGN_TYPED_DATA_V4) {
-    const safeProvider = this.protocolKit.getSafeProvider()
-    const signerAddress = await safeProvider.getSignerAddress()
-    const isPasskeySigner = await safeProvider.isPasskeySigner()
-
-    if (!signerAddress) {
-      throw new Error('There is no signer address available to sign the SafeOperation')
-    }
-
-    const isOwner = await this.protocolKit.isOwner(signerAddress)
-    const isSafeDeployed = await this.protocolKit.isSafeDeployed()
-
-    if ((!isOwner && isSafeDeployed) || (!isSafeDeployed && !isPasskeySigner && !isOwner)) {
-      throw new Error('UserOperations can only be signed by Safe owners')
-    }
-
-    let safeSignature: SafeSignature
-
-    if (isPasskeySigner) {
-      const safeOpHash = this.getHash()
-
-      if (!isSafeDeployed) {
-        const passkeySignature = await this.protocolKit.signHash(safeOpHash)
-        safeSignature = new EthSafeSignature(this.options.sharedSigner, passkeySignature.data, true)
-      } else {
-        safeSignature = await this.protocolKit.signHash(safeOpHash)
-      }
-    } else {
-      if (
-        [
-          SigningMethod.ETH_SIGN_TYPED_DATA_V4,
-          SigningMethod.ETH_SIGN_TYPED_DATA_V3,
-          SigningMethod.ETH_SIGN_TYPED_DATA
-        ].includes(signingMethod)
-      ) {
-        const signer = await safeProvider.getExternalSigner()
-
-        if (!signer) {
-          throw new Error('No signer found')
-        }
-
-        const signerAddress = signer.account.address
-        const safeOperation = this.getSafeOperation()
-        const signature = await signer.signTypedData({
-          domain: {
-            chainId: Number(this.options.chainId),
-            verifyingContract: this.options.moduleAddress
-          },
-          types: this.getEIP712Type(),
-          message: {
-            ...safeOperation,
-            nonce: BigInt(safeOperation.nonce),
-            validAfter: toHex(safeOperation.validAfter),
-            validUntil: toHex(safeOperation.validUntil),
-            maxFeePerGas: toHex(safeOperation.maxFeePerGas),
-            maxPriorityFeePerGas: toHex(safeOperation.maxPriorityFeePerGas)
-          },
-          primaryType: 'SafeOp'
-        })
-
-        safeSignature = new EthSafeSignature(signerAddress, signature)
-      } else {
-        const safeOpHash = this.getHash()
-
-        safeSignature = await this.protocolKit.signHash(safeOpHash)
-      }
-    }
-
-    this.addSignature(safeSignature)
-  }
-
   getSignature(signer: string): SafeSignature | undefined {
     return this.signatures.get(signer.toLowerCase())
   }

packages/relay-kit/src/packs/safe-4337/SafeOperationFactory.ts

@@ -1,4 +1,3 @@
-import Safe from '@safe-global/protocol-kit'
 import {
   UserOperation,
   UserOperationV06,
@@ -13,13 +12,12 @@ import { isEntryPointV7 } from '@safe-global/relay-kit/packs/safe-4337/utils'
 class SafeOperationFactory {
   static createSafeOperation(
     userOperation: UserOperation,
-    protocolKit: Safe,
     options: SafeOperationOptions
   ): SafeOperationBase {
     if (isEntryPointV7(options.entryPoint)) {
-      return new SafeOperationV07(userOperation as UserOperationV07, protocolKit, options)
+      return new SafeOperationV07(userOperation as UserOperationV07, options)
     } else {
-      return new SafeOperationV06(userOperation as UserOperationV06, protocolKit, options)
+      return new SafeOperationV06(userOperation as UserOperationV06, options)
     }
   }
 }

packages/relay-kit/src/packs/safe-4337/SafeOperationV06.ts

@@ -1,4 +1,3 @@
-import Safe from '@safe-global/protocol-kit'
 import {
   UserOperationV06,
   EstimateGasData,
@@ -11,8 +10,8 @@ import { EIP712_SAFE_OPERATION_TYPE_V06 } from '@safe-global/relay-kit/packs/saf
 class SafeOperationV06 extends SafeOperationBase {
   userOperation!: UserOperationV06
 
-  constructor(userOperation: UserOperationV06, protocolKit: Safe, options: SafeOperationOptions) {
-    super(userOperation, protocolKit, options)
+  constructor(userOperation: UserOperationV06, options: SafeOperationOptions) {
+    super(userOperation, options)
   }
 
   addEstimations(estimations: EstimateGasData): void {

packages/relay-kit/src/packs/safe-4337/SafeOperationV07.ts

@@ -7,13 +7,12 @@ import {
 import { concat, Hex, isAddress, pad, toHex } from 'viem'
 import SafeOperationBase from '@safe-global/relay-kit/packs/safe-4337/SafeOperationBase'
 import { EIP712_SAFE_OPERATION_TYPE_V07 } from '@safe-global/relay-kit/packs/safe-4337/constants'
-import Safe from '@safe-global/protocol-kit'
 
 class SafeOperationV07 extends SafeOperationBase {
   userOperation!: UserOperationV07
 
-  constructor(userOperation: UserOperationV07, protocolKit: Safe, options: SafeOperationOptions) {
-    super(userOperation, protocolKit, options)
+  constructor(userOperation: UserOperationV07, options: SafeOperationOptions) {
+    super(userOperation, options)
   }
 
   addEstimations(estimations: EstimateGasData): void {