From 95b996d2f5d19ca804a19854da8b7971d6a8b7db Mon Sep 17 00:00:00 2001 From: Shivaansh Kapoor Date: Thu, 4 Jul 2024 18:17:31 +0400 Subject: [PATCH 1/5] switched to custom errors and fixed postop gas calc --- contracts/common/Errors.sol | 30 ++++++++ .../SponsorshipPaymasterWithPremium.sol | 70 +++++++++---------- ...tSponsorshipPaymasterWithPremiumTest.t.sol | 30 +------- ..._TestSponsorshipPaymasterWithPremium.t.sol | 15 ---- 4 files changed, 65 insertions(+), 80 deletions(-) diff --git a/contracts/common/Errors.sol b/contracts/common/Errors.sol index 998492a..58bf40f 100644 --- a/contracts/common/Errors.sol +++ b/contracts/common/Errors.sol @@ -32,6 +32,36 @@ contract BiconomySponsorshipPaymasterErrors { */ error VerifyingSignerCanNotBeContract(); + /** + * @notice Throws when ETH withdrawal fails + */ + error WithdrawalFailed(); + + /** + * @notice Throws when insufficient funds to withdraw + */ + error InsufficientFundsInGasTank(); + + /** + * @notice Throws when invalid signature length in paymasterAndData + */ + error InvalidSignatureLength(); + + /** + * @notice Throws when invalid signature length in paymasterAndData + */ + error InvalidPriceMarkup(); + + /** + * @notice Throws when insufficient funds for paymasterid + */ + error InsufficientFundsForPaymasterId(); + + /** + * @notice Throws when calling deposit() + */ + error UseDepositForInstead(); + /** * @notice Throws when trying to withdraw to address(0) */ diff --git a/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol b/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol index 2e3abf4..3011264 100644 --- a/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol +++ b/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol @@ -39,7 +39,6 @@ contract BiconomySponsorshipPaymaster is address public verifyingSigner; address public feeCollector; - uint48 public postopCost; uint32 private constant PRICE_DENOMINATOR = 1e6; // note: could rename to PAYMASTER_ID_OFFSET @@ -55,8 +54,11 @@ contract BiconomySponsorshipPaymaster is ) BasePaymaster(_owner, _entryPoint) { - // TODO - // Check for zero address + if (_verifyingSigner == address(0)) { + revert VerifyingSignerCanNotBeZero(); + } else if (_feeCollector == address(0)) { + revert FeeCollectorCanNotBeZero(); + } verifyingSigner = _verifyingSigner; feeCollector = _feeCollector; } @@ -117,23 +119,11 @@ contract BiconomySponsorshipPaymaster is emit FeeCollectorChanged(oldFeeCollector, _newFeeCollector, msg.sender); } - /** - * @dev Set a new unaccountedEPGasOverhead value. - * @param value The new value to be set as the unaccountedEPGasOverhead. - * @notice only to be called by the owner of the contract. - */ - function setPostopCost(uint48 value) external payable onlyOwner { - require(value <= 200_000, "Gas overhead too high"); - uint256 oldValue = postopCost; - postopCost = value; - emit PostopCostChanged(oldValue, value); - } - /** * @dev Override the default implementation. */ function deposit() external payable virtual override { - revert("Use depositFor() instead"); + revert UseDepositForInstead(); } /** @@ -155,15 +145,19 @@ contract BiconomySponsorshipPaymaster is function withdrawTo(address payable withdrawAddress, uint256 amount) external override nonReentrant { if (withdrawAddress == address(0)) revert CanNotWithdrawToZeroAddress(); uint256 currentBalance = paymasterIdBalances[msg.sender]; - require(amount <= currentBalance, "Sponsorship Paymaster: Insufficient funds to withdraw from gas tank"); + if (amount > currentBalance) { + revert InsufficientFundsInGasTank(); + } paymasterIdBalances[msg.sender] = currentBalance - amount; entryPoint.withdrawTo(withdrawAddress, amount); emit GasWithdrawn(msg.sender, withdrawAddress, amount); } - function withdrawEth(address payable recipient, uint256 amount) external onlyOwner { + function withdrawEth(address payable recipient, uint256 amount) external onlyOwner nonReentrant { (bool success,) = recipient.call{ value: amount }(""); - require(success, "withdraw failed"); + if (!success) { + revert WithdrawalFailed(); + } } /** @@ -225,11 +219,13 @@ contract BiconomySponsorshipPaymaster is bytes calldata signature ) { - paymasterId = address(bytes20(paymasterAndData[VALID_PND_OFFSET:VALID_PND_OFFSET + 20])); - validUntil = uint48(bytes6(paymasterAndData[VALID_PND_OFFSET + 20:VALID_PND_OFFSET + 26])); - validAfter = uint48(bytes6(paymasterAndData[VALID_PND_OFFSET + 26:VALID_PND_OFFSET + 32])); - priceMarkup = uint32(bytes4(paymasterAndData[VALID_PND_OFFSET + 32:VALID_PND_OFFSET + 36])); - signature = paymasterAndData[VALID_PND_OFFSET + 36:]; + unchecked { + paymasterId = address(bytes20(paymasterAndData[VALID_PND_OFFSET:VALID_PND_OFFSET + 20])); + validUntil = uint48(bytes6(paymasterAndData[VALID_PND_OFFSET + 20:VALID_PND_OFFSET + 26])); + validAfter = uint48(bytes6(paymasterAndData[VALID_PND_OFFSET + 26:VALID_PND_OFFSET + 32])); + priceMarkup = uint32(bytes4(paymasterAndData[VALID_PND_OFFSET + 32:VALID_PND_OFFSET + 36])); + signature = paymasterAndData[VALID_PND_OFFSET + 36:]; + } } /// @notice Performs post-operation tasks, such as deducting the sponsored gas cost from the paymasterId's balance @@ -252,14 +248,12 @@ contract BiconomySponsorshipPaymaster is (address paymasterId, uint32 dynamicMarkup, bytes32 userOpHash) = abi.decode(context, (address, uint32, bytes32)); - uint256 balToDeduct = actualGasCost + postopCost * actualUserOpFeePerGas; - - uint256 costIncludingPremium = (balToDeduct * dynamicMarkup) / PRICE_DENOMINATOR; + uint256 costIncludingPremium = (actualGasCost * dynamicMarkup) / PRICE_DENOMINATOR; // deduct with premium paymasterIdBalances[paymasterId] -= costIncludingPremium; - uint256 actualPremium = costIncludingPremium - balToDeduct; + uint256 actualPremium = costIncludingPremium - actualGasCost; // "collect" premium paymasterIdBalances[feeCollector] += actualPremium; @@ -294,10 +288,9 @@ contract BiconomySponsorshipPaymaster is //ECDSA library supports both 64 and 65-byte long signatures. // we only "require" it here so that the revert reason on invalid signature will be of "VerifyingPaymaster", and // not "ECDSA" - require( - signature.length == 64 || signature.length == 65, - "VerifyingPaymaster: invalid signature length in paymasterAndData" - ); + if(signature.length != 64 && signature.length != 65){ + revert InvalidSignatureLength(); + } bool validSig = verifyingSigner.isValidSignatureNow( ECDSA_solady.toEthSignedMessageHash(getHash(userOp, paymasterId, validUntil, validAfter, priceMarkup)), @@ -309,18 +302,19 @@ contract BiconomySponsorshipPaymaster is return ("", _packValidationData(true, validUntil, validAfter)); } - require(priceMarkup <= 2e6 && priceMarkup > 0, "Sponsorship Paymaster: Invalid markup %"); + if (priceMarkup > 2e6 || priceMarkup == 0) { + revert InvalidPriceMarkup(); + } uint256 maxFeePerGas = userOp.unpackMaxFeePerGas(); // Send 1e6 for No markup // Send between 0 and 1e6 for discount - uint256 effectiveCost = ((requiredPreFund + (postopCost * maxFeePerGas)) * priceMarkup) / PRICE_DENOMINATOR; + uint256 effectiveCost = (requiredPreFund * priceMarkup) / PRICE_DENOMINATOR; - require( - effectiveCost <= paymasterIdBalances[paymasterId], - "Sponsorship Paymaster: paymasterId does not have enough deposit" - ); + if (effectiveCost > paymasterIdBalances[paymasterId]) { + revert InsufficientFundsForPaymasterId(); + } context = abi.encode(paymasterId, priceMarkup, userOpHash); diff --git a/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol b/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol index 058e340..4f58bbe 100644 --- a/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol +++ b/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol @@ -26,7 +26,6 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { assertEq(address(testArtifact.entryPoint()), ENTRYPOINT_ADDRESS); assertEq(testArtifact.verifyingSigner(), PAYMASTER_SIGNER.addr); assertEq(testArtifact.feeCollector(), PAYMASTER_FEE_COLLECTOR.addr); - assertEq(testArtifact.postopCost(), 0 wei); } function test_CheckInitialPaymasterState() external view { @@ -34,7 +33,6 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { assertEq(address(bicoPaymaster.entryPoint()), ENTRYPOINT_ADDRESS); assertEq(bicoPaymaster.verifyingSigner(), PAYMASTER_SIGNER.addr); assertEq(bicoPaymaster.feeCollector(), PAYMASTER_FEE_COLLECTOR.addr); - assertEq(bicoPaymaster.postopCost(), 0 wei); } function test_OwnershipTransfer() external prankModifier(PAYMASTER_OWNER.addr) { @@ -121,7 +119,7 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { } function test_RevertIf_DepositCalled() external { - vm.expectRevert("Use depositFor() instead"); + vm.expectRevert(abi.encodeWithSelector(UseDepositForInstead.selector)); bicoPaymaster.deposit{ value: 1 ether }(); } @@ -146,7 +144,7 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { } function test_RevertIf_WithdrawToExceedsBalance() external prankModifier(DAPP_ACCOUNT.addr) { - vm.expectRevert("Sponsorship Paymaster: Insufficient funds to withdraw from gas tank"); + vm.expectRevert(abi.encodeWithSelector(InsufficientFundsInGasTank.selector)); bicoPaymaster.withdrawTo(payable(DAN_ADDRESS), 1 ether); } @@ -261,32 +259,10 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { function test_RevertIf_WithdrawEthExceedsBalance() external prankModifier(PAYMASTER_OWNER.addr) { uint256 ethAmount = 10 ether; - vm.expectRevert("withdraw failed"); + vm.expectRevert(abi.encodeWithSelector(WithdrawalFailed.selector)); bicoPaymaster.withdrawEth(payable(ALICE_ADDRESS), ethAmount); } - function test_SetPostopCost() external prankModifier(PAYMASTER_OWNER.addr) { - uint48 initialPostopCost = bicoPaymaster.postopCost(); - assertEq(initialPostopCost, 0 wei); - uint48 newPostopCost = initialPostopCost + 1 wei; - - vm.expectEmit(true, true, false, true, address(bicoPaymaster)); - emit IBiconomySponsorshipPaymaster.PostopCostChanged(initialPostopCost, newPostopCost); - bicoPaymaster.setPostopCost(newPostopCost); - - uint48 resultingPostopCost = bicoPaymaster.postopCost(); - assertEq(resultingPostopCost, newPostopCost); - } - - function test_RevertIf_SetPostopCostToHigh() external prankModifier(PAYMASTER_OWNER.addr) { - uint48 initialPostopCost = bicoPaymaster.postopCost(); - assertEq(initialPostopCost, 0 wei); - uint48 newPostopCost = initialPostopCost + 200_001 wei; - - vm.expectRevert("Gas overhead too high"); - bicoPaymaster.setPostopCost(newPostopCost); - } - function test_WithdrawErc20() external prankModifier(PAYMASTER_OWNER.addr) { MockToken token = new MockToken("Token", "TKN"); uint256 mintAmount = 10 * (10 ** token.decimals()); diff --git a/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol b/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol index 17e4676..5fb19db 100644 --- a/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol +++ b/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol @@ -7,7 +7,6 @@ import { IBiconomySponsorshipPaymaster } from "../../../../contracts/interfaces/ import { BiconomySponsorshipPaymaster } from "../../../../contracts/sponsorship/SponsorshipPaymasterWithPremium.sol"; import { MockToken } from "./../../../../lib/nexus/contracts/mocks/MockToken.sol"; - contract TestFuzz_SponsorshipPaymasterWithPremium is NexusTestBase { BiconomySponsorshipPaymaster public bicoPaymaster; @@ -79,20 +78,6 @@ contract TestFuzz_SponsorshipPaymasterWithPremium is NexusTestBase { assertEq(address(bicoPaymaster).balance, 0 ether); } - function testFuzz_SetPostopCost(uint48 value) external prankModifier(PAYMASTER_OWNER.addr) { - vm.assume(value <= 200_000 wei); - uint48 initialPostopCost = bicoPaymaster.postopCost(); - assertEq(initialPostopCost, 0 wei); - uint48 newPostopCost = value; - - vm.expectEmit(true, true, false, true, address(bicoPaymaster)); - emit IBiconomySponsorshipPaymaster.PostopCostChanged(initialPostopCost, newPostopCost); - bicoPaymaster.setPostopCost(newPostopCost); - - uint48 resultingPostopCost = bicoPaymaster.postopCost(); - assertEq(resultingPostopCost, newPostopCost); - } - function testFuzz_WithdrawErc20(address target, uint256 amount) external prankModifier(PAYMASTER_OWNER.addr) { vm.assume(target != address(0)); vm.assume(amount <= 1_000_000 * (10 ** 18)); From ff05364756dfe109c1cd119c92f4c3da22b97880 Mon Sep 17 00:00:00 2001 From: Shivaansh Kapoor Date: Sat, 6 Jul 2024 18:21:11 +0400 Subject: [PATCH 2/5] fixed a few things with gas calc and wrote tests --- .../SponsorshipPaymasterWithPremium.sol | 19 ++-- test/foundry/base/NexusTestBase.sol | 91 +++---------------- ...tSponsorshipPaymasterWithPremiumTest.t.sol | 74 ++++++++++++++- 3 files changed, 95 insertions(+), 89 deletions(-) diff --git a/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol b/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol index 3011264..5c78e91 100644 --- a/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol +++ b/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol @@ -232,14 +232,11 @@ contract BiconomySponsorshipPaymaster is /// @dev This function is called after a user operation has been executed or reverted. /// @param context The context containing the token amount and user sender address. /// @param actualGasCost The actual gas cost of the transaction. - /// @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, - uint256 actualUserOpFeePerGas + uint256 ) internal override @@ -253,13 +250,15 @@ contract BiconomySponsorshipPaymaster is // deduct with premium paymasterIdBalances[paymasterId] -= costIncludingPremium; - uint256 actualPremium = costIncludingPremium - actualGasCost; - // "collect" premium - paymasterIdBalances[feeCollector] += actualPremium; + if (actualGasCost < costIncludingPremium) { + // "collect" premium + uint256 actualPremium = costIncludingPremium - actualGasCost; + paymasterIdBalances[feeCollector] += actualPremium; + // Review if we should emit balToDeduct as well + emit PremiumCollected(paymasterId, actualPremium); + } emit GasBalanceDeducted(paymasterId, costIncludingPremium, userOpHash); - // Review if we should emit balToDeduct as well - emit PremiumCollected(paymasterId, actualPremium); } } @@ -288,7 +287,7 @@ contract BiconomySponsorshipPaymaster is //ECDSA library supports both 64 and 65-byte long signatures. // we only "require" it here so that the revert reason on invalid signature will be of "VerifyingPaymaster", and // not "ECDSA" - if(signature.length != 64 && signature.length != 65){ + if (signature.length != 64 && signature.length != 65) { revert InvalidSignatureLength(); } diff --git a/test/foundry/base/NexusTestBase.sol b/test/foundry/base/NexusTestBase.sol index 6bc0df7..308bf26 100644 --- a/test/foundry/base/NexusTestBase.sol +++ b/test/foundry/base/NexusTestBase.sol @@ -8,6 +8,7 @@ import "solady/src/utils/ECDSA.sol"; import { EntryPoint } from "account-abstraction/contracts/core/EntryPoint.sol"; import { IEntryPoint } from "account-abstraction/contracts/interfaces/IEntryPoint.sol"; +import { IPaymaster } from "account-abstraction/contracts/interfaces/IPaymaster.sol"; import { PackedUserOperation } from "account-abstraction/contracts/interfaces/PackedUserOperation.sol"; import { Nexus } from "nexus/contracts/Nexus.sol"; @@ -331,87 +332,23 @@ abstract contract NexusTestBase is CheatCodes, BaseEventsAndErrors { assertTrue(res, "Pre-funding account should succeed"); } - /// @notice Calculates the gas cost of the calldata - /// @param data The calldata - /// @return calldataGas The gas cost of the calldata - function calculateCalldataCost(bytes memory data) internal pure returns (uint256 calldataGas) { - for (uint256 i = 0; i < data.length; i++) { - if (uint8(data[i]) == 0) { - calldataGas += 4; - } else { - calldataGas += 16; - } - } - } - - /// @notice Helper function to measure and log gas for simple EOA calls - /// @param description The description for the log - /// @param target The target contract address - /// @param value The value to be sent with the call - /// @param callData The calldata for the call - function measureAndLogGasEOA( - string memory description, - address target, - uint256 value, - bytes memory callData + function estimatePaymasterGasCosts( + BiconomySponsorshipPaymaster paymaster, + PackedUserOperation memory userOp, + bytes32 userOpHash, + uint256 requiredPreFund ) internal + prankModifier(ENTRYPOINT_ADDRESS) + returns (uint256 validationGasLimit, uint256 postopGasLimit) { - uint256 calldataCost = 0; - for (uint256 i = 0; i < callData.length; i++) { - if (uint8(callData[i]) == 0) { - calldataCost += 4; - } else { - calldataCost += 16; - } - } - - uint256 baseGas = 21_000; - - uint256 initialGas = gasleft(); - (bool res,) = target.call{ value: value }(callData); - uint256 gasUsed = initialGas - gasleft() + baseGas + calldataCost; - assertTrue(res); - emit log_named_uint(description, gasUsed); - } + validationGasLimit = gasleft(); + (bytes memory context,) = paymaster.validatePaymasterUserOp(userOp, userOpHash, requiredPreFund); + validationGasLimit = validationGasLimit - gasleft(); - /// @notice Helper function to calculate calldata cost and log gas usage - /// @param description The description for the log - /// @param userOps The user operations to be executed - function measureAndLogGas(string memory description, PackedUserOperation[] memory userOps) internal { - bytes memory callData = abi.encodeWithSelector(ENTRYPOINT.handleOps.selector, userOps, payable(BUNDLER.addr)); - - uint256 calldataCost = 0; - for (uint256 i = 0; i < callData.length; i++) { - if (uint8(callData[i]) == 0) { - calldataCost += 4; - } else { - calldataCost += 16; - } - } - - uint256 baseGas = 21_000; - - uint256 initialGas = gasleft(); - ENTRYPOINT.handleOps(userOps, payable(BUNDLER.addr)); - uint256 gasUsed = initialGas - gasleft() + baseGas + calldataCost; - emit log_named_uint(description, gasUsed); - } - - /// @notice Handles a user operation and measures gas usage - /// @param userOps The user operations to handle - /// @param refundReceiver The address to receive the gas refund - /// @return gasUsed The amount of gas used - function handleUserOpAndMeasureGas( - PackedUserOperation[] memory userOps, - address refundReceiver - ) - internal - returns (uint256 gasUsed) - { - uint256 gasStart = gasleft(); - ENTRYPOINT.handleOps(userOps, payable(refundReceiver)); - gasUsed = gasStart - gasleft(); + postopGasLimit = gasleft(); + paymaster.postOp(IPaymaster.PostOpMode.opSucceeded, context, 3e4, 2e9); + postopGasLimit = postopGasLimit - gasleft(); } /// @notice Generates and signs the paymaster data for a user operation. diff --git a/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol b/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol index 4f58bbe..c4b5113 100644 --- a/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol +++ b/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Unlicensed pragma solidity ^0.8.26; +import { console2 } from "forge-std/src/console2.sol"; import { NexusTestBase } from "../../base/NexusTestBase.sol"; import { IBiconomySponsorshipPaymaster } from "../../../../contracts/interfaces/IBiconomySponsorshipPaymaster.sol"; import { BiconomySponsorshipPaymaster } from "../../../../contracts/sponsorship/SponsorshipPaymasterWithPremium.sol"; @@ -28,6 +29,16 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { assertEq(testArtifact.feeCollector(), PAYMASTER_FEE_COLLECTOR.addr); } + function test_RevertIf_DeployWithSignerSetToZero() external { + vm.expectRevert(abi.encodeWithSelector(VerifyingSignerCanNotBeZero.selector)); + new BiconomySponsorshipPaymaster(PAYMASTER_OWNER.addr, ENTRYPOINT, address(0), PAYMASTER_FEE_COLLECTOR.addr); + } + + function test_RevertIf_DeployWithFeeCollectorSetToZero() external { + vm.expectRevert(abi.encodeWithSelector(FeeCollectorCanNotBeZero.selector)); + new BiconomySponsorshipPaymaster(PAYMASTER_OWNER.addr, ENTRYPOINT, PAYMASTER_SIGNER.addr, address(0)); + } + function test_CheckInitialPaymasterState() external view { assertEq(bicoPaymaster.owner(), PAYMASTER_OWNER.addr); assertEq(address(bicoPaymaster.entryPoint()), ENTRYPOINT_ADDRESS); @@ -148,7 +159,7 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { bicoPaymaster.withdrawTo(payable(DAN_ADDRESS), 1 ether); } - function test_ValidatePaymasterAndPostOp() external { + function test_ValidatePaymasterAndPostOpWithoutPremium() external { uint256 initialDappPaymasterBalance = 10 ether; bicoPaymaster.depositFor{ value: initialDappPaymasterBalance }(DAPP_ACCOUNT.addr); @@ -169,12 +180,71 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { vm.expectEmit(true, false, true, true, address(bicoPaymaster)); emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, userOpHash); + ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr)); + + uint256 resultingDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); + assertNotEq(initialDappPaymasterBalance, resultingDappPaymasterBalance); + } + + function test_ValidatePaymasterAndPostOpWithPremium() external { + uint256 initialDappPaymasterBalance = 10 ether; + bicoPaymaster.depositFor{ value: initialDappPaymasterBalance }(DAPP_ACCOUNT.addr); + uint256 initialBundlerBalance = BUNDLER.addr.balance; + + PackedUserOperation[] memory ops = new PackedUserOperation[](1); + + uint48 validUntil = uint48(block.timestamp + 1 days); + uint48 validAfter = uint48(block.timestamp); + + PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); + + // Charge a 10% premium + uint32 premium = 1e6 + 1e5; + userOp.paymasterAndData = generateAndSignPaymasterData( + userOp, PAYMASTER_SIGNER, bicoPaymaster, 3e6, 3e6, DAPP_ACCOUNT.addr, validUntil, validAfter, premium + ); + userOp.signature = signUserOp(ALICE, userOp); + + // Estimate paymaster gas limits + bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOp); + (uint256 validationGasLimit, uint256 postopGasLimit) = + estimatePaymasterGasCosts(bicoPaymaster, userOp, userOpHash, 5e4); + + // Ammend the userop to have new gas limits and signature + userOp.paymasterAndData = generateAndSignPaymasterData( + userOp, + PAYMASTER_SIGNER, + bicoPaymaster, + uint128(validationGasLimit), + uint128(postopGasLimit), + DAPP_ACCOUNT.addr, + validUntil, + validAfter, + premium + ); + userOp.signature = signUserOp(ALICE, userOp); + ops[0] = userOp; + userOpHash = ENTRYPOINT.getUserOpHash(userOp); + + // submit userops vm.expectEmit(true, false, false, true, address(bicoPaymaster)); emit IBiconomySponsorshipPaymaster.PremiumCollected(DAPP_ACCOUNT.addr, 0); + vm.expectEmit(true, false, true, true, address(bicoPaymaster)); + emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, userOpHash); ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr)); + // Check that gas fees ended up in the right wallets uint256 resultingDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); - assertNotEq(initialDappPaymasterBalance, resultingDappPaymasterBalance); + uint256 resultingFeeCollectorPaymasterBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); + uint256 resultingBundlerBalance = BUNDLER.addr.balance - initialBundlerBalance; + uint256 totalGasFeesCharged = initialDappPaymasterBalance - resultingDappPaymasterBalance; + console2.log(resultingDappPaymasterBalance); + console2.log(resultingFeeCollectorPaymasterBalance); + console2.log(resultingBundlerBalance); + console2.log(totalGasFeesCharged); + + // resultingDappPaymasterBalance + assertEq(resultingFeeCollectorPaymasterBalance, (totalGasFeesCharged * 1e5) / premium); } function test_RevertIf_ValidatePaymasterUserOpWithIncorrectSignatureLength() external { From a478815ed19ec6b4d32f6d2e19f8122553658f52 Mon Sep 17 00:00:00 2001 From: Shivaansh Kapoor Date: Sun, 7 Jul 2024 22:06:39 +0400 Subject: [PATCH 3/5] more tests for accounting for premiums --- .../SponsorshipPaymasterWithPremium.sol | 14 +--- foundry.toml | 5 +- test/foundry/base/NexusTestBase.sol | 2 +- ...tSponsorshipPaymasterWithPremiumTest.t.sol | 66 +++++++++--------- ..._TestSponsorshipPaymasterWithPremium.t.sol | 67 ++++++++++++++++++- 5 files changed, 103 insertions(+), 51 deletions(-) diff --git a/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol b/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol index 5c78e91..5b06711 100644 --- a/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol +++ b/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol @@ -232,15 +232,7 @@ contract BiconomySponsorshipPaymaster is /// @dev This function is called after a user operation has been executed or reverted. /// @param context The context containing the token amount and user sender address. /// @param actualGasCost The actual gas cost of the transaction. - function _postOp( - PostOpMode, - bytes calldata context, - uint256 actualGasCost, - uint256 - ) - internal - override - { + function _postOp(PostOpMode, bytes calldata context, uint256 actualGasCost, uint256) internal override { unchecked { (address paymasterId, uint32 dynamicMarkup, bytes32 userOpHash) = abi.decode(context, (address, uint32, bytes32)); @@ -250,7 +242,7 @@ contract BiconomySponsorshipPaymaster is // deduct with premium paymasterIdBalances[paymasterId] -= costIncludingPremium; - if (actualGasCost < costIncludingPremium) { + if (costIncludingPremium > actualGasCost) { // "collect" premium uint256 actualPremium = costIncludingPremium - actualGasCost; paymasterIdBalances[feeCollector] += actualPremium; @@ -305,8 +297,6 @@ contract BiconomySponsorshipPaymaster is revert InvalidPriceMarkup(); } - uint256 maxFeePerGas = userOp.unpackMaxFeePerGas(); - // Send 1e6 for No markup // Send between 0 and 1e6 for discount uint256 effectiveCost = (requiredPreFund * priceMarkup) / PRICE_DENOMINATOR; diff --git a/foundry.toml b/foundry.toml index 04c3656..80fe03b 100644 --- a/foundry.toml +++ b/foundry.toml @@ -5,7 +5,6 @@ block_timestamp = 1_680_220_800 # March 31, 2023 at 00:00 GMT bytecode_hash = "none" evm_version = "paris" # See https://www.evmdiff.com/features?name=PUSH0&kind=opcode - fuzz = { runs = 1_000 } gas_reports = ["*"] optimizer = true optimizer_runs = 1_000_000 @@ -19,6 +18,10 @@ gas_reports_ignore = ["LockTest"] via_ir = true +[fuzz] + runs = 1_000 + max_test_rejects = 1_000_000 + [profile.ci] fuzz = { runs = 10_000 } verbosity = 4 diff --git a/test/foundry/base/NexusTestBase.sol b/test/foundry/base/NexusTestBase.sol index 308bf26..cd7dbaf 100644 --- a/test/foundry/base/NexusTestBase.sol +++ b/test/foundry/base/NexusTestBase.sol @@ -347,7 +347,7 @@ abstract contract NexusTestBase is CheatCodes, BaseEventsAndErrors { validationGasLimit = validationGasLimit - gasleft(); postopGasLimit = gasleft(); - paymaster.postOp(IPaymaster.PostOpMode.opSucceeded, context, 3e4, 2e9); + paymaster.postOp(IPaymaster.PostOpMode.opSucceeded, context, 1e12, 3e6); postopGasLimit = postopGasLimit - gasleft(); } diff --git a/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol b/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol index c4b5113..0f6ed46 100644 --- a/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol +++ b/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol @@ -135,31 +135,6 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { } function test_WithdrawTo() external prankModifier(DAPP_ACCOUNT.addr) { - uint256 depositAmount = 10 ether; - bicoPaymaster.depositFor{ value: depositAmount }(DAPP_ACCOUNT.addr); - uint256 danInitialBalance = DAN_ADDRESS.balance; - - vm.expectEmit(true, true, true, true, address(bicoPaymaster)); - emit IBiconomySponsorshipPaymaster.GasWithdrawn(DAPP_ACCOUNT.addr, DAN_ADDRESS, depositAmount); - bicoPaymaster.withdrawTo(payable(DAN_ADDRESS), depositAmount); - - uint256 dappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); - assertEq(dappPaymasterBalance, 0 ether); - uint256 expectedDanBalance = danInitialBalance + depositAmount; - assertEq(DAN_ADDRESS.balance, expectedDanBalance); - } - - function test_RevertIf_WithdrawToZeroAddress() external prankModifier(DAPP_ACCOUNT.addr) { - vm.expectRevert(abi.encodeWithSelector(CanNotWithdrawToZeroAddress.selector)); - bicoPaymaster.withdrawTo(payable(address(0)), 0 ether); - } - - function test_RevertIf_WithdrawToExceedsBalance() external prankModifier(DAPP_ACCOUNT.addr) { - vm.expectRevert(abi.encodeWithSelector(InsufficientFundsInGasTank.selector)); - bicoPaymaster.withdrawTo(payable(DAN_ADDRESS), 1 ether); - } - - function test_ValidatePaymasterAndPostOpWithoutPremium() external { uint256 initialDappPaymasterBalance = 10 ether; bicoPaymaster.depositFor{ value: initialDappPaymasterBalance }(DAPP_ACCOUNT.addr); @@ -169,14 +144,34 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { uint48 validAfter = uint48(block.timestamp); PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); + + // No premium + uint32 premium = 1e6; userOp.paymasterAndData = generateAndSignPaymasterData( - userOp, PAYMASTER_SIGNER, bicoPaymaster, 3e6, 3e6, DAPP_ACCOUNT.addr, validUntil, validAfter, 1e6 + userOp, PAYMASTER_SIGNER, bicoPaymaster, 3e6, 3e6, DAPP_ACCOUNT.addr, validUntil, validAfter, premium ); userOp.signature = signUserOp(ALICE, userOp); + // Estimate paymaster gas limits bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOp); + (uint256 validationGasLimit, uint256 postopGasLimit) = + estimatePaymasterGasCosts(bicoPaymaster, userOp, userOpHash, 5e4); + // Ammend the userop to have new gas limits and signature + userOp.paymasterAndData = generateAndSignPaymasterData( + userOp, + PAYMASTER_SIGNER, + bicoPaymaster, + uint128(validationGasLimit), + uint128(postopGasLimit), + DAPP_ACCOUNT.addr, + validUntil, + validAfter, + premium + ); + userOp.signature = signUserOp(ALICE, userOp); ops[0] = userOp; + userOpHash = ENTRYPOINT.getUserOpHash(userOp); vm.expectEmit(true, false, true, true, address(bicoPaymaster)); emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, userOpHash); @@ -189,7 +184,6 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { function test_ValidatePaymasterAndPostOpWithPremium() external { uint256 initialDappPaymasterBalance = 10 ether; bicoPaymaster.depositFor{ value: initialDappPaymasterBalance }(DAPP_ACCOUNT.addr); - uint256 initialBundlerBalance = BUNDLER.addr.balance; PackedUserOperation[] memory ops = new PackedUserOperation[](1); @@ -226,6 +220,8 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { ops[0] = userOp; userOpHash = ENTRYPOINT.getUserOpHash(userOp); + uint256 initialFeeCollectorBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); + initialDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); // submit userops vm.expectEmit(true, false, false, true, address(bicoPaymaster)); emit IBiconomySponsorshipPaymaster.PremiumCollected(DAPP_ACCOUNT.addr, 0); @@ -233,18 +229,16 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, userOpHash); ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr)); - // Check that gas fees ended up in the right wallets uint256 resultingDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); uint256 resultingFeeCollectorPaymasterBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); - uint256 resultingBundlerBalance = BUNDLER.addr.balance - initialBundlerBalance; + uint256 totalGasFeesCharged = initialDappPaymasterBalance - resultingDappPaymasterBalance; - console2.log(resultingDappPaymasterBalance); - console2.log(resultingFeeCollectorPaymasterBalance); - console2.log(resultingBundlerBalance); - console2.log(totalGasFeesCharged); - - // resultingDappPaymasterBalance - assertEq(resultingFeeCollectorPaymasterBalance, (totalGasFeesCharged * 1e5) / premium); + uint256 premiumCollected = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance; + + uint256 expectedGasPayment = totalGasFeesCharged - premiumCollected; + uint256 expectedPremium = expectedGasPayment / 10; + + assertEq(premiumCollected, expectedPremium); } function test_RevertIf_ValidatePaymasterUserOpWithIncorrectSignatureLength() external { diff --git a/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol b/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol index 5fb19db..98fe11d 100644 --- a/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol +++ b/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol @@ -1,11 +1,13 @@ // SPDX-License-Identifier: Unlicensed pragma solidity ^0.8.26; -import { console2 } from "forge-std/src/Console2.sol"; +import { console2 } from "forge-std/src/console2.sol"; +import { stdMath } from "forge-std/src/Test.sol"; import { NexusTestBase } from "../../base/NexusTestBase.sol"; import { IBiconomySponsorshipPaymaster } from "../../../../contracts/interfaces/IBiconomySponsorshipPaymaster.sol"; import { BiconomySponsorshipPaymaster } from "../../../../contracts/sponsorship/SponsorshipPaymasterWithPremium.sol"; import { MockToken } from "./../../../../lib/nexus/contracts/mocks/MockToken.sol"; +import { PackedUserOperation } from "account-abstraction/contracts/interfaces/PackedUserOperation.sol"; contract TestFuzz_SponsorshipPaymasterWithPremium is NexusTestBase { BiconomySponsorshipPaymaster public bicoPaymaster; @@ -97,4 +99,67 @@ contract TestFuzz_SponsorshipPaymasterWithPremium is NexusTestBase { assertEq(token.balanceOf(address(bicoPaymaster)), 0); assertEq(token.balanceOf(ALICE_ADDRESS), mintAmount); } + + function testFuzz_ValidatePaymasterAndPostOpWithPremium(uint32 premium) external { + vm.assume(premium <= 2e6); + vm.assume(premium > 1e6); + + uint256 initialDappPaymasterBalance = 10 ether; + bicoPaymaster.depositFor{ value: initialDappPaymasterBalance }(DAPP_ACCOUNT.addr); + + PackedUserOperation[] memory ops = new PackedUserOperation[](1); + + uint48 validUntil = uint48(block.timestamp + 1 days); + uint48 validAfter = uint48(block.timestamp); + + PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); + + userOp.paymasterAndData = generateAndSignPaymasterData( + userOp, PAYMASTER_SIGNER, bicoPaymaster, 3e6, 3e6, DAPP_ACCOUNT.addr, validUntil, validAfter, premium + ); + userOp.signature = signUserOp(ALICE, userOp); + + // Estimate paymaster gas limits + bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOp); + (uint256 validationGasLimit, uint256 postopGasLimit) = + estimatePaymasterGasCosts(bicoPaymaster, userOp, userOpHash, 5e4); + + // Ammend the userop to have new gas limits and signature + userOp.paymasterAndData = generateAndSignPaymasterData( + userOp, + PAYMASTER_SIGNER, + bicoPaymaster, + uint128(validationGasLimit), + uint128(postopGasLimit), + DAPP_ACCOUNT.addr, + validUntil, + validAfter, + premium + ); + userOp.signature = signUserOp(ALICE, userOp); + ops[0] = userOp; + userOpHash = ENTRYPOINT.getUserOpHash(userOp); + + uint256 initialFeeCollectorBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); + initialDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); + vm.expectEmit(true, false, false, true, address(bicoPaymaster)); + emit IBiconomySponsorshipPaymaster.PremiumCollected(DAPP_ACCOUNT.addr, 0); + vm.expectEmit(true, false, true, true, address(bicoPaymaster)); + emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, userOpHash); + ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr)); + + uint256 resultingDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); + uint256 resultingFeeCollectorPaymasterBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); + + uint256 totalGasFeesCharged = initialDappPaymasterBalance - resultingDappPaymasterBalance; + uint256 premiumCollected = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance; + + uint256 expectedPremium = totalGasFeesCharged - (totalGasFeesCharged * 1e6) / premium; + + console2.log(premiumCollected); + console2.log(expectedPremium); + console2.log(stdMath.percentDelta(premiumCollected, expectedPremium)); + // less than 0.01% difference between actual and expected values + assert(stdMath.percentDelta(premiumCollected, expectedPremium) < 1e14); + } } From 7234ab1ed133279c72fd69bdb0bdd852ee9ef837 Mon Sep 17 00:00:00 2001 From: Shivaansh Kapoor Date: Sun, 7 Jul 2024 22:16:29 +0400 Subject: [PATCH 4/5] some cleanup --- ...estFuzz_TestSponsorshipPaymasterWithPremium.t.sol | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol b/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol index 98fe11d..25fe3d1 100644 --- a/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol +++ b/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: Unlicensed pragma solidity ^0.8.26; -import { console2 } from "forge-std/src/console2.sol"; -import { stdMath } from "forge-std/src/Test.sol"; import { NexusTestBase } from "../../base/NexusTestBase.sol"; import { IBiconomySponsorshipPaymaster } from "../../../../contracts/interfaces/IBiconomySponsorshipPaymaster.sol"; import { BiconomySponsorshipPaymaster } from "../../../../contracts/sponsorship/SponsorshipPaymasterWithPremium.sol"; @@ -152,14 +150,10 @@ contract TestFuzz_SponsorshipPaymasterWithPremium is NexusTestBase { uint256 resultingFeeCollectorPaymasterBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); uint256 totalGasFeesCharged = initialDappPaymasterBalance - resultingDappPaymasterBalance; - uint256 premiumCollected = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance; - uint256 expectedPremium = totalGasFeesCharged - (totalGasFeesCharged * 1e6) / premium; + uint256 premiumCollected = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance; + uint256 expectedPremium = totalGasFeesCharged - ((totalGasFeesCharged * 1e6) / premium); - console2.log(premiumCollected); - console2.log(expectedPremium); - console2.log(stdMath.percentDelta(premiumCollected, expectedPremium)); - // less than 0.01% difference between actual and expected values - assert(stdMath.percentDelta(premiumCollected, expectedPremium) < 1e14); + assertEq(premiumCollected, expectedPremium); } } From 5ad56198927d055efb57675415511c85e4d61104 Mon Sep 17 00:00:00 2001 From: Shivaansh Kapoor Date: Mon, 8 Jul 2024 11:46:24 +0400 Subject: [PATCH 5/5] make tests more modular --- .../SponsorshipPaymasterWithPremium.sol | 22 ++-- test/foundry/base/NexusTestBase.sol | 62 +++++++++++ ...tSponsorshipPaymasterWithPremiumTest.t.sol | 102 ++++-------------- ..._TestSponsorshipPaymasterWithPremium.t.sol | 49 ++------- 4 files changed, 101 insertions(+), 134 deletions(-) diff --git a/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol b/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol index 5b06711..e63b539 100644 --- a/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol +++ b/contracts/sponsorship/SponsorshipPaymasterWithPremium.sol @@ -234,23 +234,23 @@ contract BiconomySponsorshipPaymaster is /// @param actualGasCost The actual gas cost of the transaction. function _postOp(PostOpMode, bytes calldata context, uint256 actualGasCost, uint256) internal override { unchecked { - (address paymasterId, uint32 dynamicMarkup, bytes32 userOpHash) = + (address paymasterId, uint32 dynamicAdjustment, bytes32 userOpHash) = abi.decode(context, (address, uint32, bytes32)); - uint256 costIncludingPremium = (actualGasCost * dynamicMarkup) / PRICE_DENOMINATOR; + uint256 adjustedGasCost = (actualGasCost * dynamicAdjustment) / PRICE_DENOMINATOR; - // deduct with premium - paymasterIdBalances[paymasterId] -= costIncludingPremium; + // Deduct the adjusted cost + paymasterIdBalances[paymasterId] -= adjustedGasCost; - if (costIncludingPremium > actualGasCost) { - // "collect" premium - uint256 actualPremium = costIncludingPremium - actualGasCost; - paymasterIdBalances[feeCollector] += actualPremium; - // Review if we should emit balToDeduct as well - emit PremiumCollected(paymasterId, actualPremium); + if (adjustedGasCost > actualGasCost) { + // Add premium to fee + uint256 premium = adjustedGasCost - actualGasCost; + paymasterIdBalances[feeCollector] += premium; + // Review if we should emit adjustedGasCost as well + emit PremiumCollected(paymasterId, premium); } - emit GasBalanceDeducted(paymasterId, costIncludingPremium, userOpHash); + emit GasBalanceDeducted(paymasterId, adjustedGasCost, userOpHash); } } diff --git a/test/foundry/base/NexusTestBase.sol b/test/foundry/base/NexusTestBase.sol index cd7dbaf..dbe176c 100644 --- a/test/foundry/base/NexusTestBase.sol +++ b/test/foundry/base/NexusTestBase.sol @@ -351,6 +351,46 @@ abstract contract NexusTestBase is CheatCodes, BaseEventsAndErrors { postopGasLimit = postopGasLimit - gasleft(); } + function createUserOp( + Vm.Wallet memory sender, + BiconomySponsorshipPaymaster paymaster, + uint32 premium + ) + internal + returns (PackedUserOperation memory userOp, bytes32 userOpHash) + { + // Create userOp with no paymaster gas estimates + uint48 validUntil = uint48(block.timestamp + 1 days); + uint48 validAfter = uint48(block.timestamp); + + userOp = buildUserOpWithCalldata(sender, "", address(VALIDATOR_MODULE)); + + userOp.paymasterAndData = generateAndSignPaymasterData( + userOp, PAYMASTER_SIGNER, paymaster, 3e6, 3e6, DAPP_ACCOUNT.addr, validUntil, validAfter, premium + ); + userOp.signature = signUserOp(sender, userOp); + + // Estimate paymaster gas limits + userOpHash = ENTRYPOINT.getUserOpHash(userOp); + (uint256 validationGasLimit, uint256 postopGasLimit) = + estimatePaymasterGasCosts(paymaster, userOp, userOpHash, 5e4); + + // Ammend the userop to have new gas limits and signature + userOp.paymasterAndData = generateAndSignPaymasterData( + userOp, + PAYMASTER_SIGNER, + paymaster, + uint128(validationGasLimit), + uint128(postopGasLimit), + DAPP_ACCOUNT.addr, + validUntil, + validAfter, + premium + ); + userOp.signature = signUserOp(sender, userOp); + userOpHash = ENTRYPOINT.getUserOpHash(userOp); + } + /// @notice Generates and signs the paymaster data for a user operation. /// @dev This function prepares the `paymasterAndData` field for a `PackedUserOperation` with the correct signature. /// @param userOp The user operation to be signed. @@ -417,4 +457,26 @@ abstract contract NexusTestBase is CheatCodes, BaseEventsAndErrors { } return result; } + + function getPremiums( + BiconomySponsorshipPaymaster paymaster, + uint256 initialDappPaymasterBalance, + uint256 initialFeeCollectorBalance, + uint32 premium + ) + internal + view + returns (uint256 expectedPremium, uint256 actualPremium) + { + uint256 resultingDappPaymasterBalance = paymaster.getBalance(DAPP_ACCOUNT.addr); + uint256 resultingFeeCollectorPaymasterBalance = paymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); + + uint256 totalGasFeesCharged = initialDappPaymasterBalance - resultingDappPaymasterBalance; + + if (premium >= 1e6) { + //premium + expectedPremium = totalGasFeesCharged - ((totalGasFeesCharged * 1e6) / premium); + actualPremium = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance; + } else revert("Premium must be more than 1e6"); + } } diff --git a/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol b/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol index 0f6ed46..a1df8d2 100644 --- a/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol +++ b/test/foundry/unit/concrete/TestSponsorshipPaymasterWithPremiumTest.t.sol @@ -134,94 +134,40 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { bicoPaymaster.deposit{ value: 1 ether }(); } - function test_WithdrawTo() external prankModifier(DAPP_ACCOUNT.addr) { - uint256 initialDappPaymasterBalance = 10 ether; - bicoPaymaster.depositFor{ value: initialDappPaymasterBalance }(DAPP_ACCOUNT.addr); - - PackedUserOperation[] memory ops = new PackedUserOperation[](1); - - uint48 validUntil = uint48(block.timestamp + 1 days); - uint48 validAfter = uint48(block.timestamp); - - PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); - - // No premium + function test_ValidatePaymasterAndPostOpWithoutPremium() external prankModifier(DAPP_ACCOUNT.addr) { + bicoPaymaster.depositFor{ value: 10 ether }(DAPP_ACCOUNT.addr); + // No premoium uint32 premium = 1e6; - userOp.paymasterAndData = generateAndSignPaymasterData( - userOp, PAYMASTER_SIGNER, bicoPaymaster, 3e6, 3e6, DAPP_ACCOUNT.addr, validUntil, validAfter, premium - ); - userOp.signature = signUserOp(ALICE, userOp); - - // Estimate paymaster gas limits - bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOp); - (uint256 validationGasLimit, uint256 postopGasLimit) = - estimatePaymasterGasCosts(bicoPaymaster, userOp, userOpHash, 5e4); - // Ammend the userop to have new gas limits and signature - userOp.paymasterAndData = generateAndSignPaymasterData( - userOp, - PAYMASTER_SIGNER, - bicoPaymaster, - uint128(validationGasLimit), - uint128(postopGasLimit), - DAPP_ACCOUNT.addr, - validUntil, - validAfter, - premium - ); - userOp.signature = signUserOp(ALICE, userOp); + PackedUserOperation[] memory ops = new PackedUserOperation[](1); + (PackedUserOperation memory userOp, bytes32 userOpHash) = createUserOp(ALICE, bicoPaymaster, premium); ops[0] = userOp; - userOpHash = ENTRYPOINT.getUserOpHash(userOp); + + uint256 initialDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); + uint256 initialFeeCollectorBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); vm.expectEmit(true, false, true, true, address(bicoPaymaster)); emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, userOpHash); ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr)); - uint256 resultingDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); - assertNotEq(initialDappPaymasterBalance, resultingDappPaymasterBalance); + (uint256 expectedPremium, uint256 actualPremium) = + getPremiums(bicoPaymaster, initialDappPaymasterBalance, initialFeeCollectorBalance, premium); + + assertEq(expectedPremium, actualPremium); } function test_ValidatePaymasterAndPostOpWithPremium() external { - uint256 initialDappPaymasterBalance = 10 ether; - bicoPaymaster.depositFor{ value: initialDappPaymasterBalance }(DAPP_ACCOUNT.addr); - - PackedUserOperation[] memory ops = new PackedUserOperation[](1); - - uint48 validUntil = uint48(block.timestamp + 1 days); - uint48 validAfter = uint48(block.timestamp); - - PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); - - // Charge a 10% premium + bicoPaymaster.depositFor{ value: 10 ether }(DAPP_ACCOUNT.addr); + // 10% premium on gas cost uint32 premium = 1e6 + 1e5; - userOp.paymasterAndData = generateAndSignPaymasterData( - userOp, PAYMASTER_SIGNER, bicoPaymaster, 3e6, 3e6, DAPP_ACCOUNT.addr, validUntil, validAfter, premium - ); - userOp.signature = signUserOp(ALICE, userOp); - - // Estimate paymaster gas limits - bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOp); - (uint256 validationGasLimit, uint256 postopGasLimit) = - estimatePaymasterGasCosts(bicoPaymaster, userOp, userOpHash, 5e4); - // Ammend the userop to have new gas limits and signature - userOp.paymasterAndData = generateAndSignPaymasterData( - userOp, - PAYMASTER_SIGNER, - bicoPaymaster, - uint128(validationGasLimit), - uint128(postopGasLimit), - DAPP_ACCOUNT.addr, - validUntil, - validAfter, - premium - ); - userOp.signature = signUserOp(ALICE, userOp); + PackedUserOperation[] memory ops = new PackedUserOperation[](1); + (PackedUserOperation memory userOp, bytes32 userOpHash) = createUserOp(ALICE, bicoPaymaster, premium); ops[0] = userOp; - userOpHash = ENTRYPOINT.getUserOpHash(userOp); + uint256 initialDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); uint256 initialFeeCollectorBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); - initialDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); + // submit userops vm.expectEmit(true, false, false, true, address(bicoPaymaster)); emit IBiconomySponsorshipPaymaster.PremiumCollected(DAPP_ACCOUNT.addr, 0); @@ -229,16 +175,10 @@ contract TestSponsorshipPaymasterWithPremium is NexusTestBase { emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, userOpHash); ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr)); - uint256 resultingDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); - uint256 resultingFeeCollectorPaymasterBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); - - uint256 totalGasFeesCharged = initialDappPaymasterBalance - resultingDappPaymasterBalance; - uint256 premiumCollected = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance; - - uint256 expectedGasPayment = totalGasFeesCharged - premiumCollected; - uint256 expectedPremium = expectedGasPayment / 10; + (uint256 expectedPremium, uint256 actualPremium) = + getPremiums(bicoPaymaster, initialDappPaymasterBalance, initialFeeCollectorBalance, premium); - assertEq(premiumCollected, expectedPremium); + assertEq(expectedPremium, actualPremium); } function test_RevertIf_ValidatePaymasterUserOpWithIncorrectSignatureLength() external { diff --git a/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol b/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol index 25fe3d1..11b43e4 100644 --- a/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol +++ b/test/foundry/unit/fuzz/TestFuzz_TestSponsorshipPaymasterWithPremium.t.sol @@ -101,59 +101,24 @@ contract TestFuzz_SponsorshipPaymasterWithPremium is NexusTestBase { function testFuzz_ValidatePaymasterAndPostOpWithPremium(uint32 premium) external { vm.assume(premium <= 2e6); vm.assume(premium > 1e6); - - uint256 initialDappPaymasterBalance = 10 ether; - bicoPaymaster.depositFor{ value: initialDappPaymasterBalance }(DAPP_ACCOUNT.addr); + bicoPaymaster.depositFor{ value: 10 ether }(DAPP_ACCOUNT.addr); PackedUserOperation[] memory ops = new PackedUserOperation[](1); - - uint48 validUntil = uint48(block.timestamp + 1 days); - uint48 validAfter = uint48(block.timestamp); - - PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); - - userOp.paymasterAndData = generateAndSignPaymasterData( - userOp, PAYMASTER_SIGNER, bicoPaymaster, 3e6, 3e6, DAPP_ACCOUNT.addr, validUntil, validAfter, premium - ); - userOp.signature = signUserOp(ALICE, userOp); - - // Estimate paymaster gas limits - bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOp); - (uint256 validationGasLimit, uint256 postopGasLimit) = - estimatePaymasterGasCosts(bicoPaymaster, userOp, userOpHash, 5e4); - - // Ammend the userop to have new gas limits and signature - userOp.paymasterAndData = generateAndSignPaymasterData( - userOp, - PAYMASTER_SIGNER, - bicoPaymaster, - uint128(validationGasLimit), - uint128(postopGasLimit), - DAPP_ACCOUNT.addr, - validUntil, - validAfter, - premium - ); - userOp.signature = signUserOp(ALICE, userOp); + (PackedUserOperation memory userOp, bytes32 userOpHash) = createUserOp(ALICE, bicoPaymaster, premium); ops[0] = userOp; - userOpHash = ENTRYPOINT.getUserOpHash(userOp); uint256 initialFeeCollectorBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); - initialDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); + uint256 initialDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); vm.expectEmit(true, false, false, true, address(bicoPaymaster)); emit IBiconomySponsorshipPaymaster.PremiumCollected(DAPP_ACCOUNT.addr, 0); vm.expectEmit(true, false, true, true, address(bicoPaymaster)); emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, userOpHash); ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr)); - uint256 resultingDappPaymasterBalance = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr); - uint256 resultingFeeCollectorPaymasterBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr); - - uint256 totalGasFeesCharged = initialDappPaymasterBalance - resultingDappPaymasterBalance; + (uint256 expectedPremium, uint256 actualPremium) = + getPremiums(bicoPaymaster, initialDappPaymasterBalance, initialFeeCollectorBalance, premium); - uint256 premiumCollected = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance; - uint256 expectedPremium = totalGasFeesCharged - ((totalGasFeesCharged * 1e6) / premium); - - assertEq(premiumCollected, expectedPremium); + assertEq(expectedPremium, actualPremium); } + }