Merge pull request #41 from bcnmy/fix/todos-reviews

Fix Todos and Reviews
bcnmy · Nov 25, 2024 · fdea539 · fdea539
2 parents 04580de + f721e4f
commit fdea539
Show file tree

Hide file tree

Showing 16 changed files with 238 additions and 113 deletions.
diff --git a/.gitignore b/.gitignore
@@ -5,6 +5,7 @@ node_modules
 out
 docs
 storageLayout
+cache_forge
 
 # files
 *.env

diff --git a/contracts/libraries/TokenPaymasterParserLib.sol b/contracts/libraries/TokenPaymasterParserLib.sol
@@ -12,9 +12,9 @@ library TokenPaymasterParserLib {
     function parsePaymasterAndData(
         bytes calldata paymasterAndData
     )
-        external
+        internal
         pure
-        returns (IBiconomyTokenPaymaster.PaymasterMode mode, bytes memory modeSpecificData)
+        returns (IBiconomyTokenPaymaster.PaymasterMode mode, bytes calldata modeSpecificData)
     {
         unchecked {
             mode = IBiconomyTokenPaymaster.PaymasterMode(uint8(bytes1(paymasterAndData[PAYMASTER_MODE_OFFSET])));
@@ -25,15 +25,15 @@ library TokenPaymasterParserLib {
     function parseExternalModeSpecificData(
         bytes calldata modeSpecificData
     )
-        external
+        internal
         pure
         returns (
             uint48 validUntil,
             uint48 validAfter,
             address tokenAddress,
-            uint256 tokenPrice, // Review: why uint128 and not uint256. in independent mode it is uint256
+            uint256 tokenPrice, 
             uint32 externalPriceMarkup,
-            bytes memory signature
+            bytes calldata signature
         )
     {
         validUntil = uint48(bytes6(modeSpecificData[:6]));
@@ -47,7 +47,7 @@ library TokenPaymasterParserLib {
     function parseIndependentModeSpecificData(
         bytes calldata modeSpecificData
     )
-        external
+        internal
         pure
         returns (address tokenAddress)
     {

diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol
@@ -16,9 +16,7 @@ import { TokenPaymasterParserLib } from "../libraries/TokenPaymasterParserLib.so
 import { SignatureCheckerLib } from "solady/utils/SignatureCheckerLib.sol";
 import { ECDSA as ECDSA_solady } from "solady/utils/ECDSA.sol";
 import "account-abstraction/core/Helpers.sol";
-import "./swaps/Uniswapper.sol";
-// Todo: marked for removal
-import "forge-std/console2.sol";
+import { Uniswapper, IV3SwapRouter } from "./swaps/Uniswapper.sol";
 
 /**
  * @title BiconomyTokenPaymaster
@@ -72,7 +70,7 @@ contract BiconomyTokenPaymaster is
         uint256 priceExpiryDurationArg,
         uint256 nativeAssetDecimalsArg,
         IOracle nativeAssetToUsdOracleArg,
-        ISwapRouter uniswapRouterArg,
+        IV3SwapRouter uniswapRouterArg,
         address wrappedNativeArg,
         address[] memory independentTokensArg, // Array of token addresses supported by the paymaster in independent
         // mode
@@ -440,6 +438,24 @@ contract BiconomyTokenPaymaster is
         );
     }
 
+    /**
+     * @dev Get the price of a token in USD
+     * @param tokenAddress The address of the token to get the price of
+     * @return price The price of the token in USD
+     */
+    function getPrice(address tokenAddress) public view returns (uint256) {
+        return _getPrice(tokenAddress);
+    }
+
+    /**
+     * @dev Check if a token is supported
+     * @param tokenAddress The address of the token to check
+     * @return bool True if the token is supported, false otherwise
+     */
+    function isTokenSupported(address tokenAddress) public view returns (bool) {
+        return independentTokenDirectory[tokenAddress].oracle != IOracle(address(0));
+    }
+
     /**
      * @dev Validate a user operation.
      * This method is abstract in BasePaymaster and must be implemented in derived contracts.
@@ -456,7 +472,7 @@ contract BiconomyTokenPaymaster is
         override
         returns (bytes memory context, uint256 validationData)
     {
-        (PaymasterMode mode, bytes memory modeSpecificData) = userOp.paymasterAndData.parsePaymasterAndData();
+        (PaymasterMode mode, bytes calldata modeSpecificData) = userOp.paymasterAndData.parsePaymasterAndData();
 
         if (uint8(mode) > 1) {
             revert InvalidPaymasterMode();
@@ -505,7 +521,6 @@ contract BiconomyTokenPaymaster is
 
 
             uint256 tokenAmount;
-            // Review
             {
                 uint256 maxFeePerGas = UserOperationLib.unpackMaxFeePerGas(userOp);
                 tokenAmount = ((maxCost + maxPenalty + (unaccountedGas * maxFeePerGas)) * externalPriceMarkup * tokenPrice)
@@ -517,7 +532,14 @@ contract BiconomyTokenPaymaster is
 
             // deduct max penalty from the token amount we pass to the postOp
             // so we don't refund it at postOp
-            context = abi.encode(userOp.sender, tokenAddress, tokenAmount-((maxPenalty*tokenPrice*externalPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_PRICE_DENOMINATOR)), tokenPrice, externalPriceMarkup, userOpHash);
+            context = abi.encode(
+                userOp.sender,
+                tokenAddress,
+                tokenAmount-((maxPenalty*tokenPrice*externalPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_PRICE_DENOMINATOR)),
+                tokenPrice,
+                externalPriceMarkup,
+                userOpHash
+            );
             validationData = _packValidationData(false, validUntil, validAfter);
         } else if (mode == PaymasterMode.INDEPENDENT) {
             // Use only oracles for the token specified in modeSpecificData
@@ -528,12 +550,12 @@ contract BiconomyTokenPaymaster is
             // Get address for token used to pay
             address tokenAddress = modeSpecificData.parseIndependentModeSpecificData();
             uint256 tokenPrice = _getPrice(tokenAddress);
+
             if(tokenPrice == 0) {
                 revert TokenNotSupported();
             }
             uint256 tokenAmount;
 
-            // TODO: Account for penalties here
             {
                 // Calculate token amount to precharge
                 uint256 maxFeePerGas = UserOperationLib.unpackMaxFeePerGas(userOp);
@@ -545,7 +567,14 @@ contract BiconomyTokenPaymaster is
             SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount);
 
             context =
-                abi.encode(userOp.sender, tokenAddress, tokenAmount-((maxPenalty*tokenPrice*independentPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_PRICE_DENOMINATOR)), tokenPrice, independentPriceMarkup, userOpHash);
+                abi.encode(
+                    userOp.sender,
+                    tokenAddress,
+                    tokenAmount-((maxPenalty*tokenPrice*independentPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_PRICE_DENOMINATOR)),
+                    tokenPrice,
+                    independentPriceMarkup,
+                    userOpHash
+                );
             validationData = 0; // Validation success and price is valid indefinetly
         }
     }
@@ -587,7 +616,6 @@ contract BiconomyTokenPaymaster is
             emit TokensRefunded(userOpSender, tokenAddress, refundAmount, userOpHash);
         }
 
-        // Todo: Review events and what we need to emit.
         emit PaidGasInTokens(
             userOpSender, tokenAddress, actualGasCost, actualTokenAmount, appliedPriceMarkup, tokenPrice, userOpHash
         );

diff --git a/contracts/token/oracles/TwapOracle.sol b/contracts/token/oracles/TwapOracle.sol
@@ -3,8 +3,8 @@ pragma solidity ^0.8.27;
 
 import { IOracle } from "../../interfaces/oracles/IOracle.sol";
 import { IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
-import { OracleLibrary } from "@uniswap/v3-periphery/libraries/OracleLibrary.sol";
-import { IUniswapV3PoolImmutables } from "@uniswap/v3-core/interfaces/pool/IUniswapV3PoolImmutables.sol";
+import { OracleLibrary } from "@uniswap/v3-periphery/contracts/libraries/OracleLibrary.sol";
+import { IUniswapV3PoolImmutables } from "@uniswap/v3-core/contracts/interfaces/pool/IUniswapV3PoolImmutables.sol";
 
 contract TwapOracle is IOracle {
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/

diff --git a/contracts/token/swaps/Uniswapper.sol b/contracts/token/swaps/Uniswapper.sol
@@ -3,8 +3,8 @@ pragma solidity ^0.8.27;
 
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import "@uniswap/v3-periphery/contracts/interfaces/ISwapRouter.sol";
 import "@uniswap/v3-periphery/contracts/interfaces/IPeripheryPayments.sol";
+import { IV3SwapRouter } from "@uniswap/swap-router-contracts/contracts/interfaces/IV3SwapRouter.sol";
 
 /**
  * @title Uniswapper
@@ -13,8 +13,10 @@ import "@uniswap/v3-periphery/contracts/interfaces/IPeripheryPayments.sol";
  * @notice Based on Infinitism's Uniswap Helper contract
  */
 abstract contract Uniswapper {
+
+    event SwappingReverted(address tokenIn, uint256 amountIn, bytes reason);
     /// @notice The Uniswap V3 SwapRouter contract
-    ISwapRouter public immutable uniswapRouter;
+    IV3SwapRouter public immutable uniswapRouter;
 
     /// @notice The ERC-20 token that wraps the native asset for current chain
     address public immutable wrappedNative;
@@ -27,7 +29,7 @@ abstract contract Uniswapper {
     error TokensAndPoolsLengthMismatch();
 
     constructor(
-        ISwapRouter uniswapRouterArg,
+        IV3SwapRouter uniswapRouterArg,
         address wrappedNativeArg,
         address[] memory tokens,
         uint24[] memory tokenPoolFeeTiers
@@ -52,28 +54,28 @@ abstract contract Uniswapper {
     }
 
     function _swapTokenToWeth(address tokenIn, uint256 amountIn, uint256 minAmountOut) internal returns (uint256 amountOut) {
-        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams({
+        IV3SwapRouter.ExactInputSingleParams memory params = IV3SwapRouter.ExactInputSingleParams({
             tokenIn: tokenIn,
             tokenOut: wrappedNative,
             fee: tokenToPools[tokenIn],
             recipient: address(this),
-            deadline: block.timestamp,
+            //deadline: block.timestamp,
             amountIn: amountIn,
             amountOutMinimum: minAmountOut,
             sqrtPriceLimitX96: 0
         });
 
         try uniswapRouter.exactInputSingle(params) returns (uint256 _amountOut) {
             amountOut = _amountOut;
-        } catch {
-            // Review could emit an event here
-            // Uniswap Reverted
+        } catch (bytes memory reason) {
+            emit SwappingReverted(tokenIn, amountIn, reason);
             amountOut = 0;
         }
     }
 
     function _unwrapWeth(uint256 amount) internal {
         if(amount == 0) return;
+        IERC20(wrappedNative).transfer(address(uniswapRouter), amount);
         IPeripheryPayments(address(uniswapRouter)).unwrapWETH9(amount, address(this));
     }
 }
diff --git a/foundry.toml b/foundry.toml
@@ -34,6 +34,8 @@
   optimism = { key = "${API_KEY_OPTIMISTIC_ETHERSCAN}" }
   polygon = { key = "${API_KEY_POLYGONSCAN}" }
   sepolia = { key = "${API_KEY_ETHERSCAN}" }
+  base = { key = "${BASESCAN_API_KEY}" }
+  base-sepolia = { key = "${BASESCAN_API_KEY}" }
 
 [fmt]
   bracket_spacing = true
@@ -54,4 +56,8 @@
   optimism = "https://optimism-mainnet.infura.io/v3/${API_KEY_INFURA}"
   polygon = "https://polygon-mainnet.infura.io/v3/${API_KEY_INFURA}"
   sepolia = "https://sepolia.infura.io/v3/${API_KEY_INFURA}"
+  base = "${BASE_RPC_URL}"
+  base-sepolia = "${BASE_SEPOLIA_RPC_URL}"
 
+# sepolia base
+# forge script DeployTokenPMTest --rpc-url base-sepolia --broadcast --sender 0x531b827c1221EC7CE13266e8F5CB1ec6Ae470be5 --private-key $PRIVATE_KEY_BASE --verify --verifier-url https://api-sepolia.basescan.org/api --etherscan-api-key T3F3Z7T992V5BVN3TSU5KBD7NNK6P4A5D2
diff --git a/package.json b/package.json
@@ -8,33 +8,19 @@
   },
   "dependencies": {
     "@openzeppelin/contracts": "5.1.0",
-    "@rhinestone/modulekit": "^0.4.10",
+    "accountabstraction": "https://github.com/eth-infinitism/account-abstraction#develop",
+    "nexus": "https://github.com/bcnmy/nexus#dev",
+    "solady": "github:vectorized/solady",
     "@uniswap/v3-core": "https://github.com/Uniswap/v3-core#0.8",
     "@uniswap/v3-periphery": "https://github.com/Uniswap/v3-periphery#0.8",
-    "accountabstraction": "https://github.com/eth-infinitism/account-abstraction#develop",
-    "nexus": "https://github.com/bcnmy/nexus#773943fb7bf6cd14a0dc6dcb9f513db53213d1d5"
+    "@uniswap/swap-router-contracts": "https://github.com/Uniswap/swap-router-contracts",
+    "@prb/test": "^0.6.4"
   },
   "devDependencies": {
-    "@bonadocs/docgen": "^1.0.1-alpha.1",
-    "@ethersproject/abstract-provider": "^5.7.0",
-    "@openzeppelin": "https://github.com/OpenZeppelin/openzeppelin-contracts#master",
-    "@prb/test": "^0.6.4",
-    "@typechain/ethers-v6": "^0.5.1",
-    "@types/chai": "^4.3.11",
-    "@types/mocha": ">=10.0.6",
-    "@types/node": ">=20.11.19",
-    "account-abstraction": "github:eth-infinitism/account-abstraction#develop",
     "chai": "^4.3.7",
-    "codecov": "^3.8.3",
-    "erc7579": "https://github.com/erc7579/erc7579-implementation",
     "ethers": "^6.11.1",
-    "excessively-safe-call": "github:nomad-xyz/ExcessivelySafeCall",
-    "module-bases": "github:rhinestonewtf/module-bases",
-    "modulekit": "github:rhinestonewtf/modulekit",
     "prettier": "^3.2.5",
     "prettier-plugin-solidity": "^1.3.1",
-    "sentinellist": "github:rhinestonewtf/sentinellist",
-    "solady": "github:vectorized/solady",
     "solhint": "^4.1.1",
     "solhint-plugin-prettier": "^0.1.0",
     "solidity-coverage": "^0.8.7",
@@ -71,5 +57,6 @@
     "lint:ts-fix": "pnpm prettier --write 'test/**/*.ts' 'scripts/**/*.ts'",
     "lint:sol-fix": "pnpm prettier --write 'contracts/**/*.sol' && pnpm solhint 'contracts/**/*.sol' --fix --noPrompt && forge fmt",
     "lint:fix": "pnpm run lint:sol-fix && pnpm run lint:ts-fix"
-  }
-}
+  },
+  "packageManager": "[email protected]+sha512.beb9e2a803db336c10c9af682b58ad7181ca0fbd0d4119f2b33d5f2582e96d6c0d93c85b23869295b765170fbdaa92890c0da6ada457415039769edf3c959efe"
+}
diff --git a/remappings.txt b/remappings.txt
@@ -1,6 +1,4 @@
-@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/
-@openzeppelin/=node_modules/@openzeppelin/contracts
-@openzeppelin/contracts/contracts/=node_modules/@openzeppelin/contracts/
+@openzeppelin/contracts/=node_modules/@openzeppelin/contracts
 @prb/test/=node_modules/@prb/test/
 @nexus/=node_modules/nexus/
 forge-std/=lib/forge-std/src/
@@ -9,10 +7,9 @@ account-abstraction/=node_modules/account-abstraction/contracts/
 @modulekit/=node_modules/modulekit/src/
 sentinellist/=node_modules/sentinellist/src/
 solady/=node_modules/solady/src/
-@uniswap/v3-periphery/=node_modules/@uniswap/v3-periphery/contracts
-@uniswap/v3-core/=node_modules/@uniswap/v3-core/contracts/
 @uniswap/v3-periphery/contracts/=node_modules/@uniswap/v3-periphery/contracts
 @uniswap/v3-core/contracts/=node_modules/@uniswap/v3-core/contracts/
+@uniswap/swap-router-contracts/contracts/=node_modules/@uniswap/swap-router-contracts/contracts/
 solady/src/=node_modules/solady/src/
 excessively-safe-call/=node_modules/excessively-safe-call/src/
 modulekit/=node_modules/@rhinestone/modulekit/src/
@@ -21,3 +18,4 @@ erc7579/=node_modules/erc7579/src/
 kernel/=node_modules/@zerodev/kernel/src/
 @safe-global/=node_modules/@safe-global/
 solarray/=node_modules/solarray/src/
+erc7739Validator/=node_modules/erc7739-validator-base/src/
diff --git a/remappings.txt.bak b/remappings.txt.bak
@@ -0,0 +1,24 @@
+@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/
+@openzeppelin/=node_modules/@openzeppelin/contracts
+@openzeppelin/contracts/contracts/=node_modules/@openzeppelin/contracts/
+@prb/test/=node_modules/@prb/test/
+@nexus/=node_modules/nexus/
+forge-std/=lib/forge-std/src/
+account-abstraction/=node_modules/account-abstraction/contracts/
+@ERC4337/account-abstraction=node_modules/account-abstraction/
+@modulekit/=node_modules/modulekit/src/
+sentinellist/=node_modules/sentinellist/src/
+solady/=node_modules/solady/src/
+@uniswap/v3-periphery/=node_modules/@uniswap/v3-periphery/contracts
+@uniswap/v3-core/=node_modules/@uniswap/v3-core/contracts/
+@uniswap/v3-periphery/contracts/=node_modules/@uniswap/v3-periphery/contracts
+@uniswap/v3-core/contracts/=node_modules/@uniswap/v3-core/contracts/
+solady/src/=node_modules/solady/src/
+excessively-safe-call/=node_modules/excessively-safe-call/src/
+modulekit/=node_modules/@rhinestone/modulekit/src/
+module-bases/=node_modules/module-bases/src/
+erc7579/=node_modules/erc7579/src/
+kernel/=node_modules/@zerodev/kernel/src/
+@safe-global/=node_modules/@safe-global/
+solarray/=node_modules/solarray/src/
+erc7739Validator/=node_modules/erc7739-validator-base/src/
-Original file line number
+Diff line change
@@ Expand Up / @@ -5,6 +5,7 @@ node_modules @@
     out
     docs
     storageLayout
+    cache_forge
     # files
     *.env
@@ Expand Down @@