diff --git a/src/contracts/atlas/GasAccounting.sol b/src/contracts/atlas/GasAccounting.sol index 49088171c..63f9cdd48 100644 --- a/src/contracts/atlas/GasAccounting.sol +++ b/src/contracts/atlas/GasAccounting.sol @@ -175,13 +175,13 @@ abstract contract GasAccounting is SafetyLocks { /// increase transient solver deposits. /// @param owner The address of the owner from whom AtlETH is taken. /// @param amount The amount of AtlETH to be taken. - /// @param gasUsed The amount of gas used in the SolverOperation. + /// @param gasValueUsed The ETH value of gas used in the SolverOperation. /// @param solverWon A boolean indicating whether the solver won the bid. /// @return deficit The amount of AtlETH that was not repaid, if any. function _assign( address owner, uint256 amount, - uint256 gasUsed, + uint256 gasValueUsed, bool solverWon ) internal @@ -222,8 +222,8 @@ abstract contract GasAccounting is SafetyLocks { _aData.bonded -= _amt; } - // Update analytics (auctionWins, auctionFails, totalGasUsed) and lastAccessedBlock - _updateAnalytics(_aData, solverWon && deficit == 0, gasUsed); + // Update analytics (auctionWins, auctionFails, totalGasValueUsed) and lastAccessedBlock + _updateAnalytics(_aData, solverWon && deficit == 0, gasValueUsed); _aData.lastAccessedBlock = uint32(block.number); // Persist changes in the _aData memory struct back to storage @@ -236,7 +236,8 @@ abstract contract GasAccounting is SafetyLocks { /// @notice Increases the owner's bonded balance by the specified amount. /// @param owner The address of the owner whose bonded balance will be increased. /// @param amount The amount by which to increase the owner's bonded balance. - function _credit(address owner, uint256 amount, uint256 gasUsed) internal { + /// @param gasValueUsed The ETH value of gas used in the SolverOperation. + function _credit(address owner, uint256 amount, uint256 gasValueUsed) internal { uint112 _amt = SafeCast.toUint112(amount); EscrowAccountAccessData memory _aData = S_accessData[owner]; @@ -246,8 +247,8 @@ abstract contract GasAccounting is SafetyLocks { S_bondedTotalSupply += amount; - // Update analytics (auctionWins, auctionFails, totalGasUsed) - _updateAnalytics(_aData, true, gasUsed); + // Update analytics (auctionWins, auctionFails, totalGasValueUsed) + _updateAnalytics(_aData, true, gasValueUsed); // Persist changes in the _aData memory struct back to storage S_accessData[owner] = _aData; @@ -437,8 +438,15 @@ abstract contract GasAccounting is SafetyLocks { /// @dev This function is only ever called in the context of bidFind = false so no risk of doublecounting changes. /// @param aData The Solver's EscrowAccountAccessData struct to update. /// @param auctionWon A boolean indicating whether the solver's solverOp won the auction. - /// @param gasUsed The amount of gas used by the solverOp. - function _updateAnalytics(EscrowAccountAccessData memory aData, bool auctionWon, uint256 gasUsed) internal view { + /// @param gasValueUsed The ETH value of gas used by the solverOp. Should be calculated as gasUsed * tx.gasprice. + function _updateAnalytics( + EscrowAccountAccessData memory aData, + bool auctionWon, + uint256 gasValueUsed + ) + internal + pure + { if (auctionWon) { unchecked { ++aData.auctionWins; @@ -450,7 +458,7 @@ abstract contract GasAccounting is SafetyLocks { } // Track total ETH value of gas spent by solver in metacalls. Measured in gwei (1e9 digits truncated). - aData.totalGasValueUsed += uint64(gasUsed * tx.gasprice / _GAS_VALUE_DECIMALS_TO_DROP); + aData.totalGasValueUsed += SafeCast.toUint64(gasValueUsed / _GAS_VALUE_DECIMALS_TO_DROP); } /// @notice Calculates the gas cost of the calldata used to execute a SolverOperation. diff --git a/test/GasAccounting.t.sol b/test/GasAccounting.t.sol index 55a66091c..cdb2f55a2 100644 --- a/test/GasAccounting.t.sol +++ b/test/GasAccounting.t.sol @@ -43,12 +43,12 @@ contract MockGasAccounting is TestAtlas, BaseTest { // Expose access to internal functions for testing // ///////////////////////////////////////////////////////// - function assign(address owner, uint256 value, bool solverWon) external returns (uint256) { - return _assign(owner, value, value, solverWon); + function assign(address owner, uint256 amount, uint256 gasValueUsed, bool solverWon) external returns (uint256) { + return _assign(owner, amount, gasValueUsed, solverWon); } - function credit(address owner, uint256 value) external { - _credit(owner, value, value); + function credit(address owner, uint256 amount, uint256 gasValueUsed) external { + _credit(owner, amount, gasValueUsed); } function handleSolverAccounting( @@ -792,7 +792,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { uint256 bondedTotalSupplyBefore = mockGasAccounting.bondedTotalSupply(); uint256 depositsBefore = mockGasAccounting.getDeposits(); - uint256 deficit = mockGasAccounting.assign(solverOp.from, assignedAmount, true); + uint256 deficit = mockGasAccounting.assign(solverOp.from, assignedAmount, assignedAmount, true); assertEq(deficit, 0, "Deficit should be 0"); (, uint32 lastAccessedBlock,,,) = mockGasAccounting.accessData(solverOp.from); @@ -818,7 +818,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { uint256 depositsBefore = mockGasAccounting.getDeposits(); // Call the assign function and capture the deficit - uint256 deficit = mockGasAccounting.assign(solverOp.from, assignedAmount, true); + uint256 deficit = mockGasAccounting.assign(solverOp.from, assignedAmount, assignedAmount, true); assertEq(deficit, 0, "Deficit should be 0"); // Retrieve and check the updated access data @@ -850,7 +850,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { uint256 bondedTotalSupplyBefore = mockGasAccounting.bondedTotalSupply(); uint256 depositsBefore = mockGasAccounting.getDeposits(); - uint256 deficit = mockGasAccounting.assign(solverOp.from, assignedAmount, true); + uint256 deficit = mockGasAccounting.assign(solverOp.from, assignedAmount, assignedAmount, true); assertEq(deficit, assignedAmount - (unbondingAmount + bondedAmount)); (, uint32 lastAccessedBlock,,,) = mockGasAccounting.accessData(solverOp.from); assertEq(lastAccessedBlock, uint32(block.number)); @@ -862,10 +862,12 @@ contract GasAccountingTest is AtlasConstants, BaseTest { } function test_assign_reputationAnalytics() public { + // NOTE: the `amount` and `gasValueUsed` params for `_assign()` should be measured in ETH value. I.e. they should be calculated as `gasUsed * tx.gasprice`. uint256 startGasPrice = 2e9; uint256 endGasPrice = 4e9; - uint256 assignedAmount = 1_234_567; + uint256 gasUsedAmount = 1_234_567; + uint256 assignedAmount; uint24 auctionWins; uint24 auctionFails; uint64 totalGasValueUsed; @@ -879,20 +881,32 @@ contract GasAccountingTest is AtlasConstants, BaseTest { vm.txGasPrice(startGasPrice); // Set gas price to 2e9 assertEq(tx.gasprice, startGasPrice, "tx.gasprice should be 2e9"); - mockGasAccounting.assign(solverOp.from, assignedAmount, true); + assignedAmount = gasUsedAmount * tx.gasprice; + mockGasAccounting.assign({ + owner: solverOp.from, + amount: assignedAmount, + gasValueUsed: assignedAmount, + solverWon: true + }); (,, auctionWins, auctionFails, totalGasValueUsed) = mockGasAccounting.accessData(solverOp.from); - expectedTotalGasValueUsed = assignedAmount * startGasPrice / ONE_GWEI; + expectedTotalGasValueUsed = assignedAmount / ONE_GWEI; assertEq(auctionWins, 1, "auctionWins should be incremented by 1"); assertEq(auctionFails, 0, "auctionFails should remain at 0"); assertEq(totalGasValueUsed, expectedTotalGasValueUsed, "totalGasValueUsed not as expected"); vm.txGasPrice(endGasPrice); // Set gas price to 4e9 assertEq(tx.gasprice, endGasPrice, "tx.gasprice should be 4e9"); - mockGasAccounting.assign(solverOp.from, assignedAmount, false); + assignedAmount = gasUsedAmount * tx.gasprice; + mockGasAccounting.assign({ + owner: solverOp.from, + amount: assignedAmount, + gasValueUsed: assignedAmount, + solverWon: false + }); (,, auctionWins, auctionFails, totalGasValueUsed) = mockGasAccounting.accessData(solverOp.from); - expectedTotalGasValueUsed += assignedAmount * endGasPrice / ONE_GWEI; + expectedTotalGasValueUsed += assignedAmount / ONE_GWEI; assertEq(auctionWins, 1, "auctionWins should remain at 1"); assertEq(auctionFails, 1, "auctionFails should be incremented by 1"); assertEq(totalGasValueUsed, expectedTotalGasValueUsed, "totalGasValueUsed not as expected"); @@ -907,7 +921,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { uint256 depositsBefore = mockGasAccounting.getDeposits(); (uint112 unbondingBefore,) = mockGasAccounting._balanceOf(solverOp.from); vm.expectRevert(AtlasErrors.ValueTooLarge.selector); - mockGasAccounting.assign(solverOp.from, assignedAmount, true); + mockGasAccounting.assign(solverOp.from, assignedAmount, assignedAmount, true); // Check assign reverted with overflow, and accounting values did not change assertEq(mockGasAccounting.bondedTotalSupply(), bondedTotalSupplyBefore); @@ -926,7 +940,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { (, lastAccessedBlock,,,) = mockGasAccounting.accessData(solverOp.from); assertEq(lastAccessedBlock, 0); - mockGasAccounting.credit(solverOp.from, creditedAmount); + mockGasAccounting.credit(solverOp.from, creditedAmount, creditedAmount); (, lastAccessedBlock,,,) = mockGasAccounting.accessData(solverOp.from); (uint112 bondedAfter,,,,) = mockGasAccounting.accessData(solverOp.from); @@ -939,7 +953,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { // Testing uint112 boundary values for casting from uint256 to uint112 in _credit() uint256 overflowAmount = uint256(type(uint112).max) + 1; vm.expectRevert(abi.encodeWithSelector(SafeCast.SafeCastOverflowedUintDowncast.selector, 112, overflowAmount)); - mockGasAccounting.credit(solverOp.from, overflowAmount); + mockGasAccounting.credit(solverOp.from, overflowAmount, overflowAmount); } function test_handleSolverAccounting_solverNotResponsible() public { @@ -962,7 +976,7 @@ contract GasAccountingTest is AtlasConstants, BaseTest { uint256 expectedWriteoffs = initialWriteoffs + AccountingMath.withAtlasAndBundlerSurcharges(gasUsed); // Verify writeoffs have increased - assertEq(mockGasAccounting.getWriteoffs(), expectedWriteoffs, "Writeoffs mismatch"); + assertApproxEqRel(mockGasAccounting.getWriteoffs(), expectedWriteoffs, 1e15, "Writeoffs not within 0.1% error margin"); } function test_handleSolverAccounting_solverResponsible() public {