From 74368ef53995266e23b080207a489c7c9ab9e445 Mon Sep 17 00:00:00 2001 From: Ben Sparks <52714090+BenSparksCode@users.noreply.github.com> Date: Wed, 4 Dec 2024 14:00:17 +0200 Subject: [PATCH 1/3] feat: make Atlas surcharge settable by recipient --- src/contracts/atlas/AtlETH.sol | 9 +++---- src/contracts/atlas/GasAccounting.sol | 14 +++++------ src/contracts/atlas/Storage.sol | 36 ++++++++++++++++++++------- src/contracts/interfaces/IAtlas.sol | 3 +++ test/FLOnline.t.sol | 2 +- test/GasAccounting.t.sol | 2 +- test/Storage.t.sol | 8 +++++- test/TrebleSwap.t.sol | 2 +- 8 files changed, 50 insertions(+), 26 deletions(-) diff --git a/src/contracts/atlas/AtlETH.sol b/src/contracts/atlas/AtlETH.sol index 22a40fadc..9b1b89ee7 100644 --- a/src/contracts/atlas/AtlETH.sol +++ b/src/contracts/atlas/AtlETH.sol @@ -251,9 +251,7 @@ abstract contract AtlETH is Permit69 { /// @dev This function can only be called by the current surcharge recipient. /// It transfers the accumulated surcharge amount to the surcharge recipient's address. function withdrawSurcharge() external { - if (msg.sender != S_surchargeRecipient) { - revert InvalidAccess(); - } + _onlySurchargeRecipient(); uint256 _paymentAmount = S_cumulativeSurcharge; S_cumulativeSurcharge = 0; // Clear before transfer to prevent reentrancy @@ -268,10 +266,9 @@ abstract contract AtlETH is Permit69 { /// If the caller is not the current surcharge recipient, it reverts with an `InvalidAccess` error. /// @param newRecipient The address of the new surcharge recipient. function transferSurchargeRecipient(address newRecipient) external { + _onlySurchargeRecipient(); + address _surchargeRecipient = S_surchargeRecipient; - if (msg.sender != _surchargeRecipient) { - revert InvalidAccess(); - } S_pendingSurchargeRecipient = newRecipient; emit SurchargeRecipientTransferStarted(_surchargeRecipient, newRecipient); diff --git a/src/contracts/atlas/GasAccounting.sol b/src/contracts/atlas/GasAccounting.sol index 32a5484d5..286f1cfc6 100644 --- a/src/contracts/atlas/GasAccounting.sol +++ b/src/contracts/atlas/GasAccounting.sol @@ -49,7 +49,7 @@ abstract contract GasAccounting is SafetyLocks { t_claims = _rawClaims.withSurcharge(BUNDLER_SURCHARGE_RATE); // Atlas surcharge is based on the raw claims value. - t_fees = _rawClaims.getSurcharge(ATLAS_SURCHARGE_RATE); + t_fees = _rawClaims.getSurcharge(S_atlasSurchargeRate); t_deposits = msg.value; // Explicitly set other transient vars to 0 in case multiple metacalls in single tx. @@ -288,10 +288,10 @@ abstract contract GasAccounting is SafetyLocks { if (result.bundlersFault()) { // CASE: Solver is not responsible for the failure of their operation, so we blame the bundler // and reduce the total amount refunded to the bundler - t_writeoffs += _gasUsed.withSurcharges(ATLAS_SURCHARGE_RATE, BUNDLER_SURCHARGE_RATE); + t_writeoffs += _gasUsed.withSurcharges(S_atlasSurchargeRate, BUNDLER_SURCHARGE_RATE); } else { // CASE: Solver failed, so we calculate what they owe. - uint256 _gasUsedWithSurcharges = _gasUsed.withSurcharges(ATLAS_SURCHARGE_RATE, BUNDLER_SURCHARGE_RATE); + uint256 _gasUsedWithSurcharges = _gasUsed.withSurcharges(S_atlasSurchargeRate, BUNDLER_SURCHARGE_RATE); uint256 _surchargesOnly = _gasUsedWithSurcharges - _gasUsed; // In `_assign()`, the failing solver's bonded AtlETH balance is reduced by `_gasUsedWithSurcharges`. Any @@ -309,7 +309,7 @@ abstract contract GasAccounting is SafetyLocks { } function _writeOffBidFindGasCost(uint256 gasUsed) internal { - t_writeoffs += gasUsed.withSurcharges(ATLAS_SURCHARGE_RATE, BUNDLER_SURCHARGE_RATE); + t_writeoffs += gasUsed.withSurcharges(S_atlasSurchargeRate, BUNDLER_SURCHARGE_RATE); } /// @param ctx Context struct containing relevant context information for the Atlas auction. @@ -358,7 +358,7 @@ abstract contract GasAccounting is SafetyLocks { // be offset by any gas surcharge paid by failed solvers, which would have been added to deposits or // writeoffs in _handleSolverAccounting(). As such, the winning solver does not pay for surcharge on the gas // used by other solvers. - netAtlasGasSurcharge = _fees - _gasRemainder.getSurcharge(ATLAS_SURCHARGE_RATE); + netAtlasGasSurcharge = _fees - _gasRemainder.getSurcharge(S_atlasSurchargeRate); adjustedWithdrawals += netAtlasGasSurcharge; S_cumulativeSurcharge = _surcharge + netAtlasGasSurcharge; } else { @@ -366,8 +366,8 @@ abstract contract GasAccounting is SafetyLocks { uint256 _solverSurcharge = t_solverSurcharge; if (_solverSurcharge > 0) { netAtlasGasSurcharge = _solverSurcharge.getPortionFromTotalSurcharge({ - targetSurchargeRate: ATLAS_SURCHARGE_RATE, - totalSurchargeRate: ATLAS_SURCHARGE_RATE + BUNDLER_SURCHARGE_RATE + targetSurchargeRate: S_atlasSurchargeRate, + totalSurchargeRate: S_atlasSurchargeRate + BUNDLER_SURCHARGE_RATE }); // When no winning solvers, bundler max refund is 80% of metacall gas cost. The remaining 20% can be diff --git a/src/contracts/atlas/Storage.sol b/src/contracts/atlas/Storage.sol index baca20ae2..7eb375d18 100644 --- a/src/contracts/atlas/Storage.sol +++ b/src/contracts/atlas/Storage.sol @@ -18,7 +18,6 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { address public immutable SIMULATOR; address public immutable L2_GAS_CALCULATOR; uint256 public immutable ESCROW_DURATION; - uint256 public immutable ATLAS_SURCHARGE_RATE; uint256 public immutable BUNDLER_SURCHARGE_RATE; // AtlETH public constants @@ -48,15 +47,16 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { uint256 internal S_totalSupply; uint256 internal S_bondedTotalSupply; - mapping(address => EscrowAccountBalance) internal s_balanceOf; // public balanceOf will return a uint256 - mapping(address => EscrowAccountAccessData) internal S_accessData; - mapping(bytes32 => bool) internal S_solverOpHashes; // NOTE: Only used for when allowTrustedOpHash is enabled - - // atlETH GasAccounting storage + // Surcharge-related storage + uint256 internal S_atlasSurchargeRate; // Atlas surcharge rate uint256 internal S_cumulativeSurcharge; // Cumulative gas surcharges collected address internal S_surchargeRecipient; // Fastlane surcharge recipient address internal S_pendingSurchargeRecipient; // For 2-step transfer process + mapping(address => EscrowAccountBalance) internal s_balanceOf; // public balanceOf will return a uint256 + mapping(address => EscrowAccountAccessData) internal S_accessData; + mapping(bytes32 => bool) internal S_solverOpHashes; // NOTE: Only used for when allowTrustedOpHash is enabled + constructor( uint256 escrowDuration, uint256 atlasSurchargeRate, @@ -72,17 +72,31 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { SIMULATOR = simulator; L2_GAS_CALCULATOR = l2GasCalculator; ESCROW_DURATION = escrowDuration; - ATLAS_SURCHARGE_RATE = atlasSurchargeRate; BUNDLER_SURCHARGE_RATE = bundlerSurchargeRate; - // Gas Accounting - // Initialized with msg.value to seed flash loan liquidity + S_atlasSurchargeRate = atlasSurchargeRate; S_cumulativeSurcharge = msg.value; S_surchargeRecipient = initialSurchargeRecipient; emit SurchargeRecipientTransferred(initialSurchargeRecipient); } + // ---------------------------------------------------- // + // Storage Setters // + // ---------------------------------------------------- // + + function setAtlasSurchargeRate(uint256 newSurchargeRate) external { + _onlySurchargeRecipient(); + S_atlasSurchargeRate = newSurchargeRate; + // TODO consider adding event + } + + function _onlySurchargeRecipient() internal view { + if (msg.sender != S_surchargeRecipient) { + revert InvalidAccess(); + } + } + // ---------------------------------------------------- // // Storage Getters // // ---------------------------------------------------- // @@ -130,6 +144,10 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { return S_pendingSurchargeRecipient; } + function atlasSurchargeRate() external view returns (uint256) { + return S_atlasSurchargeRate; + } + // ---------------------------------------------------- // // Transient External Getters // // ---------------------------------------------------- // diff --git a/src/contracts/interfaces/IAtlas.sol b/src/contracts/interfaces/IAtlas.sol index 578da2b36..e42162a94 100644 --- a/src/contracts/interfaces/IAtlas.sol +++ b/src/contracts/interfaces/IAtlas.sol @@ -45,9 +45,11 @@ interface IAtlas { function depositAndBond(uint256 amountToBond) external payable; function unbond(uint256 amount) external; function redeem(uint256 amount) external; + function withdrawSurcharge() external; function transferSurchargeRecipient(address newRecipient) external; function becomeSurchargeRecipient() external; + function setAtlasSurchargeRate(uint256 rate) external; // Permit69.sol function transferUserERC20( @@ -101,4 +103,5 @@ interface IAtlas { function cumulativeSurcharge() external view returns (uint256); function surchargeRecipient() external view returns (address); function pendingSurchargeRecipient() external view returns (address); + function atlasSurchargeRate() external view returns (uint256); } diff --git a/test/FLOnline.t.sol b/test/FLOnline.t.sol index f96a8ba94..67822c495 100644 --- a/test/FLOnline.t.sol +++ b/test/FLOnline.t.sol @@ -1302,7 +1302,7 @@ contract FastLaneOnlineTest is BaseTest { // Calculate estimated Atlas gas surcharge taken from call above txGasUsed = estAtlasGasSurcharge - gasleft(); - estAtlasGasSurcharge = txGasUsed * defaultGasPrice * atlas.ATLAS_SURCHARGE_RATE() / atlas.SCALE(); + estAtlasGasSurcharge = txGasUsed * defaultGasPrice * atlas.atlasSurchargeRate() / atlas.SCALE(); assertTrue( result == swapCallShouldSucceed, diff --git a/test/GasAccounting.t.sol b/test/GasAccounting.t.sol index 8963862b3..82a100277 100644 --- a/test/GasAccounting.t.sol +++ b/test/GasAccounting.t.sol @@ -281,7 +281,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { uint256 rawClaims = (_gasMarker + mockGasAccounting.FIXED_GAS_OFFSET()) * tx.gasprice; claims = rawClaims * ( - mockGasAccounting.SCALE() + mockGasAccounting.ATLAS_SURCHARGE_RATE() + mockGasAccounting.SCALE() + mockGasAccounting.atlasSurchargeRate() + mockGasAccounting.BUNDLER_SURCHARGE_RATE() ) / mockGasAccounting.SCALE(); } diff --git a/test/Storage.t.sol b/test/Storage.t.sol index 366afa7f3..f9aac109d 100644 --- a/test/Storage.t.sol +++ b/test/Storage.t.sol @@ -29,7 +29,6 @@ contract StorageTest is BaseTest { assertEq(atlas.symbol(), "atlETH", "symbol set incorrectly"); assertEq(atlas.decimals(), 18, "decimals set incorrectly"); - assertEq(atlas.ATLAS_SURCHARGE_RATE(), DEFAULT_ATLAS_SURCHARGE_RATE, "ATLAS_SURCHARGE_RATE set incorrectly"); assertEq( atlas.BUNDLER_SURCHARGE_RATE(), DEFAULT_BUNDLER_SURCHARGE_RATE, "BUNDLER_SURCHARGE_RATE set incorrectly" ); @@ -143,6 +142,13 @@ contract StorageTest is BaseTest { assertEq(atlas.pendingSurchargeRecipient(), userEOA, "pendingSurchargeRecipient should be userEOA"); } + function test_storage_view_atlasSurchargeRate() public { + assertEq(atlas.atlasSurchargeRate(), DEFAULT_ATLAS_SURCHARGE_RATE, "atlasSurchargeRate set incorrectly"); + vm.prank(deployer); + atlas.setAtlasSurchargeRate(100); + assertEq(atlas.atlasSurchargeRate(), 100, "atlasSurchargeRate set incorrectly"); + } + // Transient Storage Getters and Setters function test_storage_transient_lock() public { diff --git a/test/TrebleSwap.t.sol b/test/TrebleSwap.t.sol index 88005b7a9..7f1642a66 100644 --- a/test/TrebleSwap.t.sol +++ b/test/TrebleSwap.t.sol @@ -299,7 +299,7 @@ contract TrebleSwapTest is BaseTest { // Estimate gas surcharge Atlas should have taken txGasUsed = estAtlasGasSurcharge - gasleft(); - estAtlasGasSurcharge = txGasUsed * tx.gasprice * atlas.ATLAS_SURCHARGE_RATE() / atlas.SCALE(); + estAtlasGasSurcharge = txGasUsed * tx.gasprice * atlas.atlasSurchargeRate() / atlas.SCALE(); // For benchmarking console.log("Metacall gas cost: ", txGasUsed); From d0b5ef12bda6c89aae724ed3589374a3bc7403ba Mon Sep 17 00:00:00 2001 From: Ben Sparks <52714090+BenSparksCode@users.noreply.github.com> Date: Wed, 4 Dec 2024 15:51:52 +0200 Subject: [PATCH 2/3] feat: make Bundler surcharge settable by recipient --- src/contracts/atlas/GasAccounting.sol | 35 +++++++++++------------ src/contracts/atlas/Storage.sol | 40 +++++++++++++++++++++------ src/contracts/interfaces/IAtlas.sol | 3 +- src/contracts/types/AtlasErrors.sol | 3 ++ test/GasAccounting.t.sol | 3 +- test/Storage.t.sol | 36 +++++++++++++++++++++--- 6 files changed, 88 insertions(+), 32 deletions(-) diff --git a/src/contracts/atlas/GasAccounting.sol b/src/contracts/atlas/GasAccounting.sol index 286f1cfc6..3b340e5f8 100644 --- a/src/contracts/atlas/GasAccounting.sol +++ b/src/contracts/atlas/GasAccounting.sol @@ -44,12 +44,13 @@ abstract contract GasAccounting is SafetyLocks { /// @param gasMarker The gas marker used to calculate the initial accounting values. function _initializeAccountingValues(uint256 gasMarker) internal { uint256 _rawClaims = (FIXED_GAS_OFFSET + gasMarker) * tx.gasprice; + (uint256 _atlasSurchargeRate, uint256 _bundlerSurchargeRate) = _surchargeRates(); // Set any withdraws or deposits - t_claims = _rawClaims.withSurcharge(BUNDLER_SURCHARGE_RATE); + t_claims = _rawClaims.withSurcharge(_bundlerSurchargeRate); // Atlas surcharge is based on the raw claims value. - t_fees = _rawClaims.getSurcharge(S_atlasSurchargeRate); + t_fees = _rawClaims.getSurcharge(_atlasSurchargeRate); t_deposits = msg.value; // Explicitly set other transient vars to 0 in case multiple metacalls in single tx. @@ -278,6 +279,7 @@ abstract contract GasAccounting is SafetyLocks { internal { uint256 _gasUsed = (gasWaterMark + _SOLVER_BASE_GAS_USED - gasleft()) * tx.gasprice; + (uint256 _atlasSurchargeRate, uint256 _bundlerSurchargeRate) = _surchargeRates(); if (includeCalldata) { _gasUsed += _getCalldataCost(solverOp.data.length); @@ -288,10 +290,10 @@ abstract contract GasAccounting is SafetyLocks { if (result.bundlersFault()) { // CASE: Solver is not responsible for the failure of their operation, so we blame the bundler // and reduce the total amount refunded to the bundler - t_writeoffs += _gasUsed.withSurcharges(S_atlasSurchargeRate, BUNDLER_SURCHARGE_RATE); + t_writeoffs += _gasUsed.withSurcharges(_atlasSurchargeRate, _bundlerSurchargeRate); } else { // CASE: Solver failed, so we calculate what they owe. - uint256 _gasUsedWithSurcharges = _gasUsed.withSurcharges(S_atlasSurchargeRate, BUNDLER_SURCHARGE_RATE); + uint256 _gasUsedWithSurcharges = _gasUsed.withSurcharges(_atlasSurchargeRate, _bundlerSurchargeRate); uint256 _surchargesOnly = _gasUsedWithSurcharges - _gasUsed; // In `_assign()`, the failing solver's bonded AtlETH balance is reduced by `_gasUsedWithSurcharges`. Any @@ -309,13 +311,13 @@ abstract contract GasAccounting is SafetyLocks { } function _writeOffBidFindGasCost(uint256 gasUsed) internal { - t_writeoffs += gasUsed.withSurcharges(S_atlasSurchargeRate, BUNDLER_SURCHARGE_RATE); + (uint256 _atlasSurchargeRate, uint256 _bundlerSurchargeRate) = _surchargeRates(); + t_writeoffs += gasUsed.withSurcharges(_atlasSurchargeRate, _bundlerSurchargeRate); } /// @param ctx Context struct containing relevant context information for the Atlas auction. /// @param solverGasLimit The maximum gas limit for a solver, as set in the DAppConfig /// @return adjustedWithdrawals Withdrawals of the current metacall, adjusted by adding the Atlas gas surcharge. - /// @return adjustedDeposits Deposits of the current metacall, no adjustments applied. /// @return adjustedClaims Claims of the current metacall, adjusted by subtracting the unused gas scaled to include /// bundler surcharge. /// @return adjustedWriteoffs Writeoffs of the current metacall, adjusted by adding the bundler gas overage penalty @@ -331,7 +333,6 @@ abstract contract GasAccounting is SafetyLocks { internal returns ( uint256 adjustedWithdrawals, - uint256 adjustedDeposits, uint256 adjustedClaims, uint256 adjustedWriteoffs, uint256 netAtlasGasSurcharge @@ -340,17 +341,17 @@ abstract contract GasAccounting is SafetyLocks { uint256 _surcharge = S_cumulativeSurcharge; adjustedWithdrawals = t_withdrawals; - adjustedDeposits = t_deposits; adjustedClaims = t_claims; adjustedWriteoffs = t_writeoffs; uint256 _fees = t_fees; + (uint256 _atlasSurchargeRate, uint256 _bundlerSurchargeRate) = _surchargeRates(); uint256 _gasLeft = gasleft(); // Hold this constant for the calculations // Estimate the unspent, remaining gas that the Solver will not be liable for. uint256 _gasRemainder = _gasLeft * tx.gasprice; - adjustedClaims -= _gasRemainder.withSurcharge(BUNDLER_SURCHARGE_RATE); + adjustedClaims -= _gasRemainder.withSurcharge(_bundlerSurchargeRate); if (ctx.solverSuccessful) { // If a solver was successful, calc the full Atlas gas surcharge on the gas cost of the entire metacall, and @@ -358,7 +359,7 @@ abstract contract GasAccounting is SafetyLocks { // be offset by any gas surcharge paid by failed solvers, which would have been added to deposits or // writeoffs in _handleSolverAccounting(). As such, the winning solver does not pay for surcharge on the gas // used by other solvers. - netAtlasGasSurcharge = _fees - _gasRemainder.getSurcharge(S_atlasSurchargeRate); + netAtlasGasSurcharge = _fees - _gasRemainder.getSurcharge(_atlasSurchargeRate); adjustedWithdrawals += netAtlasGasSurcharge; S_cumulativeSurcharge = _surcharge + netAtlasGasSurcharge; } else { @@ -366,14 +367,14 @@ abstract contract GasAccounting is SafetyLocks { uint256 _solverSurcharge = t_solverSurcharge; if (_solverSurcharge > 0) { netAtlasGasSurcharge = _solverSurcharge.getPortionFromTotalSurcharge({ - targetSurchargeRate: S_atlasSurchargeRate, - totalSurchargeRate: S_atlasSurchargeRate + BUNDLER_SURCHARGE_RATE + targetSurchargeRate: _atlasSurchargeRate, + totalSurchargeRate: _atlasSurchargeRate + _bundlerSurchargeRate }); // When no winning solvers, bundler max refund is 80% of metacall gas cost. The remaining 20% can be // collected through storage refunds. Any excess bundler surcharge is instead taken as Atlas surcharge. uint256 _bundlerSurcharge = _solverSurcharge - netAtlasGasSurcharge; - uint256 _maxBundlerRefund = adjustedClaims.withoutSurcharge(BUNDLER_SURCHARGE_RATE).maxBundlerRefund(); + uint256 _maxBundlerRefund = adjustedClaims.withoutSurcharge(_bundlerSurchargeRate).maxBundlerRefund(); if (_bundlerSurcharge > _maxBundlerRefund) { netAtlasGasSurcharge += _bundlerSurcharge - _maxBundlerRefund; } @@ -381,14 +382,14 @@ abstract contract GasAccounting is SafetyLocks { adjustedWithdrawals += netAtlasGasSurcharge; S_cumulativeSurcharge = _surcharge + netAtlasGasSurcharge; } - return (adjustedWithdrawals, adjustedDeposits, adjustedClaims, adjustedWriteoffs, netAtlasGasSurcharge); + return (adjustedWithdrawals, adjustedClaims, adjustedWriteoffs, netAtlasGasSurcharge); } // Calculate whether or not the bundler used an excessive amount of gas and, if so, reduce their // gas rebate. By reducing the claims, solvers end up paying less in total. if (ctx.solverCount > 0) { // Calculate the unadjusted bundler gas surcharge - uint256 _grossBundlerGasSurcharge = adjustedClaims.withoutSurcharge(BUNDLER_SURCHARGE_RATE); + uint256 _grossBundlerGasSurcharge = adjustedClaims.withoutSurcharge(_bundlerSurchargeRate); // Calculate an estimate for how much gas should be remaining // NOTE: There is a free buffer of one SolverOperation because solverIndex starts at 0. @@ -435,9 +436,9 @@ abstract contract GasAccounting is SafetyLocks { uint256 _claims; uint256 _writeoffs; uint256 _withdrawals; - uint256 _deposits; + uint256 _deposits = t_deposits; // load here, not used in adjustment function below - (_withdrawals, _deposits, _claims, _writeoffs, netAtlasGasSurcharge) = + (_withdrawals, _claims, _writeoffs, netAtlasGasSurcharge) = _adjustAccountingForFees(ctx, solverGasLimit); uint256 _amountSolverPays; diff --git a/src/contracts/atlas/Storage.sol b/src/contracts/atlas/Storage.sol index 7eb375d18..2d5fe63af 100644 --- a/src/contracts/atlas/Storage.sol +++ b/src/contracts/atlas/Storage.sol @@ -18,7 +18,6 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { address public immutable SIMULATOR; address public immutable L2_GAS_CALCULATOR; uint256 public immutable ESCROW_DURATION; - uint256 public immutable BUNDLER_SURCHARGE_RATE; // AtlETH public constants // These constants double as interface functions for the ERC20 standard, hence the lowercase naming convention. @@ -48,7 +47,7 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { uint256 internal S_bondedTotalSupply; // Surcharge-related storage - uint256 internal S_atlasSurchargeRate; // Atlas surcharge rate + uint256 internal S_surchargeRates; // left 128 bits: Atlas rate, right 128 bits: Bundler rate uint256 internal S_cumulativeSurcharge; // Cumulative gas surcharges collected address internal S_surchargeRecipient; // Fastlane surcharge recipient address internal S_pendingSurchargeRecipient; // For 2-step transfer process @@ -72,9 +71,13 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { SIMULATOR = simulator; L2_GAS_CALCULATOR = l2GasCalculator; ESCROW_DURATION = escrowDuration; - BUNDLER_SURCHARGE_RATE = bundlerSurchargeRate; - S_atlasSurchargeRate = atlasSurchargeRate; + // Check Atlas and Bundler gas surcharges fit in 128 bits each + if(atlasSurchargeRate > type(uint128).max || bundlerSurchargeRate > type(uint128).max) { + revert SurchargeRateTooHigh(); + } + + S_surchargeRates = atlasSurchargeRate << 128 | bundlerSurchargeRate; S_cumulativeSurcharge = msg.value; S_surchargeRecipient = initialSurchargeRecipient; @@ -85,10 +88,15 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { // Storage Setters // // ---------------------------------------------------- // - function setAtlasSurchargeRate(uint256 newSurchargeRate) external { + function setSurchargeRates(uint256 newAtlasRate, uint256 newBundlerRate) external { _onlySurchargeRecipient(); - S_atlasSurchargeRate = newSurchargeRate; - // TODO consider adding event + + // Check Atlas and Bundler gas surcharges fit in 128 bits each + if(newAtlasRate > type(uint128).max || newBundlerRate > type(uint128).max) { + revert SurchargeRateTooHigh(); + } + + S_surchargeRates = newAtlasRate << 128 | newBundlerRate; } function _onlySurchargeRecipient() internal view { @@ -145,7 +153,23 @@ contract Storage is AtlasEvents, AtlasErrors, AtlasConstants { } function atlasSurchargeRate() external view returns (uint256) { - return S_atlasSurchargeRate; + // Includes only the left 128 bits to isolate the Atlas rate + return S_surchargeRates >> 128; + } + + function bundlerSurchargeRate() external view returns (uint256) { + // Includes only the right 128 bits to isolate the bundler rate + return S_surchargeRates & ((1 << 128) - 1); + } + + // ---------------------------------------------------- // + // Storage Internal Getters // + // ---------------------------------------------------- // + + function _surchargeRates() internal view returns (uint256 atlasRate, uint256 bundlerRate) { + uint256 _bothRates = S_surchargeRates; + atlasRate = _bothRates >> 128; + bundlerRate = _bothRates & ((1 << 128) - 1); } // ---------------------------------------------------- // diff --git a/src/contracts/interfaces/IAtlas.sol b/src/contracts/interfaces/IAtlas.sol index e42162a94..adc003795 100644 --- a/src/contracts/interfaces/IAtlas.sol +++ b/src/contracts/interfaces/IAtlas.sol @@ -49,7 +49,7 @@ interface IAtlas { function withdrawSurcharge() external; function transferSurchargeRecipient(address newRecipient) external; function becomeSurchargeRecipient() external; - function setAtlasSurchargeRate(uint256 rate) external; + function setSurchargeRates(uint128 newAtlasRate, uint128 newBundlerRate) external; // Permit69.sol function transferUserERC20( @@ -104,4 +104,5 @@ interface IAtlas { function surchargeRecipient() external view returns (address); function pendingSurchargeRecipient() external view returns (address); function atlasSurchargeRate() external view returns (uint256); + function bundlerSurchargeRate() external view returns (uint256); } diff --git a/src/contracts/types/AtlasErrors.sol b/src/contracts/types/AtlasErrors.sol index e3046bdbc..f33c8a272 100644 --- a/src/contracts/types/AtlasErrors.sol +++ b/src/contracts/types/AtlasErrors.sol @@ -103,6 +103,9 @@ contract AtlasErrors { error NotInitialized(); error AlreadyInitialized(); + // Storage + error SurchargeRateTooHigh(); + // AtlasVerification error NoUnusedNonceInBitmap(); diff --git a/test/GasAccounting.t.sol b/test/GasAccounting.t.sol index 82a100277..cc49139f2 100644 --- a/test/GasAccounting.t.sol +++ b/test/GasAccounting.t.sol @@ -71,7 +71,6 @@ contract MockGasAccounting is TestAtlas, BaseTest { external returns ( uint256 adjustedWithdrawals, - uint256 adjustedDeposits, uint256 adjustedClaims, uint256 adjustedWriteoffs, uint256 netAtlasGasSurcharge @@ -282,7 +281,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { claims = rawClaims * ( mockGasAccounting.SCALE() + mockGasAccounting.atlasSurchargeRate() - + mockGasAccounting.BUNDLER_SURCHARGE_RATE() + + mockGasAccounting.bundlerSurchargeRate() ) / mockGasAccounting.SCALE(); } diff --git a/test/Storage.t.sol b/test/Storage.t.sol index f9aac109d..88b643e85 100644 --- a/test/Storage.t.sol +++ b/test/Storage.t.sol @@ -5,6 +5,7 @@ import "forge-std/Test.sol"; import { Storage } from "../src/contracts/atlas/Storage.sol"; import "../src/contracts/types/LockTypes.sol"; +import { AtlasErrors } from "../src/contracts/types/AtlasErrors.sol"; import { BaseTest } from "./base/BaseTest.t.sol"; @@ -29,9 +30,6 @@ contract StorageTest is BaseTest { assertEq(atlas.symbol(), "atlETH", "symbol set incorrectly"); assertEq(atlas.decimals(), 18, "decimals set incorrectly"); - assertEq( - atlas.BUNDLER_SURCHARGE_RATE(), DEFAULT_BUNDLER_SURCHARGE_RATE, "BUNDLER_SURCHARGE_RATE set incorrectly" - ); assertEq(atlas.SCALE(), DEFAULT_SCALE, "SCALE set incorrectly"); assertEq(atlas.FIXED_GAS_OFFSET(), DEFAULT_FIXED_GAS_OFFSET, "FIXED_GAS_OFFSET set incorrectly"); } @@ -145,10 +143,40 @@ contract StorageTest is BaseTest { function test_storage_view_atlasSurchargeRate() public { assertEq(atlas.atlasSurchargeRate(), DEFAULT_ATLAS_SURCHARGE_RATE, "atlasSurchargeRate set incorrectly"); vm.prank(deployer); - atlas.setAtlasSurchargeRate(100); + atlas.setSurchargeRates(100, 200); assertEq(atlas.atlasSurchargeRate(), 100, "atlasSurchargeRate set incorrectly"); } + function test_storage_view_bundlerSurchargeRate() public { + assertEq(atlas.bundlerSurchargeRate(), DEFAULT_BUNDLER_SURCHARGE_RATE, "bundlerSurchargeRate set incorrectly"); + vm.prank(deployer); + atlas.setSurchargeRates(100, 200); + assertEq(atlas.bundlerSurchargeRate(), 200, "bundlerSurchargeRate set incorrectly"); + } + + // Storage Setters + + function test_storage_setSurchargeRates() public { + uint256 tooHigh = uint256(type(uint128).max) + 1; + + vm.prank(deployer); + vm.expectRevert(AtlasErrors.SurchargeRateTooHigh.selector); + atlas.setSurchargeRates(tooHigh, 456); + + vm.prank(deployer); + vm.expectRevert(AtlasErrors.SurchargeRateTooHigh.selector); + atlas.setSurchargeRates(123, tooHigh); + + vm.prank(userEOA); + vm.expectRevert(AtlasErrors.InvalidAccess.selector); + atlas.setSurchargeRates(123, 456); + + vm.prank(deployer); + atlas.setSurchargeRates(123, 456); + assertEq(atlas.atlasSurchargeRate(), 123, "atlasSurchargeRate set incorrectly"); + assertEq(atlas.bundlerSurchargeRate(), 456, "bundlerSurchargeRate set incorrectly"); + } + // Transient Storage Getters and Setters function test_storage_transient_lock() public { From 716d068f92bf278a1ed2f2eabfc5f1191ad0fd88 Mon Sep 17 00:00:00 2001 From: Ben Sparks <52714090+BenSparksCode@users.noreply.github.com> Date: Thu, 5 Dec 2024 18:59:20 +0200 Subject: [PATCH 3/3] docs: added explanation to `_deficit()` --- src/contracts/atlas/GasAccounting.sol | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/contracts/atlas/GasAccounting.sol b/src/contracts/atlas/GasAccounting.sol index 3b340e5f8..36879d364 100644 --- a/src/contracts/atlas/GasAccounting.sol +++ b/src/contracts/atlas/GasAccounting.sol @@ -104,7 +104,24 @@ abstract contract GasAccounting is SafetyLocks { } function _deficit() internal view returns (uint256) { - return t_claims + t_withdrawals + t_fees - t_writeoffs; + // _deficit() is compared against t_deposits which includes: + // + msg.value deposited + // + gas cost + A + B (prev solver fault fails) + + // _deficit() is therefore composed of: + // + withdrawals = msg.value borrowed + // + claims = gas cost + B (full tx) + // + fees = A (full tx) + // - writeoffs = gas cost + A + B (prev bundler fault fails) + + // Such that `deficit() - t_deposits` = + // + msg.value still owed + // + gas cost + A + B (full tx) + // - gas cost + A + B (prev solver fault fails) + // - gas cost + A + B (prev bundler fault fails) + // == only what the current solver owes if they win + + return t_withdrawals + t_claims + t_fees - t_writeoffs; } /// @notice Allows a solver to settle any outstanding ETH owed, either to repay gas used by their solverOp or to @@ -438,8 +455,7 @@ abstract contract GasAccounting is SafetyLocks { uint256 _withdrawals; uint256 _deposits = t_deposits; // load here, not used in adjustment function below - (_withdrawals, _claims, _writeoffs, netAtlasGasSurcharge) = - _adjustAccountingForFees(ctx, solverGasLimit); + (_withdrawals, _claims, _writeoffs, netAtlasGasSurcharge) = _adjustAccountingForFees(ctx, solverGasLimit); uint256 _amountSolverPays; uint256 _amountSolverReceives;