From 74de3c5c7909dcab11f2c01f2b9275cf1608b657 Mon Sep 17 00:00:00 2001 From: Filipp Makarov Date: Tue, 5 Nov 2024 20:42:39 +0300 Subject: [PATCH 1/7] fix maxPenalty --- contracts/token/BiconomyTokenPaymaster.sol | 4 +++- test/base/TestBase.sol | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index 094cb30..1f6777b 100644 --- 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); + console2.log("max pnalty in validatePaymasterUserOp", maxPenalty); + + context = abi.encode(userOp.sender, tokenAddress, tokenAmount-maxPenalty, tokenPrice, externalPriceMarkup, userOpHash); validationData = _packValidationData(false, validUntil, validAfter); } else if (mode == PaymasterMode.INDEPENDENT) { // Use only oracles for the token specified in modeSpecificData diff --git a/test/base/TestBase.sol b/test/base/TestBase.sol index ba15ec1..3cf2fa4 100644 --- a/test/base/TestBase.sol +++ b/test/base/TestBase.sol @@ -471,6 +471,7 @@ 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 @@ -499,3 +500,13 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { return array; } } +/* +1226028000000 +1794876000000 + +2921664000000 +3020904000000 + +1800000000000 max penalty + +*/ \ No newline at end of file From d4345eba82d039f28fa2c57f391a221259b6ab4c Mon Sep 17 00:00:00 2001 From: Filipp Makarov Date: Tue, 5 Nov 2024 20:45:01 +0300 Subject: [PATCH 2/7] fix maxPenalty --- contracts/token/BiconomyTokenPaymaster.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index 1f6777b..0f0d2f8 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -491,8 +491,10 @@ contract BiconomyTokenPaymaster is // Transfer full amount to this address. Unused amount will be refunded in postOP SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount); - console2.log("max pnalty in validatePaymasterUserOp", maxPenalty); - + // deduct max penalty from the token amount we pass to the postOp + // so we don't refund it at postOp + // other way to do it is not adding it to the tokenAmount and just charge + // tokenAmount + maxPenalty on line 492 context = abi.encode(userOp.sender, tokenAddress, tokenAmount-maxPenalty, tokenPrice, externalPriceMarkup, userOpHash); validationData = _packValidationData(false, validUntil, validAfter); } else if (mode == PaymasterMode.INDEPENDENT) { From c562f6e6092356a35a71e80fedde3e385364f0a5 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 6 Nov 2024 16:12:08 +0530 Subject: [PATCH 3/7] fix:accounting --- contracts/token/BiconomyTokenPaymaster.sol | 4 ++-- test/base/TestBase.sol | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index 0f0d2f8..ddbc5a7 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -495,7 +495,7 @@ contract BiconomyTokenPaymaster is // so we don't refund it at postOp // other way to do it is not adding it to the tokenAmount and just charge // tokenAmount + maxPenalty on line 492 - context = abi.encode(userOp.sender, tokenAddress, tokenAmount-maxPenalty, tokenPrice, externalPriceMarkup, userOpHash); + 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 @@ -520,7 +520,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 } } diff --git a/test/base/TestBase.sol b/test/base/TestBase.sol index 3cf2fa4..cd7bd69 100644 --- a/test/base/TestBase.sol +++ b/test/base/TestBase.sol @@ -473,7 +473,6 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { 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); From 094fd2448f8c94465e08a5d3117d4b425cf81e9d Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 6 Nov 2024 16:14:12 +0530 Subject: [PATCH 4/7] chore:update price for base test --- test/unit/concrete/TestTokenPaymaster.Base.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/concrete/TestTokenPaymaster.Base.t.sol b/test/unit/concrete/TestTokenPaymaster.Base.t.sol index 766c886..0e20c73 100644 --- 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])); } From 42f8da26fbf005ada431c128b5df6c0f36e2a19d Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 6 Nov 2024 16:20:39 +0530 Subject: [PATCH 5/7] chore:refactor logging and comments --- contracts/token/BiconomyTokenPaymaster.sol | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index ddbc5a7..75b5726 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -493,8 +493,6 @@ contract BiconomyTokenPaymaster is // deduct max penalty from the token amount we pass to the postOp // so we don't refund it at postOp - // other way to do it is not adding it to the tokenAmount and just charge - // tokenAmount + maxPenalty on line 492 context = abi.encode(userOp.sender, tokenAddress, tokenAmount-((maxPenalty*tokenPrice)/1e18), tokenPrice, externalPriceMarkup, userOpHash); validationData = _packValidationData(false, validUntil, validAfter); } else if (mode == PaymasterMode.INDEPENDENT) { @@ -555,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; @@ -589,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. From 1f09a7d0332e80351e8d45ac9b6683cf726dd8a6 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 6 Nov 2024 18:15:57 +0530 Subject: [PATCH 6/7] fix: temp small fix for coverage report --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3bfb39d..5cb4abd 100644 --- a/package.json +++ b/package.json @@ -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", From 86baa0c0463744e9ea94997e605707ab69f001a5 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 6 Nov 2024 19:46:14 +0530 Subject: [PATCH 7/7] fix: nexus commit --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5cb4abd..fd27a7d 100644 --- 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",