Rounding and billing fix

foundry.toml

@@ -4,6 +4,12 @@ out = "out"
 libs = ["lib"]
 evm_version = "cancun"
 
+[profile.ci]
+src = "src"
+out = "out"
+libs = ["lib"]
+evm_version = "cancun"
+
 [profile.deploy]
 src = "src"
 out = "out"

src/AuthCaptureEscrow.sol

@@ -41,9 +41,9 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
         uint48 authorizationExpiry;
         /// @dev Timestamp when a successful payment can no longer be refunded
         uint48 refundExpiry;
-        /// @dev Minimum fee percentage in basis points
+        /// @dev Minimum fee rate in basis points; bounds the absolute fee at capture as amount * minFeeBps / 10_000
         uint16 minFeeBps;
-        /// @dev Maximum fee percentage in basis points
+        /// @dev Maximum fee rate in basis points; bounds the absolute fee at capture as amount * maxFeeBps / 10_000
         uint16 maxFeeBps;
         /// @dev Address that receives the fee portion of payments, if 0 then operator can set at capture
         address feeReceiver;
@@ -80,7 +80,7 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
         PaymentInfo paymentInfo,
         uint256 amount,
         address tokenCollector,
-        uint16 feeBps,
+        uint256 feeAmount,
         address feeReceiver
     );
 
@@ -90,7 +90,7 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
     );
 
     /// @notice Emitted when payment is captured from escrow
-    event PaymentCaptured(bytes32 indexed paymentInfoHash, uint256 amount, uint16 feeBps, address feeReceiver);
+    event PaymentCaptured(bytes32 indexed paymentInfoHash, uint256 amount, uint256 feeAmount, address feeReceiver);
 
     /// @notice Emitted when an authorized payment is voided, returning any escrowed funds to the payer
     event PaymentVoided(bytes32 indexed paymentInfoHash, uint256 amount);
@@ -128,8 +128,8 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
     /// @notice Fee bps range invalid due to min > max
     error InvalidFeeBpsRange(uint16 minFeeBps, uint16 maxFeeBps);
 
-    /// @notice Fee bps outside of allowed range
-    error FeeBpsOutOfRange(uint16 feeBps, uint16 minFeeBps, uint16 maxFeeBps);
+    /// @notice Fee amount outside of payer-approved bounds
+    error FeeAmountOutOfRange(uint256 feeAmount, uint256 minFee, uint256 maxFee);
 
     /// @notice Fee receiver is zero address with a non-zero fee
     error ZeroFeeReceiver();
@@ -195,21 +195,21 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
     /// @param amount Amount to charge and capture
     /// @param tokenCollector Address of the token collector
     /// @param collectorData Data to pass to the token collector
-    /// @param feeBps Fee percentage to apply (must be within min/max range)
+    /// @param feeAmount Absolute fee in token units (must fall within payer-approved bounds)
     /// @param feeReceiver Address to receive fees (should match the paymentInfo.feeReceiver unless that is 0 in which case it can be any address)
     function charge(
         PaymentInfo calldata paymentInfo,
         uint256 amount,
         address tokenCollector,
         bytes calldata collectorData,
-        uint16 feeBps,
+        uint256 feeAmount,
         address feeReceiver
     ) external nonReentrant onlySender(paymentInfo.operator) validAmount(amount) {
         // Check payment info valid
         _validatePayment(paymentInfo, amount);
 
         // Check fee parameters valid
-        _validateFee(paymentInfo, feeBps, feeReceiver);
+        _validateFee(paymentInfo, amount, feeAmount, feeReceiver);
 
         // Check payment not already collected
         bytes32 paymentInfoHash = getHash(paymentInfo);
@@ -218,13 +218,13 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
         // Set payment state with refundable amount
         paymentState[paymentInfoHash] =
             PaymentState({hasCollectedPayment: true, capturableAmount: 0, refundableAmount: uint120(amount)});
-        emit PaymentCharged(paymentInfoHash, paymentInfo, amount, tokenCollector, feeBps, feeReceiver);
+        emit PaymentCharged(paymentInfoHash, paymentInfo, amount, tokenCollector, feeAmount, feeReceiver);
 
         // Transfer tokens into escrow
         _collectTokens(paymentInfo, amount, tokenCollector, collectorData, TokenCollector.CollectorType.Payment);
 
         // Transfer tokens to receiver and fee receiver
-        _distributeTokens(paymentInfo.token, paymentInfo.receiver, amount, feeBps, feeReceiver);
+        _distributeTokens(paymentInfo.token, paymentInfo.receiver, amount, feeAmount, feeReceiver);
     }
 
     /// @notice Transfers funds from payer to escrow
@@ -262,16 +262,16 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
     ///
     /// @param paymentInfo PaymentInfo struct
     /// @param amount Amount to capture
-    /// @param feeBps Fee percentage to apply (must be within min/max range)
+    /// @param feeAmount Absolute fee in token units (must fall within payer-approved bounds)
     /// @param feeReceiver Address to receive fees (should match the paymentInfo.feeReceiver unless that is 0 in which case it can be any address)
-    function capture(PaymentInfo calldata paymentInfo, uint256 amount, uint16 feeBps, address feeReceiver)
+    function capture(PaymentInfo calldata paymentInfo, uint256 amount, uint256 feeAmount, address feeReceiver)
         external
         nonReentrant
         onlySender(paymentInfo.operator)
         validAmount(amount)
     {
         // Check fee parameters valid
-        _validateFee(paymentInfo, feeBps, feeReceiver);
+        _validateFee(paymentInfo, amount, feeAmount, feeReceiver);
 
         // Check before authorization expiry
         if (block.timestamp >= paymentInfo.authorizationExpiry) {
@@ -289,10 +289,10 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
         state.capturableAmount -= uint120(amount);
         state.refundableAmount += uint120(amount);
         paymentState[paymentInfoHash] = state;
-        emit PaymentCaptured(paymentInfoHash, amount, feeBps, feeReceiver);
+        emit PaymentCaptured(paymentInfoHash, amount, feeAmount, feeReceiver);
 
         // Transfer tokens to receiver and fee receiver
-        _distributeTokens(paymentInfo.token, paymentInfo.receiver, amount, feeBps, feeReceiver);
+        _distributeTokens(paymentInfo.token, paymentInfo.receiver, amount, feeAmount, feeReceiver);
     }
 
     /// @notice Permanently voids a payment authorization
@@ -392,9 +392,7 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
     /// @return The operator's token store address
     function getTokenStore(address operator) public view returns (address) {
         return LibClone.predictDeterministicAddress({
-            implementation: tokenStoreImplementation,
-            salt: bytes32(bytes20(operator)),
-            deployer: address(this)
+            implementation: tokenStoreImplementation, salt: bytes32(bytes20(operator)), deployer: address(this)
         });
     }
 
@@ -440,8 +438,7 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
         } else if (tokenStore.code.length == 0) {
             // Call failed from undeployed TokenStore, deploy and try again
             tokenStore = LibClone.cloneDeterministic({
-                implementation: tokenStoreImplementation,
-                salt: bytes32(bytes20(operator))
+                implementation: tokenStoreImplementation, salt: bytes32(bytes20(operator))
             });
             emit TokenStoreCreated(operator, tokenStore);
             TokenStore(tokenStore).sendTokens(token, recipient, amount);
@@ -459,13 +456,11 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
     /// @param token Token to transfer
     /// @param receiver Address to receive payment
     /// @param amount Total amount to split between payment and fees
-    /// @param feeBps Fee percentage in basis points
+    /// @param feeAmount Absolute fee in token units
     /// @param feeReceiver Address to receive fees
-    function _distributeTokens(address token, address receiver, uint256 amount, uint16 feeBps, address feeReceiver)
+    function _distributeTokens(address token, address receiver, uint256 amount, uint256 feeAmount, address feeReceiver)
         internal
     {
-        uint256 feeAmount = amount * feeBps / _MAX_FEE_BPS;
-
         // Send fee portion if non-zero
         if (feeAmount > 0) _sendTokens(msg.sender, token, feeReceiver, feeAmount);
 
@@ -507,18 +502,23 @@ contract AuthCaptureEscrow is ReentrancyGuardTransient {
     /// @notice Validates attempted fee adheres to constraints set by payment info
     ///
     /// @param paymentInfo PaymentInfo struct
-    /// @param feeBps Fee percentage in basis points
+    /// @param amount Capture or charge amount used to compute fee bounds
+    /// @param feeAmount Absolute fee in token units
     /// @param feeReceiver Address to receive fees
-    function _validateFee(PaymentInfo calldata paymentInfo, uint16 feeBps, address feeReceiver) internal pure {
-        uint16 minFeeBps = paymentInfo.minFeeBps;
-        uint16 maxFeeBps = paymentInfo.maxFeeBps;
+    function _validateFee(PaymentInfo calldata paymentInfo, uint256 amount, uint256 feeAmount, address feeReceiver)
+        internal
+        pure
+    {
         address configuredFeeReceiver = paymentInfo.feeReceiver;
 
-        // Check fee bps within [min, max]
-        if (feeBps < minFeeBps || feeBps > maxFeeBps) revert FeeBpsOutOfRange(feeBps, minFeeBps, maxFeeBps);
+        uint256 minFee = amount * paymentInfo.minFeeBps / _MAX_FEE_BPS;
+        uint256 maxFee = amount * paymentInfo.maxFeeBps / _MAX_FEE_BPS;
+
+        // Check fee amount within payer-approved bounds
+        if (feeAmount < minFee || feeAmount > maxFee) revert FeeAmountOutOfRange(feeAmount, minFee, maxFee);
 
-        // Check fee recipient only zero address if zero fee bps
-        if (feeReceiver == address(0) && feeBps > 0) revert ZeroFeeReceiver();
+        // Check fee recipient only zero address if zero fee
+        if (feeReceiver == address(0) && feeAmount > 0) revert ZeroFeeReceiver();
 
         // Check fee receiver matches payment info if non-zero
         if (configuredFeeReceiver != address(0) && configuredFeeReceiver != feeReceiver) {

src/collectors/ERC3009PaymentCollector.sol

@@ -39,7 +39,8 @@ contract ERC3009PaymentCollector is TokenCollector, ERC6492SignatureHandler {
         uint256 maxAmount = paymentInfo.maxAmount;
 
         // Pull tokens into this contract
-        IERC3009(token).receiveWithAuthorization({
+        IERC3009(token)
+            .receiveWithAuthorization({
             from: payer,
             to: address(this),
             value: maxAmount,

src/collectors/Permit2PaymentCollector.sol

@@ -42,7 +42,9 @@ contract Permit2PaymentCollector is TokenCollector, ERC6492SignatureHandler {
     ) internal override {
         permit2.permitTransferFrom({
             permit: ISignatureTransfer.PermitTransferFrom({
-                permitted: ISignatureTransfer.TokenPermissions({token: paymentInfo.token, amount: paymentInfo.maxAmount}),
+                permitted: ISignatureTransfer.TokenPermissions({
+                    token: paymentInfo.token, amount: paymentInfo.maxAmount
+                }),
                 nonce: uint256(_getHashPayerAgnostic(paymentInfo)),
                 deadline: paymentInfo.preApprovalExpiry
             }),

test/base/AuthCaptureEscrowBase.sol

@@ -161,8 +161,9 @@ contract AuthCaptureEscrowBase is Test, DeployPermit2 {
         uint256 validBefore,
         bytes32 nonce
     ) internal view returns (bytes32) {
-        bytes32 structHash =
-            keccak256(abi.encode(_RECEIVE_WITH_AUTHORIZATION_TYPEHASH, from, to, value, validAfter, validBefore, nonce));
+        bytes32 structHash = keccak256(
+            abi.encode(_RECEIVE_WITH_AUTHORIZATION_TYPEHASH, from, to, value, validAfter, validBefore, nonce)
+        );
         return keccak256(abi.encodePacked("\x19\x01", IERC3009(token).DOMAIN_SEPARATOR(), structHash));
     }
 
@@ -203,4 +204,8 @@ contract AuthCaptureEscrowBase is Test, DeployPermit2 {
         paymentInfo.payer = payer;
         return hash;
     }
+
+    function _feeAmount(uint256 amount, uint16 feeBps) internal pure returns (uint256) {
+        return amount * feeBps / 10_000;
+    }
 }

test/base/AuthCaptureEscrowSmartWalletBase.sol

@@ -172,11 +172,7 @@ contract AuthCaptureEscrowSmartWalletBase is AuthCaptureEscrowBase {
         uint256 nonce = uint256(hashPortion);
 
         return MagicSpend.WithdrawRequest({
-            asset: address(0),
-            amount: 0,
-            nonce: nonce,
-            expiry: type(uint48).max,
-            signature: new bytes(0)
+            asset: address(0), amount: 0, nonce: nonce, expiry: type(uint48).max, signature: new bytes(0)
         });
     }
 

test/gas/gasBenchmark.t.sol

@@ -8,6 +8,7 @@ import {AuthCaptureEscrowBase} from "../base/AuthCaptureEscrowBase.sol";
 contract GasBenchmarkBase is AuthCaptureEscrowBase {
     uint120 internal constant BENCHMARK_AMOUNT = 100e6;
     uint16 internal constant BENCHMARK_FEE_BPS = 100; // 1%
+    uint256 internal constant BENCHMARK_FEE_AMOUNT = uint256(BENCHMARK_AMOUNT) * uint256(BENCHMARK_FEE_BPS) / 10_000;
 
     AuthCaptureEscrow.PaymentInfo internal paymentInfo;
     bytes internal signature;
@@ -22,7 +23,9 @@ contract GasBenchmarkBase is AuthCaptureEscrowBase {
         mockERC3009Token.mint(payerEOA, 1e6);
         vm.startPrank(operator);
         authCaptureEscrow.authorize(warmupInfo, 1e6, address(erc3009PaymentCollector), warmupSignature);
-        authCaptureEscrow.capture(warmupInfo, 1e6, BENCHMARK_FEE_BPS, feeReceiver); // make sure token store is deployed before subsequent tests
+        authCaptureEscrow.capture(
+            warmupInfo, 1e6, (uint256(1_000_000) * uint256(BENCHMARK_FEE_BPS)) / 10_000, feeReceiver
+        ); // make sure token store is deployed before subsequent tests
         vm.stopPrank();
 
         // Create and sign payment info
@@ -45,7 +48,12 @@ contract ChargeGasBenchmark is GasBenchmarkBase {
     function test_charge_benchmark() public {
         vm.prank(operator);
         authCaptureEscrow.charge(
-            paymentInfo, BENCHMARK_AMOUNT, address(erc3009PaymentCollector), signature, BENCHMARK_FEE_BPS, feeReceiver
+            paymentInfo,
+            BENCHMARK_AMOUNT,
+            address(erc3009PaymentCollector),
+            signature,
+            BENCHMARK_FEE_AMOUNT,
+            feeReceiver
         );
     }
 }
@@ -61,7 +69,7 @@ contract CaptureGasBenchmark is GasBenchmarkBase {
 
     function test_capture_benchmark() public {
         vm.prank(operator);
-        authCaptureEscrow.capture(paymentInfo, BENCHMARK_AMOUNT, BENCHMARK_FEE_BPS, feeReceiver);
+        authCaptureEscrow.capture(paymentInfo, BENCHMARK_AMOUNT, BENCHMARK_FEE_AMOUNT, feeReceiver);
     }
 }
 
@@ -106,7 +114,7 @@ contract RefundGasBenchmark is GasBenchmarkBase {
         vm.prank(operator);
         authCaptureEscrow.authorize(paymentInfo, BENCHMARK_AMOUNT, address(erc3009PaymentCollector), signature);
         vm.prank(operator);
-        authCaptureEscrow.capture(paymentInfo, BENCHMARK_AMOUNT, BENCHMARK_FEE_BPS, feeReceiver);
+        authCaptureEscrow.capture(paymentInfo, BENCHMARK_AMOUNT, BENCHMARK_FEE_AMOUNT, feeReceiver);
 
         // Give operator tokens for refund and approve collector
         mockERC3009Token.mint(operator, BENCHMARK_AMOUNT);

test/src/PaymentEscrow/authorize.t.sol

@@ -321,9 +321,7 @@ contract AuthorizeTest is AuthCaptureEscrowSmartWalletBase {
         vm.assume(maxAmount >= amount && amount > 0);
         mockERC3009Token.mint(address(smartWalletDeployed), amount);
         AuthCaptureEscrow.PaymentInfo memory paymentInfo = _createPaymentInfo({
-            payer: address(smartWalletDeployed),
-            maxAmount: maxAmount,
-            token: address(mockERC3009Token)
+            payer: address(smartWalletDeployed), maxAmount: maxAmount, token: address(mockERC3009Token)
         });
 
         // Create and sign the spend permission