fix violation of check effect interaction pattern

WIN-1733
BitGo · Jan 23, 2024 · c018084 · c018084
1 parent 791dbfd
commit c018084
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 15 deletions.
diff --git a/contracts/Batcher.sol b/contracts/Batcher.sol
@@ -71,12 +71,12 @@ contract Batcher {
     // Revert everything if any transfer fails
     for (uint8 i = 0; i < recipients.length; i++) {
       require(recipients[i] != address(0), 'Invalid recipient address');
+      emit BatchTransfer(msg.sender, recipients[i], values[i]);
       (bool success, ) = recipients[i].call{
         value: values[i],
         gas: transferGasLimit
       }('');
       require(success, 'Send failed');
-      emit BatchTransfer(msg.sender, recipients[i], values[i]);
     }
   }
 

diff --git a/contracts/ForwarderFactoryV4.sol b/contracts/ForwarderFactoryV4.sol
@@ -67,14 +67,14 @@ contract ForwarderFactoryV4 is CloneFactory {
     bytes32 finalSalt = keccak256(abi.encodePacked(parent, feeAddress, salt));
 
     address payable clone = createClone(implementationAddress, finalSalt);
-    ForwarderV4(clone).init(
+    emit ForwarderCreated(
+      clone,
       parent,
       feeAddress,
       shouldAutoFlushERC721,
       shouldAutoFlushERC1155
     );
-    emit ForwarderCreated(
-      clone,
+    ForwarderV4(clone).init(
       parent,
       feeAddress,
       shouldAutoFlushERC721,

diff --git a/contracts/ForwarderV4.sol b/contracts/ForwarderV4.sol
@@ -89,16 +89,15 @@ contract ForwarderV4 is IERC721Receiver, ERC1155Receiver, IForwarderV4 {
       return;
     }
 
-    (bool success, ) = parentAddress.call{ value: value }('');
-    require(success, 'Flush failed');
-
     /**
      * Since we are forwarding on initialization,
      * we don't have the context of the original sender.
      * We still emit an event about the forwarding but set
      * the sender to the forwarder itself
      */
     emit ForwarderDeposited(address(this), value, msg.data);
+    (bool success, ) = parentAddress.call{ value: value }('');
+    require(success, 'Flush failed');
   }
 
   /**
@@ -359,9 +358,9 @@ contract ForwarderV4 is IERC721Receiver, ERC1155Receiver, IForwarderV4 {
       return;
     }
 
+    emit ForwarderDeposited(msg.sender, value, msg.data);
     (bool success, ) = parentAddress.call{ value: value }('');
     require(success, 'Flush failed');
-    emit ForwarderDeposited(msg.sender, value, msg.data);
   }
 
   /**

diff --git a/contracts/WalletSimple.sol b/contracts/WalletSimple.sol
@@ -207,10 +207,6 @@ contract WalletSimple is IERC721Receiver, ERC1155Receiver {
       sequenceId
     );
 
-    // Success, send the transaction
-    (bool success, ) = toAddress.call{ value: value }(data);
-    require(success, 'Call execution failed');
-
     emit Transacted(
       msg.sender,
       otherSigner,
@@ -219,6 +215,10 @@ contract WalletSimple is IERC721Receiver, ERC1155Receiver {
       value,
       data
     );
+
+    // Success, send the transaction
+    (bool success, ) = toAddress.call{ value: value }(data);
+    require(success, 'Call execution failed');
   }
 
   /**
@@ -268,8 +268,8 @@ contract WalletSimple is IERC721Receiver, ERC1155Receiver {
       sequenceId
     );
 
-    batchTransfer(recipients, values);
     emit BatchTransacted(msg.sender, otherSigner, operationHash);
+    batchTransfer(recipients, values);
   }
 
   /**
@@ -286,10 +286,10 @@ contract WalletSimple is IERC721Receiver, ERC1155Receiver {
     for (uint256 i = 0; i < recipients.length; i++) {
       require(address(this).balance >= values[i], 'Insufficient funds');
 
+      emit BatchTransfer(msg.sender, recipients[i], values[i]);
+
       (bool success, ) = recipients[i].call{ value: values[i] }('');
       require(success, 'Call failed');
-
-      emit BatchTransfer(msg.sender, recipients[i], values[i]);
     }
   }