Skip to content

feat(protocol): switch L2 fee controller to p-controller#21350

Open
linoscope wants to merge 2 commits intol2_fee_vaultfrom
l2_fee_vault_with_p_controller
Open

feat(protocol): switch L2 fee controller to p-controller#21350
linoscope wants to merge 2 commits intol2_fee_vaultfrom
l2_fee_vault_with_p_controller

Conversation

@linoscope
Copy link
Copy Markdown
Collaborator

@linoscope linoscope commented Feb 19, 2026

Overview

This PR builds on top of @ggonzalez94 's #21214 by switching the fee update logic from an EIP-1559-style multiplicative step to a proportional (P) controller.

New fee is calculated as follows:

  • effectiveBalance = max(0, balance - totalLiabilities)
  • epsilon = max(0, (targetBalanceWei - effectiveBalance) / targetBalanceWei)
  • pTerm = Kp * epsilon * (maxFeePerGasWei - minFeePerGasWei)
  • newFee = clamp(pTerm, minFeePerGasWei, maxFeePerGasWei)

This means fee reacts directly to the current deficit: when the vault is at/above target, the proportional term is zero and the final fee is bounded by min/max; when deficit grows, fee scales proportionally up to max.

Reasoning to take this approach v.s. EIP-1559 is given in the next section. A more comprehensive comparison with other approaches (e.g., PI-controller, P-controller with feed forward) will be shared separately soon.

EIP-1559-style v.s. P-Controller

The two charts below are the simulated fee and vault values based on last year's base/blob fee history.

Here is the simulator UI: Link

Some insights:

  • EIP-1559 (orange) has larger fee spikes (about 2x peak) and longer elevated periods; P-controller (blue) is smoother.
  • EIP-1559 shows clear under/overshoot around target (large swing below then well above 10 ETH). P-controller stays closer to target with much smaller excursions.
  • EIP-1559’s multiplicative update is effectively slower to correct after shocks, so it can overcharge after recovery (seen in orange staying high while vault is already above target).

This is because:

  • The EIP-1559-style controlle updates the fee multiplicatively based on previous fee on each posting event (fee_next = fee_now * (1 + error/denominator) where error = (targetVault - vault)/targetVault), so when it ramps up during a deficit it has resistance to changing quickly and can stay high even after conditions improve, which creates undershoot/overshoot in both fee and vault.
  • The P-controller instead computes fee directly from the current normalized vault deficit each step (fee = Kp * deficit), so the fee reacts quicker to decific increases, and once the deficit shrinks, the fee shrinks immediately too.
image image

@claude
Copy link
Copy Markdown

claude bot commented Feb 19, 2026

Claude finished @linoscope's task in 4m 57s —— View job


Code Review

  • Read changed files
  • Analyze implementation correctness
  • Review test coverage
  • Check for bugs, security, and gas issues

Overall: Clean, correct implementation. The P-controller is a real improvement over the EIP-1559 controller — stateless, no signed integer arithmetic, immediate response to current conditions. A few things to address below.


Issues

1. init doesn't emit AnchorUpdated (L2FeeVault.sol:192)

_setAnchor is called in init but AnchorUpdated is only emitted in setAnchor. Off-chain indexers bootstrapping from events won't see the initial anchor address.

Fix: either emit AnchorUpdated in init, or move the emit into _setAnchor. Fix this →


2. Formula "dead zone" when minFee > 0

The formula is pTerm = Kp * epsilon * (maxFee - minFee), clamped to [minFee, maxFee]. This means fee stays at minFee for any deficit below epsilon < minFee / (Kp * feeRange). With the test config (minFee=100, feeRange=1e6, Kp=2), the dead zone is ~0.005% deficit — negligible. But if someone configures minFee close to maxFee (small feeRange), the controller can become effectively disabled.

Consider documenting this, or using pTerm = minFee + Kp * epsilon * (maxFee - minFee) instead, which makes the fee smoothly transition from exactly minFee at 0% deficit to maxFee at the saturation point, eliminating the dead zone entirely.

3. No validation that kpWad > 0 (L2FeeVault.sol:172)

kpWad = 0 silently disables the controller — fee is always minFee regardless of deficit. This is probably a misconfiguration rather than an intended mode. Either add require(_kpWad > 0, InvalidValue()); in the constructor or document the behavior explicitly. Fix this →


Missing Test Coverage (L2FeeVault.t.sol)

4. No fuzz tests — the controller uses three Math.mulDiv calls with arbitrary inputs. A fuzz test verifying result ∈ [minFee, maxFee] for all valid inputs would be valuable:

function testFuzz_feeAlwaysWithinBounds(uint256 balance, uint256 liabilities) external { ... }

5. No partial claim testtest_claim_transfersAndUpdatesLiabilities only uses amount=0 (full claim). Add a case for a specific partial amount to verify claimable and totalLiabilities are both updated correctly.

6. No setAnchor access control test — there's a test for importProposalFee reverting when unauthorized, but no equivalent for setAnchor. A non-owner call should revert.


Minor Observations

  • _calcL1Cost with unchecked: Mathematically safe — uint64 * uint128 max is ~2^192 and uint32 * 2^17 * uint128 max is ~2^177, both within uint256. No actual overflow risk.
  • Steady-state error: Inherent to pure P-controllers — the vault will equilibrate at a small persistent deficit (the deficit needed to produce enough fee to cover costs). This is acceptable and presumably accepted. Worth noting in docs if not already.
  • Fee immediately snaps to minFee on surplus (unlike the old EIP-1559 controller which needed multiple steps to decay). This is an improvement — no overshoot on recovery.
  • Storage layout unchanged: The P-controller is stateless (no previous fee needed), so no new slots. The __gap[51] has capacity for future additions.
  • The removal of int256 arithmetic and the BASE_FEE_MAX_CHANGE_DENOMINATOR constant simplifies the code significantly. Good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant