Skip to content

Commit

Permalink
fix: guesstimate gas for propose
Browse files Browse the repository at this point in the history
  • Loading branch information
@LHerskind
LHerskind committed Sep 9, 2024
1 parent dc43306 commit a432843
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 a432843

Please sign in to comment.