Skip to content

Commit

Permalink
merge
Browse files Browse the repository at this point in the history
  • Loading branch information
Filipp Makarov authored and Filipp Makarov committed Oct 22, 2024
2 parents 344384f + 1087b8e commit 54d1b08
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 106 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 @@ -5,12 +5,11 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { PackedUserOperation } from "account-abstraction/core/UserOperationLib.sol";

interface IBiconomySponsorshipPaymaster {

struct WithdrawalRequest {
uint256 amount;
address to;
uint256 requestSubmittedTimestamp;
}
}

event UnaccountedGasChanged(uint256 indexed oldValue, uint256 indexed newValue);
event FixedPriceMarkupChanged(uint256 indexed oldValue, uint256 indexed newValue);
Expand Down
149 changes: 90 additions & 59 deletions contracts/sponsorship/BiconomySponsorshipPaymaster.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,17 @@ contract BiconomySponsorshipPaymaster is
uint256 private constant _UNACCOUNTED_GAS_LIMIT = 100_000;

mapping(address => uint256) public paymasterIdBalances;
mapping(address => bool) internal trustedPaymasterIds;
mapping (address paymasterId => WithdrawalRequest request) requests;
mapping(address => bool) internal _trustedPaymasterIds;
mapping(address paymasterId => WithdrawalRequest request) internal _requests;

constructor(
address owner,
IEntryPoint entryPointArg,
address verifyingSignerArg,
address feeCollectorArg,
uint256 unaccountedGasArg,
uint256 _paymasterIdWithdrawalDelay,
uint256 _minDeposit
uint256 paymasterIdWithdrawalDelayArg,
uint256 minDepositArg
)
BasePaymaster(owner, entryPointArg)
{
Expand All @@ -76,8 +76,8 @@ contract BiconomySponsorshipPaymaster is
}
feeCollector = feeCollectorArg;
unaccountedGas = unaccountedGasArg;
paymasterIdWithdrawalDelay = _paymasterIdWithdrawalDelay;
minDeposit = _minDeposit;
paymasterIdWithdrawalDelay = paymasterIdWithdrawalDelayArg;
minDeposit = minDepositArg;
}

receive() external payable {
Expand All @@ -92,8 +92,7 @@ contract BiconomySponsorshipPaymaster is
function depositFor(address paymasterId) external payable nonReentrant {
if (paymasterId == address(0)) revert PaymasterIdCanNotBeZero();
if (msg.value == 0) revert DepositCanNotBeZero();
if(paymasterIdBalances[paymasterId] + msg.value < minDeposit)
revert LowDeposit();
if (paymasterIdBalances[paymasterId] + msg.value < minDeposit) revert LowDeposit();
paymasterIdBalances[paymasterId] += msg.value;
entryPoint.depositTo{ value: msg.value }(address(this));
emit GasDeposited(paymasterId, msg.value);
Expand All @@ -118,6 +117,26 @@ contract BiconomySponsorshipPaymaster is
emit VerifyingSignerChanged(oldSigner, newVerifyingSigner, msg.sender);
}

/**
* @dev Set a new trusted paymasterId.
* Can only be called by the owner of the contract.
* @param paymasterId The paymasterId to be set as trusted.
* @param isTrusted Whether the paymasterId is trusted or not.
*/
function setTrustedPaymasterId(address paymasterId, bool isTrusted) external payable onlyOwner {
if (paymasterId == address(0)) revert PaymasterIdCanNotBeZero();
_trustedPaymasterIds[paymasterId] = isTrusted;
}

/**
* @dev Set a new minimum deposit value.
* Can only be called by the owner of the contract.
* @param newMinDeposit The new minimum deposit value to be set.
*/
function setMinDeposit(uint256 newMinDeposit) external payable onlyOwner {
minDeposit = newMinDeposit;
}

/**
* @dev Set a new fee collector address.
* Can only be called by the owner of the contract.
Expand Down Expand Up @@ -173,9 +192,9 @@ contract BiconomySponsorshipPaymaster is
if (withdrawAddress == address(0)) revert CanNotWithdrawToZeroAddress();
if (amount == 0) revert CanNotWithdrawZeroAmount();
uint256 currentBalance = paymasterIdBalances[msg.sender];
if (amount > currentBalance)
revert InsufficientFundsInGasTank();
requests[msg.sender] = WithdrawalRequest({amount: amount, to: withdrawAddress, requestSubmittedTimestamp: block.timestamp });
if (amount > currentBalance) revert InsufficientFundsInGasTank();
_requests[msg.sender] =
WithdrawalRequest({ amount: amount, to: withdrawAddress, requestSubmittedTimestamp: block.timestamp });
emit WithdrawalRequestSubmitted(withdrawAddress, amount);
}

Expand All @@ -185,16 +204,15 @@ contract BiconomySponsorshipPaymaster is
* @param paymasterId paymasterId (Dapp Depositor address)
*/
function executeWithdrawalRequest(address paymasterId) external nonReentrant {
WithdrawalRequest memory req = requests[paymasterId];
if(req.requestSubmittedTimestamp == 0) revert NoRequestSubmitted();
uint256 clearanceTimestamp = req.requestSubmittedTimestamp + getDelay(paymasterId);
if (block.timestamp < clearanceTimestamp)
revert RequestNotClearedYet(clearanceTimestamp);
WithdrawalRequest memory req = _requests[paymasterId];
if (req.requestSubmittedTimestamp == 0) revert NoRequestSubmitted();
uint256 clearanceTimestamp = req.requestSubmittedTimestamp + _getDelay(paymasterId);
if (block.timestamp < clearanceTimestamp) revert RequestNotClearedYet(clearanceTimestamp);
uint256 currentBalance = paymasterIdBalances[paymasterId];
req.amount = req.amount > currentBalance ? currentBalance : req.amount;
if(req.amount == 0) revert CanNotWithdrawZeroAmount();
paymasterIdBalances[paymasterId] = currentBalance - req.amount;
delete requests[paymasterId];
delete _requests[paymasterId];
entryPoint.withdrawTo(payable(req.to), req.amount);
emit GasWithdrawn(paymasterId, req.to, req.amount);
}
Expand All @@ -203,7 +221,7 @@ contract BiconomySponsorshipPaymaster is
* @dev Cancel a withdrawal request for the paymasterId (Dapp Depositor address)
*/
function cancelWithdrawalRequest() external {
delete requests[msg.sender];
delete _requests[msg.sender];
emit WithdrawalRequestCancelledFor(msg.sender);
}

Expand All @@ -215,6 +233,7 @@ contract BiconomySponsorshipPaymaster is
}

function withdrawTo(address payable withdrawAddress, uint256 amount) external virtual override {
(withdrawAddress, amount);
revert SubmitRequestInstead();
}

Expand Down Expand Up @@ -286,8 +305,10 @@ 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]));
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:];
}
}
Expand All @@ -305,33 +326,35 @@ contract BiconomySponsorshipPaymaster is
internal
override
{
unchecked {
(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
console2.log("overcharged ", prechargedAmount - adjustedGasCost);
paymasterIdBalances[paymasterId] += (prechargedAmount - adjustedGasCost);
} else {
// deduct what needs to be deducted from paymasterId
console2.log("undercharged ", (adjustedGasCost - prechargedAmount));
paymasterIdBalances[paymasterId] -= (adjustedGasCost - prechargedAmount);
}
// here adjustedGasCost does not account for gasPenalty. prechargedAmount accounts for penalty with maxGasPenalty
emit GasBalanceDeducted(paymasterId, adjustedGasCost, premium);
(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
console2.log("overcharged ", prechargedAmount - adjustedGasCost);

Check failure on line 345 in contracts/sponsorship/BiconomySponsorshipPaymaster.sol

View workflow job for this annotation

GitHub Actions / Lint sources 

Unexpected console statement
paymasterIdBalances[paymasterId] += (prechargedAmount - adjustedGasCost);
} else {
// deduct what needs to be deducted from paymasterId
console2.log("undercharged ", (adjustedGasCost - prechargedAmount));

Check failure on line 349 in contracts/sponsorship/BiconomySponsorshipPaymaster.sol

View workflow job for this annotation

GitHub Actions / Lint sources 

Unexpected console statement
paymasterIdBalances[paymasterId] -= (adjustedGasCost - prechargedAmount);
}
// here adjustedGasCost does not account for gasPenalty. prechargedAmount accounts for penalty with maxGasPenalty
emit GasBalanceDeducted(paymasterId, adjustedGasCost, premium);

// here adjustedGasCost does not account for gasPenalty. prechargedAmount accounts for penalty with
// maxGasPenalty
emit GasBalanceDeducted(paymasterId, adjustedGasCost, premium);
}

/**
Expand All @@ -354,10 +377,17 @@ contract BiconomySponsorshipPaymaster is
returns (bytes memory context, uint256 validationData)
{
(userOpHash);
(address paymasterId, uint48 validUntil, uint48 validAfter, uint32 priceMarkup, uint128 paymasterValidationGasLimit, uint128 paymasterPostOpGasLimit, bytes calldata signature) =
parsePaymasterAndData(userOp.paymasterAndData);
(
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"
Expand Down Expand Up @@ -387,15 +417,17 @@ contract BiconomySponsorshipPaymaster is

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

console2.log("max penalty ", maxPenalty);

Check failure on line 426 in contracts/sponsorship/BiconomySponsorshipPaymaster.sol

View workflow job for this annotation

GitHub Actions / Lint sources 

Unexpected console statement

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

if (effectiveCost + maxPenalty > paymasterIdBalances[paymasterId]) {
revert InsufficientFundsForPaymasterId();
Expand Down Expand Up @@ -431,15 +463,14 @@ contract BiconomySponsorshipPaymaster is
}
}

function _getDelay(address paymasterId) internal view returns (uint256) {
if (_trustedPaymasterIds[paymasterId]) return 0;
return paymasterIdWithdrawalDelay;
}

function _withdrawERC20(IERC20 token, address target, uint256 amount) private {
if (target == address(0)) revert CanNotWithdrawToZeroAddress();
SafeTransferLib.safeTransfer(address(token), target, amount);
emit TokensWithdrawn(address(token), target, amount, msg.sender);
}

function getDelay(address paymasterId) internal view returns (uint256) {
if (trustedPaymasterIds[paymasterId])
return 0;
return paymasterIdWithdrawalDelay;
}
}
23 changes: 12 additions & 11 deletions test/base/TestBase.sol
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// 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";
Expand All @@ -26,7 +27,6 @@ import {
} from "../../../contracts/token/BiconomyTokenPaymaster.sol";

abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {

using UserOperationLib for PackedUserOperation;

address constant ENTRYPOINT_ADDRESS = address(0x0000000071727De22E5E9d8BAf0edAc6f37da032);
Expand Down Expand Up @@ -212,14 +212,14 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
pmData.priceMarkup,
new bytes(65) // Zero signature
);

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

// Sign the hash
signature = signMessage(signer, paymasterHash);
// Sign the hash
signature = signMessage(signer, paymasterHash);
}

// Final paymaster data with the actual signature
Expand Down Expand Up @@ -333,8 +333,8 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {

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]))
uint128(uint256(userOp.accountGasLimits))
+ uint128(bytes16(userOp.paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET:_PAYMASTER_DATA_OFFSET]))
) * 10 * userOp.unpackMaxFeePerGas() / 100;
}

Expand All @@ -351,8 +351,9 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors {
internal
view
{
(uint256 expectedPriceMarkup, uint256 actualPriceMarkup) =
getPriceMarkups(bicoPaymaster, initialDappPaymasterBalance, initialFeeCollectorBalance, priceMarkup, maxPenalty);
(uint256 expectedPriceMarkup, uint256 actualPriceMarkup) = getPriceMarkups(
bicoPaymaster, initialDappPaymasterBalance, initialFeeCollectorBalance, priceMarkup, maxPenalty
);
uint256 totalGasFeePaid = BUNDLER.addr.balance - initialBundlerBalance;
uint256 gasPaidByDapp = initialDappPaymasterBalance - bicoPaymaster.getBalance(DAPP_ACCOUNT.addr);

Expand Down
28 changes: 14 additions & 14 deletions test/unit/concrete/TestSponsorshipPaymaster.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,21 @@ import "@ERC4337/account-abstraction/contracts/interfaces/IStakeManager.sol";
contract TestSponsorshipPaymasterWithPriceMarkup is TestBase {
BiconomySponsorshipPaymaster public bicoPaymaster;

uint256 public constant WITHDRAWAL_DELAY = 3600;
uint256 public constant WITHDRAWAL_DELAY = 3600;
uint256 public constant MIN_DEPOSIT = 1e15;

function setUp() public {
setupPaymasterTestEnvironment();
// Deploy Sponsorship Paymaster
bicoPaymaster = new BiconomySponsorshipPaymaster(
{
owner: PAYMASTER_OWNER.addr,
entryPointArg: ENTRYPOINT,
verifyingSignerArg: PAYMASTER_SIGNER.addr,
feeCollectorArg: PAYMASTER_FEE_COLLECTOR.addr,
unaccountedGasArg: 7e3,
_paymasterIdWithdrawalDelay: WITHDRAWAL_DELAY,
_minDeposit: MIN_DEPOSIT
}
);
bicoPaymaster = new BiconomySponsorshipPaymaster({
owner: PAYMASTER_OWNER.addr,
entryPointArg: ENTRYPOINT,
verifyingSignerArg: PAYMASTER_SIGNER.addr,
feeCollectorArg: PAYMASTER_FEE_COLLECTOR.addr,
unaccountedGasArg: 7e3,
paymasterIdWithdrawalDelayArg: WITHDRAWAL_DELAY,
minDepositArg: MIN_DEPOSIT
});
}

function test_Deploy() external {
Expand Down Expand Up @@ -56,7 +54,9 @@ contract TestSponsorshipPaymasterWithPriceMarkup is TestBase {

function test_RevertIf_DeployWithFeeCollectorSetToZero() external {
vm.expectRevert(abi.encodeWithSelector(FeeCollectorCanNotBeZero.selector));
new BiconomySponsorshipPaymaster(PAYMASTER_OWNER.addr, ENTRYPOINT, PAYMASTER_SIGNER.addr, address(0), 7e3, 3600, 1e15);
new BiconomySponsorshipPaymaster(
PAYMASTER_OWNER.addr, ENTRYPOINT, PAYMASTER_SIGNER.addr, address(0), 7e3, 3600, 1e15
);
}

function test_RevertIf_DeployWithFeeCollectorAsContract() external {
Expand Down Expand Up @@ -230,7 +230,7 @@ contract TestSponsorshipPaymasterWithPriceMarkup is TestBase {
vm.warp(block.timestamp + WITHDRAWAL_DELAY + 1);
uint256 dappPaymasterBalanceBefore = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr);
uint256 bobBalanceBefore = BOB_ADDRESS.balance;
bicoPaymaster.executeWithdrawalRequest(DAPP_ACCOUNT.addr);
bicoPaymaster.executeWithdrawalRequest(DAPP_ACCOUNT.addr);
uint256 dappPaymasterBalanceAfter = bicoPaymaster.getBalance(DAPP_ACCOUNT.addr);
uint256 bobBalanceAfter = BOB_ADDRESS.balance;
assertEq(dappPaymasterBalanceAfter, dappPaymasterBalanceBefore - depositAmount);
Expand Down
Loading

0 comments on commit 54d1b08

Please sign in to comment.