From 0163984ea4f9d7623613d2d7cd8e2e946acecdf5 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 25 Jun 2018 14:13:31 -0700 Subject: Add comments to dispatchTransferFrom --- .../Exchange/MixinAssetProxyDispatcher.sol | 86 ++++++++++++++-------- 1 file changed, 56 insertions(+), 30 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index 96763970c..3203322aa 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -20,13 +20,11 @@ pragma solidity ^0.4.24; import "../../utils/Ownable/Ownable.sol"; import "../../utils/LibBytes/LibBytes.sol"; -import "./libs/LibExchangeErrors.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "../AssetProxy/interfaces/IAssetProxy.sol"; contract MixinAssetProxyDispatcher is Ownable, - LibExchangeErrors, MAssetProxyDispatcher { using LibBytes for bytes; @@ -51,7 +49,7 @@ contract MixinAssetProxyDispatcher is address currentAssetProxy = assetProxies[assetProxyId]; require( oldAssetProxy == currentAssetProxy, - ASSET_PROXY_MISMATCH + "ASSET_PROXY_MISMATCH" ); IAssetProxy assetProxy = IAssetProxy(newAssetProxy); @@ -61,7 +59,7 @@ contract MixinAssetProxyDispatcher is bytes4 newAssetProxyId = assetProxy.getProxyId(); require( newAssetProxyId == assetProxyId, - ASSET_PROXY_ID_MISMATCH + "ASSET_PROXY_ID_MISMATCH" ); } @@ -100,61 +98,89 @@ contract MixinAssetProxyDispatcher is { // Do nothing if no amount should be transferred. if (amount > 0) { - require(assetData.length >= 4, "ASSET_DATA_LENGTH"); + // Ensure assetData length is valid + require( + assetData.length > 3, + "LENGTH_GREATER_THAN_3_REQUIRED" + ); // Lookup assetProxy bytes4 assetProxyId; assembly { - assetProxyId := and(mload(add(assetData, 32)), -0xFFFFFFFF00000000000000000000000000000000000000000000000000000000 + assetProxyId := and(mload( + add(assetData, 32)), + 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000 ) } IAssetProxy assetProxy = assetProxies[assetProxyId]; + // Ensure that assetProxy exists require( assetProxy != address(0), - ASSET_PROXY_DOES_NOT_EXIST + "ASSET_PROXY_DOES_NOT_EXIST" ); - /* - assetProxy.transferFrom( - assetData, - from, - to, - amount - ); - return; - */ - + // We construct calldata for the `assetProxy.transferFrom` ABI. + // The layout of this calldata is in the table below. + // + // | Area | Offset | Length | Contents | + // | -------- |--------|---------|-------------------------------------------- | + // | Header | 0 | 4 | function selector | + // | Params | | 4 * 32 | function parameters: | + // | | 4 | | 1. offset to assetData (*) | + // | | 36 | | 2. from | + // | | 68 | | 3. to | + // | | 100 | | 4. amount | + // | Data | | | assetData: | + // | | 132 | 32 | assetData Length | + // | | 164 | ** | assetData Contents | + bytes4 transferFromSelector = IAssetProxy(assetProxy).transferFrom.selector; - bool success; + bool success; assembly { - - let cdStart := mload(0x40) - let dataAreaLength := mul(div(add(mload(assetData), 63), 32), 32) + /////// Setup State /////// + // `cdStart` is the start of the calldata for `assetProxy.transferFrom` (equal to free memory ptr). + let cdStart := mload(64) + // `dataAreaLength` is the total number of words needed to store `assetData` + // As-per the ABI spec, this value is padded up to the nearest multiple of 32, + // and includes 32-bytes for length. + let dataAreaLength := and(add(mload(assetData), 63), 0xFFFFFFFFFFFE0) + // `cdEnd` is the end of the calldata for `assetProxy.transferFrom`. let cdEnd := add(cdStart, add(132, dataAreaLength)) + + /////// Setup Header Area /////// + // This area holds the 4-byte `transferFromSelector`. mstore(cdStart, transferFromSelector) + + /////// Setup Params Area /////// + // Each parameter is padded to 32-bytes. The entire Params Area is 128 bytes. + // Notes: + // 1. The offset to `assetData` is the length of the Params Area (128 bytes). + // 2. A 20-byte mask is applied to addresses to zero-out the unused bytes. mstore(add(cdStart, 4), 128) mstore(add(cdStart, 36), and(from, 0xffffffffffffffffffffffffffffffffffffffff)) mstore(add(cdStart, 68), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) mstore(add(cdStart, 100), amount) + /////// Setup Data Area /////// + // This area holds `assetData`. let dataArea := add(cdStart, 132) for {} lt(dataArea, cdEnd) {} { mstore(dataArea, mload(assetData)) dataArea := add(dataArea, 32) assetData := add(assetData, 32) } - + + /////// Call `assetProxy.transferFrom` using the constructed calldata /////// success := call( - gas, - assetProxy, - 0, - cdStart, - sub(cdEnd, cdStart), - cdStart, - 0 + gas, // forward all gas + assetProxy, // call address of asset proxy + 0, // don't send any ETH + cdStart, // pointer to start of input + sub(cdEnd, cdStart), // length of input + cdStart, // write output over input + 0 // output size is 0 bytes ) } require( -- cgit v1.2.3