diff --git a/contracts/ForwarderFactoryV4.sol b/contracts/ForwarderFactoryV4.sol new file mode 100644 index 0000000..18dfad9 --- /dev/null +++ b/contracts/ForwarderFactoryV4.sol @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.20; +import './ForwarderV4.sol'; +import './CloneFactory.sol'; + +/** + * @title ForwarderFactoryV4 + * @notice This contract will deploy new forwarder contracts using the create2 opcode + */ +contract ForwarderFactoryV4 is CloneFactory { + address public implementationAddress; + + /** + * @notice Event triggered when a new forwarder is deployed + * @param newForwarderAddress Address of the newly deployed forwarder + * @param parentAddress Address to which the funds should be forwarded + * @param feeAddress Address which is allowed to call methods on forwarder contract alongwith the parentAddress + * @param shouldAutoFlushERC721 Whether to automatically flush ERC721 tokens or not + * @param shouldAutoFlushERC1155 Whether to automatically flush ERC1155 tokens or not + */ + event ForwarderCreated( + address newForwarderAddress, + address parentAddress, + address feeAddress, + bool shouldAutoFlushERC721, + bool shouldAutoFlushERC1155 + ); + + constructor(address _implementationAddress) { + implementationAddress = _implementationAddress; + } + + /** + * @notice Creates a new forwarder contract + * @param parent Address to which the funds should be forwarded + * @param feeAddress Address which is allowed to call methods on forwarder contract alongwith the parentAddress + * @param salt Salt to be used while deploying the contract + */ + function createForwarder( + address parent, + address feeAddress, + bytes32 salt + ) external { + this.createForwarder(parent, feeAddress, salt, true, true); + } + + /** + * @notice Creates a new forwarder contract + * @param parent Address to which the funds should be forwarded + * @param feeAddress Address which is allowed to call methods on forwarder contract alongwith the parentAddress + * @param salt Salt to be used while deploying the contract + * @param shouldAutoFlushERC721 Whether to automatically flush ERC721 tokens or not + * @param shouldAutoFlushERC1155 Whether to automatically flush ERC1155 tokens or not + */ + function createForwarder( + address parent, + address feeAddress, + bytes32 salt, + bool shouldAutoFlushERC721, + bool shouldAutoFlushERC1155 + ) external { + /// include the parent in the salt so any contract deployed directly relies on the parent address + bytes32 finalSalt = keccak256(abi.encodePacked(parent, salt)); + + address payable clone = createClone(implementationAddress, finalSalt); + ForwarderV4(clone).init( + parent, + feeAddress, + shouldAutoFlushERC721, + shouldAutoFlushERC1155 + ); + emit ForwarderCreated( + clone, + parent, + feeAddress, + shouldAutoFlushERC721, + shouldAutoFlushERC1155 + ); + } +} diff --git a/contracts/UpdatedForwarder.sol b/contracts/ForwarderV4.sol similarity index 77% rename from contracts/UpdatedForwarder.sol rename to contracts/ForwarderV4.sol index efb3c2b..3abcd7e 100644 --- a/contracts/UpdatedForwarder.sol +++ b/contracts/ForwarderV4.sol @@ -9,21 +9,31 @@ import './TransferHelper.sol'; import './IForwarder.sol'; /** - * Contract that will forward any incoming Ether to the parent address of the contract - * + * @title ForwarderV4 + * @notice This contract will forward any incoming Ether or token to the parent address of the contract */ -contract UpdatedForwarder is IERC721Receiver, ERC1155Receiver, IForwarder { - // Address to which any funds sent to this contract will be forwarded +contract ForwarderV4 is IERC721Receiver, ERC1155Receiver, IForwarder { + /// @notice Any funds sent to this contract will be forwarded to this address address public parentAddress; - // Address which is allowed to call methods on this contract alongwith the parentAddress + /// @notice Address which is allowed to call methods on this contract alongwith the parentAddress address public feeAddress; bool public autoFlush721 = true; bool public autoFlush1155 = true; + /** + * @notice Event triggered when a deposit is received in the forwarder + * @param from Address from which the deposit is received + * @param value Amount of Ether received + * @param data Data sent along with the deposit + */ event ForwarderDeposited(address from, uint256 value, bytes data); /** - * Initialize the contract, and sets the destination address to that of the parent address + * @notice Initialize the contract, and sets the destination address to that of the parent address + * @param _parentAddress Address to which the funds should be forwarded + * @param _feeAddress Address which is allowed to call methods on this contract alongwith the parentAddress + * @param _autoFlush721 Whether to automatically flush ERC721 tokens or not + * @param _autoFlush1155 Whether to automatically flush ERC1155 tokens or not */ function init( address _parentAddress, @@ -38,7 +48,7 @@ contract UpdatedForwarder is IERC721Receiver, ERC1155Receiver, IForwarder { uint256 value = address(this).balance; - // set whether we want to automatically flush erc721/erc1155 tokens or not + /// @notice set whether we want to automatically flush erc721/erc1155 tokens or not autoFlush721 = _autoFlush721; autoFlush1155 = _autoFlush1155; @@ -49,15 +59,17 @@ contract UpdatedForwarder is IERC721Receiver, ERC1155Receiver, IForwarder { (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 + /** + * 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); } /** - * Modifier that will execute internal code block only if the sender is from the allowed addresses + * @notice Modifier that will execute internal code block only if the sender is from the allowed addresses */ modifier onlyAllowedAddress() { require( @@ -68,7 +80,7 @@ contract UpdatedForwarder is IERC721Receiver, ERC1155Receiver, IForwarder { } /** - * Modifier that will execute internal code block only if the contract has not been initialized yet + * @notice Modifier that will execute internal code block only if the contract has not been initialized yet */ modifier onlyUninitialized() { require(parentAddress == address(0x0), 'Already initialized'); @@ -76,14 +88,14 @@ contract UpdatedForwarder is IERC721Receiver, ERC1155Receiver, IForwarder { } /** - * Default function; Gets called when data is sent but does not match any other function + * @notice Default function; Gets called when data is sent but does not match any other function */ fallback() external payable { flush(); } /** - * Default function; Gets called when Ether is deposited with no data, and forwards it to the parent address + * @notice Default function; Gets called when Ether is deposited with no data, and forwards it to the parent address */ receive() external payable { flush(); @@ -134,13 +146,19 @@ contract UpdatedForwarder is IERC721Receiver, ERC1155Receiver, IForwarder { instance.supportsInterface(type(IERC721).interfaceId), 'The caller does not support the ERC721 interface' ); - // this won't work for ERC721 re-entrancy + /// this won't work for ERC721 re-entrancy instance.safeTransferFrom(address(this), parentAddress, _tokenId, data); } return this.onERC721Received.selector; } + /** + * @notice Method to allow for calls to other contracts. This method can only be called by the allowed address + * @param target The target contract address whose method needs to be called + * @param value The amount of Ether to be sent + * @param data The calldata to be sent + */ function callFromAllowedAddress( address target, uint256 value, @@ -304,7 +322,7 @@ contract UpdatedForwarder is IERC721Receiver, ERC1155Receiver, IForwarder { } /** - * Flush the entire balance of the contract to the parent address. + * @notice Flush the entire balance of the contract to the parent address. */ function flush() public { uint256 value = address(this).balance; diff --git a/contracts/IForwarder.sol b/contracts/IForwarder.sol index 7e5681e..d35121e 100644 --- a/contracts/IForwarder.sol +++ b/contracts/IForwarder.sol @@ -19,7 +19,7 @@ interface IForwarder is IERC165 { function setAutoFlush1155(bool autoFlush) external; /** - * Execute a token transfer of the full balance from the forwarder token to the parent address + * Execute a token transfer of the full balance from the forwarder to the parent address * * @param tokenContractAddress the address of the erc20 token contract */ diff --git a/contracts/UpdatedForwarderFactory.sol b/contracts/UpdatedForwarderFactory.sol deleted file mode 100644 index 9ce7f4a..0000000 --- a/contracts/UpdatedForwarderFactory.sol +++ /dev/null @@ -1,54 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.20; -import './UpdatedForwarder.sol'; -import './CloneFactory.sol'; - -contract UpdatedForwarderFactory is CloneFactory { - address public implementationAddress; - - event ForwarderCreated( - address newForwarderAddress, - address parentAddress, - address feeAddress, - bool shouldAutoFlushERC721, - bool shouldAutoFlushERC1155 - ); - - constructor(address _implementationAddress) { - implementationAddress = _implementationAddress; - } - - function createForwarder( - address parent, - address feeAddress, - bytes32 salt - ) external { - this.createForwarder(parent, feeAddress, salt, true, true); - } - - function createForwarder( - address parent, - address feeAddress, - bytes32 salt, - bool shouldAutoFlushERC721, - bool shouldAutoFlushERC1155 - ) external { - // include the signers in the salt so any contract deployed to a given address must have the same signers - bytes32 finalSalt = keccak256(abi.encodePacked(parent, salt)); - - address payable clone = createClone(implementationAddress, finalSalt); - UpdatedForwarder(clone).init( - parent, - feeAddress, - shouldAutoFlushERC721, - shouldAutoFlushERC1155 - ); - emit ForwarderCreated( - clone, - parent, - feeAddress, - shouldAutoFlushERC721, - shouldAutoFlushERC1155 - ); - } -} diff --git a/test/updatedForwarderFactory.js b/test/forwarderFactoryV4.js similarity index 94% rename from test/updatedForwarderFactory.js rename to test/forwarderFactoryV4.js index 3557474..1e54572 100644 --- a/test/updatedForwarderFactory.js +++ b/test/forwarderFactoryV4.js @@ -6,16 +6,16 @@ const util = require('ethereumjs-util'); const abi = require('ethereumjs-abi'); const BigNumber = require('bignumber.js'); -const UpdatedForwarder = artifacts.require('./UpdatedForwarder.sol'); -const UpdatedForwarderFactory = artifacts.require( - './UpdatedForwarderFactory.sol' +const ForwarderV4 = artifacts.require('./ForwarderV4.sol'); +const ForwarderFactoryV4 = artifacts.require( + './ForwarderFactoryV4.sol' ); const hre = require('hardhat'); const createForwarderFactory = async () => { - const forwarderContract = await UpdatedForwarder.new([], {}); - const forwarderFactory = await UpdatedForwarderFactory.new( + const forwarderContract = await ForwarderV4.new([], {}); + const forwarderFactory = await ForwarderFactoryV4.new( forwarderContract.address ); return { @@ -67,7 +67,7 @@ const createForwarder = async ( return forwarderAddress; }; -describe('UpdatedForwarderFactory', function () { +describe('ForwarderFactoryV4', function () { let accounts; before(async () => { await hre.network.provider.send('hardhat_reset'); @@ -216,7 +216,7 @@ describe('UpdatedForwarderFactory', function () { ); const forwarderContract = await hre.ethers.getContractAt( - 'UpdatedForwarder', + 'ForwarderV4', forwarderAddress ); const autoFlush721 = await forwarderContract.autoFlush721(); @@ -241,7 +241,7 @@ describe('UpdatedForwarderFactory', function () { ); const forwarderContract = await hre.ethers.getContractAt( - 'UpdatedForwarder', + 'ForwarderV4', forwarderAddress ); const autoFlush1155 = await forwarderContract.autoFlush1155(); diff --git a/test/updatedForwarder.js b/test/forwarderV4.js similarity index 98% rename from test/updatedForwarder.js rename to test/forwarderV4.js index 4803ed0..673b088 100644 --- a/test/updatedForwarder.js +++ b/test/forwarderV4.js @@ -8,9 +8,9 @@ const helpers = require('./helpers'); const BigNumber = require('bignumber.js'); const { makeInterfaceId } = require('@openzeppelin/test-helpers'); -const UpdatedForwarder = artifacts.require('./UpdatedForwarder.sol'); -const UpdatedForwarderFactory = artifacts.require( - './UpdatedForwarderFactory.sol' +const ForwarderV4 = artifacts.require('./ForwarderV4.sol'); +const ForwarderFactoryV4 = artifacts.require( + './ForwarderFactoryV4.sol' ); const ERC721 = artifacts.require('./MockERC721'); const ERC1155 = artifacts.require('./MockERC1155'); @@ -18,14 +18,14 @@ const AlwaysFalseERC165 = artifacts.require('./AlwaysFalseERC165.sol'); const ReentryForwarder = artifacts.require('./ReentryForwarder'); const createForwarder = async (creator, parent, feeAddress) => { - const forwarderContract = await UpdatedForwarder.new([], { from: creator }); + const forwarderContract = await ForwarderV4.new([], { from: creator }); await forwarderContract.init(parent, feeAddress, true, true); return forwarderContract; }; const createForwarderFactory = async () => { - const forwarderContract = await UpdatedForwarder.new([], {}); - const forwarderFactory = await UpdatedForwarderFactory.new( + const forwarderContract = await ForwarderV4.new([], {}); + const forwarderFactory = await ForwarderFactoryV4.new( forwarderContract.address ); return { @@ -83,7 +83,7 @@ const getMethodData = async function (types, values, methodName) { const FORWARDER_DEPOSITED_EVENT = 'ForwarderDeposited'; -describe('UpdatedForwarder', function () { +describe('ForwarderV4', function () { let accounts; before(async () => { await hre.network.provider.send('hardhat_reset'); @@ -302,7 +302,7 @@ describe('UpdatedForwarder', function () { baseAddress, feeAddress ); - noAutoFlushForwarder = await UpdatedForwarder.new([], { + noAutoFlushForwarder = await ForwarderV4.new([], { from: accounts[1] }); await noAutoFlushForwarder.init(baseAddress, feeAddress, false, false);