Add bit masks where needed in unpackData

contracts/libraries/ArbitraryMessage.sol

@@ -17,17 +17,6 @@ library ArbitraryMessage {
     * offset 111/143/112     : X bytes  :: bytes   - source chain id
     * offset 111/143/112 + X : Y bytes  :: bytes   - destination chain id
 
-    * bytes 1 to 32 are 0 because message length is stored as little endian.
-    * mload always reads 32 bytes.
-    * so we can and have to start reading recipient at offset 20 instead of 32.
-    * if we were to read at 32 the address would contain part of value and be corrupted.
-    * when reading from offset 20 mload will read 12 zero bytes followed
-    * by the 20 recipient address bytes and correctly convert it into an address.
-    * this saves some storage/gas over the alternative solution
-    * which is padding address to 32 bytes and reading recipient at offset 32.
-    * for more details see discussion in:
-    * https://github.com/paritytech/parity-bridge/issues/61
-
     * NOTE: when message structure is changed, make sure that MESSAGE_PACKING_VERSION from VersionableAMB is updated as well
     * NOTE: assembly code uses calldatacopy, make sure that message is passed as the first argument in the calldata
     * @param _data encoded message
@@ -51,48 +40,56 @@ library ArbitraryMessage {
         uint256 datasize;
 
         assembly {
-            messageId := mload(add(_data, 32))
-            sender := mload(add(_data, 52))
-            executor := mload(add(_data, 72))
-            gasLimit := mload(add(_data, 76))
+            messageId := mload(add(_data, 32)) // 32 bytes
+            sender := and(mload(add(_data, 52)), 0xffffffffffffffffffffffffffffffffffffffff) // 20 bytes
+
+            // executor (20 bytes) + gasLimit (4 bytes) + srcChainIdLength (1 byte) + srcChainIdLength (1 bytes) + dataType (1 byte) + remainder (5 bytes)
+            let blob := mload(add(_data, 84))
 
-            let srcChainIdLength := and(mload(add(_data, 77)), 0xff)
-            let dstChainIdLength := and(mload(add(_data, 78)), 0xff)
+            // after bit shift left 12 bytes are zeros automatically
+            executor := shr(96, blob)
+            gasLimit := and(shr(64, blob), 0xffffffff)
 
-            dataType := and(mload(add(_data, 110)), 0xFF00000000000000000000000000000000000000000000000000000000000000)
+            // load source chain id length
+            let chainIdLength := and(shr(56, blob), 0xff)
+
+            dataType := and(shl(208, blob), 0xFF00000000000000000000000000000000000000000000000000000000000000)
             switch dataType
                 case 0x0000000000000000000000000000000000000000000000000000000000000000 {
                     gasPrice := 0
                 }
                 case 0x0100000000000000000000000000000000000000000000000000000000000000 {
                     gasPrice := mload(add(_data, 111)) // 32
-                    srcdataptr := add(srcdataptr, 0x20)
+                    srcdataptr := add(srcdataptr, 32)
                 }
                 case 0x0200000000000000000000000000000000000000000000000000000000000000 {
                     gasPrice := 0
-                    srcdataptr := add(srcdataptr, 0x01)
+                    srcdataptr := add(srcdataptr, 1)
                 }
 
             // at this moment srcdataptr points to sourceChainId
 
             // mask for sourceChainId
             // e.g. length X -> (1 << (X * 8)) - 1
-            let mask := sub(shl(shl(3, srcChainIdLength), 1), 1)
+            let mask := sub(shl(shl(3, chainIdLength), 1), 1)
 
             // increase payload offset by length of source chain id
-            srcdataptr := add(srcdataptr, srcChainIdLength)
+            srcdataptr := add(srcdataptr, chainIdLength)
 
             // write sourceChainId
             mstore(chainIds, and(mload(add(_data, srcdataptr)), mask))
 
             // at this moment srcdataptr points to destinationChainId
 
+            // load destination chain id length
+            chainIdLength := and(shr(48, blob), 0xff)
+
             // mask for destinationChainId
             // e.g. length X -> (1 << (X * 8)) - 1
-            mask := sub(shl(shl(3, dstChainIdLength), 1), 1)
+            mask := sub(shl(shl(3, chainIdLength), 1), 1)
 
             // increase payload offset by length of destination chain id
-            srcdataptr := add(srcdataptr, dstChainIdLength)
+            srcdataptr := add(srcdataptr, chainIdLength)
 
             // write destinationChainId
             mstore(add(chainIds, 32), and(mload(add(_data, srcdataptr)), mask))