Skip to content

Commit

Permalink
remove ecdsa sig verification from paymaster
Browse files Browse the repository at this point in the history
  • Loading branch information
@jackchuma
jackchuma committed Feb 1, 2025
1 parent e2ee1f3 commit b80dbc8
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 45 deletions.
18 changes: 2 additions & 16 deletions contracts/src/Paymaster.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ pragma solidity 0.8.24;
import {IEntryPoint} from "account-abstraction/interfaces/IEntryPoint.sol";
import {IPaymaster} from "account-abstraction/interfaces/IPaymaster.sol";
import {PackedUserOperation} from "account-abstraction/interfaces/PackedUserOperation.sol";
import {ECDSA} from "solady/utils/ECDSA.sol";
import {SafeTransferLib} from "solady/utils/SafeTransferLib.sol";

import {IUserOpPrecheck} from "./interfaces/IUserOpPrecheck.sol";
Expand All @@ -16,7 +15,6 @@ import {IUserOpPrecheck} from "./interfaces/IUserOpPrecheck.sol";
/// @notice This contract is used as a hook for fulfillers to provide funds for requested transactions when the
/// cross-chain call(s) are ERC-4337 User Operations
abstract contract Paymaster is IPaymaster {
using ECDSA for bytes32;
using SafeTransferLib for address payable;

/// @notice The context structure returned by validatePaymasterUserOp
Expand Down Expand Up @@ -229,10 +227,9 @@ abstract contract Paymaster is IPaymaster {
returns (bytes memory context, uint256 validationData)
{
_settleBalanceDiff(maxCost);
(uint256 ethAmount, bytes memory signature, address precheckContract) =
abi.decode(userOp.paymasterAndData[52:], (uint256, bytes, address));
(uint256 ethAmount, address precheckContract) = abi.decode(userOp.paymasterAndData[52:], (uint256, address));

address fulfiller = _genDigest(userOp, ethAmount).toEthSignedMessageHash().recover(signature);
address fulfiller = tx.origin;
uint256 balance = _magicSpendBalance[fulfiller];
uint256 gasBalance = _ethForGas[fulfiller];

Expand Down Expand Up @@ -397,15 +394,4 @@ abstract contract Paymaster is IPaymaster {

return totalTrackedEthBalance_ - entryPointBalance;
}

/// @notice A function that generates the digest that the fulfiller's signature is for
///
/// @param userOp The User Operation to generate a digest for
/// @param ethAmount The amount of eth in the User Operation
///
/// @return digest The digest for the fulfiller's signature
function _genDigest(PackedUserOperation calldata userOp, uint256 ethAmount) private view returns (bytes32) {
uint256 dstChainId = block.chainid;
return keccak256(abi.encode(userOp.sender, userOp.nonce, userOp.callData, ethAmount, dstChainId));
}
}
50 changes: 22 additions & 28 deletions contracts/test/Paymaster.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ pragma solidity 0.8.24;

import {EntryPoint, IEntryPoint, PackedUserOperation, UserOperationLib} from "account-abstraction/core/EntryPoint.sol";
import {Vm} from "forge-std/Vm.sol";
import {ECDSA} from "solady/utils/ECDSA.sol";
import {Ownable} from "solady/auth/Ownable.sol";

import {BaseTest} from "./BaseTest.t.sol";
Expand All @@ -15,7 +14,6 @@ import {MockUserOpPrecheck} from "./mocks/MockUserOpPrecheck.sol";

contract PaymasterTest is BaseTest, MockEndpoint {
using UserOperationLib for PackedUserOperation;
using ECDSA for bytes32;

IEntryPoint entryPoint;
MockAccount mockAccount;
Expand Down Expand Up @@ -317,7 +315,7 @@ contract PaymasterTest is BaseTest, MockEndpoint {
fundAccount(signer.addr, amount)
fundPaymaster(signer.addr, amount)
{
PackedUserOperation[] memory userOps = _generateUserOps(signer.privateKey, amount, address(0), 0);
PackedUserOperation[] memory userOps = _generateUserOps(amount, address(0), 0);
uint256 maxCost = this.calculateMaxCost(userOps[0]);

_deposit(amount);
Expand All @@ -332,6 +330,7 @@ contract PaymasterTest is BaseTest, MockEndpoint {
abi.encodeWithSelector(Paymaster.InsufficientMagicSpendBalance.selector, signer.addr, 0, amount)
)
);
vm.prank(signer.addr, signer.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));
}

Expand All @@ -340,7 +339,7 @@ contract PaymasterTest is BaseTest, MockEndpoint {
fundAccount(signer.addr, amount)
fundPaymaster(signer.addr, amount)
{
PackedUserOperation[] memory userOps = _generateUserOps(otherSigner.privateKey, amount, address(0), 0);
PackedUserOperation[] memory userOps = _generateUserOps(amount, address(0), 0);
uint256 maxCost = this.calculateMaxCost(userOps[0]);

vm.assume(maxCost <= amount && amount <= type(uint256).max - maxCost);
Expand All @@ -355,22 +354,24 @@ contract PaymasterTest is BaseTest, MockEndpoint {
abi.encodeWithSelector(Paymaster.InsufficientGasBalance.selector, otherSigner.addr, 0, maxCost)
)
);
vm.prank(otherSigner.addr, otherSigner.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));
}

function test_validatePaymasterUserOp_revertsIfFulfillerHasInsufficientGasBalanceOnSecondTry(
uint256 amount,
uint256 ethAmount
) public fundAccount(signer.addr, amount) fundPaymaster(signer.addr, amount) {
PackedUserOperation[] memory userOps = _generateUserOps(signer.privateKey, ethAmount, address(0), 0);
PackedUserOperation[] memory userOps = _generateUserOps(ethAmount, address(0), 0);
uint256 maxCost = this.calculateMaxCost(userOps[0]);

vm.assume(ethAmount < type(uint256).max - maxCost * 2 && ethAmount + maxCost * 2 < amount);
_deposit(maxCost);

vm.prank(signer.addr, signer.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));

userOps = _generateUserOps(otherSigner.privateKey, ethAmount, address(0), 1);
userOps = _generateUserOps(ethAmount, address(0), 1);
maxCost = this.calculateMaxCost(userOps[0]);
_deposit(maxCost);

Expand All @@ -382,6 +383,7 @@ contract PaymasterTest is BaseTest, MockEndpoint {
abi.encodeWithSelector(Paymaster.InsufficientGasBalance.selector, otherSigner.addr, 0, maxCost)
)
);
vm.prank(otherSigner.addr, otherSigner.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));
}

Expand All @@ -391,12 +393,13 @@ contract PaymasterTest is BaseTest, MockEndpoint {
fundPaymaster(signer.addr, amount)
{
assertEq(paymaster.getGasBalance(signer.addr), address(entryPoint).balance);
PackedUserOperation[] memory userOps = _generateUserOps(signer.privateKey, ethAmount, address(0), 0);
PackedUserOperation[] memory userOps = _generateUserOps(ethAmount, address(0), 0);
uint256 maxCost = this.calculateMaxCost(userOps[0]);

vm.assume(ethAmount < type(uint256).max - maxCost && ethAmount + maxCost < amount);
_deposit(maxCost);

vm.prank(signer.addr, signer.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));

assertEq(paymaster.getGasBalance(signer.addr), address(entryPoint).balance);
Expand All @@ -409,13 +412,14 @@ contract PaymasterTest is BaseTest, MockEndpoint {
{
vm.assume(ethAmount > 0);

PackedUserOperation[] memory userOps = _generateUserOps(signer.privateKey, ethAmount, precheckAddress, 0);
PackedUserOperation[] memory userOps = _generateUserOps(ethAmount, precheckAddress, 0);
uint256 maxCost = this.calculateMaxCost(userOps[0]);

vm.assume(ethAmount < type(uint256).max - maxCost && ethAmount + maxCost < amount);
_deposit(maxCost);

vm.expectRevert(abi.encodeWithSelector(IEntryPoint.FailedOpWithRevert.selector, 0, "AA33 reverted", ""));
vm.prank(signer.addr, signer.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));
}

Expand All @@ -426,12 +430,13 @@ contract PaymasterTest is BaseTest, MockEndpoint {
{
vm.assume(ethAmount > 0);

PackedUserOperation[] memory userOps = _generateUserOps(signer.privateKey, ethAmount, address(0), 0);
PackedUserOperation[] memory userOps = _generateUserOps(ethAmount, address(0), 0);
uint256 maxCost = this.calculateMaxCost(userOps[0]);

vm.assume(ethAmount < type(uint256).max - maxCost && ethAmount + maxCost < amount);
_deposit(maxCost);

vm.prank(signer.addr, signer.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));

assertEq(paymaster.requestHash(), entryPoint.getUserOpHash(userOps[0]));
Expand All @@ -444,12 +449,13 @@ contract PaymasterTest is BaseTest, MockEndpoint {
fundPaymaster(signer.addr, amount)
{
uint256 ethAmount = 0;
PackedUserOperation[] memory userOps = _generateUserOps(signer.privateKey, ethAmount, address(0), 0);
PackedUserOperation[] memory userOps = _generateUserOps(ethAmount, address(0), 0);
uint256 maxCost = this.calculateMaxCost(userOps[0]);

vm.assume(ethAmount < type(uint256).max - maxCost && ethAmount + maxCost < amount);
_deposit(maxCost);

vm.prank(signer.addr, signer.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));

assertEq(paymaster.requestHash(), bytes32(0));
Expand All @@ -463,19 +469,20 @@ contract PaymasterTest is BaseTest, MockEndpoint {
{
vm.assume(ethAmount > 0);

PackedUserOperation[] memory userOps = _generateUserOps(signer.privateKey, ethAmount, address(0), 0);
PackedUserOperation[] memory userOps = _generateUserOps(ethAmount, address(0), 0);
uint256 maxCost = this.calculateMaxCost(userOps[0]);

vm.assume(ethAmount < type(uint256).max - maxCost && ethAmount + maxCost < amount);
_deposit(maxCost);
uint256 initialBalance = paymaster.getMagicSpendBalance(signer.addr);

vm.prank(signer.addr, signer.addr);
entryPoint.handleOps(userOps, payable(BUNDLER));

assertEq(paymaster.getMagicSpendBalance(signer.addr), initialBalance - ethAmount);
}

function _generateUserOps(uint256 signerKey, uint256 ethAmount, address precheck, uint256 nonce)
function _generateUserOps(uint256 ethAmount, address precheck, uint256 nonce)
private
view
returns (PackedUserOperation[] memory)
Expand All @@ -489,27 +496,14 @@ contract PaymasterTest is BaseTest, MockEndpoint {
accountGasLimits: bytes32(abi.encodePacked(uint128(1000000), uint128(1000000))),
preVerificationGas: 100000,
gasFees: bytes32(abi.encodePacked(uint128(1000000), uint128(1000000))),
paymasterAndData: "",
paymasterAndData: _encodePaymasterAndData(ethAmount, precheck),
signature: abi.encode(0)
});
(uint8 v, bytes32 r, bytes32 s) = vm.sign(signerKey, _genDigest(userOps[0], ethAmount).toEthSignedMessageHash());
userOps[0].paymasterAndData = _encodePaymasterAndData(abi.encodePacked(r, s, v), ethAmount, precheck);
return userOps;
}

function _encodePaymasterAndData(bytes memory signature, uint256 ethAmount, address precheck)
private
view
returns (bytes memory)
{
return abi.encodePacked(
address(paymaster), uint128(1000000), uint128(1000000), abi.encode(ethAmount, signature, precheck)
);
}

function _genDigest(PackedUserOperation memory userOp, uint256 ethAmount) private view returns (bytes32) {
uint256 dstChainId = block.chainid;
return keccak256(abi.encode(userOp.sender, userOp.nonce, userOp.callData, ethAmount, dstChainId));
function _encodePaymasterAndData(uint256 ethAmount, address precheck) private view returns (bytes memory) {
return abi.encodePacked(address(paymaster), uint128(1000000), uint128(1000000), abi.encode(ethAmount, precheck));
}

function calculateMaxCost(PackedUserOperation calldata userOp) public view returns (uint256) {
Expand Down
2 changes: 1 addition & 1 deletion contracts/test/mocks/MockUserOpPrecheck.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {IUserOpPrecheck} from "../../src/interfaces/IUserOpPrecheck.sol";

contract MockUserOpPrecheck is IUserOpPrecheck {
function precheckUserOp(PackedUserOperation calldata userOp, address fulfiller) external pure {
address expectedCaller = abi.decode(userOp.paymasterAndData[188:], (address));
address expectedCaller = abi.decode(userOp.paymasterAndData[104:], (address));

if (expectedCaller != fulfiller) {
revert();
Expand Down

0 comments on commit b80dbc8

Please sign in to comment.