From c0180846e061ea332af2daf9d5356098207f76b9 Mon Sep 17 00:00:00 2001 From: Akash Gianchandani Date: Tue, 23 Jan 2024 10:51:43 +0530 Subject: [PATCH] fix violation of check effect interaction pattern WIN-1733 --- contracts/Batcher.sol | 2 +- contracts/ForwarderFactoryV4.sol | 6 +++--- contracts/ForwarderV4.sol | 7 +++---- contracts/WalletSimple.sol | 14 +++++++------- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/contracts/Batcher.sol b/contracts/Batcher.sol index cdc2ef7..1813866 100644 --- 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 index 2c986bc..15e47bb 100644 --- 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 index 8037fe7..0d00a6a 100644 --- a/contracts/ForwarderV4.sol +++ b/contracts/ForwarderV4.sol @@ -89,9 +89,6 @@ 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. @@ -99,6 +96,8 @@ contract ForwarderV4 is IERC721Receiver, ERC1155Receiver, IForwarderV4 { * 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 index 02e31fe..03c7844 100644 --- 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]); } }