Add Greg's documentation to MixinErc721Transfer

1 files changed, 44 insertions, 33 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
index af986adfa..c170917ea 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
@@ -27,7 +27,9 @@ contract MixinERC721Transfer is
     LibTransferErrors
 {
     using LibBytes for bytes;
-     bytes4 constant SAFE_TRANSFER_FROM_SELECTOR = bytes4(keccak256("safeTransferFrom(address,address,uint256,bytes)"));
+    
+    bytes4 constant SAFE_TRANSFER_FROM_SELECTOR = bytes4(keccak256("safeTransferFrom(address,address,uint256,bytes)"));
+    
     /// @dev Internal version of `transferFrom`.
     /// @param assetData Encoded byte array.
     /// @param from Address to transfer asset from.
@@ -57,44 +59,54 @@ contract MixinERC721Transfer is
         // We construct calldata for the `token.safeTransferFrom` 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. from                                   |
-        // |          | 36     |         |   2. to                                     |
-        // |          | 68     |         |   3. tokenId                                |
-        // |          | 100    |         |   4. offset to receiverData (*)             |
-        // | Data     |        |         | receiverData:                               |
-        // |          | 132    | 32      | receiverData Length                         |
-        // |          | 164    | **      | receiverData Contents                       |
+        // | Area     | Offset | Length  | Contents                            |
+        // |----------|--------|---------|-------------------------------------|
+        // | Header   | 0      | 4       | function selector                   |
+        // | Params   |        | 4 * 32  | function parameters:                |
+        // |          | 4      |         |   1. from                           |
+        // |          | 36     |         |   2. to                             |
+        // |          | 68     |         |   3. tokenId                        |
+        // |          | 100    |         |   4. offset to receiverData (*)     |
+        // | Data     |        |         | receiverData:                       |
+        // |          | 132    | 32      | receiverData Length                 |
+        // |          | 164    | **      | receiverData Contents               |
 
         bytes4 safeTransferFromSelector = SAFE_TRANSFER_FROM_SELECTOR;
         bool success;
         assembly {
             /////// Setup State ///////
-            // `cdStart` is the start of the calldata for `token.safeTransferFrom` (equal to free memory ptr).
+            // `cdStart` is the start of the calldata for
+            // `token.safeTransferFrom` (equal to free memory ptr).
             let cdStart := mload(64)
-            // `dataAreaLength` is the total number of words needed to store `receiverData`
-            //  As-per the ABI spec, this value is padded up to the nearest multiple of 32,
-            //  and includes 32-bytes for length.
-            //  It's calculated as folows:
+            // `dataAreaLength` is the total number of words
+            // needed to store `receiverData`
+            // As-per the ABI spec, this value is padded up to
+            // the nearest multiple of 32,
+            // and includes 32-bytes for length.
+            // It's calculated as folows:
             //      - Unpadded length in bytes = `mload(receiverData) + 32`
-            //      - Add 31 to this value then divide by 32 to get the length in words.
-            //      - Multiply this value by 32 to get the padded length in bytes.
-            let dataAreaLength := mul(div(add(mload(receiverData), 63), 32), 32)
+            //      - Add 31 to convert rounding down to rounding up.
+            //        Combined with the previous and this is `63`.
+            //      - Round down to nearest multiple of 32 by clearing
+            //        bits 0x1F. This is done with `and` and a mask.
+            let dataAreaLength := and(add(mload(receiverData), 63), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0)
             // `cdEnd` is the end of the calldata for `token.safeTransferFrom`.
             let cdEnd := add(cdStart, add(132, dataAreaLength))
 
             /////// Setup Header Area ///////
             // This area holds the 4-byte `transferFromSelector`.
+            // Any trailing data in transferFromSelector will be
+            // overwritten in the next `mstore` call.
             mstore(cdStart, safeTransferFromSelector)
             
             /////// Setup Params Area ///////
-            // Each parameter is padded to 32-bytes. The entire Params Area is 128 bytes.
+            // Each parameter is padded to 32-bytes.
+            // The entire Params Area is 128 bytes.
             // Notes:
-            //   1. A 20-byte mask is applied to addresses to zero-out the unused bytes.
-            //   2. The offset to `receiverData` is the length of the Params Area (128 bytes).
+            //   1. A 20-byte mask is applied to addresses
+            //      to zero-out the unused bytes.
+            //   2. The offset to `receiverData` is the length
+            //      of the Params Area (128 bytes).
             mstore(add(cdStart, 4), and(from, 0xffffffffffffffffffffffffffffffffffffffff))
             mstore(add(cdStart, 36), and(to, 0xffffffffffffffffffffffffffffffffffffffff))
             mstore(add(cdStart, 68), tokenId)
@@ -108,19 +120,18 @@ contract MixinERC721Transfer is
                 dataArea := add(dataArea, 32)
                 receiverData := add(receiverData, 32)
             }
-
-            /////// Call `token.safeTransferFrom` using the constructed calldata ///////
+            
+            /////// Call `token.safeTransferFrom` using the calldata ///////
             success := call(
-                gas,
-                token,
-                0,
-                cdStart,
-                sub(cdEnd, cdStart),
-                cdStart,
-                0
+                gas,                    // forward all gas
+                token,                  // call address of token contract
+                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(
             success,
             TRANSFER_FAILED