Skip to content

Commit

Permalink
clean up tests and withdrawableFunds
Browse files Browse the repository at this point in the history
  • Loading branch information
wilsoncusack committed Mar 14, 2024
1 parent dfac046 commit b71735a
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 37 deletions.
16 changes: 8 additions & 8 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
PostOpTest:test_RevertsIfPostOpFailed(uint256,uint256,uint256) (runs: 256, μ: 94902, ~: 97095)
PostOpTest:test_RevertsIfPostOpFailed(uint256,uint256,uint256) (runs: 256, μ: 94981, ~: 97095)
PostOpTest:test_entryPointAddStake(uint112,uint32) (runs: 256, μ: 56233, ~: 56233)
PostOpTest:test_entryPointDeposit(uint112) (runs: 256, μ: 32405, ~: 32962)
PostOpTest:test_entryPointDeposit(uint112) (runs: 256, μ: 32331, ~: 32962)
PostOpTest:test_entryPointUnlockStake() (gas: 54910)
PostOpTest:test_entryPointWithdraw(uint112) (runs: 256, μ: 63200, ~: 65300)
PostOpTest:test_entryPointWithdraw(uint112) (runs: 256, μ: 63500, ~: 65300)
PostOpTest:test_entryPointWithdrawStake() (gas: 71825)
PostOpTest:test_paymasterPaysForOp() (gas: 208402)
PostOpTest:test_transfersExcess(uint256,uint256,uint256,uint256) (runs: 256, μ: 110036, ~: 111224)
PostOpTest:test_transfersExcess(uint256,uint256,uint256,uint256) (runs: 256, μ: 110216, ~: 111227)
Simulate:test() (gas: 12114)
ValidatePaymasterUserOpTest:test_emitsCorrectly(address,uint256,uint256) (runs: 256, μ: 102602, ~: 103835)
ValidatePaymasterUserOpTest:test_emitsCorrectly(address,uint256,uint256) (runs: 256, μ: 102564, ~: 103835)
ValidatePaymasterUserOpTest:test_recordsNonceUsed(uint256) (runs: 256, μ: 109936, ~: 111025)
ValidatePaymasterUserOpTest:test_returns1sIfWrongSignature() (gas: 94076)
ValidatePaymasterUserOpTest:test_returnsCorrectly() (gas: 88920)
Expand All @@ -17,10 +17,10 @@ ValidatePaymasterUserOpTest:test_revertsIfWithdrawAssetNotZero() (gas: 55974)
ValidateTest:test_receive() (gas: 14687)
WithdrawGasExcess:test_RevertsIfNoExcess(uint256) (runs: 256, μ: 68914, ~: 69177)
WithdrawGasExcess:test_transferExcess(uint256,uint256,uint256) (runs: 256, μ: 102074, ~: 102074)
WithdrawTest:test_emitsCorrectly(address,uint256,uint256) (runs: 256, μ: 116118, ~: 119073)
WithdrawTest:test_emitsCorrectlyERC20Withdraw(address,uint256,uint256) (runs: 256, μ: 158012, ~: 160513)
WithdrawTest:test_emitsCorrectly(address,uint256,uint256) (runs: 256, μ: 115868, ~: 119073)
WithdrawTest:test_emitsCorrectlyERC20Withdraw(address,uint256,uint256) (runs: 256, μ: 158052, ~: 160513)
WithdrawTest:test_recordsNonceUsed(uint256) (runs: 256, μ: 109078, ~: 110167)
WithdrawTest:test_revertsIfExpired(uint48,uint256) (runs: 256, μ: 59425, ~: 59535)
WithdrawTest:test_revertsIfExpired(uint48,uint256) (runs: 256, μ: 59403, ~: 59535)
WithdrawTest:test_revertsIfNonceUsed() (gas: 93506)
WithdrawTest:test_revertsIfWrongSignature() (gas: 61536)
WithdrawTest:test_transfersERC20Successfully(uint256) (runs: 256, μ: 136385, ~: 138825)
Expand Down
14 changes: 7 additions & 7 deletions src/MagicSpend.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ contract MagicSpend is Ownable, IPaymaster {
uint48 expiry;
}

/// @notice Track the funds available to be withdrawn per user.
mapping(address user => uint256 amount) public withdrawableFunds;
/// @notice Track the ETH available to be withdrawn per user.
mapping(address user => uint256 amount) internal _withdrawableETH;

/// @dev Mappings keeping track of already used nonces per user to prevent replays of withdraw requests.
mapping(uint256 nonce => mapping(address user => bool used)) internal _nonceUsed;
Expand Down Expand Up @@ -135,7 +135,7 @@ contract MagicSpend is Ownable, IPaymaster {
}

// NOTE: Do not include the gas part in withdrawable funds as it will be handled in `postOp()`.
withdrawableFunds[userOp.sender] += withdrawAmount - maxCost;
_withdrawableETH[userOp.sender] += withdrawAmount - maxCost;
context = abi.encode(maxCost, userOp.sender);
}

Expand All @@ -153,10 +153,10 @@ contract MagicSpend is Ownable, IPaymaster {

// Compute the total remaining funds available for the user accout.
// NOTE: Take into account the user operation gas that was not consummed.
uint256 withdrawable = withdrawableFunds[account] + (maxGasCost - actualGasCost);
uint256 withdrawable = _withdrawableETH[account] + (maxGasCost - actualGasCost);

// Send the all remaining funds to the user accout.
delete withdrawableFunds[account];
delete _withdrawableETH[account];
if (withdrawable > 0) {
SafeTransferLib.forceSafeTransferETH(account, withdrawable, SafeTransferLib.GAS_STIPEND_NO_STORAGE_WRITES);
}
Expand All @@ -167,11 +167,11 @@ contract MagicSpend is Ownable, IPaymaster {
/// @dev Can be called back during the `UserOperation` execution to sponsor funds for non-gas related
/// use cases (e.g., swap or mint).
function withdrawGasExcess() external {
uint256 amount = withdrawableFunds[msg.sender];
uint256 amount = _withdrawableETH[msg.sender];
// we could allow 0 value transfers, but prefer to be explicit
if (amount == 0) revert NoExcess();

delete withdrawableFunds[msg.sender];
delete _withdrawableETH[msg.sender];
_withdraw(address(0), msg.sender, amount);
}

Expand Down
22 changes: 0 additions & 22 deletions test/ValidatePaymasterUserOp.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,28 +40,6 @@ contract ValidatePaymasterUserOpTest is PaymasterMagicSpendBaseTest, ValidateTes
assertEq(uint160(validationData), 1);
}

// function test_setsExcess(uint256 amount_, uint256 maxCost_, uint256 actualCost) public {
// vm.assume(maxCost_ <= amount_);
// vm.assume(actualCost <= maxCost_);
// amount = amount_;
// magic.validatePaymasterUserOp(_getUserOp(), userOpHash, maxCost_);
// uint256 expected = amount - maxCost_;
// assertEq(magic.fundsExcessBalance(withdrawer), expected);
// }

// function test_doesNotOverwriteExcess(uint256 amount_, uint256 maxCost_, uint256 actualCost) public {
// maxCost_ = bound(maxCost_, 0, amount_);
// actualCost = bound(actualCost, 0, maxCost_);
// uint256 expected = amount_ - maxCost_;
// vm.assume(expected < type(uint256).max / 2);
// amount = amount_;
// magic.validatePaymasterUserOp(_getUserOp(), userOpHash, maxCost_);
// assertEq(magic.fundsExcessBalance(withdrawer), expected);
// nonce += 1;
// magic.validatePaymasterUserOp(_getUserOp(), userOpHash, maxCost_);
// assertEq(magic.fundsExcessBalance(withdrawer), expected * 2);
// }

function test_emitsCorrectly(address, uint256 amount_, uint256 nonce_) public override {
maxCost = amount_;
super.test_emitsCorrectly(magic.entryPoint(), amount_, nonce_);
Expand Down

0 comments on commit b71735a

Please sign in to comment.