-
Notifications
You must be signed in to change notification settings - Fork 615
fix: blob fees & l1-publisher logging #11029
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.
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
Changes from 1 commit
8484168
eb311d5
86b6693
53b354e
ce614fe
e7b3f71
aa4e4a9
dc303b4
258a904
9981f97
36b6582
6e9e8ac
83c27f9
c286d82
39de22e
a6a6308
7b92aaf
d1442f7
c4783eb
e553adc
db53a66
5e73f7e
3bb7df0
aa0e244
5737628
9a4fef2
1da6523
11e448c
b04b1b1
379e286
6e95160
9fd9ec2
cfbe1d0
89d1ec5
9d1e93d
7140c1e
da68f1a
dec90b6
45ee7f8
fa98417
99f4fe6
5cca260
b52c269
b0b4f4e
00a9b72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { Blob } from '@aztec/foundation/blob'; | ||
| import { EthAddress } from '@aztec/foundation/eth-address'; | ||
| import { createLogger } from '@aztec/foundation/log'; | ||
| import { sleep } from '@aztec/foundation/sleep'; | ||
|
|
@@ -106,8 +107,9 @@ describe('GasUtils', () => { | |
| await cheatCodes.setAutomine(false); | ||
| await cheatCodes.setIntervalMining(0); | ||
|
|
||
| // Ensure initial base fee is low | ||
| await cheatCodes.setNextBlockBaseFeePerGas(initialBaseFee); | ||
| // Add blob data | ||
| const blobData = new Uint8Array(131072).fill(1); | ||
| const kzg = Blob.getViemKzgInstance(); | ||
|
|
||
| const request = { | ||
| to: '0x1234567890123456789012345678901234567890' as `0x${string}`, | ||
|
|
@@ -119,12 +121,16 @@ describe('GasUtils', () => { | |
|
|
||
| const originalMaxFeePerGas = WEI_CONST * 10n; | ||
| const originalMaxPriorityFeePerGas = WEI_CONST; | ||
| const originalMaxFeePerBlobGas = WEI_CONST * 10n; | ||
|
|
||
| const txHash = await walletClient.sendTransaction({ | ||
| ...request, | ||
| gas: estimatedGas, | ||
| maxFeePerGas: originalMaxFeePerGas, | ||
| maxPriorityFeePerGas: originalMaxPriorityFeePerGas, | ||
| blobs: [blobData], | ||
| kzg, | ||
| maxFeePerBlobGas: originalMaxFeePerBlobGas, | ||
| }); | ||
|
|
||
| const rawTx = await cheatCodes.getRawTransaction(txHash); | ||
|
|
@@ -142,11 +148,12 @@ describe('GasUtils', () => { | |
| params: [rawTx], | ||
| }); | ||
|
|
||
| // keeping auto-mining disabled to simulate a stuck transaction | ||
| // The monitor should detect the stall and create a replacement tx | ||
|
|
||
| // Monitor should detect stall and replace with higher gas price | ||
| const monitorFn = gasUtils.monitorTransaction(request, txHash, { gasLimit: estimatedGas }); | ||
| const monitorFn = gasUtils.monitorTransaction(request, txHash, { gasLimit: estimatedGas }, undefined, { | ||
| blobs: [blobData], | ||
| kzg, | ||
| maxFeePerBlobGas: WEI_CONST * 20n, | ||
| }); | ||
|
|
||
| await sleep(2000); | ||
| // re-enable mining | ||
|
|
@@ -156,11 +163,12 @@ describe('GasUtils', () => { | |
| // Verify that a replacement transaction was created | ||
| expect(receipt.transactionHash).not.toBe(txHash); | ||
|
|
||
| // Get details of replacement tx to verify higher gas price | ||
| // Get details of replacement tx to verify higher gas prices | ||
| const replacementTx = await publicClient.getTransaction({ hash: receipt.transactionHash }); | ||
|
|
||
| expect(replacementTx.maxFeePerGas!).toBeGreaterThan(originalMaxFeePerGas); | ||
| expect(replacementTx.maxPriorityFeePerGas!).toBeGreaterThan(originalMaxPriorityFeePerGas); | ||
| expect(replacementTx.maxFeePerBlobGas!).toBeGreaterThan(originalMaxFeePerBlobGas); | ||
| }, 20_000); | ||
|
|
||
| it('respects max gas price limits during spikes', async () => { | ||
|
|
@@ -299,4 +307,53 @@ describe('GasUtils', () => { | |
| const expectedEstimate = baseEstimate + (baseEstimate * 20n) / 100n; | ||
| expect(bufferedEstimate).toBe(expectedEstimate); | ||
| }); | ||
|
|
||
| it('correctly handles transactions with blobs', async () => { | ||
| // Create a sample blob | ||
| const blobData = new Uint8Array(131072).fill(1); // 128KB blob | ||
| const kzg = Blob.getViemKzgInstance(); | ||
|
|
||
| const receipt = await gasUtils.sendAndMonitorTransaction( | ||
| { | ||
| to: '0x1234567890123456789012345678901234567890', | ||
| data: '0x', | ||
| value: 0n, | ||
| }, | ||
| undefined, | ||
| { | ||
| blobs: [blobData], | ||
| kzg, | ||
| maxFeePerBlobGas: 10000000000n, // 10 gwei | ||
| }, | ||
| ); | ||
|
|
||
| expect(receipt.status).toBe('success'); | ||
| expect(receipt.blobGasUsed).toBeDefined(); | ||
| expect(receipt.blobGasPrice).toBeDefined(); | ||
| }, 20_000); | ||
|
|
||
| it('estimates gas correctly for blob transactions', async () => { | ||
| // Create a sample blob | ||
| const blobData = new Uint8Array(131072).fill(1); // 128KB blob | ||
| const kzg = Blob.getViemKzgInstance(); | ||
|
|
||
| const request = { | ||
| to: '0x1234567890123456789012345678901234567890' as `0x${string}`, | ||
| data: '0x' as `0x${string}`, | ||
| value: 0n, | ||
| }; | ||
|
|
||
| // Estimate gas without blobs first | ||
| const baseEstimate = await gasUtils.estimateGas(walletClient.account!, request); | ||
|
|
||
| // Estimate gas with blobs | ||
| const blobEstimate = await gasUtils.estimateGas(walletClient.account!, request, undefined, { | ||
| blobs: [blobData], | ||
| kzg, | ||
| maxFeePerBlobGas: 10000000000n, | ||
| }); | ||
|
|
||
| // Blob transactions should require more gas | ||
| expect(blobEstimate).toBeGreaterThan(baseEstimate); | ||
|
Comment on lines
+359
to
+360
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious: why is this? Does
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems in our |
||
| }, 20_000); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,6 +149,7 @@ export interface L1BlobInputs { | |
| interface GasPrice { | ||
| maxFeePerGas: bigint; | ||
| maxPriorityFeePerGas: bigint; | ||
| maxFeePerBlobGas?: bigint; | ||
| } | ||
|
|
||
| export class L1TxUtils { | ||
|
|
@@ -301,7 +302,11 @@ export class L1TxUtils { | |
| gasConfig, | ||
| attempts, | ||
| tx.maxFeePerGas && tx.maxPriorityFeePerGas | ||
| ? { maxFeePerGas: tx.maxFeePerGas, maxPriorityFeePerGas: tx.maxPriorityFeePerGas } | ||
| ? { | ||
| maxFeePerGas: tx.maxFeePerGas, | ||
| maxPriorityFeePerGas: tx.maxPriorityFeePerGas, | ||
| maxFeePerBlobGas: tx.maxFeePerBlobGas, | ||
| } | ||
| : undefined, | ||
| ); | ||
|
|
||
|
|
@@ -365,6 +370,15 @@ export class L1TxUtils { | |
| const block = await this.publicClient.getBlock({ blockTag: 'latest' }); | ||
| const baseFee = block.baseFeePerGas ?? 0n; | ||
|
|
||
| // Get blob base fee if available | ||
| let blobBaseFee = 0n; | ||
| try { | ||
| const blobBaseFeeHex = await this.publicClient.request({ method: 'eth_blobBaseFee' }); | ||
| blobBaseFee = BigInt(blobBaseFeeHex); | ||
|
spalladino marked this conversation as resolved.
|
||
| } catch { | ||
| // Ignore if not supported | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's at least log this as a |
||
| } | ||
|
Comment on lines
+428
to
+435
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we just skip all this if |
||
|
|
||
| // Get initial priority fee from the network | ||
| let priorityFee = await this.publicClient.estimateMaxPriorityFeePerGas(); | ||
| let maxFeePerGas = baseFee; | ||
|
|
@@ -405,14 +419,28 @@ export class L1TxUtils { | |
| // Ensure priority fee doesn't exceed max fee | ||
| const maxPriorityFeePerGas = priorityFee > maxFeePerGas ? maxFeePerGas : priorityFee; | ||
|
|
||
| if (attempt > 0 && previousGasPrice?.maxFeePerBlobGas) { | ||
| const bumpPercentage = | ||
| gasConfig.priorityFeeRetryBumpPercentage! > MIN_REPLACEMENT_BUMP_PERCENTAGE | ||
| ? gasConfig.priorityFeeRetryBumpPercentage! | ||
| : MIN_REPLACEMENT_BUMP_PERCENTAGE; | ||
|
|
||
| blobBaseFee = (previousGasPrice.maxFeePerBlobGas * (100n + bumpPercentage)) / 100n; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we use the Also, we need to account for the current blob base fee, and do something similar to what we do for the calldata fee: use the minimum between the current chain blob base fee (bumped by 12.5% per stall time) and geth's minimum bump (10%) wrt the previous price.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for that 🙏
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yup. I learned that the hard way. Anything related to mempool management is out of protocol, so it depends on the clients, and is awfully undocumented. |
||
| } | ||
|
|
||
| this.logger?.debug(`Computed gas price`, { | ||
| attempt, | ||
| baseFee: formatGwei(baseFee), | ||
| maxFeePerGas: formatGwei(maxFeePerGas), | ||
| maxPriorityFeePerGas: formatGwei(maxPriorityFeePerGas), | ||
| ...(blobBaseFee && { maxFeePerBlobGas: formatGwei(blobBaseFee) }), | ||
| }); | ||
|
|
||
| return { maxFeePerGas, maxPriorityFeePerGas }; | ||
| return { | ||
| maxFeePerGas, | ||
| maxPriorityFeePerGas, | ||
| ...(blobBaseFee && { maxFeePerBlobGas: blobBaseFee }), | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.