code optimisation + dev notes

contracts/token/BiconomyTokenPaymaster.sol

@@ -6,7 +6,8 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 import { IEntryPoint } from "@account-abstraction/contracts/interfaces/IEntryPoint.sol";
 import { UserOperation } from "@account-abstraction/contracts/interfaces/UserOperation.sol";
-import { UserOperationLib } from "@account-abstraction/contracts/interfaces/UserOperation.sol";import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
+import { UserOperationLib } from "@account-abstraction/contracts/interfaces/UserOperation.sol";
+import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import { BasePaymaster } from "../BasePaymaster.sol";
 import { IOracleAggregator } from "./oracles/IOracleAggregator.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
@@ -34,14 +35,6 @@ contract BiconomyTokenPaymaster is BasePaymaster, ReentrancyGuard, TokenPaymaste
     using UserOperationLib for UserOperation;
     using SafeERC20 for IERC20;
 
-    // todo: Marked for removal
-    // In case we also add gasless aspect and more (hybrid) (based on paymasterAndData)
-    /*enum PaymentMode {
-      GASLESS,
-      ERC20,
-      FIXED_FEE
-    }*/
-
     enum ExchangeRateSource {
         EXTERNAL_EXCHANGE_RATE,
         CHAINLINK_PRICE_ORACLE_BASED
@@ -54,8 +47,7 @@ contract BiconomyTokenPaymaster is BasePaymaster, ReentrancyGuard, TokenPaymaste
       uint48 validAfter;
       IERC20 feeToken; 
       uint256 exchangeRate;
-      uint256 fee;
-      // PaymentMode mode; //todo: cleanup
+      uint256 fee; // review instead of flat fee in token terms it could also be a fee multiplier set at token level
       bytes signature;
     }
 
@@ -79,7 +71,7 @@ contract BiconomyTokenPaymaster is BasePaymaster, ReentrancyGuard, TokenPaymaste
     // address public immutable smartAccountFactory;
 
     // review
-    // notice: Since it's always verified by the signing sevrice, below gated mapping state could be avoided.
+    // notice: Since it's always verified by the signing service, below gated mapping state could be avoided.
     mapping(address => bool) private supportedTokens;
 
     IOracleAggregator public oracleAggregator;
@@ -225,7 +217,7 @@ contract BiconomyTokenPaymaster is BasePaymaster, ReentrancyGuard, TokenPaymaste
      */
     function exchangePrice(
         address _token
-    ) external view virtual returns (uint256 exchangeRate) {
+    ) public view virtual returns (uint256 exchangeRate) {
         // get price from oracle aggregator. could be in yul / staticcall then try catch / if else on success and data
         exchangeRate = IOracleAggregator(oracleAggregator).getTokenValueOfOneEth(_token);
         // exchangeRate = (exchangeRate * 99) / 100; // 1% conver chainlink `Deviation threshold`
@@ -244,18 +236,6 @@ contract BiconomyTokenPaymaster is BasePaymaster, ReentrancyGuard, TokenPaymaste
         token.safeTransfer(target, amount);
     }
 
-    function pack(UserOperation calldata userOp) internal pure returns (bytes memory ret) {
-        bytes calldata pnd = userOp.paymasterAndData;
-        // solhint-disable-next-line no-inline-assembly
-        assembly {
-            let ofs := userOp
-            let len := sub(sub(pnd.offset, ofs), 32)
-            ret := mload(0x40)
-            mstore(0x40, add(ret, add(len, 32)))
-            mstore(ret, len)
-            calldatacopy(add(ret, 32), ofs, len)
-        }
-    }
 
      /**
      * @dev This method is called by the off-chain service, to sign the request.
@@ -274,12 +254,18 @@ contract BiconomyTokenPaymaster is BasePaymaster, ReentrancyGuard, TokenPaymaste
         uint256 fee
     ) public view returns (bytes32) {
         //can't use userOp.hash(), since it contains also the paymasterAndData itself.
-        address sender = userOp.getSender();
         return
             keccak256(
                 abi.encode(
-                    // todo: review could remove pack and use just like verifying paymaster and use --via-ir flag while compiling
-                    pack(userOp),
+                    userOp.getSender(),
+                    userOp.nonce,
+                    keccak256(userOp.initCode),
+                    keccak256(userOp.callData),
+                    userOp.callGasLimit,
+                    userOp.verificationGasLimit,
+                    userOp.preVerificationGas,
+                    userOp.maxFeePerGas,
+                    userOp.maxPriorityFeePerGas,
                     block.chainid,
                     address(this),
                     priceSource,
@@ -350,32 +336,35 @@ contract BiconomyTokenPaymaster is BasePaymaster, ReentrancyGuard, TokenPaymaste
         }
 
         address account = userOp.getSender();
-        uint256 gasPriceUserOp = userOp.gasPrice();
 
         require(_isSupportedToken(feeToken), "TokenPaymaster: token is not supported as fee token") ;
 
-        uint256 costOfPost = userOp.gasPrice() * UNACCOUNTED_COST; // unaccountedEPGasOverhead
+        uint256 costOfPost = userOp.maxFeePerGas * UNACCOUNTED_COST; // unaccountedEPGasOverhead
 
         // This model assumes irrespective of priceSource exchangeRate is always sent from outside
         // for below checks you would either need maxCost or some exchangeRate
         uint256 tokenRequiredPreFund = ((requiredPreFund + costOfPost) * exchangeRate) / 10 ** 18;
 
-        if (userOp.initCode.length != 0) {
+        // todo 
+        // review: allowance check possibly can be marked for removal and just rely on balance check
+        // postOp would fail anyway if user removes allowance or doesn't have balance
+        /*if (userOp.initCode.length != 0) {
         _validateConstructor(userOp, feeToken, tokenRequiredPreFund + fee);
         } else {
             require(
              IERC20(feeToken).allowance(account, address(this)) >=
                 (tokenRequiredPreFund + fee),
                 "Paymaster: not enough allowance"
             );
-        }
+        }*/
 
         require(
             IERC20(feeToken).balanceOf(account) >= (tokenRequiredPreFund + fee),
             "Paymaster: not enough balance"
         );
 
-        context = abi.encode(account, feeToken, priceSource, exchangeRate, fee, gasPriceUserOp);
+        context = abi.encode(account, feeToken, priceSource, exchangeRate, fee, userOp.maxFeePerGas,
+                userOp.maxPriorityFeePerGas);
 
         return (context, Helpers._packValidationData(false, validUntil, validAfter));
     }
@@ -392,18 +381,29 @@ contract BiconomyTokenPaymaster is BasePaymaster, ReentrancyGuard, TokenPaymaste
         uint256 actualGasCost
     ) internal virtual override {
 
-        (address account, IERC20 feeToken, ExchangeRateSource priceSource, uint256 exchangeRate, uint256 fee, uint256 gasPriceUserOp) = abi
-            .decode(context, (address, IERC20, ExchangeRateSource, uint256, uint256, uint256));
+        (address account, IERC20 feeToken, ExchangeRateSource priceSource, uint256 exchangeRate, uint256 fee, uint256 maxFeePerGas, uint256 maxPriorityFeePerGas) = abi
+            .decode(context, (address, IERC20, ExchangeRateSource, uint256, uint256, uint256, uint256));
 
         uint256 effectiveExchangeRate = exchangeRate;
 
         if (priceSource == ExchangeRateSource.CHAINLINK_PRICE_ORACLE_BASED) {
-            effectiveExchangeRate = this.exchangePrice(address(feeToken));
+            effectiveExchangeRate = exchangePrice(address(feeToken));
         } 
 
+        uint256 gasPriceUserOp = maxFeePerGas;
+        // review Could do below if we're okay to touch BASEFEE in postOp call
+        /*unchecked {
+            if (maxFeePerGas == maxPriorityFeePerGas) {
+            } else {
+                gasPriceUserOp = Math.min(maxFeePerGas, maxPriorityFeePerGas + block.basefee);
+            }
+        }*/
+
         uint256 actualTokenCost = ((actualGasCost + (UNACCOUNTED_COST * gasPriceUserOp)) * effectiveExchangeRate) / 1e18;
         if (mode != PostOpMode.postOpReverted) {
             // review if below silently fails should notify in event accordingly
+            // failure example
+            // https://dashboard.tenderly.co/yashasvi/project/tx/mumbai/0xa01f4f57eb28b430b287e55e9baf2e2fd7f45b565ee932b0d96d28211d0272ff?trace=0.3.1.1.2.1.0 
             feeToken.safeTransferFrom(account, feeReceiver, actualTokenCost + fee);
             emit TokenPaymasterOperation(account, address(feeToken), actualTokenCost + fee);
         } 

contracts/token/feemanager/FeeManager.sol

@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.17;
+
+import "@openzeppelin/contracts/access/Ownable.sol";
+
+// optional aid contract
+contract FeeManager is Ownable {
+
+    constructor(address _owner) {
+        _transferOwnership(_owner);
+    }
+
+}

contracts/token/oracles/OracleAggregator.sol

@@ -19,6 +19,10 @@ contract OracleAggregator is Ownable{
 
     mapping(address => TokenInfo) internal tokensInfo;
 
+    constructor(address _owner) {
+        _transferOwnership(_owner);
+    }
+
     function setTokenOracle(address token, address callAddress, uint8 decimals, bytes calldata callData, bool signed) external onlyOwner{
         require(callAddress != address(0),"OracleAggregator:: call address can not be zero");
         require(token != address(0),"OracleAggregator:: token address can not be zero");

test/token-paymaster/biconomy-token-paymaster-specs.ts

@@ -111,7 +111,7 @@ describe("Biconomy Token Paymaster", function () {
     // const offchainSignerAddress = await deployer.getAddress();
     const walletOwnerAddress = await walletOwner.getAddress();
 
-    oracleAggregator = await new OracleAggregator__factory(deployer).deploy();
+    oracleAggregator = await new OracleAggregator__factory(deployer).deploy(walletOwnerAddress);
 
     const MockToken = await ethers.getContractFactory("MockToken");
     token = await MockToken.deploy();
@@ -286,14 +286,8 @@ describe("Biconomy Token Paymaster", function () {
       const userOp1 = await fillAndSign(
         {
           sender: walletAddress,
-          verificationGasLimit: 5000000,
+          // verificationGasLimit: 500000,
           // initCode: hexConcat([walletFactory.address, deploymentData]),
-          paymasterAndData: ethers.utils.hexConcat([
-            paymasterAddress,
-            ethers.utils.hexlify(1).slice(0, 4),
-            encodePaymasterData(token.address, MOCK_FX),
-            "0x" + "00".repeat(65),
-          ]),
           // nonce: 0,
           /* callData: encodeERC20Approval(
             userSCW,

yarn.lock

@@ -2,9 +2,9 @@
 # yarn lockfile v1
 
 
-"@0xsequence/create3@git+https://github.com/0xsequence/create3.git":
+"@0xsequence/create3@https://github.com/0xsequence/create3":
   version "3.0.0"
-  resolved "git+https://github.com/0xsequence/create3.git#acc4703a21ec1d71dc2a99db088c4b1f467530fd"
+  resolved "https://github.com/0xsequence/create3#acc4703a21ec1d71dc2a99db088c4b1f467530fd"
   dependencies:
     csv-writer "^1.6.0"
 
@@ -1311,10 +1311,10 @@
   resolved "https://registry.yarnpkg.com/@types/ms/-/ms-0.7.31.tgz#31b7ca6407128a3d2bbc27fe2d21b345397f6197"
   integrity sha512-iiUgKzV9AuaEkZqkOLDIvlQiL6ltuZd9tGcW3gwpnX8JbuiuhFlEGmmFXEXkN50Cvq7Os88IY2v0dkDqXYWVgA==
 
-"@types/node@*", "@types/node@^18.16.0":
-  version "18.16.3"
-  resolved "https://registry.yarnpkg.com/@types/node/-/node-18.16.3.tgz#6bda7819aae6ea0b386ebc5b24bdf602f1b42b01"
-  integrity sha512-OPs5WnnT1xkCBiuQrZA4+YAV4HEJejmHneyraIaxsbev5yCEr6KMwINNFP9wQeFIw8FWcoTqF3vQsa5CDaI+8Q==
+"@types/node@*":
+  version "20.1.0"
+  resolved "https://registry.yarnpkg.com/@types/node/-/node-20.1.0.tgz#258805edc37c327cf706e64c6957f241ca4c4c20"
+  integrity sha512-O+z53uwx64xY7D6roOi4+jApDGFg0qn6WHcxe5QeqjMaTezBO/mxdfFXIVAVVyNWKx84OmPB3L8kbVYOTeN34A==
 
 "@types/node@^10.0.3":
   version "10.17.60"
@@ -1326,6 +1326,11 @@
   resolved "https://registry.yarnpkg.com/@types/node/-/node-12.20.55.tgz#c329cbd434c42164f846b909bd6f85b5537f6240"
   integrity sha512-J8xLz7q2OFulZ2cyGTLE1TbbZcjpno7FaN6zdJNrgAdrJ+DZzh/uFR6YrTb4C+nXakvud8Q4+rbhoIWlYQbUFQ==
 
+"@types/node@^18.16.0":
+  version "18.16.5"
+  resolved "https://registry.yarnpkg.com/@types/node/-/node-18.16.5.tgz#bf64e42719dc2e74da24709a2e1c0b50a966120a"
+  integrity sha512-seOA34WMo9KB+UA78qaJoCO20RJzZGVXQ5Sh6FWu0g/hfT44nKXnej3/tCQl7FL97idFpBhisLYCTB50S0EirA==
+
 "@types/node@^8.0.0":
   version "8.10.66"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-8.10.66.tgz#dd035d409df322acc83dff62a602f12a5783bbb3"
@@ -2079,9 +2084,9 @@ camelcase@^6.0.0:
   integrity sha512-Gmy6FhYlCY7uOElZUSbxo2UCDH8owEk996gkbrpsgGtrJLM3J7jGxl9Ic7Qwwj4ivOE5AWZWRMecDdF7hqGjFA==
 
 caniuse-lite@^1.0.30001449:
-  version "1.0.30001482"
-  resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001482.tgz#8b3fad73dc35b2674a5c96df2d4f9f1c561435de"
-  integrity sha512-F1ZInsg53cegyjroxLNW9DmrEQ1SuGRTO1QlpA0o2/6OpQ0gFeDRoq1yFmnr8Sakn9qwwt9DmbxHB6w167OSuQ==
+  version "1.0.30001485"
+  resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001485.tgz#026bb7319f1e483391872dc303a973d4f513f619"
+  integrity sha512-8aUpZ7sjhlOyiNsg+pgcrTTPUXKh+rg544QYHSvQErljVEKJzvkYkCR/hUFeeVoEfTToUtY9cUKNRC7+c45YkA==
 
 case@^1.6.3:
   version "1.6.3"
@@ -2407,9 +2412,9 @@ cookie@^0.4.1:
   integrity sha512-aSWTXFzaKWkvHO1Ny/s+ePFpvKsPnjc551iI41v3ny/ow6tBG5Vd+FuqGNhh1LxOmVzOlGUriIlOaokOvhaStA==
 
 core-js-compat@^3.25.1:
-  version "3.30.1"
-  resolved "https://registry.yarnpkg.com/core-js-compat/-/core-js-compat-3.30.1.tgz#961541e22db9c27fc48bfc13a3cafa8734171dfe"
-  integrity sha512-d690npR7MC6P0gq4npTl5n2VQeNAmUrJ90n+MHiKS7W2+xno4o3F5GDEuylSdi6EJ3VssibSGXOa1r3YXD3Mhw==
+  version "3.30.2"
+  resolved "https://registry.yarnpkg.com/core-js-compat/-/core-js-compat-3.30.2.tgz#83f136e375babdb8c80ad3c22d67c69098c1dd8b"
+  integrity sha512-nriW1nuJjUgvkEjIot1Spwakz52V9YkYHZAQG6A1eCgC8AA1p0zngrQEP9R0+V6hji5XilWKG1Bd0YRppmGimA==
   dependencies:
     browserslist "^4.21.5"
 
@@ -2683,9 +2688,9 @@ [email protected]:
   integrity sha512-WMwm9LhRUo+WUaRN+vRuETqG89IgZphVSNkdFgeb6sS/E4OrDIN7t48CAewSHXc6C8lefD8KKfr5vY61brQlow==
 
 electron-to-chromium@^1.4.284:
-  version "1.4.380"
-  resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.380.tgz#195dc59d930c6b74efbee6f0e6a267ce4af5ed91"
-  integrity sha512-XKGdI4pWM78eLH2cbXJHiBnWUwFSzZM7XujsB6stDiGu9AeSqziedP6amNLpJzE3i0rLTcfAwdCTs5ecP5yeSg==
+  version "1.4.385"
+  resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.385.tgz#1afd8d6280d510145148777b899ff481c65531ff"
+  integrity sha512-L9zlje9bIw0h+CwPQumiuVlfMcV4boxRjFIWDcLfFqTZNbkwOExBzfmswytHawObQX4OUhtNv8gIiB21kOurIg==
 
 [email protected], elliptic@^6.4.0, elliptic@^6.5.2, elliptic@^6.5.4:
   version "6.5.4"