Merge pull request safe-global#245 from safe-global/execTransaction-i…

…mmutable Make executeTransaction immutable
fdarian · Aug 24, 2022 · b206089 · b206089
2 parents 1450835 + c40f96b
commit b206089
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 7 deletions.
diff --git a/packages/safe-core-sdk/src/Safe.ts b/packages/safe-core-sdk/src/Safe.ts
@@ -605,28 +605,33 @@ class Safe {
     safeTransaction: SafeTransaction,
     options?: TransactionOptions
   ): Promise<TransactionResult> {
-    const txHash = await this.getTransactionHash(safeTransaction)
+    const signedSafeTransaction = await this.createTransaction(safeTransaction.data)
+    safeTransaction.signatures.forEach((signature) => {
+      signedSafeTransaction.addSignature(signature)
+    })
+
+    const txHash = await this.getTransactionHash(signedSafeTransaction)
     const ownersWhoApprovedTx = await this.getOwnersWhoApprovedTx(txHash)
     for (const owner of ownersWhoApprovedTx) {
-      safeTransaction.addSignature(generatePreValidatedSignature(owner))
+      signedSafeTransaction.addSignature(generatePreValidatedSignature(owner))
     }
     const owners = await this.getOwners()
     const signerAddress = await this.#ethAdapter.getSignerAddress()
     if (owners.includes(signerAddress)) {
-      safeTransaction.addSignature(generatePreValidatedSignature(signerAddress))
+      signedSafeTransaction.addSignature(generatePreValidatedSignature(signerAddress))
     }
 
     const threshold = await this.getThreshold()
-    if (threshold > safeTransaction.signatures.size) {
-      const signaturesMissing = threshold - safeTransaction.signatures.size
+    if (threshold > signedSafeTransaction.signatures.size) {
+      const signaturesMissing = threshold - signedSafeTransaction.signatures.size
       throw new Error(
         `There ${signaturesMissing > 1 ? 'are' : 'is'} ${signaturesMissing} signature${
           signaturesMissing > 1 ? 's' : ''
         } missing`
       )
     }
 
-    const value = BigNumber.from(safeTransaction.data.value)
+    const value = BigNumber.from(signedSafeTransaction.data.value)
     if (!value.isZero()) {
       const balance = await this.getBalance()
       if (value.gt(BigNumber.from(balance))) {
@@ -637,7 +642,7 @@ class Safe {
     if (options?.gas && options?.gasLimit) {
       throw new Error('Cannot specify gas and gasLimit together in transaction options')
     }
-    const txResponse = await this.#contractManager.safeContract.execTransaction(safeTransaction, {
+    const txResponse = await this.#contractManager.safeContract.execTransaction(signedSafeTransaction, {
       from: signerAddress,
       ...options
     })

diff --git a/packages/safe-core-sdk/tests/execution.test.ts b/packages/safe-core-sdk/tests/execution.test.ts
@@ -188,7 +188,10 @@ describe('Transactions execution', () => {
         data: '0x'
       }
       const tx = await safeSdk1.createTransaction(txDataPartial)
+      const initNumSignatures = tx.signatures.size
       const txResponse = await safeSdk1.executeTransaction(tx)
+      const finalNumSignatures = tx.signatures.size
+      chai.expect(initNumSignatures).to.be.eq(finalNumSignatures)
       await waitSafeTxReceipt(txResponse)
       const safeFinalBalance = await safeSdk1.getBalance()
       chai