From ee90f4b242bd78d696c6018ba8b78bce263ddfca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Mart=C3=ADnez?= Date: Tue, 23 Aug 2022 23:34:23 +0200 Subject: [PATCH 1/2] Make executeTransaction immutable --- packages/safe-core-sdk/src/Safe.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/safe-core-sdk/src/Safe.ts b/packages/safe-core-sdk/src/Safe.ts index b95b1236a..4605a700c 100644 --- a/packages/safe-core-sdk/src/Safe.ts +++ b/packages/safe-core-sdk/src/Safe.ts @@ -605,20 +605,25 @@ class Safe { safeTransaction: SafeTransaction, options?: TransactionOptions ): Promise { - 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' : '' @@ -626,7 +631,7 @@ class Safe { ) } - 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 }) From c40f96b59e4aaaf06b4ff400a9eb46bd31192e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Mart=C3=ADnez?= Date: Tue, 23 Aug 2022 23:41:18 +0200 Subject: [PATCH 2/2] Tests executeTransaction immutability --- packages/safe-core-sdk/tests/execution.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/safe-core-sdk/tests/execution.test.ts b/packages/safe-core-sdk/tests/execution.test.ts index efa14132b..11ac4fba1 100644 --- 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