Converter/Bridge Feedback

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

packages/loopring_v3/contracts/amm/libamm/AmmExitRequest.sol

@@ -4,13 +4,17 @@ pragma solidity ^0.7.0;
 pragma experimental ABIEncoderV2;
 
 import "../../lib/EIP712.sol";
+import "../../lib/TransferUtil.sol";
 import "./AmmData.sol";
 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;

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;

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

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

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

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
-            }
-        }
-    }
 }