feat: remove unchecked increments in for loops (#40)

rhinestonewtf · Oct 31, 2023 · c7e1c31 · c7e1c31
1 parent 67c485f
commit c7e1c31
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 36 deletions.
diff --git a/foundry.toml b/foundry.toml
@@ -1,5 +1,5 @@
 [profile.default]
-solc_version = "0.8.21"
+solc_version = "0.8.22"
 src = 'src'
 out = 'out'
 libs = ['lib']

diff --git a/src/Common.sol b/src/Common.sol
@@ -16,18 +16,6 @@ error InvalidLength();
 error InvalidSignature();
 error NotFound();
 
-/**
- * @dev A helper function to work with unchecked iterators in loops.
- * @param i The current index.
- *
- * @return j The next index.
- */
-function uncheckedInc(uint256 i) pure returns (uint256 j) {
-    unchecked {
-        j = i + 1;
-    }
-}
-
 /**
  * @dev Returns the current's block timestamp. This method is overridden during tests and used to simulate the
  * current block time.

diff --git a/src/base/Attestation.sol b/src/base/Attestation.sol
@@ -24,7 +24,6 @@ import {
     NotFound,
     ZERO_TIMESTAMP,
     InvalidLength,
-    uncheckedInc,
     InvalidSchema,
     _time
 } from "../Common.sol";
@@ -97,7 +96,7 @@ abstract contract Attestation is IAttestation, AttestationResolve, ReentrancyGua
         ModuleRecord storage moduleRecord =
             _getModule({ moduleAddress: multiRequests[0].data[0].subject });
 
-        for (uint256 i; i < length; i = uncheckedInc(i)) {
+        for (uint256 i; i < length; ++i) {
             // The last batch is handled slightly differently: if the total available ETH wasn't spent in full and there
             // is a remainder - it will be refunded back to the attester (something that we can only verify during the
             // last and final batch).
@@ -165,7 +164,7 @@ abstract contract Attestation is IAttestation, AttestationResolve, ReentrancyGua
         uint256 requestsLength = multiRequests.length;
 
         // should cache length
-        for (uint256 i; i < requestsLength; i = uncheckedInc(i)) {
+        for (uint256 i; i < requestsLength; ++i) {
             // The last batch is handled slightly differently: if the total available ETH wasn't spent in full and there
             // is a remainder - it will be refunded back to the attester (something that we can only verify during the
             // last and final batch).
@@ -237,7 +236,7 @@ abstract contract Attestation is IAttestation, AttestationResolve, ReentrancyGua
         uint256[] memory values = new uint256[](length);
 
         // write every attesatation provided to registry's storage
-        for (uint256 i; i < length; i = uncheckedInc(i)) {
+        for (uint256 i; i < length; ++i) {
             (attestationRecords[i], values[i]) = _writeAttestation({
                 schemaUID: schemaUID,
                 resolverUID: resolverUID,
@@ -400,7 +399,7 @@ abstract contract Attestation is IAttestation, AttestationResolve, ReentrancyGua
         );
         uint256[] memory values = new uint256[](length);
 
-        for (uint256 i; i < length; i = uncheckedInc(i)) {
+        for (uint256 i; i < length; ++i) {
             RevocationRequestData memory revocationRequests = revocationRequestDatas[i];
 
             attestationRecords[i] =

diff --git a/src/base/AttestationDelegation.sol b/src/base/AttestationDelegation.sol
@@ -20,7 +20,6 @@ import {
     NotFound,
     ZERO_TIMESTAMP,
     InvalidLength,
-    uncheckedInc,
     InvalidSchema,
     _time
 } from "../Common.sol";
@@ -96,7 +95,7 @@ abstract contract AttestationDelegation is IAttestation, Attestation {
         // Its possible that the MultiAttestationRequests is attesting different modules, that thus have different resolvers
         // gas bad
 
-        for (uint256 i; i < length; i = uncheckedInc(i)) {
+        for (uint256 i; i < length; ++i) {
             // The last batch is handled slightly differently: if the total available ETH wasn't spent in full and there
             // is a remainder - it will be refunded back to the attester (something that we can only verify during the
             // last and final batch).
@@ -116,7 +115,7 @@ abstract contract AttestationDelegation is IAttestation, Attestation {
             }
 
             // Verify signatures. Note that the signatures are assumed to be signed with increasing nonces.
-            for (uint256 j; j < dataLength; j = uncheckedInc(j)) {
+            for (uint256 j; j < dataLength; ++j) {
                 _verifyAttest(
                     DelegatedAttestationRequest({
                         schemaUID: multiDelegatedRequest.schemaUID,
@@ -186,7 +185,7 @@ abstract contract AttestationDelegation is IAttestation, Attestation {
         ModuleRecord memory moduleRecord =
             _getModule({ moduleAddress: multiDelegatedRequests[0].data[0].subject });
 
-        for (uint256 i; i < length; i = uncheckedInc(i)) {
+        for (uint256 i; i < length; ++i) {
             // The last batch is handled slightly differently: if the total available ETH wasn't spent in full and there
             // is a remainder - it will be refunded back to the attester (something that we can only verify during the
             // last and final batch).
@@ -205,7 +204,7 @@ abstract contract AttestationDelegation is IAttestation, Attestation {
             }
 
             // Verify EIP712 signatures. Please note that the signatures are assumed to be signed with increasing nonces.
-            for (uint256 j; j < dataLength; j = uncheckedInc(j)) {
+            for (uint256 j; j < dataLength; ++i) {
                 _verifyRevoke(
                     DelegatedRevocationRequest({
                         schemaUID: multiDelegatedRequest.schemaUID,

diff --git a/src/base/AttestationResolve.sol b/src/base/AttestationResolve.sol
@@ -15,7 +15,7 @@ import {
 } from "../interface/IAttestation.sol";
 import { EIP712Verifier } from "./EIP712Verifier.sol";
 
-import { ZERO_ADDRESS, AccessDenied, uncheckedInc } from "../Common.sol";
+import { ZERO_ADDRESS, AccessDenied } from "../Common.sol";
 import { AttestationDataRef, writeAttestationData, readAttestationData } from "../DataTypes.sol";
 
 /**
@@ -129,7 +129,7 @@ abstract contract AttestationResolve is IAttestation, EIP712Verifier {
         IResolver resolverContract = resolver.resolver;
         if (address(resolverContract) == ZERO_ADDRESS) {
             // Ensure that we don't accept payments if there is no resolver.
-            for (uint256 i; i < length; i = uncheckedInc(i)) {
+            for (uint256 i; i < length; ++i) {
                 if (values[i] != 0) revert NotPayable();
             }
 
@@ -138,7 +138,7 @@ abstract contract AttestationResolve is IAttestation, EIP712Verifier {
 
         uint256 totalUsedValue;
 
-        for (uint256 i; i < length; i = uncheckedInc(i)) {
+        for (uint256 i; i < length; ++i) {
             uint256 value = values[i];
 
             // Ensure that we don't accept payments which can't be forwarded to the resolver.

diff --git a/src/base/Query.sol b/src/base/Query.sol
@@ -13,7 +13,7 @@ import {
     ModuleRecord
 } from "./Attestation.sol";
 
-import { AccessDenied, NotFound, ZERO_TIMESTAMP, InvalidLength, uncheckedInc } from "../Common.sol";
+import { AccessDenied, NotFound, ZERO_TIMESTAMP, InvalidLength } from "../Common.sol";
 
 /**
  * @title Query
@@ -68,7 +68,7 @@ abstract contract Query is IQuery {
         uint256 timeNow = block.timestamp;
         attestedAtArray = new uint256[](attestersLength);
 
-        for (uint256 i; i < attestersLength; i = uncheckedInc(i)) {
+        for (uint256 i; i < attestersLength; ++i) {
             AttestationRecord storage attestation =
                 _getAttestation({ moduleAddress: module, attester: attesters[i] });
             if (attestation.revocationTime != ZERO_TIMESTAMP) {
@@ -110,7 +110,7 @@ abstract contract Query is IQuery {
         uint256 timeNow = block.timestamp;
         attestedAtArray = new uint256[](attestersLength);
 
-        for (uint256 i; i < attestersLength; i = uncheckedInc(i)) {
+        for (uint256 i; i < attestersLength; ++i) {
             AttestationRecord storage attestation =
                 _getAttestation({ moduleAddress: module, attester: attesters[i] });
 
@@ -159,7 +159,7 @@ abstract contract Query is IQuery {
     {
         uint256 attesterssLength = attesters.length;
         attestations = new AttestationRecord[](attesterssLength);
-        for (uint256 i; i < attesterssLength; i = uncheckedInc(i)) {
+        for (uint256 i; i < attesterssLength; ++i) {
             attestations[i] = findAttestation(module, attesters[i]);
         }
     }

diff --git a/src/external/ResolverBase.sol b/src/external/ResolverBase.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 pragma solidity ^0.8.19;
 
-import { AccessDenied, ZERO_TIMESTAMP, NotFound, ZERO_ADDRESS, uncheckedInc } from "../Common.sol";
+import { AccessDenied, ZERO_TIMESTAMP, NotFound, ZERO_ADDRESS } from "../Common.sol";
 import { AttestationRecord, ModuleRecord } from "../DataTypes.sol";
 import { IResolver } from "./IResolver.sol";
 
@@ -102,7 +102,7 @@ abstract contract ResolverBase is IResolver {
         // possible to send too much ETH anyway.
         uint256 remainingValue = msg.value;
 
-        for (uint256 i; i < length; i = uncheckedInc(i)) {
+        for (uint256 i; i < length; ++i) {
             // Ensure that the attester/revoker doesn't try to spend more than available.
             uint256 value = values[i];
             if (value > remainingValue) {
@@ -155,7 +155,7 @@ abstract contract ResolverBase is IResolver {
         // possible to send too much ETH anyway.
         uint256 remainingValue = msg.value;
 
-        for (uint256 i; i < length; i = uncheckedInc(i)) {
+        for (uint256 i; i < length; ++i) {
             // Ensure that the attester/revoker doesn't try to spend more than available.
             uint256 value = values[i];
             if (value > remainingValue) {

diff --git a/src/integrations/MockRegistry.sol b/src/integrations/MockRegistry.sol
@@ -3,7 +3,6 @@ pragma solidity ^0.8.19;
 
 import { IQuery } from "../interface/IQuery.sol";
 import { AttestationRecord } from "../DataTypes.sol";
-import { uncheckedInc } from "../Common.sol";
 
 /**
  * @title MockRegistry
@@ -35,7 +34,7 @@ contract MockRegistry is IQuery {
     {
         uint256 attestersLength = attesters.length;
         uint256[] memory attestedAtArray = new uint256[](attestersLength);
-        for (uint256 i; i < attestersLength; i = uncheckedInc(i)) {
+        for (uint256 i; i < attestersLength; ++i) {
             attestedAtArray[i] = uint256(1234);
         }
         return attestedAtArray;
@@ -53,7 +52,7 @@ contract MockRegistry is IQuery {
     {
         uint256 attestersLength = attesters.length;
         uint256[] memory attestedAtArray = new uint256[](attestersLength);
-        for (uint256 i; i < attestersLength; i = uncheckedInc(i)) {
+        for (uint256 i; i < attestersLength; ++i) {
             attestedAtArray[i] = uint256(1234);
         }
         return attestedAtArray;