Skip to content

Commit 5f7e700

Browse files
spalladinoclaude
andcommitted
fix(sequencer-client): reduce prediction window from LAG+1 to LAG entries
Addresses PR #22116 review comment 9: the last entry in the LAG+1 window could be invalidated by a concurrent oracle update. LAG entries are all guaranteed stable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 17446ed commit 5f7e700

3 files changed

Lines changed: 15 additions & 17 deletions

File tree

yarn-project/sequencer-client/src/global_variable_builder/README.md

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ oracle state and an assumed mana usage pattern.
55

66
## Prediction Window
77

8-
The prediction covers `LAG + 1 = 3` entries (the next available slot plus 2 more).
9-
The window is LAG slots because a new oracle update can be enqueued at any time (if
10-
the cooldown has passed), and its new values activate LAG slots later. Beyond LAG
11-
slots, the L1 fees could change unpredictably.
8+
The prediction covers `LAG = 2` entries (the next available slot plus 1 more).
9+
A new oracle update can activate at `startSlot + LAG`, so only the first LAG entries
10+
are guaranteed stable. Beyond that, L1 fees could change unpredictably.
1211

1312
Each entry uses:
1413

@@ -17,12 +16,11 @@ Each entry uses:
1716
- A **configurable congestion assumption** (`ManaUsageEstimate`): none (0 mana), target
1817
(steady state), or limit (worst case, 2x target).
1918

20-
The client picks `max(predicted[0..LAG])` as their `maxFeesPerGas`.
19+
The client picks `max(predicted[0..LAG-1])` as their `maxFeesPerGas`.
2120

2221
```
2322
predicted[0] → fee at next slot (current oracle + current congestion)
2423
predicted[1] → fee at next slot + 1 (congestion may change based on usage estimate)
25-
predicted[2] → fee at next slot + 2 (oracle may transition here)
2624
```
2725

2826
## Why LAG and not LIFETIME?
@@ -42,5 +40,5 @@ With a LIFETIME-sized window, we'd give a false sense of coverage: the predictio
4240
would look like it covers 6 slots, but slots beyond LAG could be wrong if an update
4341
is enqueued after the prediction is computed.
4442

45-
By limiting to LAG, we guarantee that the predicted L1 fees are the ones that will
46-
actually be used — no oracle update can change them within this window.
43+
By limiting to LAG entries, we guarantee that all predicted L1 fees are the ones that
44+
will actually be used — no oracle update can change them within this window.

yarn-project/sequencer-client/src/global_variable_builder/fee_predictor.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,10 @@ describe('FeePredictor', () => {
125125
expect(predicted[0].feePerL2Gas).toBe(l1Fee);
126126
});
127127

128-
it('returns exactly FEE_ORACLE_LAG + 1 entries', async () => {
128+
it('returns exactly FEE_ORACLE_LAG entries', async () => {
129129
const predictor = new FeePredictor(rollup, slotDuration, l1GenesisTime);
130130
const predicted = await predictor.getPredictedMinFees(publicClient, ManaUsageEstimate.Target);
131-
expect(predicted.length).toBe(FEE_ORACLE_LAG + 1);
131+
expect(predicted.length).toBe(FEE_ORACLE_LAG);
132132
});
133133

134134
it.each([
@@ -275,7 +275,7 @@ describe('FeePredictor', () => {
275275
const predictor = new FeePredictor(rollup, slotDuration, l1GenesisTime);
276276
const predicted = await predictor.getPredictedMinFees(publicClient, ManaUsageEstimate.None);
277277

278-
expect(predicted.length).toBe(FEE_ORACLE_LAG + 1);
278+
expect(predicted.length).toBe(FEE_ORACLE_LAG);
279279

280280
const startSlot = await getPredictionStartSlot();
281281
for (let i = 0; i < predicted.length; i++) {

yarn-project/sequencer-client/src/global_variable_builder/fee_predictor.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ type FeeOracleState = {
1818
};
1919

2020
/**
21-
* Predicts min fees for the current slot and next LAG slots based on the L1 oracle state.
22-
* The prediction window is LAG slots because a new oracle update can activate after LAG slots,
23-
* making predictions beyond that unreliable.
24-
* Caches L1 queries per L1 block and recomputes predictions for each mana usage estimate.
21+
* Predicts min fees for LAG upcoming slots based on the L1 oracle state.
22+
* A new oracle update can activate at startSlot + LAG, so only the first LAG entries
23+
* are guaranteed stable. Caches L1 queries per L1 block and recomputes predictions
24+
* for each mana usage estimate.
2525
*/
2626
export class FeePredictor {
2727
private cachedState: Promise<FeeOracleState> | undefined;
@@ -70,7 +70,7 @@ export class FeePredictor {
7070
const feeHeader = lastCheckpoint.feeHeader;
7171

7272
const slotConfig = { slotDuration: this.slotDuration, l1GenesisTime: this.l1GenesisTime };
73-
const slotCount = FEE_ORACLE_LAG + 1;
73+
const slotCount = FEE_ORACLE_LAG;
7474
const timestamps = times(slotCount, i => getTimestampForSlot(SlotNumber.add(nextSlot, i), slotConfig));
7575
const l1FeesBySlot = await Promise.all(timestamps.map(ts => this.rollupContract.getL1FeesAt(ts)));
7676

@@ -97,7 +97,7 @@ export class FeePredictor {
9797
// Slot 0: current state (next available slot after last checkpoint)
9898
result.push(this.computeGasFees(state, excessMana, ethPerFeeAsset, state.l1FeesBySlot[0]));
9999

100-
// Slots 1..LAG: advance excessMana with the assumed mana usage per checkpoint,
100+
// Slots 1..LAG-1: advance excessMana with the assumed mana usage per checkpoint,
101101
// and decay ethPerFeeAsset by MAX_FEE_ASSET_PRICE_MODIFIER_BPS per slot for conservative estimates.
102102
// Lower ethPerFeeAsset means higher fees in fee asset terms.
103103
for (let i = 1; i < state.l1FeesBySlot.length; i++) {

0 commit comments

Comments
 (0)