Skip to content

Commit 1db868c

Browse files
authored
fix(sequencer): anchor fee asset price modifier to predicted parent (#23113)
## Motivation Under proposer pipelining, checkpoint N's fee asset price modifier is computed in slot N-1 before checkpoint N-1 has landed on L1. The proposer was reading `rollupContract.getEthPerFeeAsset()`, which still reflects the latest published checkpoint (commonly N-2), while L1 later applies the modifier against checkpoint N-1's `ethPerFeeAsset`. The mismatch produced a 1-checkpoint drift between the proposer's intended new price and the price L1 actually stored, causing the e2e price-convergence test to oscillate around the target instead of converging. ## Approach Threads the predicted parent's `ethPerFeeAsset` through to the modifier computation. `buildPipelinedParentSimulationOverridesPlan` already derives that fee header for global-variable simulation overrides; the pending fee header on the resulting plan is used as the reference price for the bps calculation. Non-pipelined paths and genesis (checkpoint < 2) fall back to today's `getEthPerFeeAsset()` read. ## Changes - **ethereum**: `FeeAssetPriceOracle.computePriceModifier()` now takes an optional `currentPriceE12`; when supplied, the L1 read is skipped. - **sequencer-client**: `SequencerPublisher.getFeeAssetPriceModifier()` forwards the optional predicted price; `checkpoint_proposal_job` reads it from the pipelined simulation overrides plan and passes it through. - **end-to-end**: enables proposer pipelining + `inboxLag: 2` in `fee_asset_price_oracle_gossip.test.ts`. - **ethereum (tests)**: adds 3 unit tests covering the L1-read fallback, the predicted-price short-circuit, and concrete-value consistency with `RollupContract.computeChildFeeHeader` (asserting modifier truncation leaves a sub-bp gap to target). Note: if pipelining is enabled at checkpoint ≥ 2 but `proposedCheckpointData` is missing, the predicted-parent will be `undefined` and the code silently falls back to the stale L1 read. That's a pre-existing failure-mode behavior, not introduced here.
1 parent 6338c3f commit 1db868c

5 files changed

Lines changed: 94 additions & 10 deletions

File tree

yarn-project/end-to-end/src/e2e_p2p/fee_asset_price_oracle_gossip.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ describe('e2e_p2p_network', () => {
6363
slashingRoundSizeInEpochs: 2,
6464
slashingQuorum: 5,
6565
listenAddress: '127.0.0.1',
66+
enableProposerPipelining: true,
67+
inboxLag: 2,
6668
},
6769
});
6870

yarn-project/ethereum/src/contracts/fee_asset_price_oracle.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1+
import { jest } from '@jest/globals';
2+
import { type MockProxy, mock } from 'jest-mock-extended';
3+
4+
import type { ViemClient } from '../types.js';
15
import {
6+
FeeAssetPriceOracle,
27
MAX_FEE_ASSET_PRICE_MODIFIER_BPS,
38
sqrtPriceX96ToEthPerFeeAssetE12,
49
validateFeeAssetPriceModifier,
510
} from './fee_asset_price_oracle.js';
11+
import { RollupContract } from './rollup.js';
612

713
describe('Uniswap Price Oracle', () => {
814
describe('sqrtPriceX96ToEthPerFeeAssetE12', () => {
@@ -52,6 +58,70 @@ describe('Uniswap Price Oracle', () => {
5258
});
5359
});
5460

61+
describe('computePriceModifier', () => {
62+
let client: MockProxy<ViemClient>;
63+
let rollupContract: MockProxy<RollupContract>;
64+
let oracle: FeeAssetPriceOracle;
65+
const oraclePriceE12 = 10n ** 7n;
66+
67+
beforeEach(() => {
68+
client = mock<ViemClient>();
69+
rollupContract = mock<RollupContract>();
70+
oracle = new FeeAssetPriceOracle(client, rollupContract);
71+
72+
// Inject a stub uniswap oracle so we can drive the oracle price without touching the
73+
// real Uniswap V4 StateView contract.
74+
jest.spyOn(oracle, 'getUniswapOracle').mockResolvedValue({
75+
getMeanEthPerFeeAssetE12: () => Promise.resolve(oraclePriceE12),
76+
} as never);
77+
});
78+
79+
it('reads ethPerFeeAsset from L1 when no predicted price is provided', async () => {
80+
const onChainPriceE12 = 9_950_000n;
81+
rollupContract.getEthPerFeeAsset.mockResolvedValue(onChainPriceE12);
82+
83+
const modifier = await oracle.computePriceModifier();
84+
85+
expect(rollupContract.getEthPerFeeAsset).toHaveBeenCalledTimes(1);
86+
expect(modifier).toBe(oracle.computePriceModifierBps(onChainPriceE12, oraclePriceE12));
87+
});
88+
89+
it('uses the supplied predicted price and skips the L1 read when provided', async () => {
90+
const predictedPriceE12 = 10_050_000n;
91+
const modifier = await oracle.computePriceModifier(predictedPriceE12);
92+
93+
expect(rollupContract.getEthPerFeeAsset).not.toHaveBeenCalled();
94+
expect(modifier).toBe(oracle.computePriceModifierBps(predictedPriceE12, oraclePriceE12));
95+
});
96+
97+
it('emits the modifier that drives the predicted parent toward (but not exactly to) the target', async () => {
98+
// Pick a target within the ±100 bps cap so the test exercises truncation rather than the clamp.
99+
// P=10_100_000, T=10_150_000:
100+
// raw bps = floor((T - P) * 10_000 / P) = floor(50_000 * 10_000 / 10_100_000) = 49
101+
// child = floor(P * (10_000 + 49) / 10_000) = 10_149_490
102+
// Note 10_149_490 != 10_150_000 — bps truncation leaves the child ~510 LSB (~0.5 bp) shy
103+
// of the target. The e2e test depends on this sub-bp gap: convergence is monotonic and
104+
// within sub-bp of target, not exact equality.
105+
const predictedParentE12 = 10_100_000n;
106+
const targetE12 = 10_150_000n;
107+
jest.spyOn(oracle, 'getUniswapOracle').mockResolvedValue({
108+
getMeanEthPerFeeAssetE12: () => Promise.resolve(targetE12),
109+
} as never);
110+
111+
const modifier = await oracle.computePriceModifier(predictedParentE12);
112+
expect(modifier).toBe(49n);
113+
114+
const child = RollupContract.computeChildFeeHeader(
115+
{ excessMana: 0n, manaUsed: 0n, ethPerFeeAsset: predictedParentE12, congestionCost: 0n, proverCost: 0n },
116+
0n,
117+
modifier,
118+
100n,
119+
);
120+
expect(child.ethPerFeeAsset).toBe(10_149_490n);
121+
expect(child.ethPerFeeAsset).not.toBe(targetE12);
122+
});
123+
});
124+
55125
describe('validateFeeAssetPriceModifier', () => {
56126
it('accepts 0 modifier', () => {
57127
expect(validateFeeAssetPriceModifier(0n)).toBe(true);

yarn-project/ethereum/src/contracts/fee_asset_price_oracle.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,28 +63,33 @@ export class FeeAssetPriceOracle {
6363
*
6464
* Returns 0 if not on mainnet or if the oracle query fails.
6565
*
66+
* @param currentPriceE12 - Optional override for the parent checkpoint's eth-per-fee-asset
67+
* (E12 scale). When omitted, the latest published value is read from L1. Pipelined
68+
* proposers should supply the predicted parent value so the modifier they emit aligns
69+
* with the parent fee header L1 will see when the previous pipelined checkpoint lands.
6670
* @returns The price modifier in basis points (positive to increase price, negative to decrease)
6771
*/
68-
async computePriceModifier(): Promise<bigint> {
72+
async computePriceModifier(currentPriceE12?: bigint): Promise<bigint> {
6973
const uniswapOracle = await this.getUniswapOracle();
7074
if (!uniswapOracle) {
7175
return 0n;
7276
}
7377

7478
try {
75-
// Get current on-chain price (ETH per fee asset, E12)
76-
const currentPriceE12 = await this.rollupContract.getEthPerFeeAsset();
79+
// Get current on-chain price (ETH per fee asset, E12), preferring the caller-supplied value.
80+
const resolvedCurrentPriceE12 = currentPriceE12 ?? (await this.rollupContract.getEthPerFeeAsset());
7781

7882
// Get oracle price (median of last N blocks, ETH per fee asset, E12)
7983
const oraclePriceE12 = await uniswapOracle.getMeanEthPerFeeAssetE12();
8084

8185
// Compute modifier in basis points
82-
const modifier = this.computePriceModifierBps(currentPriceE12, oraclePriceE12);
86+
const modifier = this.computePriceModifierBps(resolvedCurrentPriceE12, oraclePriceE12);
8387

8488
this.log.debug('Computed price modifier', {
85-
currentPriceE12: currentPriceE12.toString(),
89+
currentPriceE12: resolvedCurrentPriceE12.toString(),
8690
oraclePriceE12: oraclePriceE12.toString(),
8791
modifierBps: modifier.toString(),
92+
currentPriceFromCaller: currentPriceE12 !== undefined,
8893
});
8994

9095
return modifier;

yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,14 @@ export class SequencerPublisher {
287287

288288
/**
289289
* Gets the fee asset price modifier from the oracle.
290-
* Returns 0n if the oracle query fails.
290+
*
291+
* @param predictedParentEthPerFeeAssetE12 - Optional predicted parent eth-per-fee-asset (E12).
292+
* Pipelined proposers should pass the value from the predicted parent fee header so the
293+
* modifier matches the parent L1 will use when applying it.
294+
* @returns The fee asset price modifier in basis points, or 0n if the oracle query fails.
291295
*/
292-
public getFeeAssetPriceModifier(): Promise<bigint> {
293-
return this.feeAssetPriceOracle.computePriceModifier();
296+
public getFeeAssetPriceModifier(predictedParentEthPerFeeAssetE12?: bigint): Promise<bigint> {
297+
return this.feeAssetPriceOracle.computePriceModifier(predictedParentEthPerFeeAssetE12);
294298
}
295299

296300
public getSenderAddress() {

yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -587,8 +587,11 @@ export class CheckpointProposalJob implements Traceable {
587587
.filter(c => c.checkpointNumber < this.checkpointNumber)
588588
.map(c => c.checkpointOutHash);
589589

590-
// Get the fee asset price modifier from the oracle
591-
const feeAssetPriceModifier = await this.publisher.getFeeAssetPriceModifier();
590+
// Anchor the modifier to the predicted parent fee header: L1 will apply it against
591+
// that, not against the latest published checkpoint (which lags by one under pipelining).
592+
const predictedParentEthPerFeeAssetE12 =
593+
this.pipelinedParentSimulationOverridesPlan?.pendingCheckpointState?.feeHeader?.ethPerFeeAsset;
594+
const feeAssetPriceModifier = await this.publisher.getFeeAssetPriceModifier(predictedParentEthPerFeeAssetE12);
592595

593596
// Create a long-lived forked world state for the checkpoint builder
594597
await using fork = await this.worldState.fork(this.syncedToBlockNumber, { closeDelayMs: 12_000 });

0 commit comments

Comments
 (0)