Merge pull request #30 from bcnmy/feat/refactor-events-and-tests

feaT:refactor event + add more info in parse pm data

contracts/interfaces/IBiconomySponsorshipPaymaster.sol

@@ -9,9 +9,9 @@ interface IBiconomySponsorshipPaymaster {
     event FixedPriceMarkupChanged(uint256 indexed oldValue, uint256 indexed newValue);
     event VerifyingSignerChanged(address indexed oldSigner, address indexed newSigner, address indexed actor);
     event FeeCollectorChanged(address indexed oldFeeCollector, address indexed newFeeCollector, address indexed actor);
-    event GasDeposited(address indexed paymasterId, uint256 indexed value);
-    event GasWithdrawn(address indexed paymasterId, address indexed to, uint256 indexed value);
-    event GasBalanceDeducted(address indexed paymasterId, uint256 actualGasCost, uint256 indexed adjustedGasCost, bytes32 indexed userOpHash);
+    event GasDeposited(address indexed _paymasterId, uint256 indexed _value);
+    event GasWithdrawn(address indexed _paymasterId, address indexed _to, uint256 indexed _value);
+    event GasBalanceDeducted(address indexed _paymasterId, uint256 indexed _charge, uint256 indexed _premium);
     event Received(address indexed sender, uint256 value);
     event TokensWithdrawn(address indexed token, address indexed to, uint256 indexed amount, address actor);
 
@@ -50,6 +50,8 @@ interface IBiconomySponsorshipPaymaster {
             uint48 validUntil,
             uint48 validAfter,
             uint32 priceMarkup,
+            uint128 paymasterValidationGasLimit,
+            uint128 paymasterPostOpGasLimit,
             bytes calldata signature
         );
 }

contracts/sponsorship/BiconomySponsorshipPaymaster.sol

@@ -235,6 +235,8 @@ contract BiconomySponsorshipPaymaster is
             uint48 validUntil,
             uint48 validAfter,
             uint32 priceMarkup,
+            uint128 paymasterValidationGasLimit,
+            uint128 paymasterPostOpGasLimit,
             bytes calldata signature
         )
     {
@@ -243,6 +245,8 @@ contract BiconomySponsorshipPaymaster is
             validUntil = uint48(bytes6(paymasterAndData[_PAYMASTER_ID_OFFSET + 20:_PAYMASTER_ID_OFFSET + 26]));
             validAfter = uint48(bytes6(paymasterAndData[_PAYMASTER_ID_OFFSET + 26:_PAYMASTER_ID_OFFSET + 32]));
             priceMarkup = uint32(bytes4(paymasterAndData[_PAYMASTER_ID_OFFSET + 32:_PAYMASTER_ID_OFFSET + 36]));
+            paymasterValidationGasLimit = uint128(bytes16(paymasterAndData[_PAYMASTER_VALIDATION_GAS_OFFSET:_PAYMASTER_POSTOP_GAS_OFFSET]));
+            paymasterPostOpGasLimit = uint128(bytes16(paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET : _PAYMASTER_DATA_OFFSET]));
             signature = paymasterAndData[_PAYMASTER_ID_OFFSET + 36:];
         }
     }
@@ -261,25 +265,29 @@ contract BiconomySponsorshipPaymaster is
         override
     {
         unchecked {
-            (address paymasterId, uint32 priceMarkup, bytes32 userOpHash, uint256 prechargedAmount) =
-                abi.decode(context, (address, uint32, bytes32, uint256));
+            (address paymasterId, uint32 priceMarkup, uint256 prechargedAmount) =
+                abi.decode(context, (address, uint32, uint256));
 
             // Include unaccountedGas since EP doesn't include this in actualGasCost
             // unaccountedGas = postOpGas + EP overhead gas + estimated penalty
             actualGasCost = actualGasCost + (unaccountedGas * actualUserOpFeePerGas);
             // Apply the price markup
             uint256 adjustedGasCost = (actualGasCost * priceMarkup) / _PRICE_DENOMINATOR;
 
+            uint256 premium = adjustedGasCost - actualGasCost;
+
+            // Add priceMarkup to fee collector balance
+            paymasterIdBalances[feeCollector] += premium;
+
             if (prechargedAmount > adjustedGasCost) {
                 // If overcharged refund the excess
                 paymasterIdBalances[paymasterId] += (prechargedAmount - adjustedGasCost);
+            } else {
+                // deduct what needs to be deducted from paymasterId
+                paymasterIdBalances[paymasterId] -= (adjustedGasCost - prechargedAmount);                
             }
-
-            // Add priceMarkup to fee collector balance
-            paymasterIdBalances[feeCollector] += adjustedGasCost - actualGasCost;
-
-            // premium = adjustedGasCost - actualGasCost => do not need to emit it explicitly
-            emit GasBalanceDeducted(paymasterId, actualGasCost, adjustedGasCost, userOpHash);
+            // here adjustedGasCost does not account for gasPenalty. prechargedAmount accounts for penalty with maxGasPenalty
+            emit GasBalanceDeducted(paymasterId, adjustedGasCost, premium);
         }
     }
 
@@ -302,8 +310,11 @@ contract BiconomySponsorshipPaymaster is
         override
         returns (bytes memory context, uint256 validationData)
     {
-        (address paymasterId, uint48 validUntil, uint48 validAfter, uint32 priceMarkup, bytes calldata signature) =
+        (userOpHash);
+        (address paymasterId, uint48 validUntil, uint48 validAfter, uint32 priceMarkup, uint128 paymasterValidationGasLimit, uint128 paymasterPostOpGasLimit, bytes calldata signature) =
             parsePaymasterAndData(userOp.paymasterAndData);
+        (paymasterValidationGasLimit, paymasterPostOpGasLimit);
+
         //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"
@@ -347,7 +358,7 @@ contract BiconomySponsorshipPaymaster is
 
         paymasterIdBalances[paymasterId] -= (effectiveCost + maxPenalty);
 
-        context = abi.encode(paymasterId, priceMarkup, userOpHash, effectiveCost);
+        context = abi.encode(paymasterId, priceMarkup, effectiveCost);
 
         //no need for other on-chain validation: entire UserOp should have been checked
         // by the external service prior to signing it.

test/unit/concrete/TestSponsorshipPaymaster.t.sol

@@ -224,7 +224,7 @@ contract TestSponsorshipPaymasterWithPriceMarkup is TestBase {
 
         // submit userops
         vm.expectEmit(true, false, false, false, address(bicoPaymaster));
-        emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, 0, userOpHash);
+        emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, 0);
         startPrank(BUNDLER.addr);
         ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr));
         stopPrank();
@@ -260,7 +260,7 @@ contract TestSponsorshipPaymasterWithPriceMarkup is TestBase {
 
         // submit userops
         vm.expectEmit(true, false, false, false, address(bicoPaymaster));
-        emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, 0, userOpHash);
+        emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, 0);
 
         startPrank(BUNDLER.addr);
         ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr));
@@ -403,13 +403,17 @@ contract TestSponsorshipPaymasterWithPriceMarkup is TestBase {
             uint48 parsedValidUntil,
             uint48 parsedValidAfter,
             uint32 parsedPriceMarkup,
+            uint128 parsedPaymasterValidationGasLimit,
+            uint128 parsedPaymasterPostOpGasLimit,
             bytes memory parsedSignature
         ) = bicoPaymaster.parsePaymasterAndData(paymasterAndData);
 
         assertEq(paymasterId, parsedPaymasterId);
         assertEq(validUntil, parsedValidUntil);
         assertEq(validAfter, parsedValidAfter);
         assertEq(priceMarkup, parsedPriceMarkup);
+        assertEq(pmData.validationGasLimit, parsedPaymasterValidationGasLimit);
+        assertEq(pmData.postOpGasLimit, parsedPaymasterPostOpGasLimit);
         assertEq(signature, parsedSignature);
     }
 }

test/unit/fuzz/TestFuzz_TestSponsorshipPaymaster.t.sol

@@ -112,7 +112,7 @@ contract TestFuzz_SponsorshipPaymasterWithPriceMarkup is TestBase {
         uint256 initialFeeCollectorBalance = bicoPaymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr);
 
         vm.expectEmit(true, false, false, false, address(bicoPaymaster));
-        emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, 0, userOpHash);
+        emit IBiconomySponsorshipPaymaster.GasBalanceDeducted(DAPP_ACCOUNT.addr, 0, 0);
 
         startPrank(BUNDLER.addr);
         ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr));
@@ -156,6 +156,8 @@ contract TestFuzz_SponsorshipPaymasterWithPriceMarkup is TestBase {
             uint48 parsedValidUntil,
             uint48 parsedValidAfter,
             uint32 parsedPriceMarkup,
+            uint128 parsedPaymasterValidationGasLimit,
+            uint128 parsedPaymasterPostOpGasLimit,
             bytes memory parsedSignature
         ) = bicoPaymaster.parsePaymasterAndData(paymasterAndData);
 
@@ -164,5 +166,7 @@ contract TestFuzz_SponsorshipPaymasterWithPriceMarkup is TestBase {
         assertEq(validAfter, parsedValidAfter);
         assertEq(priceMarkup, parsedPriceMarkup);
         assertEq(signature, parsedSignature);
+        assertEq(pmData.validationGasLimit, parsedPaymasterValidationGasLimit);
+        assertEq(pmData.postOpGasLimit, parsedPaymasterPostOpGasLimit);
     }
 }