fix violation of check effect interaction pattern

WIN-1733

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]);
     }
   }
 

contracts/Forwarder.sol

@@ -39,14 +39,14 @@ contract Forwarder is IERC721Receiver, ERC1155Receiver, IForwarder {
       return;
     }
 
-    (bool success, ) = parentAddress.call{ value: value }('');
-    require(success, 'Flush failed');
-
     // NOTE: 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');
   }
 
   /**
@@ -303,9 +303,9 @@ contract Forwarder is IERC721Receiver, ERC1155Receiver, IForwarder {
       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);
   }
 
   /**

contracts/ForwarderFactory.sol

@@ -31,13 +31,14 @@ contract ForwarderFactory is CloneFactory {
     bytes32 finalSalt = keccak256(abi.encodePacked(parent, salt));
 
     address payable clone = createClone(implementationAddress, finalSalt);
-    Forwarder(clone).init(
+
+    emit ForwarderCreated(
+      clone,
       parent,
       shouldAutoFlushERC721,
       shouldAutoFlushERC1155
     );
-    emit ForwarderCreated(
-      clone,
+    Forwarder(clone).init(
       parent,
       shouldAutoFlushERC721,
       shouldAutoFlushERC1155

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,

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);
   }
 
   /**

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]);
     }
   }
 

test/batcher.js

@@ -530,7 +530,7 @@ describe('Batcher', () => {
         values: randVals,
         // costs roughly 40,000 gas to get to beginning of `distributeBatch`
         // and then another 10,000 gas for each subsequent iteration
-        gasLimit: 9e4
+        gasLimit: 10e4
       };
       await runTestBatcherDriver(params);
     });