Skip to content

Gas optimize transfers in TransferExecutor#

Informational

Done

Handled according to the recommendation.

The transfer function in the TransferExecutor contract is using proxy contracts to transfer ERC20 tokens, ERC721 tokens, ERC1155 tokens and ERC721 bundles.

Here's an estimate of the additional gas cost of using proxies to do a transfer:

  1. Read the proxies state variable to get the address of a proxy contract - 2100 gas.
  2. Make an external function call to the proxy - 2600 gas
  3. The proxy contract is an upgradeable contract that works by reading a state variable (2100 gas) to get a logic contract and execute a function (2600 gas) using delegatecall -- 4700 gas
  4. The proxy contract reads the operators state variable to check that the caller has permission to execute the transfer function - 2100 gas.

Total: 11,500 gas

The gas numbers come from EIP-2929.

Recommendation#

It would be more gas efficient if the transfer function did the transfers for ERC20 tokens, ERC721 tokens, ERC1155 tokens directly rather than using proxy contracts.

The proxy contracts enable the system to change how the transfers work for ERC20 tokens, ERC721 tokens, ERC1155, however how transfers work for these is standardized.

The proxy contracts also enable the system to be extended to different kinds of tokens in the future. Therefore I suggest not using proxy contracts to transfer ERC20 tokens, ERC721 tokens and ERC1155 tokens since they can be implemented directly but let other future tokens use proxies.

Here is the current transfer function: 

 function transfer(
        LibAsset.Asset memory asset,
        address from,
        address to,
        bytes4 transferDirection,
        bytes4 transferType
    ) internal override {
        if (asset.assetType.assetClass == LibAsset.ETH_ASSET_CLASS) {
            to.transferEth(asset.value);
        } else if (asset.assetType.assetClass == LibAsset.ERC20_ASSET_CLASS) {
            (address token) = abi.decode(asset.assetType.data, (address));
            IERC20TransferProxy(proxies[LibAsset.ERC20_ASSET_CLASS]).erc20safeTransferFrom(IERC20Upgradeable(token), from, to, asset.value);
        } else if (asset.assetType.assetClass == LibAsset.ERC721_ASSET_CLASS) {
            (address token, uint tokenId) = abi.decode(asset.assetType.data, (address, uint256));
            require(asset.value == 1, "erc721 value error");
            INftTransferProxy(proxies[LibAsset.ERC721_ASSET_CLASS]).erc721safeTransferFrom(IERC721Upgradeable(token), from, to, tokenId);
        } else if (asset.assetType.assetClass == LibAsset.ERC1155_ASSET_CLASS) {
            (address token, uint tokenId) = abi.decode(asset.assetType.data, (address, uint256));
            INftTransferProxy(proxies[LibAsset.ERC1155_ASSET_CLASS]).erc1155safeTransferFrom(IERC1155Upgradeable(token), from, to, tokenId, asset.value, "");
        } else if (asset.assetType.assetClass == LibAsset.ERC721_BUNDLE_ASSET_CLASS) {
            (INftTransferProxy.ERC721BundleItem[] memory erc721BundleItems) = abi.decode(asset.assetType.data, (INftTransferProxy.ERC721BundleItem[]));
            require(asset.value > 1 && asset.value <= maxBundleSize, "erc721 value error");
            INftTransferProxy(proxies[LibAsset.ERC721_BUNDLE_ASSET_CLASS]).erc721BundleSafeTransferFrom(erc721BundleItems, from, to);
        } else {
            ITransferProxy(proxies[asset.assetType.assetClass]).transfer(asset, from, to);
        }
        emit Transfer(asset, from, to, transferDirection, transferType);
    }

Here's the suggested change: 

    function transfer(
        LibAsset.Asset memory asset,
        address from,
        address to,
        bytes4 transferDirection,
        bytes4 transferType
    ) internal override {
        if (asset.assetType.assetClass == LibAsset.ETH_ASSET_CLASS) {
            to.transferEth(asset.value);
        } else if (asset.assetType.assetClass == LibAsset.ERC20_ASSET_CLASS) {
            (address token) = abi.decode(asset.assetType.data, (address));
            SafeERC20.safeTransferFrom(token, from, to, asset.value);            
        } else if (asset.assetType.assetClass == LibAsset.ERC721_ASSET_CLASS) {
            (address token, uint tokenId) = abi.decode(asset.assetType.data, (address, uint256));
            require(asset.value == 1, "erc721 value error");            
            IERC721Upgradeable(token).safeTransferFrom( from, to, tokenId);
        } else if (asset.assetType.assetClass == LibAsset.ERC1155_ASSET_CLASS) {
            (address token, uint tokenId) = abi.decode(asset.assetType.data, (address, uint256));            
            IERC1155Upgradeable(token).safeTransferFrom(from, to, tokenId, asset.value, "");
        } else if (asset.assetType.assetClass == LibAsset.ERC721_BUNDLE_ASSET_CLASS) {
            (INftTransferProxy.ERC721BundleItem[] memory erc721BundleItems) = abi.decode(asset.assetType.data, (INftTransferProxy.ERC721BundleItem[]));
            require(asset.value > 1 && asset.value <= maxBundleSize, "erc721 value error");            
            for (uint256 i = 0; i < erc721BundleItems.length; i++) {
                for (uint256 j = 0; j < erc721BundleItems[i].tokenIds.length; j++){
                    IERC721Upgradeable(erc721BundleItems[i].tokenAddress).safeTransferFrom(from, to, erc721BundleItems[i].tokenIds[j]);
                } 
            }
        } else {
            ITransferProxy(proxies[asset.assetType.assetClass]).transfer(asset, from, to);
        }
        emit Transfer(asset, from, to, transferDirection, transferType);
    }

Note that this change would require people to approve the UniverseMarketplace contract for transfers, rather than proxy transfer contracts.