rename and add tests

.gas-snapshot

@@ -4,14 +4,17 @@ IsValidWithdrawalSignature:test_returnsTrueWithValidSignature() (gas: 51016)
 OwnerWithdrawTest:test_revertsIfNotOwner() (gas: 15305)
 OwnerWithdrawTest:test_transfersERC20Successfully(uint256) (runs: 256, μ: 97271, ~: 99711)
 OwnerWithdrawTest:test_transfersETHSuccessfully(uint256) (runs: 256, μ: 51418, ~: 53415)
-PostOpTest:test_RevertsIfPostOpFailed(uint256,uint256,uint256) (runs: 256, μ: 97167, ~: 99371)
-PostOpTest:test_entryPointAddStake(uint112,uint32) (runs: 256, μ: 56233, ~: 56233)
-PostOpTest:test_entryPointDeposit(uint112) (runs: 256, μ: 32516, ~: 32962)
-PostOpTest:test_entryPointUnlockStake() (gas: 54888)
-PostOpTest:test_entryPointWithdraw(uint112) (runs: 256, μ: 63200, ~: 65300)
-PostOpTest:test_entryPointWithdrawStake() (gas: 71803)
+PostOpTest:test_RevertsIfPostOpFailed(uint256,uint256,uint256) (runs: 256, μ: 97250, ~: 99371)
+PostOpTest:test_entryPointAddStake(uint112,uint32) (runs: 256, μ: 56278, ~: 56278)
+PostOpTest:test_entryPointDeposit(uint112) (runs: 256, μ: 32383, ~: 32940)
+PostOpTest:test_entryPointUnlockStake() (gas: 54933)
+PostOpTest:test_entryPointWithdraw(uint112) (runs: 256, μ: 63521, ~: 65321)
+PostOpTest:test_entryPointWithdrawStake() (gas: 71848)
 PostOpTest:test_paymasterPaysForOp() (gas: 210663)
-PostOpTest:test_transfersExcess(uint256,uint256,uint256,uint256) (runs: 256, μ: 112465, ~: 113488)
+PostOpTest:test_transfersExcess(uint256,uint256,uint256,uint256) (runs: 256, μ: 112468, ~: 113488)
+SetMaxWithdrawPercent:test_emitsCorrectly(uint256) (runs: 256, μ: 19851, ~: 20191)
+SetMaxWithdrawPercent:test_reverts_whenNotCalledByOwner() (gas: 12257)
+SetMaxWithdrawPercent:test_setsMaxWithdrawPercent(uint256) (runs: 256, μ: 19209, ~: 19549)
 Simulate:test() (gas: 12114)
 ValidatePaymasterUserOpTest:test_emitsCorrectly(address,uint256,uint256) (runs: 256, μ: 104847, ~: 106080)
 ValidatePaymasterUserOpTest:test_recordsNonceUsed(uint256) (runs: 256, μ: 112158, ~: 113247)
@@ -20,16 +23,16 @@ ValidatePaymasterUserOpTest:test_returnsCorrectly() (gas: 91185)
 ValidatePaymasterUserOpTest:test_revertsIfMaxCostMoreThanRequested() (gas: 39141)
 ValidatePaymasterUserOpTest:test_revertsIfNonceUsed() (gas: 103502)
 ValidatePaymasterUserOpTest:test_revertsIfWithdrawAssetNotZero() (gas: 55952)
-ValidatePaymasterUserOpTest:test_reverts_whenWithdrawExceedsMaxAllowed(uint256,uint256,uint256) (runs: 256, μ: 100811, ~: 100992)
+ValidatePaymasterUserOpTest:test_reverts_whenWithdrawExceedsMaxAllowed(uint256,uint256,uint256) (runs: 256, μ: 100800, ~: 100992)
 ValidateTest:test_receive() (gas: 14687)
-WithdrawGasExcess:test_RevertsIfNoExcess(uint256) (runs: 256, μ: 71133, ~: 71396)
+WithdrawGasExcess:test_RevertsIfNoExcess(uint256) (runs: 256, μ: 71122, ~: 71396)
 WithdrawGasExcess:test_transferExcess(uint256,uint256,uint256) (runs: 256, μ: 104279, ~: 104290)
-WithdrawTest:test_emitsCorrectly(address,uint256,uint256) (runs: 256, μ: 118413, ~: 121368)
-WithdrawTest:test_emitsCorrectlyERC20Withdraw(address,uint256,uint256) (runs: 256, μ: 162446, ~: 164773)
+WithdrawTest:test_emitsCorrectly(address,uint256,uint256) (runs: 256, μ: 118270, ~: 121368)
+WithdrawTest:test_emitsCorrectlyERC20Withdraw(address,uint256,uint256) (runs: 256, μ: 162408, ~: 164773)
 WithdrawTest:test_recordsNonceUsed(uint256) (runs: 256, μ: 111372, ~: 112461)
 WithdrawTest:test_revertsIfExpired(uint48,uint256) (runs: 256, μ: 61664, ~: 61785)
 WithdrawTest:test_revertsIfNonceUsed() (gas: 95844)
 WithdrawTest:test_revertsIfWrongSignature() (gas: 63830)
-WithdrawTest:test_reverts_whenWithdrawExceedsMaxAllowed(uint256,uint256,uint256) (runs: 256, μ: 85124, ~: 85096)
+WithdrawTest:test_reverts_whenWithdrawExceedsMaxAllowed(uint256,uint256,uint256) (runs: 256, μ: 85143, ~: 85096)
 WithdrawTest:test_transfersERC20Successfully(uint256) (runs: 256, μ: 140626, ~: 143066)
 WithdrawTest:test_transfersETHSuccessfully(uint256) (runs: 256, μ: 91692, ~: 93689)

src/MagicSpend.sol

@@ -20,7 +20,7 @@ contract MagicSpend is Ownable, IPaymaster {
     struct WithdrawRequest {
         /// @dev The signature associated with this withdraw request.
         bytes signature;
-        /// @dev The asset to withdraw. NOTE: Only ETH (associated with zero address) is supported for now.
+        /// @dev The asset to withdraw.
         address asset;
         /// @dev The requested amount to withdraw.
         uint256 amount;
@@ -30,9 +30,12 @@ contract MagicSpend is Ownable, IPaymaster {
         uint48 expiry;
     }
 
-    /// @notice The maximum percentage of address.balance a single withdraw.
+    /// @notice The maximum percentage of address.balance a single withdraw can use.
+    ///
+    /// @dev Only applies to native asset withdraws.
     /// @dev Helps prevent withdraws in the same transaction leading to reverts and hurting paymaster reputation.
-    uint256 public maxWithdrawDenominator;
+    /// @dev Percent in whole units; 20 = 1/20 = 20%
+    uint256 public maxWithdrawPercent;
 
     /// @notice Track the ETH available to be withdrawn per user.
     mapping(address user => uint256 amount) internal _withdrawableETH;
@@ -48,10 +51,10 @@ contract MagicSpend is Ownable, IPaymaster {
     /// @param nonce   The request nonce.
     event MagicSpendWithdrawal(address indexed account, address indexed asset, uint256 amount, uint256 nonce);
 
-    /// @notice Emitted when the `maxWithdrawDenominator` is set.
+    /// @notice Emitted when the `maxWithdrawPercent` is set.
     ///
-    /// @param newDenominator The new maxWithdrawDenominator value.
-    event MaxWithdrawDenominatorSet(uint256 newDenominator);
+    /// @param newPercent The new maxWithdrawPercent value.
+    event MaxWithdrawPercentSet(uint256 newPercent);
 
     /// @notice Thrown when the withdraw request signature is invalid.
     ///
@@ -81,7 +84,7 @@ contract MagicSpend is Ownable, IPaymaster {
     /// @param asset The requested asset.
     error UnsupportedPaymasterAsset(address asset);
 
-    /// @notice Thrown if WithdrawRequest.amount exceeds address(this).balance / maxWithdrawDenominator.
+    /// @notice Thrown if WithdrawRequest.amount exceeds address(this).balance / maxWithdrawPercent.
     ///
     /// @param requestedAmount The requested amount excluding gas.
     /// @param maxAllowed      The current max allowed withdraw.
@@ -106,9 +109,9 @@ contract MagicSpend is Ownable, IPaymaster {
     /// @notice Deploy the contract and set its initial owner.
     ///
     /// @param owner_ The initial owner of this contract.
-    constructor(address owner_, uint256 maxWithdrawDenominator_) {
+    constructor(address owner_, uint256 maxWithdrawPercent_) {
         Ownable._initializeOwner(owner_);
-        _setMaxWithdrawDenominator(maxWithdrawDenominator_);
+        _setMaxWithdrawPercent(maxWithdrawPercent_);
     }
 
     /// @notice Receive function allowing ETH to be deposited in this contract.
@@ -147,7 +150,7 @@ contract MagicSpend is Ownable, IPaymaster {
         onlyEntryPoint
     {
         // `PostOpMode.postOpReverted` should never happen.
-        // The flow here can only revert if there are > maxWithdrawDenominator
+        // The flow here can only revert if there are > maxWithdrawPercent
         // withdraws in the same transaction, which should be highly unlikely.
         // If the ETH transfer fails, the entire bundle will revert due an issue in the EntryPoint
         // https://github.com/eth-infinitism/account-abstraction/pull/293
@@ -255,8 +258,15 @@ contract MagicSpend is Ownable, IPaymaster {
         IEntryPoint(entryPoint()).withdrawStake(to);
     }
 
-    function setMaxWithdrawDenominator(uint256 newDenominator) external onlyOwner {
-        _setMaxWithdrawDenominator(newDenominator);
+    /// @notice sets maxWithdrawPercent
+    ///
+    /// @dev Reverts if not called by the owner of the contract.
+    ///
+    /// @param newPercent The new value for maxWithdrawPercent
+    /// Percent expressed in whole units, e.g. 20 means no single withdraw
+    /// can exceed 20% of address.balance
+    function setMaxWithdrawPercent(uint256 newPercent) external onlyOwner {
+        _setMaxWithdrawPercent(newPercent);
     }
 
     /// @notice Returns whether the `withdrawRequest` signature is valid for the given `account`.
@@ -315,10 +325,10 @@ contract MagicSpend is Ownable, IPaymaster {
         return 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789;
     }
 
-    function _setMaxWithdrawDenominator(uint256 newDenominator) internal {
-        maxWithdrawDenominator = newDenominator;
+    function _setMaxWithdrawPercent(uint256 newPercent) internal {
+        maxWithdrawPercent = newPercent;
 
-        emit MaxWithdrawDenominatorSet(newDenominator);
+        emit MaxWithdrawPercentSet(newPercent);
     }
 
     /// @notice Validate the `withdrawRequest` against the given `account`.
@@ -333,7 +343,7 @@ contract MagicSpend is Ownable, IPaymaster {
             revert InvalidNonce(withdrawRequest.nonce);
         }
 
-        uint256 maxAllowed = address(this).balance / maxWithdrawDenominator;
+        uint256 maxAllowed = address(this).balance / maxWithdrawPercent;
         if (withdrawRequest.asset == address(0) && withdrawRequest.amount > maxAllowed) {
             revert WithdrawTooLarge(withdrawRequest.amount, maxAllowed);
         }

test/OwnerWithdraw.t.sol

@@ -2,7 +2,7 @@
 pragma solidity >=0.8.21;
 
 import {Ownable} from "./MagicSpend.t.sol";
-import {MagicSpendTest} from "./Validate.t.sol";
+import {MagicSpendTest} from "./MagicSpend.t.sol";
 import {MockERC20} from "solady/test/utils/mocks/MockERC20.sol";
 
 contract OwnerWithdrawTest is MagicSpendTest {

test/SetMaxWithdrawPercent.t.sol

@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.23;
+
+import "./MagicSpend.t.sol";
+import {Ownable} from "./MagicSpend.t.sol";
+
+contract SetMaxWithdrawPercent is MagicSpendTest {
+    function test_reverts_whenNotCalledByOwner() public {
+        vm.prank(makeAddr("fake"));
+        vm.expectRevert(Ownable.Unauthorized.selector);
+        magic.setMaxWithdrawPercent(20);
+    }
+
+    function test_setsMaxWithdrawPercent(uint256 newPercent) public {
+        vm.prank(owner);
+        magic.setMaxWithdrawPercent(newPercent);
+        assertEq(magic.maxWithdrawPercent(), newPercent);
+    }
+
+    function test_emitsCorrectly(uint256 newPercent) public {
+        vm.expectEmit(false, false, false, true);
+        emit MagicSpend.MaxWithdrawPercentSet(newPercent);
+        vm.prank(owner);
+        magic.setMaxWithdrawPercent(newPercent);
+    }
+}

test/Validate.t.sol

@@ -41,24 +41,22 @@ abstract contract ValidateTest is MagicSpendTest {
     function test_reverts_whenWithdrawExceedsMaxAllowed(
         uint256 accountBalance,
         uint256 withdraw,
-        uint256 maxWithdrawDenominator
+        uint256 maxWithdrawPercent
     ) public virtual {
-        vm.assume(maxWithdrawDenominator > 0);
+        vm.assume(maxWithdrawPercent > 0);
         accountBalance = bound(accountBalance, 0, type(uint256).max - 1);
-        withdraw = bound(withdraw, accountBalance / maxWithdrawDenominator + 1, type(uint256).max);
+        withdraw = bound(withdraw, accountBalance / maxWithdrawPercent + 1, type(uint256).max);
         amount = withdraw;
 
         vm.stopPrank();
         vm.startPrank(owner);
-        magic.setMaxWithdrawDenominator(maxWithdrawDenominator);
+        magic.setMaxWithdrawPercent(maxWithdrawPercent);
         magic.ownerWithdraw(address(0), address(1), address(magic).balance);
         vm.stopPrank();
 
         vm.deal(address(magic), accountBalance);
         vm.expectRevert(
-            abi.encodeWithSelector(
-                MagicSpend.WithdrawTooLarge.selector, withdraw, accountBalance / maxWithdrawDenominator
-            )
+            abi.encodeWithSelector(MagicSpend.WithdrawTooLarge.selector, withdraw, accountBalance / maxWithdrawPercent)
         );
         vm.startPrank(invoker);
         _validateInvokingCall();

test/ValidatePaymasterUserOp.t.sol

@@ -49,10 +49,10 @@ contract ValidatePaymasterUserOpTest is PaymasterMagicSpendBaseTest, ValidateTes
     function test_reverts_whenWithdrawExceedsMaxAllowed(
         uint256 accountBalance,
         uint256 withdraw,
-        uint256 maxWithdrawDenominator
+        uint256 maxWithdrawPercent
     ) public override {
         maxCost = withdraw;
-        super.test_reverts_whenWithdrawExceedsMaxAllowed(accountBalance, withdraw, maxWithdrawDenominator);
+        super.test_reverts_whenWithdrawExceedsMaxAllowed(accountBalance, withdraw, maxWithdrawPercent);
     }
 
     function _validateInvokingCall() internal override {