AA-293 N-05 using uint instead of uint256 (eth-infinitism#417)

drortirosh · shahafn · web-flow · commit 0146bf79ef59 · 2024-01-30T18:50:10.000+02:00
Co-authored-by: shahafn &lt;shahaflol@gmail.com&gt;
diff --git a/.solhint.json b/.solhint.json
@@ -4,7 +4,7 @@
   "compiler-version": ["error",">=0.7.5"],
     "func-visibility": ["off",{"ignoreConstructors":true}],
     "custom-errors": ["off"],
-    "explicit-types": ["off"],
+    "explicit-types": ["warn", "explicit"],
     "no-global-import": ["off"],
     "immutable-vars-naming": ["off"],
     "mark-callable-contracts": ["off"]
diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol
@@ -37,7 +37,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
 
     //compensate for innerHandleOps' emit message and deposit refund.
     // allow some slack for future gas price changes.
-    uint private constant INNER_GAS_OVERHEAD = 10000;
+    uint256 private constant INNER_GAS_OVERHEAD = 10000;
 
     // Marker for inner call revert on out of gas
     bytes32 private constant INNER_OUT_OF_GAS = hex"deaddead";
@@ -85,7 +85,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
         bytes memory context = getMemoryBytesFromOffset(opInfo.contextOffset);
         bool success;
         {
-            uint saveFreePtr;
+            uint256 saveFreePtr;
             assembly ("memory-safe") {
                 saveFreePtr := mload(0x40)
             }
diff --git a/contracts/core/EntryPointSimulations.sol b/contracts/core/EntryPointSimulations.sol
@@ -180,7 +180,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations {
         unchecked{
         // silly code, to waste some gas to make sure depositTo is always little more
         // expensive than on-chain call
-            uint x = 1;
+            uint256 x = 1;
             while (x < 5) {
                 x++;
             }
diff --git a/contracts/interfaces/IEntryPoint.sol b/contracts/interfaces/IEntryPoint.sol
@@ -105,7 +105,7 @@ interface IEntryPoint is IStakeManager, INonceManager {
     /**
      * A custom revert error of handleOps, to report a revert by account or paymaster.
      * @param opIndex - Index into the array of ops to the failed one (in simulateValidation, this is always zero).
-     * @param reason  - Revert reason. see FailedOp(uint,string), above
+     * @param reason  - Revert reason. see FailedOp(uint256,string), above
      * @param inner   - data from inner cought revert reason
      * @dev note that inner is truncated to 2048 bytes
      */
diff --git a/contracts/samples/LegacyTokenPaymaster.sol b/contracts/samples/LegacyTokenPaymaster.sol
@@ -32,7 +32,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 {
         _mint(address(this), 1);
 
         //owner is allowed to withdraw tokens from the paymaster's balance
-        _approve(address(this), msg.sender, type(uint).max);
+        _approve(address(this), msg.sender, type(uint256).max);
     }
 
 
@@ -55,7 +55,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 {
         _approve(address(this), owner(), 0);
         super.transferOwnership(newOwner);
         // new owner is allowed to withdraw tokens from the paymaster's balance
-        _approve(address(this), newOwner, type(uint).max);
+        _approve(address(this), newOwner, type(uint256).max);
     }
 
     //Note: this method assumes a fixed ratio of token-to-eth. subclass should override to supply oracle
@@ -102,7 +102,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 {
      * the user's TX , back to the state it was before the transaction started (before the validatePaymasterUserOp),
      * and the transaction should succeed there.
      */
-    function _postOp(PostOpMode mode, bytes calldata context, uint256 actualGasCost, uint actualUserOpFeePerGas) internal override {
+    function _postOp(PostOpMode mode, bytes calldata context, uint256 actualGasCost, uint256 actualUserOpFeePerGas) internal override {
         //we don't really care about the mode, we just pay the gas with the user's tokens.
         (mode);
         address sender = abi.decode(context, (address));
diff --git a/contracts/samples/SimpleAccountFactory.sol b/contracts/samples/SimpleAccountFactory.sol
@@ -27,7 +27,7 @@ contract SimpleAccountFactory {
      */
     function createAccount(address owner,uint256 salt) public returns (SimpleAccount ret) {
         address addr = getAddress(owner, salt);
-        uint codeSize = addr.code.length;
+        uint256 codeSize = addr.code.length;
         if (codeSize > 0) {
             return SimpleAccount(payable(addr));
         }
diff --git a/contracts/samples/TokenPaymaster.sol b/contracts/samples/TokenPaymaster.sol
@@ -152,7 +152,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper {
     /// @param actualUserOpFeePerGas - the gas price this UserOp pays. This value is based on the UserOp's maxFeePerGas
     //      and maxPriorityFee (and basefee)
     //      It is not the same as tx.gasprice, which is what the bundler pays.
-    function _postOp(PostOpMode, bytes calldata context, uint256 actualGasCost, uint actualUserOpFeePerGas) internal override {
+    function _postOp(PostOpMode, bytes calldata context, uint256 actualGasCost, uint256 actualUserOpFeePerGas) internal override {
         unchecked {
             uint256 priceMarkup = tokenPaymasterConfig.priceMarkup;
             (
diff --git a/contracts/samples/bls/BLSAccountFactory.sol b/contracts/samples/bls/BLSAccountFactory.sol
@@ -31,12 +31,12 @@ contract BLSAccountFactory {
     function createAccount(uint256 salt, uint256[4] calldata aPublicKey) public returns (BLSAccount) {
 
         // the BLSSignatureAggregator depends on the public-key being the last 4 uint256 of msg.data.
-        uint slot;
+        uint256 slot;
         assembly {slot := aPublicKey}
         require(slot == msg.data.length - 128, "wrong pubkey offset");
 
         address addr = getAddress(salt, aPublicKey);
-        uint codeSize = addr.code.length;
+        uint256 codeSize = addr.code.length;
         if (codeSize > 0) {
             return BLSAccount(payable(addr));
         }
diff --git a/contracts/samples/bls/BLSHelper.sol b/contracts/samples/bls/BLSHelper.sol
@@ -6,8 +6,8 @@ pragma solidity ^0.8.23;
 library  BLSHelper {
 
     struct XY {
-        uint x;
-        uint y;
+        uint256 x;
+        uint256 y;
     }
     /**
      * sum all the points in the array
@@ -17,11 +17,11 @@ library  BLSHelper {
      * @return ret the sum of all points
      */
     function sum(XY[] memory points, uint256 _pp) internal pure returns (XY memory ret){
-        uint x = points[0].x;
-        uint y = points[0].y;
-        uint z = 1;
+        uint256 x = points[0].x;
+        uint256 y = points[0].y;
+        uint256 z = 1;
 
-        for (uint i = 1; i < points.length; i++) {
+        for (uint256 i = 1; i < points.length; i++) {
             (x, y, z) = jacAdd(x, y, z, points[i].x, points[i].y, 1, _pp);
         }
         (x, y) = toAffine(x, y, z, _pp);
@@ -54,7 +54,7 @@ library  BLSHelper {
             return (_x1, _y1, _z1);
 
         // We follow the equations described in https://pdfs.semanticscholar.org/5c64/29952e08025a9649c2b0ba32518e9a7fb5c2.pdf Section 5
-        uint[4] memory zs;
+        uint256[4] memory zs;
         // z1^2, z1^3, z2^2, z2^3
         zs[0] = mulmod(_z1, _z1, _pp);
         zs[1] = mulmod(_z1, zs[0], _pp);
@@ -72,7 +72,7 @@ library  BLSHelper {
         // In case of zs[0] == zs[2] && zs[1] == zs[3], double function should be used
         require(zs[0] != zs[2] || zs[1] != zs[3], "Use jacDouble function instead");
 
-        uint[4] memory hr;
+        uint256[4] memory hr;
         //h
         hr[0] = addmod(zs[2], _pp - zs[0], _pp);
         //r
@@ -160,9 +160,9 @@ library  BLSHelper {
         uint256 z = mulmod(_z, _z, _pp); //z1^2
 
         // s
-        uint s = mulmod(4, mulmod(_x, y, _pp), _pp);
+        uint256 s = mulmod(4, mulmod(_x, y, _pp), _pp);
         // m
-        uint m = addmod(mulmod(3, x, _pp), mulmod(_aa, mulmod(z, z, _pp), _pp), _pp);
+        uint256 m = addmod(mulmod(3, x, _pp), mulmod(_aa, mulmod(z, z, _pp), _pp), _pp);
 
         // x, y, z at this point will be reassigned and rather represent qx, qy, qz from the paper
         // This allows to reduce the gas cost and stack footprint of the algorithm
@@ -193,9 +193,9 @@ library  BLSHelper {
         uint256 _pp)
     internal pure returns (uint256, uint256)
     {
-        uint x = 0;
-        uint y = 0;
-        uint z = 0;
+        uint256 x = 0;
+        uint256 y = 0;
+        uint256 z = 0;
 
         // Double if x1==x2 else add
         if (_x1 == _x2) {
diff --git a/contracts/samples/bls/BLSSignatureAggregator.sol b/contracts/samples/bls/BLSSignatureAggregator.sol
@@ -45,7 +45,7 @@ contract BLSSignatureAggregator is IAggregator {
      * return the trailing 4 words of input data
      */
     function getTrailingPublicKey(bytes memory data) public pure returns (uint256[4] memory publicKey) {
-        uint len = data.length;
+        uint256 len = data.length;
         require(len > 32 * 4, "data too short for sig");
 
         /* solhint-disable-next-line no-inline-assembly */
@@ -65,7 +65,7 @@ contract BLSSignatureAggregator is IAggregator {
         require(signature.length == 64, "BLS: invalid signature");
         (uint256[2] memory blsSignature) = abi.decode(signature, (uint256[2]));
 
-        uint userOpsLen = userOps.length;
+        uint256 userOpsLen = userOps.length;
         uint256[4][] memory blsPublicKeys = new uint256[4][](userOpsLen);
         uint256[2][] memory messages = new uint256[2][](userOpsLen);
         for (uint256 i = 0; i < userOpsLen; i++) {
@@ -152,7 +152,7 @@ contract BLSSignatureAggregator is IAggregator {
      */
     function aggregateSignatures(PackedUserOperation[] calldata userOps) external pure returns (bytes memory aggregatedSignature) {
         BLSHelper.XY[] memory points = new BLSHelper.XY[](userOps.length);
-        for (uint i = 0; i < points.length; i++) {
+        for (uint256 i = 0; i < points.length; i++) {
             (uint256 x, uint256 y) = abi.decode(userOps[i].signature, (uint256, uint256));
             points[i] = BLSHelper.XY(x, y);
         }
diff --git a/contracts/test/BrokenBlsAccount.sol b/contracts/test/BrokenBlsAccount.sol
@@ -60,10 +60,10 @@ contract BrokenBLSAccountFactory {
      * This method returns an existing account address so that entryPoint.getSenderAddress() would work even after account creation
      * Also note that out BLSSignatureAggregator requires that the public-key is the last parameter
      */
-    function createAccount(uint salt, uint256[4] memory aPublicKey) public returns (BrokenBLSAccount) {
+    function createAccount(uint256 salt, uint256[4] memory aPublicKey) public returns (BrokenBLSAccount) {
 
         address addr = getAddress(salt, aPublicKey);
-        uint codeSize = addr.code.length;
+        uint256 codeSize = addr.code.length;
         if (codeSize > 0) {
             return BrokenBLSAccount(payable(addr));
         }
@@ -76,7 +76,7 @@ contract BrokenBLSAccountFactory {
     /**
      * calculate the counterfactual address of this account as it would be returned by createAccount()
      */
-    function getAddress(uint salt, uint256[4] memory aPublicKey) public view returns (address) {
+    function getAddress(uint256 salt, uint256[4] memory aPublicKey) public view returns (address) {
         return Create2.computeAddress(bytes32(salt), keccak256(abi.encodePacked(
                 type(ERC1967Proxy).creationCode,
                 abi.encode(
diff --git a/contracts/test/TestAggregatedAccountFactory.sol b/contracts/test/TestAggregatedAccountFactory.sol
@@ -26,7 +26,7 @@ contract TestAggregatedAccountFactory {
      */
     function createAccount(address owner,uint256 salt) public returns (TestAggregatedAccount ret) {
         address addr = getAddress(owner, salt);
-        uint codeSize = addr.code.length;
+        uint256 codeSize = addr.code.length;
         if (codeSize > 0) {
             return TestAggregatedAccount(payable(addr));
         }
diff --git a/contracts/test/TestExecAccount.sol b/contracts/test/TestExecAccount.sol
@@ -50,7 +50,7 @@ contract TestExecAccountFactory {
 
     function createAccount(address owner, uint256 salt) public returns (address ret) {
         address addr = getAddress(owner, salt);
-        uint codeSize = addr.code.length;
+        uint256 codeSize = addr.code.length;
         if (codeSize > 0) {
             return addr;
         }
diff --git a/contracts/test/TestHelpers.sol b/contracts/test/TestHelpers.sol
@@ -5,7 +5,7 @@ import "../core/Helpers.sol";
 
 contract TestHelpers {
 
-    function parseValidationData(uint validationData) public pure returns (ValidationData memory) {
+    function parseValidationData(uint256 validationData) public pure returns (ValidationData memory) {
         return _parseValidationData(validationData);
     }
 
diff --git a/contracts/test/TestSignatureAggregator.sol b/contracts/test/TestSignatureAggregator.sol
@@ -15,13 +15,13 @@ contract TestSignatureAggregator is IAggregator {
 
     /// @inheritdoc IAggregator
     function validateSignatures(PackedUserOperation[] calldata userOps, bytes calldata signature) external pure override {
-        uint sum = 0;
-        for (uint i = 0; i < userOps.length; i++) {
-            uint nonce = userOps[i].nonce;
+        uint256 sum = 0;
+        for (uint256 i = 0; i < userOps.length; i++) {
+            uint256 nonce = userOps[i].nonce;
             sum += nonce;
         }
-        require(signature.length == 32, "TestSignatureValidator: sig must be uint");
-        (uint sig) = abi.decode(signature, (uint));
+        require(signature.length == 32, "TestSignatureValidator: sig must be uint256");
+        (uint256 sig) = abi.decode(signature, (uint256));
         require(sig == sum, "TestSignatureValidator: aggregated signature mismatch (nonce sum)");
     }
 
@@ -35,8 +35,8 @@ contract TestSignatureAggregator is IAggregator {
      * dummy test aggregator: sum all nonce values of UserOps.
      */
     function aggregateSignatures(PackedUserOperation[] calldata userOps) external pure returns (bytes memory aggregatedSignature) {
-        uint sum = 0;
-        for (uint i = 0; i < userOps.length; i++) {
+        uint256 sum = 0;
+        for (uint256 i = 0; i < userOps.length; i++) {
             sum += userOps[i].nonce;
         }
         return abi.encode(sum);
diff --git a/contracts/test/TestWarmColdAccount.sol b/contracts/test/TestWarmColdAccount.sol
@@ -10,7 +10,7 @@ import "../core/Helpers.sol";
 // COLD_ACCOUNT_ACCESS_COST == 2600, COLD_SLOAD_COST == 2100, WARM_STORAGE_READ_COST == 100
 contract TestWarmColdAccount is IAccount {
     IEntryPoint private ep;
-    uint public state = 1;
+    uint256 public state = 1;
     constructor(IEntryPoint _ep) payable {
         ep = _ep;
     }
diff --git a/contracts/test/TestWrappedNativeToken.sol b/contracts/test/TestWrappedNativeToken.sol
@@ -20,7 +20,7 @@ contract TestWrappedNativeToken is ERC20 {
         _mint(msg.sender, msg.value);
     }
 
-    function withdraw(uint amount) public {
+    function withdraw(uint256 amount) public {
         _burn(msg.sender, amount);
         // solhint-disable-next-line avoid-low-level-calls
         (bool success,) = msg.sender.call{value:amount}("");
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt

Original file line number	Diff line number	Diff line change
`@@ -180,7 +180,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations {`
`180`	`180`	`unchecked{`
`181`	`181`	`// silly code, to waste some gas to make sure depositTo is always little more`
`182`	`182`	`// expensive than on-chain call`
`183`		`- uint x = 1;`
	`183`	`+ uint256 x = 1;`
`184`	`184`	`while (x < 5) {`
`185`	`185`	`x++;`
`186`	`186`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ contract SimpleAccountFactory {`
`27`	`27`	`*/`
`28`	`28`	`function createAccount(address owner,uint256 salt) public returns (SimpleAccount ret) {`
`29`	`29`	`address addr = getAddress(owner, salt);`
`30`		`- uint codeSize = addr.code.length;`
	`30`	`+ uint256 codeSize = addr.code.length;`
`31`	`31`	`if (codeSize > 0) {`
`32`	`32`	`return SimpleAccount(payable(addr));`
`33`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper {`
`152`	`152`	`/// @param actualUserOpFeePerGas - the gas price this UserOp pays. This value is based on the UserOp's maxFeePerGas`
`153`	`153`	`// and maxPriorityFee (and basefee)`
`154`	`154`	`// It is not the same as tx.gasprice, which is what the bundler pays.`
`155`		`- function _postOp(PostOpMode, bytes calldata context, uint256 actualGasCost, uint actualUserOpFeePerGas) internal override {`
	`155`	`+ function _postOp(PostOpMode, bytes calldata context, uint256 actualGasCost, uint256 actualUserOpFeePerGas) internal override {`
`156`	`156`	`unchecked {`
`157`	`157`	`uint256 priceMarkup = tokenPaymasterConfig.priceMarkup;`
`158`	`158`	`(`