From 5063089285be32a34f2769107aecc0c874a60319 Mon Sep 17 00:00:00 2001 From: Brechtpd Date: Sun, 18 Apr 2021 00:47:36 +0200 Subject: [PATCH] Feedback --- .../contracts/amm/libamm/AmmExitProcess.sol | 4 +- .../contracts/amm/libamm/AmmExitRequest.sol | 13 ++- .../amm/libamm/AmmTransactionReceiver.sol | 4 +- .../contracts/amm/libamm/AmmUpdateProcess.sol | 32 +++++-- .../contracts/amm/libamm/AmmUtil.sol | 49 ++++------ .../contracts/amm/libamm/AmmWithdrawal.sol | 9 +- .../aux/agents/FastWithdrawalAgent.sol | 24 +---- .../contracts/aux/bridge/Bridge.sol | 57 +++++++----- .../FastWithdrawalLiquidityProvider.sol | 10 +- .../contracts/converters/BaseConverter.sol | 15 +-- .../contracts/core/impl/ExchangeV3.sol | 14 +-- .../loopring_v3/contracts/lib/Drainable.sol | 15 +-- .../loopring_v3/contracts/lib/LPERC20.sol | 2 +- .../contracts/lib/TransferUtil.sol | 91 +++++++++++++++++++ 14 files changed, 209 insertions(+), 130 deletions(-) create mode 100644 packages/loopring_v3/contracts/lib/TransferUtil.sol diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmExitProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmExitProcess.sol index ecd4e06a4..ace5707ea 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmExitProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmExitProcess.sol @@ -10,6 +10,7 @@ import "../../lib/ERC20SafeTransfer.sol"; import "../../lib/MathUint.sol"; import "../../lib/MathUint96.sol"; import "../../lib/SignatureUtil.sol"; +import "../../lib/TransferUtil.sol"; import "../../thirdparty/SafeCast.sol"; import "./AmmUtil.sol"; import "./AmmData.sol"; @@ -29,6 +30,7 @@ library AmmExitProcess using SafeCast for uint; using SignatureUtil for bytes32; using TransactionReader for ExchangeData.Block; + using TransferUtil for address; event ForcedExitProcessed(address owner, uint96 burnAmount, uint96[] amounts); @@ -66,7 +68,7 @@ library AmmExitProcess if (isForcedExit) { if (!slippageOK) { - AmmUtil.transferOut(address(this), exit.burnAmount, exit.owner); + address(this).transferOut(exit.owner, exit.burnAmount); emit ForcedExitProcessed(exit.owner, 0, new uint96[](0)); return; } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmExitRequest.sol b/packages/loopring_v3/contracts/amm/libamm/AmmExitRequest.sol index 7510bafea..8bfd9ff9d 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmExitRequest.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmExitRequest.sol @@ -4,6 +4,7 @@ pragma solidity ^0.7.0; pragma experimental ABIEncoderV2; import "../../lib/EIP712.sol"; +import "../../lib/TransferUtil.sol"; import "./AmmData.sol"; import "./AmmUtil.sol"; @@ -11,6 +12,9 @@ import "./AmmUtil.sol"; /// @title AmmExitRequest library AmmExitRequest { + using TransferUtil for address; + using TransferUtil for address payable; + bytes32 constant public POOLEXIT_TYPEHASH = keccak256( "PoolExit(address owner,uint96 burnAmount,uint32 burnStorageID,uint96[] exitMinAmounts,uint96 fee,uint32 validUntil)" ); @@ -37,20 +41,21 @@ library AmmExitRequest validUntil: uint32(block.timestamp + S.sharedConfig.maxForcedExitAge()) }); + address eth = address(0); if (force) { require(S.forcedExit[msg.sender].validUntil == 0, "DUPLICATE"); require(S.forcedExitCount < S.sharedConfig.maxForcedExitCount(), "TOO_MANY_FORCED_EXITS"); - AmmUtil.transferIn(address(this), burnAmount); + address(this).transferIn(msg.sender, burnAmount); uint feeAmount = S.sharedConfig.forcedExitFee(); - AmmUtil.transferIn(address(0), feeAmount); - AmmUtil.transferOut(address(0), feeAmount, S.exchange.owner()); + eth.transferIn(msg.sender, feeAmount); + eth.transferOut(S.exchange.owner(), feeAmount); S.forcedExit[msg.sender] = exit; S.forcedExitCount++; } else { - AmmUtil.transferIn(address(0), 0); + eth.transferIn(msg.sender, 0); bytes32 txHash = hash(S.domainSeparator, exit); S.approvedTx[txHash] = true; diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol b/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol index f0a89a664..d791cd44c 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol @@ -50,7 +50,8 @@ library AmmTransactionReceiver returns (AmmData.Context memory) { uint size = S.tokens.length; - uint txsDataPtr = 23; + // Get the position of the txsData in the calldata + uint txsDataPtr = 0; assembly { txsDataPtr := sub(add(txsData.offset, txsDataPtr), 32) } @@ -74,6 +75,7 @@ library AmmTransactionReceiver ) private { + // abi.decode(callbackData, (AmmData.PoolTx)); // Manually decode the encoded PoolTx in `callbackData` AmmData.PoolTxType txType; bytes calldata data; diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmUpdateProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmUpdateProcess.sol index f00dc34a4..6f4f0fcf6 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmUpdateProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmUpdateProcess.sol @@ -20,27 +20,47 @@ library AmmUpdateProcess internal view { - uint txsDataPtr = ctx.txsDataPtr + 5; + // Start by reading the first 28 bytes into packedData + uint txsDataPtr = ctx.txsDataPtr + 28; for (uint i = 0; i < ctx.tokens.length; i++) { - // txType | owner | accountID | tokenID | feeBips + AmmData.Token memory token = ctx.tokens[i]; + + /* + AmmUpdateTransaction.readTx(txsData, ctx.txIdx++ * ExchangeData.TX_DATA_AVAILABILITY_SIZE, update); + + require( + update.owner == address(this) && + update.accountID == ctx.accountID && + update.tokenID == token.tokenID && + update.feeBips == ctx.feeBips && + update.tokenWeight == token.weight, + "INVALID_AMM_UPDATE_TX_DATA" + ); + */ + + // txType (1) | owner (20) | accountID (4) | tokenID (2) | feeBips (1) uint packedDataA; - // tokenWeight | nonce | balance + // tokenWeight (12) | nonce (4) | balance (12) uint packedDataB; assembly { packedDataA := calldataload(txsDataPtr) packedDataB := calldataload(add(txsDataPtr, 28)) } - AmmData.Token memory token = ctx.tokens[i]; - require( + // txType == ExchangeData.TransactionType.AMM_UPDATE && + // update.owner == address(this) + // update.accountID == ctx.accountID && + // update.tokenID == token.tokenID && + // update.feeBips == ctx.feeBips && packedDataA & 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffff == (uint(ExchangeData.TransactionType.AMM_UPDATE) << 216) | (uint(address(this)) << 56) | (ctx.accountID << 24) | (token.tokenID << 8) | ctx.feeBips && + // update.tokenWeight == token.weight (packedDataB >> 128) & 0xffffffffffffffffffffffff == token.weight, "INVALID_AMM_UPDATE_TX_DATA" ); - ctx.tokenBalancesL2[i] = uint96(packedDataB & 0xffffffffffffffffffffffff); + ctx.tokenBalancesL2[i] = /*tokenWeight*/uint96(packedDataB & 0xffffffffffffffffffffffff); txsDataPtr += ExchangeData.TX_DATA_AVAILABILITY_SIZE; } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmUtil.sol b/packages/loopring_v3/contracts/amm/libamm/AmmUtil.sol index 1fdab5adc..1050ac3bb 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmUtil.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmUtil.sol @@ -35,8 +35,23 @@ library AmmUtil // Check the signature type require(signature.toUint8Unsafe(0) == L2_SIGNATURE_TYPE, "INVALID_SIGNATURE_TYPE"); + /* // Read the signature verification transaction - uint txsDataPtr = ctx.txsDataPtr - 2; + SignatureVerificationTransaction.SignatureVerification memory verification; + SignatureVerificationTransaction.readTx(txsData, ctx.txIdx++ * ExchangeData.TX_DATA_AVAILABILITY_SIZE, verification); + + // Verify that the hash was signed on L2 + require( + verification.owner == owner && + verification.data == uint(txHash) >> 3, + "INVALID_OFFCHAIN_L2_APPROVAL" + ); + */ + + // Read the signature verification transaction + // Start by reading the first 21 bytes into packedData + uint txsDataPtr = ctx.txsDataPtr + 21; + // packedData: txType (1) | owner (20) uint packedData; uint data; assembly { @@ -60,7 +75,10 @@ library AmmUtil pure returns (uint packedData, address to, address from) { - uint txsDataPtr = ctx.txsDataPtr; + // TransferTransaction.readTx(txsData, ctx.txIdx++ * ExchangeData.TX_DATA_AVAILABILITY_SIZE, transfer); + + // Start by reading the first 23 bytes into packedData + uint txsDataPtr = ctx.txsDataPtr + 23; // packedData: txType (1) | type (1) | fromAccountID (4) | toAccountID (4) | tokenID (2) | amount (3) | feeTokenID (2) | fee (2) | storageID (4) assembly { packedData := calldataload(txsDataPtr) @@ -102,31 +120,4 @@ library AmmUtil return (1000 - 5) <= ratio && ratio <= (1000 + 5); } } - - function transferIn( - address token, - uint amount - ) - internal - { - if (token == address(0)) { - require(msg.value == amount, "INVALID_ETH_VALUE"); - } else if (amount > 0) { - token.safeTransferFromAndVerify(msg.sender, address(this), amount); - } - } - - function transferOut( - address token, - uint amount, - address to - ) - internal - { - if (token == address(0)) { - to.sendETHAndVerify(amount, gasleft()); - } else { - token.safeTransferAndVerify(to, amount); - } - } } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawal.sol b/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawal.sol index 6e0e064bb..037ecc0e2 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawal.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawal.sol @@ -5,6 +5,7 @@ pragma experimental ABIEncoderV2; import "../../lib/ERC20.sol"; import "../../lib/MathUint.sol"; +import "../../lib/TransferUtil.sol"; import "./AmmData.sol"; import "./AmmPoolToken.sol"; import "./AmmStatus.sol"; @@ -17,6 +18,7 @@ library AmmWithdrawal using AmmPoolToken for AmmData.State; using AmmStatus for AmmData.State; using MathUint for uint; + using TransferUtil for address; function withdrawWhenOffline( AmmData.State storage S @@ -44,12 +46,9 @@ library AmmWithdrawal uint totalSupply = S.totalSupply(); for (uint i = 0; i < S.tokens.length; i++) { address token = S.tokens[i].addr; - uint balance = token == address(0) ? - address(this).balance : - ERC20(token).balanceOf(address(this)); - + uint balance = token.selfBalance(); uint amount = balance.mul(poolAmount) / totalSupply; - AmmUtil.transferOut(token, amount, msg.sender); + token.transferOut(msg.sender, amount); } S._totalSupply = S._totalSupply.sub(poolAmount); diff --git a/packages/loopring_v3/contracts/aux/agents/FastWithdrawalAgent.sol b/packages/loopring_v3/contracts/aux/agents/FastWithdrawalAgent.sol index dc4ff1496..47162ef8d 100644 --- a/packages/loopring_v3/contracts/aux/agents/FastWithdrawalAgent.sol +++ b/packages/loopring_v3/contracts/aux/agents/FastWithdrawalAgent.sol @@ -7,10 +7,10 @@ import "../../core/iface/IAgentRegistry.sol"; import "../../core/iface/IExchangeV3.sol"; import "../../lib/Claimable.sol"; import "../../lib/EIP712.sol"; -import "../../lib/ERC20SafeTransfer.sol"; import "../../lib/MathUint.sol"; import "../../lib/ReentrancyGuard.sol"; import "../../lib/SignatureUtil.sol"; +import "../../lib/TransferUtil.sol"; /// @title Fast withdrawal agent implementation. With the help of liquidity providers (LPs), /// exchange operators can convert any normal withdrawals into fast withdrawals. @@ -45,8 +45,8 @@ contract FastWithdrawalAgent is ReentrancyGuard, IAgent { using AddressUtil for address; using AddressUtil for address payable; - using ERC20SafeTransfer for address; using MathUint for uint; + using TransferUtil for address; event Processed( address exchange, @@ -111,10 +111,9 @@ contract FastWithdrawalAgent is ReentrancyGuard, IAgent ) { // Transfer the tokens immediately to the requested address // using funds from the liquidity provider (`msg.sender`). - transfer( + withdrawal.token.transferFromOut( liquidityProvider, withdrawal.to, - withdrawal.token, withdrawal.amount ); success = true; @@ -132,21 +131,4 @@ contract FastWithdrawalAgent is ReentrancyGuard, IAgent success ); } - - function transfer( - address from, - address to, - address token, - uint amount - ) - internal - { - if (amount > 0) { - if (token == address(0)) { - to.sendETHAndVerify(amount, gasleft()); // ETH - } else { - token.safeTransferFromAndVerify(from, to, amount); // ERC20 token - } - } - } } \ No newline at end of file diff --git a/packages/loopring_v3/contracts/aux/bridge/Bridge.sol b/packages/loopring_v3/contracts/aux/bridge/Bridge.sol index b23c1ab38..c26360ad5 100644 --- a/packages/loopring_v3/contracts/aux/bridge/Bridge.sol +++ b/packages/loopring_v3/contracts/aux/bridge/Bridge.sol @@ -12,6 +12,7 @@ import "../../lib/ERC20.sol"; import "../../lib/MathUint.sol"; import "../../lib/MathUint96.sol"; import "../../lib/ReentrancyGuard.sol"; +import "../../lib/TransferUtil.sol"; import "./IBridge.sol"; @@ -26,6 +27,7 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable using ERC20SafeTransfer for address; using MathUint for uint; using MathUint96 for uint96; + using TransferUtil for address; // Transfers packed as: // - address owner : 20 bytes @@ -155,7 +157,8 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable external onlyFromExchangeOwner { - uint txsDataPtr = 23; + // Get the offset to txsData in the calldata + uint txsDataPtr = 0; assembly { txsDataPtr := sub(add(txsData.offset, txsDataPtr), 32) } @@ -188,7 +191,7 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable for (uint i = 0; i < transfers.length; i++) { InternalBridgeTransfer memory transfer = transfers[i]; - // Pack the transfer data to compare agains batch deposit hash + // Pack the transfer data to compare against batch deposit hash address owner = transfer.owner; uint16 tokenID = transfer.tokenID; uint amount = transfer.amount; @@ -217,10 +220,9 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable address tokenAddress = exchange.getTokenAddress(transfers[idx].tokenID); - _transferOut( - tokenAddress, - transfers[idx].amount, - transfers[idx].owner + tokenAddress.transferOut( + transfers[idx].owner, + transfers[idx].amount ); } } @@ -339,6 +341,7 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable function _processTransactions(Context memory ctx) internal { + // abi.decode(callbackData, (BridgeOperations)) // Get the calldata structs directly from the encoded calldata bytes data TransferBatch[] calldata transferBatches; ConnectorCalls[] calldata connectorCalls; @@ -574,8 +577,11 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable ); // Verify withdrawal data - uint txsDataPtr = ctx.txsDataPtr - 21; + // Start by reading the first 2 bytes into header + uint txsDataPtr = ctx.txsDataPtr + 2; + // header: txType (1) | type (1) uint header; + // packedData: tokenID (2) | amount (12) | feeTokenID (2) | fee (2) uint packedData; bytes20 dataHash; assembly { @@ -705,20 +711,6 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable } } - function _transferOut( - address token, - uint amount, - address to - ) - internal - { - if (token == address(0)) { - to.sendETHAndVerify(amount, gasleft()); - } else { - token.safeTransferAndVerify(to, amount); - } - } - function _hashTransferBatch( bytes memory transfers ) @@ -845,7 +837,10 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable pure returns (uint packedData, address to, address from) { - uint txsDataPtr = ctx.txsDataPtr; + // TransferTransaction.readTx(txsData, ctx.txIdx++ * ExchangeData.TX_DATA_AVAILABILITY_SIZE, transfer); + + // Start by reading the first 23 bytes into packedData + uint txsDataPtr = ctx.txsDataPtr + 23; // packedData: txType (1) | type (1) | fromAccountID (4) | toAccountID (4) | tokenID (2) | amount (3) | feeTokenID (2) | fee (2) | storageID (4) assembly { packedData := calldataload(txsDataPtr) @@ -864,8 +859,24 @@ contract Bridge is IBridge, ReentrancyGuard, Claimable internal pure { + /* + // Read the signature verification transaction + SignatureVerificationTransaction.SignatureVerification memory verification; + SignatureVerificationTransaction.readTx(txsData, ctx.txIdx++ * ExchangeData.TX_DATA_AVAILABILITY_SIZE, verification); + + // Verify that the hash was signed on L2 + require( + verification.owner == owner && + verification.accountID == ctx.accountID && + verification.data == uint(txHash) >> 3, + "INVALID_OFFCHAIN_L2_APPROVAL" + ); + */ + // Read the signature verification transaction - uint txsDataPtr = ctx.txsDataPtr + 2; + // Start by reading the first 25 bytes into packedDate + uint txsDataPtr = ctx.txsDataPtr + 25; + // packedData: txType (1) | owner (20) | accountID (4) uint packedData; uint data; assembly { diff --git a/packages/loopring_v3/contracts/aux/fast-withdrawal/FastWithdrawalLiquidityProvider.sol b/packages/loopring_v3/contracts/aux/fast-withdrawal/FastWithdrawalLiquidityProvider.sol index cbc2050e7..f1c0b82f0 100644 --- a/packages/loopring_v3/contracts/aux/fast-withdrawal/FastWithdrawalLiquidityProvider.sol +++ b/packages/loopring_v3/contracts/aux/fast-withdrawal/FastWithdrawalLiquidityProvider.sol @@ -7,10 +7,10 @@ import "../agents/FastWithdrawalAgent.sol"; import "../../lib/OwnerManagable.sol"; import "../../lib/EIP712.sol"; import "../../lib/ERC20.sol"; -import "../../lib/ERC20SafeTransfer.sol"; import "../../lib/MathUint.sol"; import "../../lib/ReentrancyGuard.sol"; import "../../lib/SignatureUtil.sol"; +import "../../lib/TransferUtil.sol"; /// @title Basic contract storing funds for a liquidity provider. @@ -19,9 +19,9 @@ contract FastWithdrawalLiquidityProvider is ReentrancyGuard, OwnerManagable { using AddressUtil for address; using AddressUtil for address payable; - using ERC20SafeTransfer for address; using MathUint for uint; using SignatureUtil for bytes32; + using TransferUtil for address; struct FastWithdrawalApproval { @@ -91,11 +91,7 @@ contract FastWithdrawalLiquidityProvider is ReentrancyGuard, OwnerManagable nonReentrant onlyOwner { - if (token == address(0)) { - to.sendETHAndVerify(amount, gasleft()); // ETH - } else { - token.safeTransferAndVerify(to, amount); // ERC20 token - } + token.transferOut(to, amount); } // Allows the LP to enable the necessary ERC20 approvals diff --git a/packages/loopring_v3/contracts/converters/BaseConverter.sol b/packages/loopring_v3/contracts/converters/BaseConverter.sol index 96ce17df0..07eb1b2d8 100644 --- a/packages/loopring_v3/contracts/converters/BaseConverter.sol +++ b/packages/loopring_v3/contracts/converters/BaseConverter.sol @@ -10,6 +10,7 @@ import "../lib/AddressUtil.sol"; import "../lib/ERC20SafeTransfer.sol"; import "../lib/MathUint.sol"; import "../lib/LPERC20.sol"; +import "../lib/TransferUtil.sol"; /// @author Brecht Devos - @@ -18,6 +19,7 @@ abstract contract BaseConverter is LPERC20, Claimable, Drainable using AddressUtil for address; using ERC20SafeTransfer for address; using MathUint for uint; + using TransferUtil for address; event ConversionSuccess (uint amountIn, uint amountOut); event ConversionFailed (string reason); @@ -106,12 +108,7 @@ abstract contract BaseConverter is LPERC20, Claimable, Drainable address token = failed ? tokenIn : tokenOut; // Current balance in the contract - uint balance = 0; - if (token == address(0)) { - balance = address(this).balance; - } else { - balance = ERC20(token).balanceOf(address(this)); - } + uint balance = token.selfBalance(); // Share to withdraw uint amount = balance.mul(poolAmount) / totalSupply; @@ -126,11 +123,7 @@ abstract contract BaseConverter is LPERC20, Claimable, Drainable // Send remaining amount to `to` uint amountToSend = amount.sub(repayAmount); - if (token == address(0)) { - to.sendETHAndVerify(amountToSend, gasleft()); // ETH - } else { - token.safeTransferAndVerify(to, amountToSend); // ERC20 token - } + token.transferOut(to, amountToSend); } // Wrapper around `convert` which enforces only self calls. diff --git a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol index b29642839..2951799dc 100644 --- a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol @@ -3,11 +3,11 @@ pragma solidity ^0.7.0; pragma experimental ABIEncoderV2; -import "../../lib/AddressUtil.sol"; import "../../lib/EIP712.sol"; import "../../lib/ERC20SafeTransfer.sol"; import "../../lib/MathUint.sol"; import "../../lib/ReentrancyGuard.sol"; +import "../../lib/TransferUtil.sol"; import "../../thirdparty/proxies/OwnedUpgradabilityProxy.sol"; import "../iface/IAgentRegistry.sol"; import "../iface/IExchangeV3.sol"; @@ -30,9 +30,8 @@ import "./libtransactions/TransferTransaction.sol"; /// @author Daniel Wang - contract ExchangeV3 is IExchangeV3, ReentrancyGuard { - using AddressUtil for address; - using ERC20SafeTransfer for address; using MathUint for uint; + using TransferUtil for address; using ExchangeAdmins for ExchangeData.State; using ExchangeBalances for ExchangeData.State; using ExchangeBlocks for ExchangeData.State; @@ -163,13 +162,8 @@ contract ExchangeV3 is IExchangeV3, ReentrancyGuard { require(recipient != address(0), "INVALID_ADDRESS"); - if (token == address(0)) { - uint amount = address(this).balance; - recipient.sendETHAndVerify(amount, gasleft()); - } else { - uint amount = ERC20(token).balanceOf(address(this)); - token.safeTransferAndVerify(recipient, amount); - } + uint amount = token.selfBalance(); + token.transferOut(recipient, amount); } function isUserOrAgent(address owner) diff --git a/packages/loopring_v3/contracts/lib/Drainable.sol b/packages/loopring_v3/contracts/lib/Drainable.sol index 0cdf4e1e6..8251fd102 100644 --- a/packages/loopring_v3/contracts/lib/Drainable.sol +++ b/packages/loopring_v3/contracts/lib/Drainable.sol @@ -3,8 +3,7 @@ pragma solidity ^0.7.0; import "./AddressUtil.sol"; -import "./ERC20.sol"; -import "./ERC20SafeTransfer.sol"; +import "./TransferUtil.sol"; /// @title Drainable @@ -12,8 +11,7 @@ import "./ERC20SafeTransfer.sol"; /// @dev Standard functionality to allow draining funds from a contract. abstract contract Drainable { - using AddressUtil for address; - using ERC20SafeTransfer for address; + using TransferUtil for address; event Drained( address to, @@ -30,13 +28,8 @@ abstract contract Drainable { require(canDrain(msg.sender, token), "UNAUTHORIZED"); - if (token == address(0)) { - amount = address(this).balance; - to.sendETHAndVerify(amount, gasleft()); // ETH - } else { - amount = ERC20(token).balanceOf(address(this)); - token.safeTransferAndVerify(to, amount); // ERC20 token - } + amount = token.balanceOf(msg.sender); + token.transferOut(to, amount); emit Drained(to, token, amount); } diff --git a/packages/loopring_v3/contracts/lib/LPERC20.sol b/packages/loopring_v3/contracts/lib/LPERC20.sol index f753c4029..1e0cc1dae 100644 --- a/packages/loopring_v3/contracts/lib/LPERC20.sol +++ b/packages/loopring_v3/contracts/lib/LPERC20.sol @@ -36,7 +36,7 @@ contract LPERC20 is ERC20 internal { DOMAIN_SEPARATOR = EIP712.hash(EIP712.Domain( - "LPERC20", + _name, "1.0", address(this) )); diff --git a/packages/loopring_v3/contracts/lib/TransferUtil.sol b/packages/loopring_v3/contracts/lib/TransferUtil.sol new file mode 100644 index 000000000..fb716a75e --- /dev/null +++ b/packages/loopring_v3/contracts/lib/TransferUtil.sol @@ -0,0 +1,91 @@ + +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; +pragma experimental ABIEncoderV2; + +import "./AddressUtil.sol"; +import "./ERC20.sol"; +import "./ERC20SafeTransfer.sol"; + + +/// @title TransferUtil +library TransferUtil +{ + using AddressUtil for address; + using ERC20SafeTransfer for address; + + function selfBalance( + address token + ) + internal + view + returns (uint amount) + { + amount = balanceOf(token, address(this)); + } + + function balanceOf( + address token, + address addr + ) + internal + view + returns (uint amount) + { + if (token == address(0)) { + amount = addr.balance; + } else { + amount = ERC20(token).balanceOf(addr); + } + } + + function transferIn( + address token, + address from, + uint amount + ) + internal + { + if (token == address(0)) { + require(msg.value == amount, "INVALID_ETH_VALUE"); + } else if (amount > 0) { + token.safeTransferFromAndVerify(from, address(this), amount); + } + } + + function transferOut( + address token, + address to, + uint amount + ) + internal + { + if (amount == 0) { + return; + } + if (token == address(0)) { + to.sendETHAndVerify(amount, gasleft()); + } else { + token.safeTransferAndVerify(to, amount); + } + } + + function transferFromOut( + address token, + address from, + address to, + uint amount + ) + internal + { + if (amount == 0) { + return; + } + if (token == address(0)) { + to.sendETHAndVerify(amount, gasleft()); // ETH + } else { + token.safeTransferFromAndVerify(from, to, amount); // ERC20 token + } + } +}