[eth] Wrap parsing method with unchecked (#378)

* Move parsing method to unchecked

There is no overflow possible there

* Update another for loop to be unchecked

ethereum/contracts/pyth/Pyth.sol

@@ -37,8 +37,10 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         uint requiredFee = getUpdateFee(updateData);
         require(msg.value >= requiredFee, "insufficient paid fee amount");
  
-        for(uint i = 0; i < updateData.length; i++) {
+        for(uint i = 0; i < updateData.length; ) {
             updatePriceBatchFromVm(updateData[i]);
+
+            unchecked { i++; }
         }
 
         emit UpdatePriceFeeds(msg.sender, updateData.length, requiredFee);
@@ -58,158 +60,168 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
     }
 
     function parseAndProcessBatchPriceAttestation(IWormhole.VM memory vm) internal {
-        bytes memory encoded = vm.payload;    
-        uint index = 0;
-
-        // Check header
-        {
-            uint32 magic = encoded.toUint32(index);
-            index += 4;
-            require(magic == 0x50325748, "invalid magic value");
+        // Most of the math operations below are simple additions.
+        // In the places that there is more complex operation there is
+        // a comment explaining why it is safe. Also, byteslib
+        // operations have proper require.
+        unchecked {
+            bytes memory encoded = vm.payload;    
+            uint index = 0;
+
+            // Check header
+            {
+                uint32 magic = encoded.toUint32(index);
+                index += 4;
+                require(magic == 0x50325748, "invalid magic value");
+
+                uint16 versionMajor = encoded.toUint16(index);
+                index += 2;
+                require(versionMajor == 3, "invalid version major, expected 3");
+
+                uint16 versionMinor = encoded.toUint16(index);
+                index += 2;
+                require(versionMinor >= 0, "invalid version minor, expected 0 or more");
+
+                uint16 hdrSize = encoded.toUint16(index);
+                index += 2;
+
+                // NOTE(2022-04-19): Currently, only payloadId comes after
+                // hdrSize. Future extra header fields must be read using a
+                // separate offset to respect hdrSize, i.e.:
+                //
+                // uint hdrIndex = 0;
+                // bpa.header.payloadId = encoded.toUint8(index + hdrIndex);
+                // hdrIndex += 1;
+                //
+                // bpa.header.someNewField = encoded.toUint32(index + hdrIndex);
+                // hdrIndex += 4;
+                //
+                // // Skip remaining unknown header bytes
+                // index += bpa.header.hdrSize;
+
+                uint8 payloadId = encoded.toUint8(index);
+
+                // Skip remaining unknown header bytes
+                index += hdrSize;
+
+                // Payload ID of 2 required for batch headerBa
+                require(payloadId == 2, "invalid payload ID, expected 2 for BatchPriceAttestation");
+            }
 
-            uint16 versionMajor = encoded.toUint16(index);
+            // Parse the number of attestations
+            uint16 nAttestations = encoded.toUint16(index);
             index += 2;
-            require(versionMajor == 3, "invalid version major, expected 3");
 
-            uint16 versionMinor = encoded.toUint16(index);
+            // Parse the attestation size
+            uint16 attestationSize = encoded.toUint16(index);
             index += 2;
-            require(versionMinor >= 0, "invalid version minor, expected 0 or more");
 
-            uint16 hdrSize = encoded.toUint16(index);
-            index += 2;
+            // Given the message is valid the arithmetic below should not overflow, and
+            // even if it overflows then the require would fail.
+            require(encoded.length == (index + (attestationSize * nAttestations)), "invalid BatchPriceAttestation size");
 
-            // NOTE(2022-04-19): Currently, only payloadId comes after
-            // hdrSize. Future extra header fields must be read using a
-            // separate offset to respect hdrSize, i.e.:
-            //
-            // uint hdrIndex = 0;
-            // bpa.header.payloadId = encoded.toUint8(index + hdrIndex);
-            // hdrIndex += 1;
-            //
-            // bpa.header.someNewField = encoded.toUint32(index + hdrIndex);
-            // hdrIndex += 4;
-            //
-            // // Skip remaining unknown header bytes
-            // index += bpa.header.hdrSize;
-
-            uint8 payloadId = encoded.toUint8(index);
-
-            // Skip remaining unknown header bytes
-            index += hdrSize;
-
-            // Payload ID of 2 required for batch headerBa
-            require(payloadId == 2, "invalid payload ID, expected 2 for BatchPriceAttestation");
-        }
+            PythInternalStructs.PriceInfo memory info;
+            bytes32 priceId;
+            uint freshPrices = 0;
+            
+            // Deserialize each attestation
+            for (uint j=0; j < nAttestations; j++) {
+                // NOTE: We don't advance the global index immediately.
+                // attestationIndex is an attestation-local offset used
+                // for readability and easier debugging.
+                uint attestationIndex = 0;
 
-        // Parse the number of attestations
-        uint16 nAttestations = encoded.toUint16(index);
-        index += 2;
+                // Unused bytes32 product id
+                attestationIndex += 32;
 
-        // Parse the attestation size
-        uint16 attestationSize = encoded.toUint16(index);
-        index += 2;
-        require(encoded.length == (index + (attestationSize * nAttestations)), "invalid BatchPriceAttestation size");
+                priceId = encoded.toBytes32(index + attestationIndex);
+                attestationIndex += 32;
 
-        PythInternalStructs.PriceInfo memory info;
-        bytes32 priceId;
-        uint freshPrices = 0;
-        
-        // Deserialize each attestation
-        for (uint j=0; j < nAttestations; j++) {
-            // NOTE: We don't advance the global index immediately.
-            // attestationIndex is an attestation-local offset used
-            // for readability and easier debugging.
-            uint attestationIndex = 0;
+                info.price.price = int64(encoded.toUint64(index + attestationIndex));
+                attestationIndex += 8;
 
-            // Unused bytes32 product id
-            attestationIndex += 32;
+                info.price.conf = encoded.toUint64(index + attestationIndex);
+                attestationIndex += 8;
 
-            priceId = encoded.toBytes32(index + attestationIndex);
-            attestationIndex += 32;
+                info.price.expo = int32(encoded.toUint32(index + attestationIndex));
+                info.emaPrice.expo = info.price.expo;
+                attestationIndex += 4;
 
-            info.price.price = int64(encoded.toUint64(index + attestationIndex));
-            attestationIndex += 8;
+                info.emaPrice.price = int64(encoded.toUint64(index + attestationIndex));
+                attestationIndex += 8;
 
-            info.price.conf = encoded.toUint64(index + attestationIndex);
-            attestationIndex += 8;
+                info.emaPrice.conf = encoded.toUint64(index + attestationIndex);
+                attestationIndex += 8;
 
-            info.price.expo = int32(encoded.toUint32(index + attestationIndex));
-            info.emaPrice.expo = info.price.expo;
-            attestationIndex += 4;
+                {
+                    // Status is an enum (encoded as uint8) with the following values:
+                    // 0 = UNKNOWN: The price feed is not currently updating for an unknown reason.
+                    // 1 = TRADING: The price feed is updating as expected.
+                    // 2 = HALTED: The price feed is not currently updating because trading in the product has been halted.
+                    // 3 = AUCTION: The price feed is not currently updating because an auction is setting the price.
+                    uint8 status = encoded.toUint8(index + attestationIndex);
+                    attestationIndex += 1;
 
-            info.emaPrice.price = int64(encoded.toUint64(index + attestationIndex));
-            attestationIndex += 8;
+                    // Unused uint32 numPublishers
+                    attestationIndex += 4;
 
-            info.emaPrice.conf = encoded.toUint64(index + attestationIndex);
-            attestationIndex += 8;
+                    // Unused uint32 numPublishers
+                    attestationIndex += 4;
 
-            {
-                // Status is an enum (encoded as uint8) with the following values:
-                // 0 = UNKNOWN: The price feed is not currently updating for an unknown reason.
-                // 1 = TRADING: The price feed is updating as expected.
-                // 2 = HALTED: The price feed is not currently updating because trading in the product has been halted.
-                // 3 = AUCTION: The price feed is not currently updating because an auction is setting the price.
-                uint8 status = encoded.toUint8(index + attestationIndex);
-                attestationIndex += 1;
-
-                // Unused uint32 numPublishers
-                attestationIndex += 4;
+                    // Unused uint64 attestationTime
+                    attestationIndex += 8;
 
-                // Unused uint32 numPublishers
-                attestationIndex += 4;
+                    info.price.publishTime = encoded.toUint64(index + attestationIndex);
+                    info.emaPrice.publishTime = info.price.publishTime;
+                    attestationIndex += 8;
 
-                // Unused uint64 attestationTime
-                attestationIndex += 8;
+                    if (status == 1) { // status == TRADING
+                        attestationIndex += 24;
+                    } else {
+                        // If status is not trading then the latest available price is
+                        // the previous price info that are passed here.
 
-                info.price.publishTime = encoded.toUint64(index + attestationIndex);
-                info.emaPrice.publishTime = info.price.publishTime;
-                attestationIndex += 8;
+                        // Previous publish time
+                        info.price.publishTime = encoded.toUint64(index + attestationIndex);
+                        attestationIndex += 8;
 
-                if (status == 1) { // status == TRADING
-                    attestationIndex += 24;
-                } else {
-                    // If status is not trading then the latest available price is
-                    // the previous price info that are passed here.
+                        // Previous price
+                        info.price.price = int64(encoded.toUint64(index + attestationIndex));
+                        attestationIndex += 8;
 
-                    // Previous publish time
-                    info.price.publishTime = encoded.toUint64(index + attestationIndex);
-                    attestationIndex += 8;
+                        // Previous confidence
+                        info.price.conf = encoded.toUint64(index + attestationIndex);
+                        attestationIndex += 8;
 
-                    // Previous price
-                    info.price.price = int64(encoded.toUint64(index + attestationIndex));
-                    attestationIndex += 8;
+                        // The EMA is last updated when the aggregate had trading status,
+                        // so, we use previous publish time here too.
+                        info.emaPrice.publishTime = info.price.publishTime;
+                    }
+                }
 
-                    // Previous confidence
-                    info.price.conf = encoded.toUint64(index + attestationIndex);
-                    attestationIndex += 8;
 
-                    // The EMA is last updated when the aggregate had trading status,
-                    // so, we use previous publish time here too.
-                    info.emaPrice.publishTime = info.price.publishTime;
-                }
-            }
+                require(attestationIndex <= attestationSize, "INTERNAL: Consumed more than `attestationSize` bytes");
 
-            require(attestationIndex <= attestationSize, "INTERNAL: Consumed more than `attestationSize` bytes");
+                // Respect specified attestation size for forward-compat
+                index += attestationSize;
 
-            // Respect specified attestation size for forward-compat
-            index += attestationSize;
+                // Store the attestation
+                uint64 latestPublishTime = latestPriceInfoPublishTime(priceId);
 
-            // Store the attestation
-            uint64 latestPublishTime = latestPriceInfoPublishTime(priceId);
+                bool fresh = false;
+                if(info.price.publishTime > latestPublishTime) {
+                    freshPrices += 1;
+                    fresh = true;
+                    setLatestPriceInfo(priceId, info);
+                }
 
-            bool fresh = false;
-            if(info.price.publishTime > latestPublishTime) {
-                freshPrices += 1;
-                fresh = true;
-                setLatestPriceInfo(priceId, info);
+                emit PriceFeedUpdate(priceId, fresh, vm.emitterChainId, vm.sequence, latestPublishTime,
+                    info.price.publishTime, info.price.price, info.price.conf);
             }
 
-            emit PriceFeedUpdate(priceId, fresh, vm.emitterChainId, vm.sequence, latestPublishTime,
-                info.price.publishTime, info.price.price, info.price.conf);
-        }
-
 
-        emit BatchPriceFeedUpdate(vm.emitterChainId, vm.sequence, nAttestations, freshPrices);
+            emit BatchPriceFeedUpdate(vm.emitterChainId, vm.sequence, nAttestations, freshPrices);
+        }
     }
 
     function queryPriceFeed(bytes32 id) public view override returns (PythStructs.PriceFeed memory priceFeed){