diff --git a/.gas-snapshot b/.gas-snapshot index 1e4e94f..267a42d 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,9 +1,40 @@ -GasbackTest:testConvertGasback() (gas: 73039) -GasbackTest:testConvertGasback(uint256,uint256) (runs: 256, μ: 434549, ~: 294047) -GasbackTest:testConvertGasbackBaseFeeVault() (gas: 27070) -GasbackTest:testConvertGasbackMaxBaseFee() (gas: 44525) -GasbackTest:testConvertGasbackMinVaultBalance() (gas: 26953) -GasbackTest:testConvertGasbackWithAccruedToAccruedRecipient() (gas: 69305) -GasbackTest:test__codesize() (gas: 9846) +FeeVaultSplitterTest:testFuzz_balances_after_multiple_payments(uint8,uint256[9]) (runs: 256, μ: 4605383, ~: 2803798) +FeeVaultSplitterTest:testFuzz_balances_after_payment(uint8,uint256) (runs: 256, μ: 2752114, ~: 1760765) +FeeVaultSplitterTest:test__codesize() (gas: 30291) +FeeVaultSplitterTest:test_balances_after_payment() (gas: 261991) +FeeVaultSplitterTest:test_distribute_clamps_end_to_payees_length() (gas: 245124) +FeeVaultSplitterTest:test_distribute_invariants_with_failed_payee() (gas: 1148434) +FeeVaultSplitterTest:test_distribute_noop_start_gte_end() (gas: 25396) +FeeVaultSplitterTest:test_externalPayees_length_matches_payee_count() (gas: 1034697) +FeeVaultSplitterTest:test_failed_payee_accounting_invariants() (gas: 1156750) +FeeVaultSplitterTest:test_multiple_payments_accounting_is_cumulative() (gas: 308742) +FeeVaultSplitterTest:test_read_public_variables() (gas: 59354) +FeeVaultSplitterTest:test_receive_allows_small_payment() (gas: 64848) +FeeVaultSplitterTest:test_receive_emits_payment_received() (gas: 1251992) +FeeVaultSplitterTest:test_receive_reverts_on_reentrant_payee() (gas: 1285807) +FeeVaultSplitterTest:test_receive_skips_failed_payee_emits_failure() (gas: 1152566) +FeeVaultSplitterTest:test_release_after_dust_payment() (gas: 131877) +FeeVaultSplitterTest:test_revert_deploy_duplicate_payee() (gas: 181846) +FeeVaultSplitterTest:test_revert_deploy_empty_payees() (gas: 39046) +FeeVaultSplitterTest:test_revert_deploy_length_mismatch_more_payees() (gas: 46640) +FeeVaultSplitterTest:test_revert_deploy_length_mismatch_more_shares() (gas: 43716) +FeeVaultSplitterTest:test_revert_deploy_zero_address_payee() (gas: 131946) +FeeVaultSplitterTest:test_revert_deploy_zero_shares() (gas: 134065) +FeeVaultSplitterTest:test_revert_release_account_has_no_shares() (gas: 11672) +FeeVaultSplitterTest:test_revert_release_account_not_due_payment() (gas: 20633) +FeeVaultSplitterTest:test_revert_release_failed_to_send_value() (gas: 1043983) +FeeVaultSplitterTest:test_revert_release_insufficient_balance() (gas: 36608) +GasbackTest:testConvertGasback() (gas: 56997) +GasbackTest:testConvertGasback(uint256,uint256) (runs: 256, μ: 414710, ~: 262761) +GasbackTest:testConvertGasbackBaseFeeVault() (gas: 29386) +GasbackTest:testConvertGasbackMaxBaseFee() (gas: 24715) +GasbackTest:testSetBaseFeeVaultShareNumeratorRevertsWhenValueAboveDenominator() (gas: 13001) +GasbackTest:testSetBaseFeeVaultShareNumeratorRevertsWhenValueBelowGasbackRatio() (gas: 13602) +GasbackTest:testSetGasbackRatioNumeratorRevertsWhenValueAboveBaseFeeVaultShare() (gas: 13418) +GasbackTest:testSetGasbackRatioNumeratorRevertsWhenValueAboveDenominator() (gas: 9778) +GasbackTest:testWithdrawAccruedRevertsWhenCallerUnauthorized() (gas: 84792) +GasbackTest:testWithdrawLeavesAccruedWhenBufferCovers() (gas: 117411) +GasbackTest:testWithdrawReconcilesAccruedDownToBalance() (gas: 117693) +GasbackTest:test__codesize() (gas: 14273) SoladyTest:test__codesize() (gas: 4099) TestPlus:test__codesize() (gas: 393) \ No newline at end of file diff --git a/.gitignore b/.gitignore index 198f4cb..1634c36 100644 --- a/.gitignore +++ b/.gitignore @@ -40,4 +40,6 @@ wake-coverage.cov create2 # Coverage report -report \ No newline at end of file +report + +dependencies/ \ No newline at end of file diff --git a/foundry.toml b/foundry.toml index 06c721a..8087f33 100644 --- a/foundry.toml +++ b/foundry.toml @@ -4,7 +4,7 @@ # The Default Profile [profile.default] -solc_version = "0.8.30" +solc_version = "0.8.34" evm_version = "prague" auto_detect_solc = false optimizer = true @@ -15,7 +15,8 @@ always_use_create_2_factory = true remappings = [ "murky=lib/murky", "dn404/=lib/dn404/src", - "solady=lib/solady/src" + "solady=lib/solady/src", + "@openzeppelin/contracts/=dependencies/@openzeppelin-contracts-4.9.5/" ] [fmt] diff --git a/soldeer.lock b/soldeer.lock new file mode 100644 index 0000000..98ae23f --- /dev/null +++ b/soldeer.lock @@ -0,0 +1,6 @@ +[[dependencies]] +name = "@openzeppelin-contracts" +version = "4.9.5" +url = "https://soldeer-revisions.s3.amazonaws.com/@openzeppelin-contracts/4_9_5_22-01-2024_13:13:56_contracts.zip" +checksum = "23d102f257e57f95e4a4a55f981f8f7781f3a68c36fa77e80640812480334b27" +integrity = "27c0919f5274f868b39a294a81d73dd061ef2518d08148a454bc16095088380e" diff --git a/soldeer.toml b/soldeer.toml new file mode 100644 index 0000000..da77d45 --- /dev/null +++ b/soldeer.toml @@ -0,0 +1,2 @@ +[dependencies] +"@openzeppelin-contracts" = "4.9.5" diff --git a/src/FeeVaultSplitter.sol b/src/FeeVaultSplitter.sol new file mode 100644 index 0000000..c825c58 --- /dev/null +++ b/src/FeeVaultSplitter.sol @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.34; + +import {PaymentSplitter} from "@openzeppelin/contracts/finance/PaymentSplitter.sol"; +import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol"; + +/** + * @title FeeVaultSplitter + * @dev This contract, implements OpenZeppelin's PaymentSplitter, supports splitting Ether payments among a group of accounts. + * + * FeeVaultSplitter follows a _push payment_ model. Incoming Ether triggers an attempt to release funds to all payees. + */ +contract FeeVaultSplitter is PaymentSplitter, ReentrancyGuard { + event PaymentFailed(address to, uint256 amount, bytes reason); + + address[] public externalPayees; + + /** + * @dev Creates an instance of `PaymentSplitter` where each account in `payees` is assigned the number of shares at + * the matching position in the `shares` array. + * + * All addresses in `payees` must be non-zero. Both arrays must have the same non-zero length, and there must be no + * duplicates in `payees`. + */ + constructor(address[] memory payees_, uint256[] memory shares_) + payable + PaymentSplitter(payees_, shares_) + { + for (uint256 i = 0; i < payees_.length; i++) { + externalPayees.push(payees_[i]); + } + } + + /** + * @dev The Ether received will be logged with {PaymentReceived} events. Note that these events are not fully + * reliable: it's possible for a contract to receive Ether without triggering this function. This only affects the + * reliability of the events, and not the actual splitting of Ether. + * + * To learn more about this see the Solidity documentation for + * https://solidity.readthedocs.io/en/latest/contracts.html#fallback-function[fallback + * functions]. + * + * SECURITY / DoS NOTE (push-payment model): this function attempts to release to every payee in + * `externalPayees` in a single call. Its gas cost therefore scales with the payee count, and a payee whose + * `receive`/fallback consumes a large amount of gas (rather than cheaply reverting, which is caught and skipped) + * can push this call out of gas and make it revert. Because the OP base fee vault's `withdraw()` sends fees to + * this contract (triggering `receive`), such a revert would block that withdrawal and strand base fees in the + * vault until resolved. To bound this risk: keep the payee set small and trusted (it is fixed at deployment). + * If a deposit's auto-distribution is ever blocked, funds are not lost — anyone can call {distribute} with a + * bounded `[start, end)` slice to release payees in chunks and recover. + */ + receive() external payable override(PaymentSplitter) nonReentrant { + emit PaymentReceived(_msgSender(), msg.value); + + _distribute(0, externalPayees.length); + } + + /** + * @dev Attempts to release payments for a slice of payees, skipping zero-due payees and emitting failures instead of + * reverting on send failures. + */ + function distribute(uint256 start, uint256 end) public { + _distribute(start, end); + } + + /** + * @dev Attempt to pay a slice of payees without reverting the whole call. + * Skips zero-due accounts and emits failures for accounts that revert on receive. + */ + function _distribute(uint256 start, uint256 end) private { + uint256 payeesLength = externalPayees.length; + if (end > payeesLength) { + end = payeesLength; + } + if (start >= end) { + return; + } + + for (uint256 i = start; i < end; i++) { + address payable account = payable(externalPayees[i]); + uint256 payment = releasable(account); + if (payment == 0) { + continue; + } + + try this.release(account) {} + catch (bytes memory reason) { + emit PaymentFailed(account, payment, reason); + } + } + } +} diff --git a/src/Gasback.sol b/src/Gasback.sol index 504438e..0a8cd33 100644 --- a/src/Gasback.sol +++ b/src/Gasback.sol @@ -30,14 +30,12 @@ contract Gasback { // recipient of the base fee vault, it can be configured to auto-pull // funds from the base fee vault when it runs out of ETH. address baseFeeVault; - // The minimum balance of the base fee vault. - uint256 minVaultBalance; - // The amount of ETH accrued by taking a cut from the gas burned. + // The amount of ETH accrued by taking a cut from the gas burned (after the base fee vault share has been taken). uint256 accrued; - // The recipient of the accrued ETH. - address accruedRecipient; // A mapping of addresses authorized to withdraw the accrued ETH. - mapping(address => bool) accuralWithdrawers; + mapping(address => bool) accrualWithdrawers; + // The numerator for the share of the base fee vault. + uint256 baseFeeVaultShareNumerator; } /// @dev Returns a pointer to the storage struct. @@ -56,11 +54,10 @@ contract Gasback { constructor() payable { GasbackStorage storage $ = _getGasbackStorage(); - $.gasbackRatioNumerator = 0.8 ether; + $.gasbackRatioNumerator = 0.6 ether; $.gasbackMaxBaseFee = type(uint256).max; $.baseFeeVault = 0x4200000000000000000000000000000000000019; - $.minVaultBalance = 0.42 ether; - $.accruedRecipient = 0x4200000000000000000000000000000000000019; + $.baseFeeVaultShareNumerator = 0.6 ether; } /*«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-*/ @@ -82,13 +79,13 @@ contract Gasback { return _getGasbackStorage().baseFeeVault; } - /// @dev The minimum balance of the base fee vault that allows a pull withdrawal. - function minVaultBalance() public view virtual returns (uint256) { - return _getGasbackStorage().minVaultBalance; + /// @dev The numerator for the share of the base fee vault. + function baseFeeVaultShareNumerator() public view virtual returns (uint256) { + return _getGasbackStorage().baseFeeVaultShareNumerator; } /*«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-«-*/ - /* ACCURAL FUNCTIONS */ + /* ACCRUAL FUNCTIONS */ /*-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»*/ /// @dev Returns the amount of ETH accrued. @@ -98,7 +95,7 @@ contract Gasback { /// @dev Withdraws from the accrued amount. function withdrawAccrued(address to, uint256 amount) public virtual returns (bool) { - require(_getGasbackStorage().accuralWithdrawers[msg.sender]); + require(_getGasbackStorage().accrualWithdrawers[msg.sender]); // Checked math prevents underflow. _getGasbackStorage().accrued -= amount; /// @solidity memory-safe-assembly @@ -109,38 +106,17 @@ contract Gasback { } /// @dev Returns whether `addr` is authorized to call `withdrawAccrued`. - function isAuthorizedAccuralWithdrawer(address addr) public view virtual returns (bool) { - return _getGasbackStorage().accuralWithdrawers[addr]; + function isAuthorizedAccrualWithdrawer(address addr) public view virtual returns (bool) { + return _getGasbackStorage().accrualWithdrawers[addr]; } /// @dev Set whether `addr` is authorized to call `withdrawAccrued`. - function setAccuralWithdrawer(address addr, bool authorized) + function setAccrualWithdrawer(address addr, bool authorized) public onlySystemOrThis returns (bool) { - _getGasbackStorage().accuralWithdrawers[addr] = authorized; - return true; - } - - /// @dev Withdraws from the accrued amount to the accrued recipient. - function withdrawAccruedToAccruedRecipient(uint256 amount) public virtual returns (bool) { - // Checked math prevents underflow. - _getGasbackStorage().accrued -= amount; - - address accruedRecipient = _getGasbackStorage().accruedRecipient; - /// @solidity memory-safe-assembly - assembly { - if iszero(call(gas(), accruedRecipient, amount, 0x00, 0x00, 0x00, 0x00)) { - revert(0x00, 0x00) - } - } - return true; - } - - /// @dev Sets the accrued recipient. - function setAccruedRecipient(address value) public onlySystemOrThis returns (bool) { - _getGasbackStorage().accruedRecipient = value; + _getGasbackStorage().accrualWithdrawers[addr] = authorized; return true; } @@ -149,18 +125,25 @@ contract Gasback { /*-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»-»*/ /// @dev Withdraws ETH from this contract. + /// @dev Reconciles the accrued ledger so it never overstates the ETH backing it: if this + /// withdrawal leaves the balance below `accrued`, `accrued` is lowered to the remaining balance. function withdraw(address to, uint256 amount) public onlySystemOrThis returns (bool) { /// @solidity memory-safe-assembly assembly { if iszero(call(gas(), to, amount, 0x00, 0x00, 0x00, 0x00)) { revert(0x00, 0x00) } } + GasbackStorage storage $ = _getGasbackStorage(); + uint256 balanceAfter = address(this).balance; + if ($.accrued > balanceAfter) $.accrued = balanceAfter; return true; } /// @dev Sets the numerator for the gasback ratio. function setGasbackRatioNumerator(uint256 value) public onlySystemOrThis returns (bool) { + GasbackStorage storage $ = _getGasbackStorage(); require(value <= GASBACK_RATIO_DENOMINATOR); - _getGasbackStorage().gasbackRatioNumerator = value; + require(value <= $.baseFeeVaultShareNumerator); + $.gasbackRatioNumerator = value; return true; } @@ -176,12 +159,59 @@ contract Gasback { return true; } - /// @dev Sets the minimum balance of the base fee vault. - function setMinVaultBalance(uint256 value) public onlySystemOrThis returns (bool) { - _getGasbackStorage().minVaultBalance = value; + /// @dev Sets the numerator for the share of the base fee vault. + /// @dev When the base fee vault's recipient is a splitter exposing `shares`/`totalShares`, + /// `value` must equal this contract's live share `(shares(this) * GASBACK_RATIO_DENOMINATOR) / + /// totalShares`, so the fallback's `expectedShare` estimate stays consistent with the ETH the + /// splitter actually routes back. The check is skipped when no such splitter is detectable + /// (unset/codeless vault, no `recipient()`, a non-splitter recipient, or the EIP-7702 setup + /// where the recipient is this contract). Re-point the vault before calling this, and re-call + /// this after changing the vault, to keep the numerator consistent. + function setBaseFeeVaultShareNumerator(uint256 value) public onlySystemOrThis returns (bool) { + GasbackStorage storage $ = _getGasbackStorage(); + require(value <= GASBACK_RATIO_DENOMINATOR); + require(value >= $.gasbackRatioNumerator); + (bool applicable, uint256 expected) = _expectedBaseFeeVaultShareNumerator(); + require(!applicable || value == expected); + $.baseFeeVaultShareNumerator = value; return true; } + /// @dev Returns this contract's expected base fee vault share numerator derived from live + /// on-chain state: `(splitter.shares(this) * GASBACK_RATIO_DENOMINATOR) / splitter.totalShares()`, + /// where the splitter is the base fee vault's `recipient()`. `applicable` is false (and `expected` + /// must be ignored) when there is no readable splitter topology, namely: the base fee vault is + /// unset or has no code; it has no readable `recipient()`; the recipient is this contract (the + /// EIP-7702 setup) or has no code; or `totalShares()`/`shares(address)` is unreadable or zero. + /// All external reads are guarded `staticcall`s so an absent/incompatible recipient never reverts + /// this path; it simply makes the consistency check inapplicable. + function _expectedBaseFeeVaultShareNumerator() + internal + view + returns (bool applicable, uint256 expected) + { + address vault = _getGasbackStorage().baseFeeVault; + if (vault == address(0) || vault.code.length == 0) return (false, 0); + + (bool ok, bytes memory data) = vault.staticcall(abi.encodeWithSignature("recipient()")); + if (!ok || data.length != 32) return (false, 0); + address splitter = abi.decode(data, (address)); + if (splitter == address(this) || splitter == address(0) || splitter.code.length == 0) { + return (false, 0); + } + + (ok, data) = splitter.staticcall(abi.encodeWithSignature("totalShares()")); + if (!ok || data.length != 32) return (false, 0); + uint256 totalShares = abi.decode(data, (uint256)); + if (totalShares == 0) return (false, 0); + + (ok, data) = splitter.staticcall(abi.encodeWithSignature("shares(address)", address(this))); + if (!ok || data.length != 32) return (false, 0); + uint256 gasbackShares = abi.decode(data, (uint256)); + + return (true, (gasbackShares * GASBACK_RATIO_DENOMINATOR) / totalShares); + } + /// @dev A noop function. function noop() public payable returns (bool) { return true; @@ -216,23 +246,23 @@ contract Gasback { GasbackStorage storage $ = _getGasbackStorage(); uint256 ethFromGas = gasToBurn * block.basefee; + uint256 ethFromVaultShare = + (ethFromGas * $.baseFeeVaultShareNumerator) / GASBACK_RATIO_DENOMINATOR; uint256 ethToGive = (ethFromGas * $.gasbackRatioNumerator) / GASBACK_RATIO_DENOMINATOR; uint256 selfBalance = address(this).balance; // If the contract has insufficient ETH, try to pull from the base fee vault. - if (ethToGive > selfBalance) { + if (ethToGive > selfBalance && block.basefee <= $.gasbackMaxBaseFee) { address vault = $.baseFeeVault; - uint256 minBalance = $.minVaultBalance; + uint256 shortfall = ethToGive - selfBalance; + uint256 vaultBalance = vault.balance; + uint256 expectedShare = + (vaultBalance * $.baseFeeVaultShareNumerator) / GASBACK_RATIO_DENOMINATOR; /// @solidity memory-safe-assembly assembly { - if extcodesize(vault) { - // If the vault has sufficient ETH, pull from it. - if gt(balance(vault), add(sub(ethToGive, selfBalance), minBalance)) { - mstore(0x00, 0x3ccfd60b) // `withdraw()`. - pop(call(gas(), vault, 0, 0x1c, 0x04, 0x00, 0x00)) - // Return ETH to vault to ensure that it has `minBalance`. - pop(call(gas(), vault, minBalance, 0x00, 0x00, 0x00, 0x00)) - } + if and(extcodesize(vault), iszero(lt(expectedShare, shortfall))) { + mstore(0x00, 0x3ccfd60b) // `withdraw()`. + pop(call(gas(), vault, 0, 0x1c, 0x04, 0x00, 0x00)) } } } @@ -244,8 +274,10 @@ contract Gasback { gasToBurn = 0; } - unchecked { - $.accrued += ethFromGas - ethToGive; + if (gasToBurn != 0) { + unchecked { + $.accrued += ethFromVaultShare - ethToGive; + } } /// @solidity memory-safe-assembly diff --git a/test/FeeVaultSplitter.t.sol b/test/FeeVaultSplitter.t.sol new file mode 100644 index 0000000..31fb644 --- /dev/null +++ b/test/FeeVaultSplitter.t.sol @@ -0,0 +1,585 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.4; + +import "./utils/SoladyTest.sol"; +import {FeeVaultSplitter} from "../src/FeeVaultSplitter.sol"; + +contract RejectingPayee { + receive() external payable { + revert("I reject ETH"); + } +} + +contract ReentrantPayee { + FeeVaultSplitter public splitter; + bool public didReenter; + + function setSplitter(FeeVaultSplitter splitter_) external { + splitter = splitter_; + } + + receive() external payable { + if (!didReenter) { + didReenter = true; + (bool success,) = address(splitter).call{value: 1}(""); + require(success, "reenter failed"); + } + } +} + +contract FeeVaultSplitterTest is SoladyTest { + event PaymentFailed(address to, uint256 amount, bytes reason); + event PaymentReceived(address from, uint256 amount); + event PaymentReleased(address to, uint256 amount); + + FeeVaultSplitter public splitter; + + /// @dev fuzz helpers + + // Struct to reduce stack depth in fuzz tests + struct FuzzTestState { + address[] fuzzPayees; + uint256[] fuzzShares; + uint256[] initialBalances; + uint256 totalSharesSum; + uint256 cumulativeTotalPaid; + FeeVaultSplitter fuzzSplitter; + } + + function _deployDefaultSplitter() internal returns (FeeVaultSplitter) { + return new FeeVaultSplitter(payees, shares); + } + + function _addressSendValueRevertReason() internal pure returns (bytes memory) { + return abi.encodeWithSignature( + "Error(string)", "Address: unable to send value, recipient may have reverted" + ); + } + + function _createFuzzTestState(uint8 numPayees, uint256 addrOffset) + internal + returns (FuzzTestState memory state) + { + state.fuzzPayees = new address[](numPayees); + state.fuzzShares = new uint256[](numPayees); + state.initialBalances = new uint256[](numPayees); + + for (uint256 i = 0; i < numPayees; i++) { + state.fuzzPayees[i] = vm.addr(i + addrOffset); + state.fuzzShares[i] = (i % 100) + 1; + state.totalSharesSum += state.fuzzShares[i]; + } + + state.fuzzSplitter = new FeeVaultSplitter(state.fuzzPayees, state.fuzzShares); + + for (uint256 i = 0; i < numPayees; i++) { + state.initialBalances[i] = state.fuzzPayees[i].balance; + } + } + + function _sendPaymentAndUpdateState(FuzzTestState memory state, uint256 paymentAmount) + internal + { + state.cumulativeTotalPaid += paymentAmount; + vm.deal(address(this), paymentAmount); + (bool success,) = address(state.fuzzSplitter).call{value: paymentAmount}(""); + assertTrue(success); + } + + function _verifyPayeeBalances(FuzzTestState memory state, uint8 numPayees) internal view { + for (uint256 i = 0; i < numPayees; i++) { + uint256 actualReceived = state.fuzzPayees[i].balance - state.initialBalances[i]; + uint256 expectedReceived = + (state.cumulativeTotalPaid * state.fuzzShares[i]) / state.totalSharesSum; + assertEq(actualReceived, expectedReceived); + } + } + + address[] public payees = new address[](3); + uint256[] public shares = new uint256[](3); + + uint256 private _deployerKey = 1; + + uint256 private _payee1Key = 2; + uint256 private _payee2Key = 3; + uint256 private _payee3Key = 4; + + address private deployer = vm.addr(_deployerKey); + + address private payee1 = vm.addr(_payee1Key); + address private payee2 = vm.addr(_payee2Key); + address private payee3 = vm.addr(_payee3Key); + + uint256 public shares1 = 48; + uint256 public shares2 = 42; + uint256 public shares3 = 10; + + function setUp() public { + payees[0] = payee1; + payees[1] = payee2; + payees[2] = payee3; + + shares[0] = shares1; + shares[1] = shares2; + shares[2] = shares3; + + splitter = new FeeVaultSplitter(payees, shares); + } + + function test_read_public_variables() public { + assertEq(splitter.externalPayees(0), payee1); + assertEq(splitter.externalPayees(1), payee2); + assertEq(splitter.externalPayees(2), payee3); + assertEq(splitter.totalShares(), 100); + assertEq(splitter.shares(payee1), shares1); + assertEq(splitter.shares(payee2), shares2); + assertEq(splitter.shares(payee3), shares3); + assertEq(splitter.payee(0), payee1); + assertEq(splitter.payee(1), payee2); + assertEq(splitter.payee(2), payee3); + } + + function test_externalPayees_length_matches_payee_count() public { + splitter = _deployDefaultSplitter(); + + // Regression guard for the constructor double-fill bug: externalPayees must have exactly + // one entry per payee (length N), not 2N with leading zero-address slots. + assertEq(splitter.externalPayees(0), payee1); + assertEq(splitter.externalPayees(1), payee2); + assertEq(splitter.externalPayees(2), payee3); + vm.expectRevert(); // index 3 is out of bounds => length is exactly 3 + splitter.externalPayees(3); + } + + function test_balances_after_payment() public { + uint256 paymentAmount = 10 ether; + + // Record balances before + uint256 balanceBefore1 = payee1.balance; + uint256 balanceBefore2 = payee2.balance; + uint256 balanceBefore3 = payee3.balance; + + // Send ETH to the splitter (triggers receive() which releases to all payees) + vm.deal(address(this), paymentAmount); + (bool success,) = address(splitter).call{value: paymentAmount}(""); + assertTrue(success, "Payment to splitter failed"); + + // Record balances after + uint256 balanceAfter1 = payee1.balance; + uint256 balanceAfter2 = payee2.balance; + uint256 balanceAfter3 = payee3.balance; + + // Calculate expected amounts based on shares + uint256 totalShares = splitter.totalShares(); + uint256 expectedPayment1 = (paymentAmount * shares1) / totalShares; + uint256 expectedPayment2 = (paymentAmount * shares2) / totalShares; + uint256 expectedPayment3 = (paymentAmount * shares3) / totalShares; + + // Verify balance changes match expected payments + assertEq( + balanceAfter1 - balanceBefore1, expectedPayment1, "Payee1 received incorrect amount" + ); + assertEq( + balanceAfter2 - balanceBefore2, expectedPayment2, "Payee2 received incorrect amount" + ); + assertEq( + balanceAfter3 - balanceBefore3, expectedPayment3, "Payee3 received incorrect amount" + ); + + // Verify the exact amounts (48%, 42%, 10% of 10 ether) + assertEq(balanceAfter1 - balanceBefore1, 4.8 ether, "Payee1 should receive 4.8 ether"); + assertEq(balanceAfter2 - balanceBefore2, 4.2 ether, "Payee2 should receive 4.2 ether"); + assertEq(balanceAfter3 - balanceBefore3, 1 ether, "Payee3 should receive 1 ether"); + assertEq(address(splitter).balance, 0); + } + + function test_receive_emits_payment_received() public { + splitter = _deployDefaultSplitter(); + + uint256 paymentAmount = 10 ether; + + vm.deal(address(this), paymentAmount); + // receive() emits PaymentReceived first, then PaymentReleased per payee during _distribute. + vm.expectEmit(true, true, true, true, address(splitter)); + emit PaymentReceived(address(this), paymentAmount); + vm.expectEmit(true, true, true, true, address(splitter)); + emit PaymentReleased(payee1, 4.8 ether); + vm.expectEmit(true, true, true, true, address(splitter)); + emit PaymentReleased(payee2, 4.2 ether); + vm.expectEmit(true, true, true, true, address(splitter)); + emit PaymentReleased(payee3, 1 ether); + + (bool success,) = address(splitter).call{value: paymentAmount}(""); + assertTrue(success, "Payment to splitter failed"); + } + + function test_receive_allows_small_payment() public { + uint256 paymentAmount = 1 wei; + + uint256 balanceBefore1 = payee1.balance; + uint256 balanceBefore2 = payee2.balance; + uint256 balanceBefore3 = payee3.balance; + + vm.deal(address(this), paymentAmount); + (bool success,) = address(splitter).call{value: paymentAmount}(""); + assertTrue(success, "Payment to splitter failed"); + + assertEq(payee1.balance, balanceBefore1); + assertEq(payee2.balance, balanceBefore2); + assertEq(payee3.balance, balanceBefore3); + assertEq(address(splitter).balance, paymentAmount); + } + + function test_receive_skips_failed_payee_emits_failure() public { + RejectingPayee rejecter = new RejectingPayee(); + + address[] memory localPayees = new address[](2); + localPayees[0] = address(rejecter); + localPayees[1] = payee1; + + uint256[] memory localShares = new uint256[](2); + localShares[0] = 50; + localShares[1] = 50; + + FeeVaultSplitter localSplitter = new FeeVaultSplitter(localPayees, localShares); + + uint256 paymentAmount = 1 ether; + uint256 payee1Before = payee1.balance; + + vm.deal(address(this), paymentAmount); + vm.expectEmit(true, true, true, true); + emit PaymentFailed(address(rejecter), 0.5 ether, _addressSendValueRevertReason()); + + (bool success,) = address(localSplitter).call{value: paymentAmount}(""); + assertTrue(success, "Payment to splitter failed"); + + assertEq(payee1.balance - payee1Before, 0.5 ether); + assertEq(address(localSplitter).balance, 0.5 ether); + } + + function test_distribute_noop_start_gte_end() public { + uint256 paymentAmount = 1 ether; + + uint256 balanceBefore1 = payee1.balance; + uint256 balanceBefore2 = payee2.balance; + uint256 balanceBefore3 = payee3.balance; + + vm.deal(address(splitter), paymentAmount); + splitter.distribute(2, 2); + + assertEq(payee1.balance, balanceBefore1); + assertEq(payee2.balance, balanceBefore2); + assertEq(payee3.balance, balanceBefore3); + assertEq(address(splitter).balance, paymentAmount); + } + + function test_distribute_clamps_end_to_payees_length() public { + uint256 paymentAmount = 1 ether; + + uint256 balanceBefore1 = payee1.balance; + uint256 balanceBefore2 = payee2.balance; + uint256 balanceBefore3 = payee3.balance; + + vm.deal(address(splitter), paymentAmount); + splitter.distribute(0, 10); + + uint256 totalShares = splitter.totalShares(); + uint256 expectedPayment1 = (paymentAmount * shares1) / totalShares; + uint256 expectedPayment2 = (paymentAmount * shares2) / totalShares; + uint256 expectedPayment3 = (paymentAmount * shares3) / totalShares; + + assertEq(payee1.balance - balanceBefore1, expectedPayment1); + assertEq(payee2.balance - balanceBefore2, expectedPayment2); + assertEq(payee3.balance - balanceBefore3, expectedPayment3); + assertEq(address(splitter).balance, 0); + } + + function test_distribute_invariants_with_failed_payee() public { + RejectingPayee rejecter = new RejectingPayee(); + + address[] memory localPayees = new address[](2); + localPayees[0] = address(rejecter); + localPayees[1] = payee1; + + uint256[] memory localShares = new uint256[](2); + localShares[0] = 50; + localShares[1] = 50; + + FeeVaultSplitter localSplitter = new FeeVaultSplitter(localPayees, localShares); + + uint256 paymentAmount = 1 ether; + uint256 payee1Before = payee1.balance; + + vm.deal(address(localSplitter), paymentAmount); + localSplitter.distribute(0, 2); + + assertEq(payee1.balance - payee1Before, 0.5 ether); + assertEq(localSplitter.released(payee1), 0.5 ether); + assertEq(localSplitter.released(address(rejecter)), 0); + assertEq(localSplitter.totalReleased(), 0.5 ether); + assertEq(address(localSplitter).balance, 0.5 ether); + assertEq(localSplitter.releasable(address(rejecter)), 0.5 ether); + assertEq(localSplitter.releasable(payee1), 0); + } + + function testFuzz_balances_after_payment(uint8 numPayees, uint256 paymentAmount) public { + numPayees = uint8(bound(numPayees, 1, 50)); + paymentAmount = bound(paymentAmount, 1 ether, 1000 ether); + + FuzzTestState memory state = _createFuzzTestState(numPayees, 100); + + _sendPaymentAndUpdateState(state, paymentAmount); + _verifyPayeeBalances(state, numPayees); + + assertLe(address(state.fuzzSplitter).balance, uint256(numPayees)); + } + + function testFuzz_balances_after_multiple_payments( + uint8 numPayees, + uint256[9] memory paymentAmounts + ) public { + numPayees = uint8(bound(numPayees, 1, 50)); + + FuzzTestState memory state = _createFuzzTestState(numPayees, 200); + + for (uint256 p = 0; p < 9; p++) { + uint256 paymentAmount = bound(paymentAmounts[p], 0.1 ether, 10 ether); + _sendPaymentAndUpdateState(state, paymentAmount); + _verifyPayeeBalances(state, numPayees); + } + + assertLe(address(state.fuzzSplitter).balance, uint256(numPayees) * 9); + } + + /// @dev deployment revert tests + + function test_revert_deploy_empty_payees() public { + address[] memory emptyPayees = new address[](0); + uint256[] memory emptyShares = new uint256[](0); + + vm.expectRevert("PaymentSplitter: no payees"); + new FeeVaultSplitter(emptyPayees, emptyShares); + } + + function test_revert_deploy_length_mismatch_more_payees() public { + address[] memory morePayees = new address[](3); + morePayees[0] = payee1; + morePayees[1] = payee2; + morePayees[2] = payee3; + + uint256[] memory fewerShares = new uint256[](2); + fewerShares[0] = 50; + fewerShares[1] = 50; + + vm.expectRevert("PaymentSplitter: payees and shares length mismatch"); + new FeeVaultSplitter(morePayees, fewerShares); + } + + function test_revert_deploy_length_mismatch_more_shares() public { + address[] memory fewerPayees = new address[](2); + fewerPayees[0] = payee1; + fewerPayees[1] = payee2; + + uint256[] memory moreShares = new uint256[](3); + moreShares[0] = 40; + moreShares[1] = 40; + moreShares[2] = 20; + + vm.expectRevert("PaymentSplitter: payees and shares length mismatch"); + new FeeVaultSplitter(fewerPayees, moreShares); + } + + function test_revert_deploy_zero_address_payee() public { + address[] memory badPayees = new address[](2); + badPayees[0] = payee1; + badPayees[1] = address(0); + + uint256[] memory validShares = new uint256[](2); + validShares[0] = 50; + validShares[1] = 50; + + vm.expectRevert("PaymentSplitter: account is the zero address"); + new FeeVaultSplitter(badPayees, validShares); + } + + function test_revert_deploy_zero_shares() public { + address[] memory validPayees = new address[](2); + validPayees[0] = payee1; + validPayees[1] = payee2; + + uint256[] memory badShares = new uint256[](2); + badShares[0] = 100; + badShares[1] = 0; + + vm.expectRevert("PaymentSplitter: shares are 0"); + new FeeVaultSplitter(validPayees, badShares); + } + + function test_revert_deploy_duplicate_payee() public { + address[] memory duplicatePayees = new address[](3); + duplicatePayees[0] = payee1; + duplicatePayees[1] = payee2; + duplicatePayees[2] = payee1; // duplicate + + uint256[] memory validShares = new uint256[](3); + validShares[0] = 40; + validShares[1] = 40; + validShares[2] = 20; + + vm.expectRevert("PaymentSplitter: account already has shares"); + new FeeVaultSplitter(duplicatePayees, validShares); + } + + function test_revert_release_account_has_no_shares() public { + address nonPayee = vm.addr(999); + + vm.expectRevert("PaymentSplitter: account has no shares"); + splitter.release(payable(nonPayee)); + } + + function test_revert_release_account_not_due_payment() public { + // No ETH sent to splitter, so payee1 has 0 releasable + vm.expectRevert("PaymentSplitter: account is not due payment"); + splitter.release(payable(payee1)); + } + + function test_revert_release_insufficient_balance() public { + // Manipulate storage to create an impossible state where totalReleased > 0 but balance = 0 + // _totalReleased is at storage slot 1 + vm.store(address(splitter), bytes32(uint256(1)), bytes32(uint256(100 ether))); + + // Now releasable(payee1) = (0 + 100 ether) * 48 / 100 - 0 = 48 ether + // But balance is 0, so _sendValue will revert + vm.expectRevert("Address: insufficient balance"); + splitter.release(payable(payee1)); + } + + function test_revert_release_failed_to_send_value() public { + // Create a contract that rejects ETH + RejectingPayee rejecter = new RejectingPayee(); + + address[] memory rejectorPayees = new address[](1); + rejectorPayees[0] = address(rejecter); + + uint256[] memory rejectorShares = new uint256[](1); + rejectorShares[0] = 100; + + FeeVaultSplitter rejectorSplitter = new FeeVaultSplitter(rejectorPayees, rejectorShares); + + // Send ETH to the splitter - it should emit a failure but not revert + vm.deal(address(this), 1 ether); + (bool success,) = address(rejectorSplitter).call{value: 1 ether}(""); + assertTrue(success); + + // Direct release should still revert since the payee rejects ETH + vm.expectRevert("Address: unable to send value, recipient may have reverted"); + rejectorSplitter.release(payable(address(rejecter))); + } + + function test_receive_reverts_on_reentrant_payee() public { + ReentrantPayee reentrant = new ReentrantPayee(); + + address[] memory localPayees = new address[](2); + localPayees[0] = address(reentrant); + localPayees[1] = payee1; + + uint256[] memory localShares = new uint256[](2); + localShares[0] = 1; + localShares[1] = 1; + + FeeVaultSplitter localSplitter = new FeeVaultSplitter(localPayees, localShares); + reentrant.setSplitter(localSplitter); + + uint256 paymentAmount = 1 ether; + uint256 payee1Before = payee1.balance; + + vm.deal(address(this), paymentAmount); + vm.expectEmit(true, true, true, true, address(localSplitter)); + emit PaymentFailed(address(reentrant), 0.5 ether, _addressSendValueRevertReason()); + + (bool success,) = address(localSplitter).call{value: paymentAmount}(""); + assertTrue(success, "Payment to splitter failed"); + + assertEq(address(reentrant).balance, 0); + assertEq(payee1.balance - payee1Before, 0.5 ether); + assertEq(localSplitter.released(address(reentrant)), 0); + assertEq(localSplitter.released(payee1), 0.5 ether); + assertEq(localSplitter.totalReleased(), 0.5 ether); + assertEq(address(localSplitter).balance, 0.5 ether); + assertEq(localSplitter.releasable(address(reentrant)), 0.5 ether); + } + + function test_release_after_dust_payment() public { + vm.deal(address(this), 1 wei); + (bool success,) = address(splitter).call{value: 1 wei}(""); + assertTrue(success, "Payment to splitter failed"); + + uint256 payee1Before = payee1.balance; + vm.deal(address(splitter), 3 wei); + + splitter.release(payable(payee1)); + + assertEq(payee1.balance - payee1Before, 1 wei); + assertEq(splitter.released(payee1), 1 wei); + assertEq(address(splitter).balance, 2 wei); + } + + function test_failed_payee_accounting_invariants() public { + RejectingPayee rejecter = new RejectingPayee(); + + address[] memory localPayees = new address[](2); + localPayees[0] = address(rejecter); + localPayees[1] = payee1; + + uint256[] memory localShares = new uint256[](2); + localShares[0] = 50; + localShares[1] = 50; + + FeeVaultSplitter localSplitter = new FeeVaultSplitter(localPayees, localShares); + + uint256 paymentAmount = 1 ether; + uint256 payee1Before = payee1.balance; + + vm.deal(address(this), paymentAmount); + (bool success,) = address(localSplitter).call{value: paymentAmount}(""); + assertTrue(success, "Payment to splitter failed"); + + assertEq(payee1.balance - payee1Before, 0.5 ether); + assertEq(localSplitter.released(payee1), 0.5 ether); + assertEq(localSplitter.released(address(rejecter)), 0); + assertEq(localSplitter.totalReleased(), 0.5 ether); + assertEq(address(localSplitter).balance, 0.5 ether); + assertEq(localSplitter.releasable(address(rejecter)), 0.5 ether); + assertEq(localSplitter.releasable(payee1), 0); + } + + function test_multiple_payments_accounting_is_cumulative() public { + uint256 balanceBefore1 = payee1.balance; + uint256 balanceBefore2 = payee2.balance; + uint256 balanceBefore3 = payee3.balance; + + vm.deal(address(this), 1 ether); + (bool success1,) = address(splitter).call{value: 1 ether}(""); + assertTrue(success1, "First payment to splitter failed"); + + vm.deal(address(this), 2 ether); + (bool success2,) = address(splitter).call{value: 2 ether}(""); + assertTrue(success2, "Second payment to splitter failed"); + + uint256 cumulativePayment = 3 ether; + uint256 totalShares = splitter.totalShares(); + uint256 expectedPayment1 = (cumulativePayment * shares1) / totalShares; + uint256 expectedPayment2 = (cumulativePayment * shares2) / totalShares; + uint256 expectedPayment3 = (cumulativePayment * shares3) / totalShares; + + assertEq(payee1.balance - balanceBefore1, expectedPayment1); + assertEq(payee2.balance - balanceBefore2, expectedPayment2); + assertEq(payee3.balance - balanceBefore3, expectedPayment3); + assertEq(splitter.released(payee1), expectedPayment1); + assertEq(splitter.released(payee2), expectedPayment2); + assertEq(splitter.released(payee3), expectedPayment3); + assertEq(splitter.totalReleased(), cumulativePayment); + assertEq(address(splitter).balance, 0); + } +} + diff --git a/test/Gasback.t.sol b/test/Gasback.t.sol index 1710cc2..13c1185 100644 --- a/test/Gasback.t.sol +++ b/test/Gasback.t.sol @@ -21,7 +21,11 @@ contract GasbackTest is SoladyTest { vm.prank(pranker); (bool success,) = address(gasback).call(abi.encode(gasToBurn)); assertTrue(success); - assertEq(pranker.balance, (gasToBurn * baseFee * 0.8 ether) / 1 ether); + assertEq( + pranker.balance, + (gasToBurn * baseFee * gasback.gasbackRatioNumerator()) + / gasback.GASBACK_RATIO_DENOMINATOR() + ); } function testConvertGasback() public { @@ -60,45 +64,104 @@ contract GasbackTest is SoladyTest { assertEq(pranker.balance, 0); } - function testConvertGasbackMinVaultBalance() public { + function testWithdrawAccruedRevertsWhenCallerUnauthorized() public { address system = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; - uint256 minVaultBalance = 50 ether; + // Lower the ratio below the vault share numerator so the fallback accrues a cut. vm.prank(system); - gasback.setMinVaultBalance(minVaultBalance); - - uint256 gasToBurn = 333; - - address pranker = address(111); - assertEq(pranker.balance, 0); - vm.prank(pranker); - (bool success,) = address(gasback).call(abi.encode(gasToBurn)); + gasback.setGasbackRatioNumerator(0.5 ether); + vm.fee(100); + vm.prank(address(111)); + (bool success,) = address(gasback).call(abi.encode(uint256(1000))); assertTrue(success); - assertEq(pranker.balance, 0); + uint256 accruedAmount = gasback.accrued(); + assertGt(accruedAmount, 0); + + // With `accrued` and the contract balance both covering `amount`, only the + // authorization check can fail. + address unauthorized = address(0xBAD); + assertFalse(gasback.isAuthorizedAccrualWithdrawer(unauthorized)); + vm.prank(unauthorized); + vm.expectRevert(); + gasback.withdrawAccrued(address(0xCAFE), accruedAmount); + + assertEq(gasback.accrued(), accruedAmount); } - function testConvertGasbackWithAccruedToAccruedRecipient() public { + function _accrueCut() internal returns (uint256 accruedAmount) { address system = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; + // Lower the ratio below the vault share so the fallback accrues a cut. vm.prank(system); - gasback.setAccruedRecipient(address(42)); + gasback.setGasbackRatioNumerator(0.5 ether); + vm.fee(100); + vm.prank(address(111)); + (bool ok,) = address(gasback).call(abi.encode(uint256(1000))); + assertTrue(ok); + accruedAmount = gasback.accrued(); + assertGt(accruedAmount, 0); + } - uint256 baseFee = 1 ether; - uint256 gasToBurn = 333; + function testWithdrawReconcilesAccruedDownToBalance() public { + address system = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; + uint256 accruedAmount = _accrueCut(); - address pranker = address(111); - vm.fee(baseFee); - vm.deal(pranker, 1000 ether); + // No buffer: balance exactly backs accrued. Withdrawing part must lower accrued to match. + vm.deal(address(gasback), accruedAmount); + vm.prank(system); + assertTrue(gasback.withdraw(address(0xCAFE), accruedAmount / 4)); - vm.prank(pranker); - (bool success,) = address(gasback).call(abi.encode(gasToBurn)); - assertTrue(success); + uint256 remaining = accruedAmount - accruedAmount / 4; + assertEq(gasback.accrued(), remaining); + assertEq(address(gasback).balance, remaining); + } - uint256 accrued = gasback.accrued(); + function testWithdrawLeavesAccruedWhenBufferCovers() public { + address system = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; + uint256 accruedAmount = _accrueCut(); - assertNotEq(accrued, 0); + // Buffer present: balance stays above accrued after the withdrawal, so accrued is untouched. + vm.deal(address(gasback), accruedAmount * 10); + vm.prank(system); + assertTrue(gasback.withdraw(address(0xCAFE), accruedAmount)); - vm.prank(pranker); - gasback.withdrawAccruedToAccruedRecipient(accrued); + assertEq(gasback.accrued(), accruedAmount); + assertEq(address(gasback).balance, accruedAmount * 9); + } + + function testSetGasbackRatioNumeratorRevertsWhenValueAboveDenominator() public { + address system = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; + uint256 value = gasback.GASBACK_RATIO_DENOMINATOR() + 1; + vm.prank(system); + vm.expectRevert(); + gasback.setGasbackRatioNumerator(value); + } + + function testSetGasbackRatioNumeratorRevertsWhenValueAboveBaseFeeVaultShare() public { + address system = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; + // Within the denominator, so only the vault share check fails. + uint256 value = gasback.baseFeeVaultShareNumerator() + 1; + assertTrue(value <= gasback.GASBACK_RATIO_DENOMINATOR()); + vm.prank(system); + vm.expectRevert(); + gasback.setGasbackRatioNumerator(value); + } - assertEq(address(42).balance, accrued); + function testSetBaseFeeVaultShareNumeratorRevertsWhenValueAboveDenominator() public { + address system = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; + // Above the gasback ratio numerator, so only the denominator check fails. + uint256 value = gasback.GASBACK_RATIO_DENOMINATOR() + 1; + assertTrue(value >= gasback.gasbackRatioNumerator()); + vm.prank(system); + vm.expectRevert(); + gasback.setBaseFeeVaultShareNumerator(value); + } + + function testSetBaseFeeVaultShareNumeratorRevertsWhenValueBelowGasbackRatio() public { + address system = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; + // Within the denominator, so only the gasback ratio check fails. + uint256 value = gasback.gasbackRatioNumerator() - 1; + assertTrue(value <= gasback.GASBACK_RATIO_DENOMINATOR()); + vm.prank(system); + vm.expectRevert(); + gasback.setBaseFeeVaultShareNumerator(value); } }