Skip to content

Commit

Permalink
Merge branch 'develop' into fix/chainlight-005-withdraw-delay-and-min…
Browse files Browse the repository at this point in the history 
…-deposit
  • Loading branch information
Filipp Makarov authored and Filipp Makarov committed Oct 21, 2024
2 parents 3af5790 + 6c81989 commit 09f243d
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 141 deletions.
3 changes: 1 addition & 2 deletions contracts/interfaces/IBiconomySponsorshipPaymaster.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ interface IBiconomySponsorshipPaymaster {
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 indexed charge, bytes32 indexed userOpHash);
event PriceMarkupCollected(address indexed paymasterId, uint256 indexed priceMarkup);
event GasBalanceDeducted(address indexed paymasterId, uint256 actualGasCost, uint256 indexed adjustedGasCost, bytes32 indexed userOpHash);

Check warning on line 21 in contracts/interfaces/IBiconomySponsorshipPaymaster.sol

View workflow job for this annotation

GitHub Actions / Lint sources 

Line length must be no more than 120 but current length is 142
event Received(address indexed sender, uint256 value);
event TokensWithdrawn(address indexed token, address indexed to, uint256 indexed amount, address actor);
event WithdrawalRequestSubmitted(address withdrawAddress, uint256 amount);
Expand Down
28 changes: 14 additions & 14 deletions contracts/sponsorship/BiconomySponsorshipPaymaster.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,18 +289,12 @@ contract BiconomySponsorshipPaymaster is
// If overcharged refund the excess
paymasterIdBalances[paymasterId] += (prechargedAmount - adjustedGasCost);
}

// Add priceMarkup to fee collector balance
paymasterIdBalances[feeCollector] += adjustedGasCost - actualGasCost;

// Should always be true
// if (adjustedGasCost > actualGasCost) {
// Apply priceMarkup to fee collector balance
uint256 premium = adjustedGasCost - actualGasCost;
paymasterIdBalances[feeCollector] += premium;
// Review: if we should emit adjustedGasCost as well
emit PriceMarkupCollected(paymasterId, premium);
// }

// Review: emit min required information
emit GasBalanceDeducted(paymasterId, adjustedGasCost, userOpHash);
// premium = adjustedGasCost - actualGasCost => do not need to emit it explicitly
emit GasBalanceDeducted(paymasterId, actualGasCost, adjustedGasCost, userOpHash);
}
}

Expand Down Expand Up @@ -352,15 +346,21 @@ contract BiconomySponsorshipPaymaster is
revert InvalidPriceMarkup();
}

// callGasLimit + paymasterPostOpGas
uint256 maxPenalty = (
uint128(uint256(userOp.accountGasLimits)) +
uint128(bytes16(userOp.paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET : _PAYMASTER_DATA_OFFSET]))
) * 10 * userOp.unpackMaxFeePerGas() / 100;

// Deduct the max gas cost.
uint256 effectiveCost =
((requiredPreFund + unaccountedGas * userOp.unpackMaxFeePerGas()) * priceMarkup) / _PRICE_DENOMINATOR;
((requiredPreFund + unaccountedGas * userOp.unpackMaxFeePerGas()) * priceMarkup / _PRICE_DENOMINATOR);

if (effectiveCost > paymasterIdBalances[paymasterId]) {
if (effectiveCost + maxPenalty > paymasterIdBalances[paymasterId]) {
revert InsufficientFundsForPaymasterId();
}

paymasterIdBalances[paymasterId] -= effectiveCost;
paymasterIdBalances[paymasterId] -= (effectiveCost + maxPenalty);

context = abi.encode(paymasterId, priceMarkup, userOpHash, effectiveCost);

Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@
"test": "pnpm run test:forge",
"test:gas:forge": "forge test --gas-report",
"test:gas": "pnpm test:gas:forge",
"coverage:forge": "forge coverage --ir-minimum",
"coverage:forge": "forge coverage --ir-minimum --gas-limit 50000000 --gas-price 1000000000",
"coverage": "pnpm run coverage:forge",
"coverage:report": "forge coverage --ir-minimum --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 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",
Expand Down
184 changes: 100 additions & 84 deletions test/base/TestBase.sol
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.27;

import { Test } from "forge-std/Test.sol";
import { Vm } from "forge-std/Vm.sol";
import { console2 } from "forge-std/console2.sol";

import "solady/utils/ECDSA.sol";
import "./TestHelper.sol";
import "account-abstraction/core/UserOperationLib.sol";

import { IAccount } from "account-abstraction/interfaces/IAccount.sol";
import { Exec } from "account-abstraction/utils/Exec.sol";
Expand All @@ -26,17 +26,30 @@ import {
} from "../../../contracts/token/BiconomyTokenPaymaster.sol";

abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
address constant ENTRYPOINT_ADDRESS = address(0x0000000071727De22E5E9d8BAf0edAc6f37da032);

address constant WRAPPED_NATIVE_ADDRESS = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
using UserOperationLib for PackedUserOperation;

address constant ENTRYPOINT_ADDRESS = address(0x0000000071727De22E5E9d8BAf0edAc6f37da032);
address constant WRAPPED_NATIVE_ADDRESS = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
address constant SWAP_ROUTER_ADDRESS = address(0xE592427A0AEce92De3Edee1F18E0157C05861564);

Vm.Wallet internal PAYMASTER_OWNER;
Vm.Wallet internal PAYMASTER_SIGNER;
Vm.Wallet internal PAYMASTER_FEE_COLLECTOR;
Vm.Wallet internal DAPP_ACCOUNT;

uint256 internal constant _PAYMASTER_POSTOP_GAS_OFFSET = UserOperationLib.PAYMASTER_POSTOP_GAS_OFFSET;
uint256 internal constant _PAYMASTER_DATA_OFFSET = UserOperationLib.PAYMASTER_DATA_OFFSET;

struct PaymasterData {
uint128 validationGasLimit;
uint128 postOpGasLimit;
address paymasterId;
uint48 validUntil;
uint48 validAfter;
uint32 priceMarkup;
}

// Used to buffer user op gas limits
// GAS_LIMIT = (ESTIMATED_GAS * GAS_BUFFER_RATIO) / 100
uint8 private constant GAS_BUFFER_RATIO = 110;
Expand Down Expand Up @@ -80,11 +93,11 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
bytes32 userOpHash = ENTRYPOINT.getUserOpHash(userOp);
verificationGasUsed = gasleft();
IAccount(userOp.sender).validateUserOp(userOp, userOpHash, 0);
verificationGasUsed = verificationGasUsed - gasleft();
verificationGasUsed = verificationGasUsed - gasleft(); //+ 21000;

callGasUsed = gasleft();
bool success = Exec.call(userOp.sender, 0, userOp.callData, 3e6);
callGasUsed = callGasUsed - gasleft();
callGasUsed = callGasUsed - gasleft(); //+ 21000;
assert(success);

verificationGasLimit = (verificationGasUsed * GAS_BUFFER_RATIO) / 100;
Expand All @@ -108,68 +121,65 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
// Estimate gas used
validationGasUsed = gasleft();
(context,) = paymaster.validatePaymasterUserOp(userOp, userOpHash, requiredPreFund);
validationGasUsed = validationGasUsed - gasleft();
validationGasUsed = validationGasUsed - gasleft(); //+ 21000;

postopGasUsed = gasleft();
paymaster.postOp(IPaymaster.PostOpMode.opSucceeded, context, 1e12, 3e6);
postopGasUsed = (postopGasUsed - gasleft());
postopGasUsed = (postopGasUsed - gasleft()); //+ 21000;

validationGasLimit = (validationGasUsed * GAS_BUFFER_RATIO) / 100;
postopGasLimit = (postopGasUsed * GAS_BUFFER_RATIO) / 100;
}

// Note: we can use externally provided gas limits to override.
// Note: we can pass callData and callGasLimit as args to test with more tx types
// Note: we can pass Nexus instance instead of sender EOA and comuting counterfactual within buildUserOpWithCalldata

function createUserOp(
Vm.Wallet memory sender,
BiconomySponsorshipPaymaster paymaster,
uint32 priceMarkup
uint32 priceMarkup,
uint128 postOpGasLimitOverride
)
internal
returns (PackedUserOperation memory userOp, bytes32 userOpHash)
{
// Create userOp with no 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, 8e3, DAPP_ACCOUNT.addr, validUntil, validAfter, priceMarkup
);
PaymasterData memory pmData = PaymasterData({
validationGasLimit: 100_000,
postOpGasLimit: uint128(postOpGasLimitOverride),
paymasterId: DAPP_ACCOUNT.addr,
validUntil: uint48(block.timestamp + 1 days),
validAfter: uint48(block.timestamp),
priceMarkup: priceMarkup
});
(userOp.paymasterAndData,) = generateAndSignPaymasterData(userOp, PAYMASTER_SIGNER, paymaster, pmData);
userOp.signature = signUserOp(sender, userOp);

(,, uint256 verificationGasLimit, uint256 callGasLimit) = estimateUserOpGasCosts(userOp);
// Estimate paymaster gas limits
(, uint256 postopGasUsed, uint256 validationGasLimit, uint256 postopGasLimit) =
estimatePaymasterGasCosts(paymaster, userOp, 5e4);

// console2.log("postOpGasUsed");
// console2.logUint(postopGasUsed);

// uint256 prevValUnaccountedGas = paymaster.unaccountedGas();
// console2.logUint(prevValUnaccountedGas);

// Below may not be needed if unaccountedGas is set correctly
vm.startPrank(paymaster.owner());
// Set unaccounted gas to be gas used in postop + 1000 for EP overhead and penalty
paymaster.setUnaccountedGas(postopGasUsed + 500);
vm.stopPrank();

// uint256 currentValUnaccountedGas = paymaster.unaccountedGas();
// console2.logUint(currentValUnaccountedGas);

// Ammend the userop to have new gas limits and signature
userOp.accountGasLimits = bytes32(abi.encodePacked(uint128(verificationGasLimit), uint128(callGasLimit)));
(userOp.paymasterAndData,) = generateAndSignPaymasterData(
userOp,
PAYMASTER_SIGNER,
paymaster,
uint128(validationGasLimit),
uint128(postopGasLimit),
// Estimate account gas limits
// (,, uint256 verificationGasLimit, uint256 callGasLimit) = estimateUserOpGasCosts(userOp);
// // Estimate paymaster gas limits
// (, uint256 postopGasUsed, uint256 validationGasLimit, uint256 postopGasLimit) =
// estimatePaymasterGasCosts(paymaster, userOp, 5e4);

// vm.startPrank(paymaster.owner());
// paymaster.setUnaccountedGas(postopGasUsed + 500);
// vm.stopPrank();

// Ammend the userop to have updated / overridden gas limits
userOp.accountGasLimits = bytes32(abi.encodePacked(uint128(100_000), uint128(0)));
PaymasterData memory pmDataNew = PaymasterData(
uint128(100_000),
uint128(postOpGasLimitOverride),
DAPP_ACCOUNT.addr,
validUntil,
validAfter,
uint48(block.timestamp + 1 days),
uint48(block.timestamp),
priceMarkup
);

(userOp.paymasterAndData,) = generateAndSignPaymasterData(userOp, PAYMASTER_SIGNER, paymaster, pmDataNew);
userOp.signature = signUserOp(sender, userOp);
userOpHash = ENTRYPOINT.getUserOpHash(userOp);
}
Expand All @@ -185,52 +195,48 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
PackedUserOperation memory userOp,
Vm.Wallet memory signer,
BiconomySponsorshipPaymaster paymaster,
uint128 paymasterValGasLimit,
uint128 paymasterPostOpGasLimit,
address paymasterId,
uint48 validUntil,
uint48 validAfter,
uint32 priceMarkup
PaymasterData memory pmData
)
internal
view
returns (bytes memory finalPmData, bytes memory signature)
{
// Initial paymaster data with zero signature
bytes memory initialPmData = abi.encodePacked(
userOp.paymasterAndData = abi.encodePacked(
address(paymaster),
paymasterValGasLimit,
paymasterPostOpGasLimit,
paymasterId,
validUntil,
validAfter,
priceMarkup,
pmData.validationGasLimit,
pmData.postOpGasLimit,
pmData.paymasterId,
pmData.validUntil,
pmData.validAfter,
pmData.priceMarkup,
new bytes(65) // Zero signature
);

// Update user operation with initial paymaster data
userOp.paymasterAndData = initialPmData;


{
// Generate hash to be signed
bytes32 paymasterHash = paymaster.getHash(userOp, paymasterId, validUntil, validAfter, priceMarkup);
bytes32 paymasterHash =
paymaster.getHash(userOp, pmData.paymasterId, pmData.validUntil, pmData.validAfter, pmData.priceMarkup);

// Sign the hash
signature = signMessage(signer, paymasterHash);
require(signature.length == 65, "Invalid Paymaster Signature length");
}

// Final paymaster data with the actual signature
finalPmData = abi.encodePacked(
address(paymaster),
paymasterValGasLimit,
paymasterPostOpGasLimit,
paymasterId,
validUntil,
validAfter,
priceMarkup,
pmData.validationGasLimit,
pmData.postOpGasLimit,
pmData.paymasterId,
pmData.validUntil,
pmData.validAfter,
pmData.priceMarkup,
signature
);
}

// Note: Token paymaster could also get into stack deep issues.
// TODO: Refactor to reduce stack depth
/// @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.
Expand Down Expand Up @@ -308,7 +314,8 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
BiconomySponsorshipPaymaster paymaster,
uint256 initialDappPaymasterBalance,
uint256 initialFeeCollectorBalance,
uint32 priceMarkup
uint32 priceMarkup,
uint256 maxPenalty
)
internal
view
Expand All @@ -318,45 +325,54 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
uint256 resultingFeeCollectorPaymasterBalance = paymaster.getBalance(PAYMASTER_FEE_COLLECTOR.addr);

uint256 totalGasFeesCharged = initialDappPaymasterBalance - resultingDappPaymasterBalance;
uint256 accountableGasFeesCharged = totalGasFeesCharged - maxPenalty;

if (priceMarkup >= 1e6) {
//priceMarkup
expectedPriceMarkup = totalGasFeesCharged - ((totalGasFeesCharged * 1e6) / priceMarkup);
actualPriceMarkup = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance;
} else {
revert("PriceMarkup must be more than 1e6");
}
expectedPriceMarkup = accountableGasFeesCharged - ((accountableGasFeesCharged * 1e6) / priceMarkup);
actualPriceMarkup = resultingFeeCollectorPaymasterBalance - initialFeeCollectorBalance;
}

function getMaxPenalty(PackedUserOperation calldata userOp) public view returns (uint256) {
return (
uint128(uint256(userOp.accountGasLimits)) +
uint128(bytes16(userOp.paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET : _PAYMASTER_DATA_OFFSET]))
) * 10 * userOp.unpackMaxFeePerGas() / 100;
}

// Note: can pack values into one struct
function calculateAndAssertAdjustments(
BiconomySponsorshipPaymaster bicoPaymaster,
uint256 initialDappPaymasterBalance,
uint256 initialFeeCollectorBalance,
uint256 initialBundlerBalance,
uint256 initialPaymasterEpBalance,
uint32 priceMarkup
uint32 priceMarkup,
uint256 maxPenalty
)
internal
view
{
(uint256 expectedPriceMarkup, uint256 actualPriceMarkup) =
getPriceMarkups(bicoPaymaster, initialDappPaymasterBalance, initialFeeCollectorBalance, priceMarkup);
getPriceMarkups(bicoPaymaster, initialDappPaymasterBalance, initialFeeCollectorBalance, priceMarkup, maxPenalty);
uint256 totalGasFeePaid = BUNDLER.addr.balance - initialBundlerBalance;
uint256 gasPaidByDapp = initialDappPaymasterBalance - bicoPaymaster.getBalance(DAPP_ACCOUNT.addr);

console2.log("1");
// Assert that what paymaster paid is the same as what the bundler received
assertEq(totalGasFeePaid, initialPaymasterEpBalance - bicoPaymaster.getDeposit());

console2.log("2");
// Assert that adjustment collected (if any) is correct
assertEq(expectedPriceMarkup, actualPriceMarkup);

console2.log("3");
// Gas paid by dapp is higher than paymaster
// Guarantees that EP always has sufficient deposit to pay back dapps

// TODO
// Review: fix this properly. avoid out of stack errors
assertGt(gasPaidByDapp, BUNDLER.addr.balance - initialBundlerBalance);
// Ensure that max 1% difference between total gas paid + the adjustment premium and gas paid by dapp (from

console2.log("4");
// Ensure that max 2% difference between total gas paid + the adjustment premium and gas paid by dapp (from
// paymaster)
assertApproxEqRel(totalGasFeePaid + actualPriceMarkup, gasPaidByDapp, 0.01e18);
assertApproxEqRel(totalGasFeePaid + actualPriceMarkup + maxPenalty, gasPaidByDapp, 0.02e18);
}

function _toSingletonArray(address addr) internal pure returns (address[] memory) {
Expand Down
Loading

0 comments on commit 09f243d

Please sign in to comment.