Merge pull request #35 from bcnmy/fix/token-pm-chargeSA

Fix how token PM charges SA
bcnmy · Nov 7, 2024 · d113079 · d113079
2 parents 6143d26 + 86baa0c
commit d113079
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 16 deletions.
diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol
@@ -491,7 +491,9 @@ contract BiconomyTokenPaymaster is
             // Transfer full amount to this address. Unused amount will be refunded in postOP
             SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount);
 
-            context = abi.encode(userOp.sender, tokenAddress, tokenAmount, tokenPrice, externalPriceMarkup, userOpHash);
+            // 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)/1e18), tokenPrice, externalPriceMarkup, userOpHash);
             validationData = _packValidationData(false, validUntil, validAfter);
         } else if (mode == PaymasterMode.INDEPENDENT) {
             // Use only oracles for the token specified in modeSpecificData
@@ -516,7 +518,7 @@ contract BiconomyTokenPaymaster is
             SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount);
 
             context =
-                abi.encode(userOp.sender, tokenAddress, tokenAmount, tokenPrice, independentPriceMarkup, userOpHash);
+                abi.encode(userOp.sender, tokenAddress, tokenAmount-((maxPenalty*tokenPrice)/1e18), tokenPrice, independentPriceMarkup, userOpHash);
             validationData = 0; // Validation success and price is valid indefinetly
         }
     }
@@ -551,13 +553,6 @@ contract BiconomyTokenPaymaster is
         uint256 actualTokenAmount = (
             (actualGasCost + (unaccountedGas * actualUserOpFeePerGas)) * appliedPriceMarkup * tokenPrice
         ) / (1e18 * _PRICE_DENOMINATOR);
-        console2.log("tokenPrice", tokenPrice);
-        console2.log("appliedPriceMarkup", appliedPriceMarkup);
-        console2.log("actualGasCost", actualGasCost);
-        console2.log("actualUserOpFeePerGas", actualUserOpFeePerGas);
-        console2.log("actualTokenAmount", actualTokenAmount);
-        console2.log("prechargedAmount", prechargedAmount);
-
         if (prechargedAmount > actualTokenAmount) {
             // If the user was overcharged, refund the excess tokens
             uint256 refundAmount = prechargedAmount - actualTokenAmount;
@@ -585,12 +580,9 @@ contract BiconomyTokenPaymaster is
         // Calculate price by using token and native oracle
         uint192 tokenPrice = _fetchPrice(tokenInfo.oracle);
         uint192 nativeAssetPrice = _fetchPrice(nativeAssetToUsdOracle);
-        console2.log("tokenPrice oracle", tokenPrice);
-        console2.log("nativeAssetPrice oracle", nativeAssetPrice);
 
         // Adjust to token  decimals
         price = (nativeAssetPrice * tokenInfo.decimals) / tokenPrice;
-        console2.log("derived & used price", price);
     }
 
     /// @notice Fetches the latest price from the given oracle.

diff --git a/package.json b/package.json
@@ -12,7 +12,7 @@
     "@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#dev"
+    "nexus": "https://github.com/bcnmy/nexus#773943fb7bf6cd14a0dc6dcb9f513db53213d1d5"
   },
   "devDependencies": {
     "@bonadocs/docgen": "^1.0.1-alpha.1",
@@ -62,7 +62,7 @@
     "test:gas": "pnpm test:gas:forge",
     "coverage:forge": "forge coverage --ir-minimum --gas-limit 50000000 --gas-price 1000000000",
     "coverage": "pnpm run coverage:forge",
-    "coverage:report": "forge coverage --ir-minimum --gas-limit 50000000 --gas-price 1000000000 --report lcov && genhtml lcov.info --branch-coverage --output-dir coverage/foundry && mv lcov.info coverage/foundry",
+    "coverage:report": "forge coverage --ir-minimum --gas-limit 50000000 --gas-price 1000000000 --report lcov && genhtml --ignore-errors inconsistent lcov.info --branch-coverage --output-dir coverage/foundry && mv lcov.info coverage/foundry",
     "deploy:forge": "forge script scripts/solidity/Deploy.s.sol --broadcast --rpc-url http://localhost:8545",
     "lint:sol": "pnpm solhint 'contracts/**/*.sol'",
     "format:check": "forge fmt --check",

diff --git a/test/base/TestBase.sol b/test/base/TestBase.sol
@@ -471,8 +471,8 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
         console2.log("gasCollectedInERC20ByPaymaster", gasCollectedInERC20ByPaymaster);
         console2.log("maxPenalty", maxPenalty);
         console2.log("totalGasFeePaid", totalGasFeePaid);
+        console2.log(uint256(1226028000000) + uint256(1794876000000));
 
-        // Note: yet to figure out why we're charging too low in tokens vs bundler is paying high gas fees!
         // Review we will also need to update premium numbers in below if there is premium: multiply by 1e6 / premium
         assertGt(gasPaidBySAInERC20 * 1e18 / tokenPrice, BUNDLER.addr.balance - initialBundlerBalance);
 
@@ -499,3 +499,13 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
         return array;
     }
 }
+/* 
+1226028000000
+1794876000000
+
+2921664000000
+3020904000000
+
+1800000000000 max penalty 
+
+*/
diff --git a/test/unit/concrete/TestTokenPaymaster.Base.t.sol b/test/unit/concrete/TestTokenPaymaster.Base.t.sol
@@ -128,7 +128,7 @@ contract TestTokenPaymasterBase is TestBase {
             initialPaymasterEpBalance, 
             initialUserTokenBalance, 
             initialPaymasterTokenBalance,
-            2672598177,
+            2624042830,
             100000,
             this.getMaxPenalty(ops[0]));
     }