From 392ba0e3e43497e72fd346b1ba4831f60194cdbc Mon Sep 17 00:00:00 2001
From: Ben Sparks <52714090+BenSparksCode@users.noreply.github.com>
Date: Thu, 12 Sep 2024 19:07:20 +0200
Subject: [PATCH] fix: record accurate `solverSurcharge` figure

---
 src/contracts/atlas/GasAccounting.sol | 28 ++++++++++++++++++++-------
 test/GasAccounting.t.sol              | 10 ----------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/contracts/atlas/GasAccounting.sol b/src/contracts/atlas/GasAccounting.sol
index 613027eeb..5b04b288d 100644
--- a/src/contracts/atlas/GasAccounting.sol
+++ b/src/contracts/atlas/GasAccounting.sol
@@ -286,8 +286,19 @@ abstract contract GasAccounting is SafetyLocks {
         } else {
             // CASE: Solver failed, so we calculate what they owe.
             uint256 _gasUsedWithSurcharges = _gasUsed.withAtlasAndBundlerSurcharges();
-            _setSolverSurcharge(solverSurcharge() + _gasUsedWithSurcharges - _gasUsed);
-            _assign(solverOp.from, _gasUsedWithSurcharges, _gasUsedWithSurcharges, false);
+            uint256 _surchargesOnly = _gasUsedWithSurcharges - _gasUsed;
+
+            // In `_assign()`, the failing solver's bonded AtlETH balance is reduced by `_gasUsedWithSurcharges`. Any
+            // deficit from that operation is added to `writeoffs` and returned as `_assignDeficit` below. The portion
+            // that can be covered by the solver's AtlETH is added to `deposits`, to account that it has been paid.
+            uint256 _assignDeficit = _assign(solverOp.from, _gasUsedWithSurcharges, _gasUsedWithSurcharges, false);
+
+            // We track the surcharges (in excess of deficit - so the actual AtlETH that can be collected) separately,
+            // so that in the event of no successful solvers, any `_assign()`ed surcharges can be attributed to an
+            // increase in Atlas' cumulative surcharge.
+            if (_surchargesOnly > _assignDeficit) {
+                _setSolverSurcharge(solverSurcharge() + (_surchargesOnly - _assignDeficit));
+            }
         }
     }
 
@@ -336,15 +347,18 @@ abstract contract GasAccounting is SafetyLocks {
         adjustedClaims -= _gasRemainder.withBundlerSurcharge();
 
         if (ctx.solverSuccessful) {
-            // Calculate the preadjusted netAtlasGasSurcharge
+            // If a solver was successful, calc the full Atlas gas surcharge on the gas cost of the entire metacall, and
+            // add it to withdrawals so that the cost is assigned to winning solver by the end of _settle(). This will
+            // 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.getAtlasSurcharge();
-
             adjustedWithdrawals += netAtlasGasSurcharge;
-
-            S_cumulativeSurcharge = _surcharge + netAtlasGasSurcharge; // Update the cumulative surcharge
+            S_cumulativeSurcharge = _surcharge + netAtlasGasSurcharge;
         } else {
+            // If no successful solvers, only collect partial surcharges from solver's fault failures (if any)
             netAtlasGasSurcharge = solverSurcharge();
-            S_cumulativeSurcharge = _surcharge + netAtlasGasSurcharge; // Update the cumulative surcharge
+            if (netAtlasGasSurcharge > 0) S_cumulativeSurcharge = _surcharge + netAtlasGasSurcharge;
             return (adjustedWithdrawals, adjustedDeposits, adjustedClaims, adjustedWriteoffs, netAtlasGasSurcharge);
         }
 
diff --git a/test/GasAccounting.t.sol b/test/GasAccounting.t.sol
index 39f62bfb0..adc23846e 100644
--- a/test/GasAccounting.t.sol
+++ b/test/GasAccounting.t.sol
@@ -27,7 +27,6 @@ import { BaseTest } from "test/base/BaseTest.t.sol";
 contract MockGasAccounting is TestAtlas, BaseTest {
     uint256 public constant MOCK_SOLVER_GAS_LIMIT = 500_000;
     
-
     constructor(
         uint256 _escrowDuration,
         address _verification,
@@ -1030,15 +1029,6 @@ contract GasAccountingTest is AtlasConstants, BaseTest {
         assertEq(unbonding, unbondingBefore);
     }
 
-    function test_settle_withFailedsolver_reverts() public {
-        // Setup context with initial claims and deposits
-        Context memory ctx = setupContext(1 ether, 1 ether, 4000 ether, 1000 ether, false);
-
-        // Expect a revert due to insufficient total balance for a failed solver
-        vm.expectRevert();
-        mockGasAccounting.settle(ctx);
-    }
-
     function test_settle_with_deposits() public {
         Context memory ctx = setupContext(1 ether, 0.5 ether, 4000 ether, 1000 ether, true);
         // Check initial balances