Skip to content

Commit

Permalink
fix: guesstimate gas for propose (#8445)
Browse files Browse the repository at this point in the history 
Fixes #8444 by making a guesstimate on the amount of gas that we need.
We need to do these gymnastics instead of the classic "gasEstimate"
directly on the rollup contract as timestamps and simulations are not
the best of friends.
  • Loading branch information
@LHerskind
LHerskind authored Sep 10, 2024
1 parent 9bf81c0 commit bff0338
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 90 deletions.
3 changes: 3 additions & 0 deletions yarn-project/end-to-end/src/composed/integration_l1_publisher.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ describe('L1Publisher integration', () => {
`0x${block.header.toBuffer().toString('hex')}`,
`0x${block.archive.root.toBuffer().toString('hex')}`,
`0x${block.header.hash().toBuffer().toString('hex')}`,
[],
`0x${block.body.toBuffer().toString('hex')}`,
],
});
Expand Down Expand Up @@ -534,6 +535,7 @@ describe('L1Publisher integration', () => {
`0x${block.header.toBuffer().toString('hex')}`,
`0x${block.archive.root.toBuffer().toString('hex')}`,
`0x${block.header.hash().toBuffer().toString('hex')}`,
[],
`0x${block.body.toBuffer().toString('hex')}`,
],
})
Expand All @@ -544,6 +546,7 @@ describe('L1Publisher integration', () => {
`0x${block.header.toBuffer().toString('hex')}`,
`0x${block.archive.root.toBuffer().toString('hex')}`,
`0x${block.header.hash().toBuffer().toString('hex')}`,
[],
],
});
expect(ethTx.input).toEqual(expectedData);
Expand Down
29 changes: 20 additions & 9 deletions yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ interface MockAvailabilityOracleWrite {
publish: (args: readonly [`0x${string}`], options: { account: PrivateKeyAccount }) => Promise<`0x${string}`>;
}

interface MockAvailabilityOracleEstimate {
publish: (args: readonly [`0x${string}`], options: { account: PrivateKeyAccount }) => Promise<bigint>;
}

interface MockAvailabilityOracleRead {
isAvailable: (args: readonly [`0x${string}`]) => Promise<boolean>;
}
Expand All @@ -21,6 +25,7 @@ class MockAvailabilityOracle {
constructor(
public write: MockAvailabilityOracleWrite,
public simulate: MockAvailabilityOracleWrite,
public estimateGas: MockAvailabilityOracleEstimate,
public read: MockAvailabilityOracleRead,
) {}
}
Expand Down Expand Up @@ -69,6 +74,7 @@ describe('L1Publisher', () => {
let availabilityOracleRead: MockProxy<MockAvailabilityOracleRead>;
let availabilityOracleWrite: MockProxy<MockAvailabilityOracleWrite>;
let availabilityOracleSimulate: MockProxy<MockAvailabilityOracleWrite>;
let availabilityOracleEstimate: MockProxy<MockAvailabilityOracleEstimate>;
let availabilityOracle: MockAvailabilityOracle;

let publicClient: MockProxy<MockPublicClient>;
Expand All @@ -88,6 +94,8 @@ describe('L1Publisher', () => {

let publisher: L1Publisher;

const GAS_GUESS = 300_000n;

beforeEach(() => {
l2Block = L2Block.random(42);

Expand All @@ -97,7 +105,7 @@ describe('L1Publisher', () => {
body = l2Block.body.toBuffer();

processTxHash = `0x${Buffer.from('txHashProcess').toString('hex')}`; // random tx hash
proposeTxHash = `0x${Buffer.from('txHashpropose').toString('hex')}`; // random tx hash
proposeTxHash = `0x${Buffer.from('txHashPropose').toString('hex')}`; // random tx hash

processTxReceipt = {
transactionHash: processTxHash,
Expand All @@ -118,9 +126,11 @@ describe('L1Publisher', () => {
availabilityOracleWrite = mock<MockAvailabilityOracleWrite>();
availabilityOracleRead = mock<MockAvailabilityOracleRead>();
availabilityOracleSimulate = mock<MockAvailabilityOracleWrite>();
availabilityOracleEstimate = mock<MockAvailabilityOracleEstimate>();
availabilityOracle = new MockAvailabilityOracle(
availabilityOracleWrite,
availabilityOracleSimulate,
availabilityOracleEstimate,
availabilityOracleRead,
);

Expand All @@ -146,13 +156,15 @@ describe('L1Publisher', () => {
account = (publisher as any)['account'];

rollupContractRead.getCurrentSlot.mockResolvedValue(l2Block.header.globalVariables.slotNumber.toBigInt());
availabilityOracleEstimate.publish.mockResolvedValueOnce(GAS_GUESS);
publicClient.getBlock.mockResolvedValue({ timestamp: 12n });
});

it('publishes and propose l2 block to l1', async () => {
rollupContractRead.archive.mockResolvedValue(l2Block.header.lastArchive.root.toString() as `0x${string}`);
rollupContractWrite.propose.mockResolvedValueOnce(proposeTxHash);
rollupContractSimulate.propose.mockResolvedValueOnce(proposeTxHash);

publicClient.getTransactionReceipt.mockResolvedValueOnce(proposeTxReceipt);

const result = await publisher.processL2Block(l2Block);
Expand All @@ -163,12 +175,13 @@ describe('L1Publisher', () => {
`0x${header.toString('hex')}`,
`0x${archive.toString('hex')}`,
`0x${blockHash.toString('hex')}`,
[],
`0x${body.toString('hex')}`,
] as const;
if (!L1Publisher.SKIP_SIMULATION) {
expect(rollupContractSimulate.propose).toHaveBeenCalledWith(args, { account: account });
}
expect(rollupContractWrite.propose).toHaveBeenCalledWith(args, { account: account });
expect(rollupContractWrite.propose).toHaveBeenCalledWith(args, {
account: account,
gas: L1Publisher.PROPOSE_GAS_GUESS + GAS_GUESS * 2n,
});
expect(publicClient.getTransactionReceipt).toHaveBeenCalledWith({ hash: proposeTxHash });
});

Expand All @@ -186,11 +199,9 @@ describe('L1Publisher', () => {
`0x${header.toString('hex')}`,
`0x${archive.toString('hex')}`,
`0x${blockHash.toString('hex')}`,
[],
] as const;
if (!L1Publisher.SKIP_SIMULATION) {
expect(rollupContractSimulate.propose).toHaveBeenCalledWith(args, { account });
}
expect(rollupContractWrite.propose).toHaveBeenCalledWith(args, { account });
expect(rollupContractWrite.propose).toHaveBeenCalledWith(args, { account, gas: L1Publisher.PROPOSE_GAS_GUESS });
expect(publicClient.getTransactionReceipt).toHaveBeenCalledWith({ hash: processTxHash });
});

Expand Down
127 changes: 46 additions & 81 deletions yarn-project/sequencer-client/src/publisher/l1-publisher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,6 @@ export type L1SubmitProofArgs = {
* Adapted from https://github.com/AztecProtocol/aztec2-internal/blob/master/falafel/src/rollup_publisher.ts.
*/
export class L1Publisher {
// @note If we want to simulate in the future, we have to skip the viem simulations and use `reads` instead
// This is because the viem simulations are not able to simulate the future, only the current state.
// This means that we will be simulating as if `block.timestamp` is the same for the next block
// as for the last block.
// Nevertheless, it can be quite useful for figuring out why exactly the transaction is failing
// as a middle ground right now, we will be skipping the simulation and just sending the transaction
// but only after we have done a successful run of the `validateHeader` for the timestamp in the future.
public static SKIP_SIMULATION = true;

private interruptibleSleep = new InterruptibleSleep();
private sleepTimeMs: number;
private interrupted = false;
Expand All @@ -123,6 +114,8 @@ export class L1Publisher {
private publicClient: PublicClient<HttpTransport, chains.Chain>;
private account: PrivateKeyAccount;

public static PROPOSE_GAS_GUESS: bigint = 500_000n;

constructor(config: TxSenderConfig & PublisherConfig, client: TelemetryClient) {
this.sleepTimeMs = config?.l1PublishRetryIntervalMS ?? 60_000;
this.metrics = new L1PublisherMetrics(client, 'L1Publisher');
Expand Down Expand Up @@ -400,11 +393,9 @@ export class L1Publisher {
`0x${proof.toString('hex')}`,
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.submitBlockRootProof(args, {
account: this.account,
});
}
await this.rollupContract.simulate.submitBlockRootProof(args, {
account: this.account,
});

return await this.rollupContract.write.submitBlockRootProof(args, {
account: this.account,
Expand Down Expand Up @@ -439,36 +430,20 @@ export class L1Publisher {
private async sendProposeWithoutBodyTx(encodedData: L1ProcessArgs): Promise<string | undefined> {
if (!this.interrupted) {
try {
if (encodedData.attestations) {
const attestations = encodedData.attestations.map(attest => attest.toViemSignature());
const args = [
`0x${encodedData.header.toString('hex')}`,
`0x${encodedData.archive.toString('hex')}`,
`0x${encodedData.blockHash.toString('hex')}`,
attestations,
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.propose(args, { account: this.account });
}

return await this.rollupContract.write.propose(args, {
account: this.account,
});
} else {
const args = [
`0x${encodedData.header.toString('hex')}`,
`0x${encodedData.archive.toString('hex')}`,
`0x${encodedData.blockHash.toString('hex')}`,
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.propose(args, { account: this.account });
}
return await this.rollupContract.write.propose(args, {
account: this.account,
});
}
const attestations = encodedData.attestations
? encodedData.attestations.map(attest => attest.toViemSignature())
: [];
const args = [
`0x${encodedData.header.toString('hex')}`,
`0x${encodedData.archive.toString('hex')}`,
`0x${encodedData.blockHash.toString('hex')}`,
attestations,
] as const;

return await this.rollupContract.write.propose(args, {
account: this.account,
gas: L1Publisher.PROPOSE_GAS_GUESS,
});
} catch (err) {
this.log.error(`Rollup publish failed`, err);
return undefined;
Expand All @@ -479,43 +454,33 @@ export class L1Publisher {
private async sendProposeTx(encodedData: L1ProcessArgs): Promise<string | undefined> {
if (!this.interrupted) {
try {
if (encodedData.attestations) {
const attestations = encodedData.attestations.map(attest => attest.toViemSignature());
const args = [
`0x${encodedData.header.toString('hex')}`,
`0x${encodedData.archive.toString('hex')}`,
`0x${encodedData.blockHash.toString('hex')}`,
attestations,
`0x${encodedData.body.toString('hex')}`,
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.propose(args, {
account: this.account,
});
}

return await this.rollupContract.write.propose(args, {
account: this.account,
});
} else {
const args = [
`0x${encodedData.header.toString('hex')}`,
`0x${encodedData.archive.toString('hex')}`,
`0x${encodedData.blockHash.toString('hex')}`,
`0x${encodedData.body.toString('hex')}`,
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.propose(args, {
account: this.account,
});
}

return await this.rollupContract.write.propose(args, {
account: this.account,
});
}
const publishGas = await this.availabilityOracleContract.estimateGas.publish([
`0x${encodedData.body.toString('hex')}`,
]);
const min = (a: bigint, b: bigint) => (a > b ? b : a);

// @note We perform this guesstimate instead of the usual `gasEstimate` since
// viem will use the current state to simulate against, which means that
// we will fail estimation in the case where we are simulating for the
// first ethereum block within our slot (as current time is not in the
// slot yet).
const gasGuesstimate = min(publishGas * 2n + L1Publisher.PROPOSE_GAS_GUESS, 15_000_000n);

const attestations = encodedData.attestations
? encodedData.attestations.map(attest => attest.toViemSignature())
: [];
const args = [
`0x${encodedData.header.toString('hex')}`,
`0x${encodedData.archive.toString('hex')}`,
`0x${encodedData.blockHash.toString('hex')}`,
attestations,
`0x${encodedData.body.toString('hex')}`,
] as const;

return await this.rollupContract.write.propose(args, {
account: this.account,
gas: gasGuesstimate,
});
} catch (err) {
this.log.error(`Rollup publish failed`, err);
return undefined;
Expand Down

0 comments on commit bff0338

Please sign in to comment.