Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: guesstimate gas for propose #8445

Merged
merged 1 commit into from
Sep 10, 2024
Merged

fix: guesstimate gas for propose #8445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions yarn-project/end-to-end/src/composed/integration_l1_publisher.test.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading