Merge pull request #19 from dodger213/improvement-add-transaction-pre…

…-hooks Improvement add transaction pre hooks
dodger213 · Aug 18, 2024 · be4cab8 · be4cab8
2 parents 02f766c + 0544259
commit be4cab8
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 59 deletions.
diff --git a/contracts/Safe.sol b/contracts/Safe.sol
@@ -132,8 +132,8 @@ contract Safe is
         address gasToken,
         address payable refundReceiver,
         bytes memory signatures
-    ) external payable override returns (bool success) {
-        onBeforeExecTransaction(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, signatures);
+    ) public payable virtual override returns (bool success) {
+        onBeforeExecTransaction(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, signatures, success);
         bytes32 txHash;
         // Use scope here to limit variable lifetime and prevent `stack too deep` errors
         {
@@ -467,6 +467,8 @@ contract Safe is
      * @param value Ether value of module transaction.
      * @param data Data payload of module transaction.
      * @param operation Operation type of module transaction.
+     * @param success Boolean flag indicating if the call succeeded.
+
      */
     function onBeforeExecTransaction(
         address to,
@@ -478,6 +480,8 @@ contract Safe is
         uint256 gasPrice,
         address gasToken,
         address payable refundReceiver,
-        bytes memory signatures
+        bytes memory signatures,
+        bool success
+
     ) internal virtual {}
 }
diff --git a/contracts/SafeL2.sol b/contracts/SafeL2.sol
@@ -37,7 +37,6 @@ contract SafeL2 is Safe {
      * @inheritdoc Safe
      */
     function onBeforeExecTransaction(
-
         address to,
         uint256 value,
         bytes calldata data,
@@ -47,7 +46,9 @@ contract SafeL2 is Safe {
         uint256 gasPrice,
         address gasToken,
         address payable refundReceiver,
-        bytes memory signatures
+        bytes memory signatures,
+        bool /*success*/
+
     ) internal override {
         bytes memory additionalInfo;
         {
@@ -72,13 +73,8 @@ contract SafeL2 is Safe {
      * @inheritdoc ModuleManager
 
      */
-    function onBeforeExecTransactionFromModule(
-        address to,
-        uint256 value,
-        bytes memory data,
-        Enum.Operation operation,
-        bytes memory /* context */
-    ) internal override {
+    function onBeforeExecTransactionFromModule(address to, uint256 value, bytes memory data, Enum.Operation operation) internal override {
+
         emit SafeModuleTransaction(msg.sender, to, value, data, operation);
 
     }

diff --git a/contracts/base/ModuleManager.sol b/contracts/base/ModuleManager.sol
@@ -172,18 +172,9 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
         uint256 value,
         bytes memory data,
         Enum.Operation operation
-    ) external override returns (bool success) {
-        success = _execTransactionFromModule(to, value, data, operation, "");
-    }
-
-    function _execTransactionFromModule(
-        address to,
-        uint256 value,
-        bytes memory data,
-        Enum.Operation operation,
-        bytes calldata context
-    ) internal returns (bool success) {
-        (address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation, context);
+    ) public override returns (bool success) {
+        onBeforeExecTransactionFromModule(to, value, data, operation);
+        (address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation);
         success = execute(to, value, data, operation, type(uint256).max);
         postModuleExecution(guard, guardHash, success);
 
@@ -197,31 +188,10 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
         uint256 value,
         bytes memory data,
         Enum.Operation operation
-    ) external override returns (bool success, bytes memory returnData) {
-        (success, returnData) = _execTransactionFromModuleReturnData(to, value, data, operation, "");
-    }
+    ) public override returns (bool success, bytes memory returnData) {
+        onBeforeExecTransactionFromModule(to, value, data, operation);
+        (address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation);
 
-    /**
-     * @inheritdoc IModuleManager
-     */
-    function execTransactionFromModuleReturnData(
-        address to,
-        uint256 value,
-        bytes memory data,
-        Enum.Operation operation,
-        bytes calldata context
-    ) external override returns (bool success, bytes memory returnData) {
-        (success, returnData) = _execTransactionFromModuleReturnData(to, value, data, operation, context);
-    }
-
-    function _execTransactionFromModuleReturnData(
-        address to,
-        uint256 value,
-        bytes memory data,
-        Enum.Operation operation,
-        bytes calldata context
-    ) internal returns (bool success, bytes memory returnData) {
-        (address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation, context);
         success = execute(to, value, data, operation, type(uint256).max);
         /* solhint-disable no-inline-assembly */
         /// @solidity memory-safe-assembly
@@ -331,17 +301,10 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
 
     /**
      * @notice A hook that gets called before execution of {execTransactionFromModule*} methods.
-
      * @param to Destination address of module transaction.
      * @param value Ether value of module transaction.
      * @param data Data payload of module transaction.
      * @param operation Operation type of module transaction.
      */
-    function onBeforeExecTransactionFromModule(
-        address to,
-        uint256 value,
-        bytes memory data,
-        Enum.Operation operation,
-        bytes calldata context
-    ) internal virtual {}
+    function onBeforeExecTransactionFromModule(address to, uint256 value, bytes memory data, Enum.Operation operation) internal virtual {}
 }
diff --git a/test/core/Safe.Execution.spec.ts b/test/core/Safe.Execution.spec.ts
@@ -204,9 +204,13 @@ describe("Safe", () => {
             await user1.sendTransaction({ to: safeAddress, value: ethers.parseEther("1") });
             const userBalance = await hre.ethers.provider.getBalance(user2.address);
             expect(await hre.ethers.provider.getBalance(safeAddress)).to.be.eq(ethers.parseEther("1"));
-
-            const executedTx = await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]);
-            await expect(executedTx).to.emit(safe, "ExecutionSuccess");
+            let executedTx: any;
+            await expect(
+                executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]).then((tx) => {
+                    executedTx = tx;
+                    return tx;
+                }),
+            ).to.emit(safe, "ExecutionSuccess");
 
             const receipt = await hre.ethers.provider.getTransactionReceipt(executedTx!.hash);
             const receiptLogs = receipt?.logs ?? [];