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.

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

fix: blob fees & l1-publisher logging #11029

Merged
merged 45 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
8484168
fix: bump blob fees on retries
spypsy Jan 3, 2025
eb311d5
PR fixes
spypsy Jan 6, 2025
86b6693
Merge branch 'master' into spy/tx-utils-blobs
spypsy Jan 6, 2025
53b354e
Merge branch 'master' into spy/tx-utils-blobs
spypsy Jan 6, 2025
ce614fe
Try info logging to get blob fees
spypsy Jan 6, 2025
e7b3f71
log existing image in bootstrap
spypsy Jan 6, 2025
aa4e4a9
actually use calculated maxFeePerBlobGas
spypsy Jan 6, 2025
dc303b4
Merge branch 'master' into spy/tx-utils-blobs
spypsy Jan 6, 2025
258a904
min bump percentage is 100 for ALL fees
spypsy Jan 6, 2025
9981f97
Merge branch 'master' into spy/tx-utils-blobs
spypsy Jan 7, 2025
36b6582
max gwei for blob fees
spypsy Jan 7, 2025
6e9e8ac
log maxFeePerBlobGas
spypsy Jan 7, 2025
83c27f9
add metadata on speed-up attempts
spypsy Jan 7, 2025
c286d82
fix conversion
spypsy Jan 7, 2025
39de22e
add some better logging
spypsy Jan 8, 2025
a6a6308
Merge branch 'master' into spy/tx-utils-blobs
spypsy Jan 8, 2025
7b92aaf
fix import
spypsy Jan 8, 2025
d1442f7
Merge remote-tracking branch 'refs/remotes/origin/spy/tx-utils-blobs'…
spypsy Jan 8, 2025
c4783eb
fix logging, add test
spypsy Jan 8, 2025
e553adc
rm unused var
spypsy Jan 8, 2025
db53a66
disable faucet on sepolia
spypsy Jan 8, 2025
5e73f7e
actually use format fn...
spypsy Jan 8, 2025
3bb7df0
allow for fixed priority fee
spypsy Jan 8, 2025
aa0e244
truncate more log fields
spypsy Jan 9, 2025
5737628
don't use fixed blob fees
spypsy Jan 9, 2025
9a4fef2
fix maybe undefined
spypsy Jan 9, 2025
1da6523
values updates
spypsy Jan 9, 2025
11e448c
update typing
spypsy Jan 9, 2025
b04b1b1
fix test
spypsy Jan 9, 2025
379e286
try fixing logging again
spypsy Jan 9, 2025
6e95160
use formatter in sendTransaction
spypsy Jan 9, 2025
9fd9ec2
fix for <1 gwei prio fee
spypsy Jan 9, 2025
cfbe1d0
just format everywhere
spypsy Jan 9, 2025
89d1ec5
Merge branch 'master' into spy/tx-utils-blobs
spypsy Jan 9, 2025
9d1e93d
allow 2 decimals in percentages, set lower gas estimate buffer
spypsy Jan 9, 2025
7140c1e
update test
spypsy Jan 10, 2025
da68f1a
another
spypsy Jan 10, 2025
dec90b6
simplify formatting
spypsy Jan 10, 2025
45ee7f8
maintain shorter hexes
spypsy Jan 10, 2025
fa98417
fix test
spypsy Jan 10, 2025
99f4fe6
throw formatted error at the source
spypsy Jan 10, 2025
5cca260
add in 'failed to send transaction'
spypsy Jan 10, 2025
b52c269
remove commented code
spypsy Jan 10, 2025
b0b4f4e
Update yarn-project/ethereum/src/l1_tx_utils.ts
spypsy Jan 10, 2025
00a9b72
Update yarn-project/ethereum/src/utils.ts
spypsy Jan 10, 2025
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
2 changes: 2 additions & 0 deletions spartan/aztec-network/templates/faucet.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{{- if not .Values.ethereum.externalHost }}
apiVersion: apps/v1
kind: Deployment
metadata:
Expand Down Expand Up @@ -127,3 +128,4 @@ spec:
- protocol: TCP
port: {{ .Values.faucet.apiServerPort }}
targetPort: {{ .Values.faucet.apiServerPort }}
{{ end }}
4 changes: 4 additions & 0 deletions spartan/aztec-network/templates/validator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,10 @@ spec:
value: {{ .Values.validator.viemPollingInterval | quote }}
- name: SEQ_VIEM_POLLING_INTERVAL_MS
value: {{ .Values.validator.viemPollingInterval | quote }}
- name: L1_FIXED_PRIORITY_FEE_PER_GAS
value: {{ .Values.validator.l1FixedPriorityFeePerGas | quote }}
- name: L1_GAS_LIMIT_BUFFER_PERCENTAGE
value: {{ .Values.validator.l1GasLimitBufferPercentage | quote }}
- name: DATA_DIRECTORY
value: "{{ .Values.validator.dataDir }}"
- name: DATA_STORE_MAP_SIZE_KB
Expand Down
2 changes: 2 additions & 0 deletions spartan/aztec-network/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ validator:
viemPollingInterval: 1000
storageSize: "1Gi"
dataDir: "/data"
l1FixedPriorityFeePerGas: ""
l1GasLimitBufferPercentage: ""

proverNode:
proverPublisherPrivateKey: "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ ethereum:
deployL1ContractsPrivateKey:

validator:
l1FixedPriorityFeePerGas: 1
l1GasLimitBufferPercentage: 5
replicas: 48
validatorKeys:
validatorAddresses:
Expand Down Expand Up @@ -73,4 +75,4 @@ proverNode:
proverPublisherPrivateKey:

bot:
txIntervalSeconds: 20
txIntervalSeconds: 5
4 changes: 2 additions & 2 deletions yarn-project/end-to-end/src/e2e_p2p/p2p_network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class P2PNetworkTest {
this.logger.info('Syncing mock system time');
const { dateProvider, deployL1ContractsValues } = this.ctx!;
// Send a tx and only update the time after the tx is mined, as eth time is not continuous
const receipt = await this.gasUtils!.sendAndMonitorTransaction({
const { receipt } = await this.gasUtils!.sendAndMonitorTransaction({
to: this.baseAccount.address,
data: '0x',
value: 1n,
Expand Down Expand Up @@ -300,7 +300,7 @@ export class P2PNetworkTest {
this.ctx.deployL1ContractsValues.walletClient,
this.logger,
{
gasLimitBufferPercentage: 20n,
gasLimitBufferPercentage: 20,
maxGwei: 500n,
minGwei: 1n,
maxAttempts: 3,
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/ethereum/src/deploy_l1_contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ export async function deployL1Contract(
} else {
// Regular deployment path
const deployData = encodeDeployData({ abi, bytecode, args });
const receipt = await l1TxUtils.sendAndMonitorTransaction({
const { receipt } = await l1TxUtils.sendAndMonitorTransaction({
to: null,
data: deployData,
});
Expand Down
137 changes: 116 additions & 21 deletions yarn-project/ethereum/src/l1_tx_utils.test.ts
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';
Expand All @@ -19,6 +20,7 @@ import { foundry } from 'viem/chains';
import { EthCheatCodes } from './eth_cheat_codes.js';
import { L1TxUtils, defaultL1TxUtilsConfig } from './l1_tx_utils.js';
import { startAnvil } from './test/start_anvil.js';
import { formatViemError } from './utils.js';

const MNEMONIC = 'test test test test test test test test test test test junk';
const WEI_CONST = 1_000_000_000n;
Expand Down Expand Up @@ -71,7 +73,7 @@ describe('GasUtils', () => {
await cheatCodes.evmMine();

gasUtils = new L1TxUtils(publicClient, walletClient, logger, {
gasLimitBufferPercentage: 20n,
gasLimitBufferPercentage: 20,
maxGwei: 500n,
minGwei: 1n,
maxAttempts: 3,
Expand All @@ -92,7 +94,7 @@ describe('GasUtils', () => {
}, 5_000);

it('sends and monitors a simple transaction', async () => {
const receipt = await gasUtils.sendAndMonitorTransaction({
const { receipt } = await gasUtils.sendAndMonitorTransaction({
to: '0x1234567890123456789012345678901234567890',
data: '0x',
value: 0n,
Expand All @@ -106,8 +108,9 @@ describe('GasUtils', () => {
await cheatCodes.setAutomine(false);
await cheatCodes.setIntervalMining(0);

// Ensure initial base fee is low
await cheatCodes.setNextBlockBaseFeePerGas(initialBaseFee);
spalladino marked this conversation as resolved.
Show resolved Hide resolved
// Add blob data
const blobData = new Uint8Array(131072).fill(1);
const kzg = Blob.getViemKzgInstance();

const request = {
to: '0x1234567890123456789012345678901234567890' as `0x${string}`,
Expand All @@ -119,12 +122,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);
Expand All @@ -142,11 +149,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
Expand All @@ -156,11 +164,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 () => {
Expand All @@ -173,11 +182,14 @@ describe('GasUtils', () => {
// Mine a new block to make the base fee change take effect
await cheatCodes.evmMine();

const receipt = await gasUtils.sendAndMonitorTransaction({
to: '0x1234567890123456789012345678901234567890',
data: '0x',
value: 0n,
});
const { receipt } = await gasUtils.sendAndMonitorTransaction(
{
to: '0x1234567890123456789012345678901234567890',
data: '0x',
value: 0n,
},
{ maxGwei },
);

expect(receipt.effectiveGasPrice).toBeLessThanOrEqual(maxGwei * WEI_CONST);
}, 60_000);
Expand All @@ -189,15 +201,15 @@ describe('GasUtils', () => {

// First deploy without any buffer
const baselineGasUtils = new L1TxUtils(publicClient, walletClient, logger, {
gasLimitBufferPercentage: 0n,
gasLimitBufferPercentage: 0,
maxGwei: 500n,
minGwei: 10n, // Increased minimum gas price
maxAttempts: 5,
checkIntervalMs: 100,
stallTimeMs: 1000,
});

const baselineTx = await baselineGasUtils.sendAndMonitorTransaction({
const { receipt: baselineTx } = await baselineGasUtils.sendAndMonitorTransaction({
to: EthAddress.ZERO.toString(),
data: SIMPLE_CONTRACT_BYTECODE,
});
Expand All @@ -209,15 +221,15 @@ describe('GasUtils', () => {

// Now deploy with 20% buffer
const bufferedGasUtils = new L1TxUtils(publicClient, walletClient, logger, {
gasLimitBufferPercentage: 20n,
gasLimitBufferPercentage: 20,
maxGwei: 500n,
minGwei: 1n,
maxAttempts: 3,
checkIntervalMs: 100,
stallTimeMs: 1000,
});

const bufferedTx = await bufferedGasUtils.sendAndMonitorTransaction({
const { receipt: bufferedTx } = await bufferedGasUtils.sendAndMonitorTransaction({
to: EthAddress.ZERO.toString(),
data: SIMPLE_CONTRACT_BYTECODE,
});
Expand Down Expand Up @@ -256,7 +268,7 @@ describe('GasUtils', () => {
const initialGasPrice = await gasUtils['getGasPrice']();

// Get retry gas price for 2nd attempt
const retryGasPrice = await gasUtils['getGasPrice'](undefined, 1, initialGasPrice);
const retryGasPrice = await gasUtils['getGasPrice'](undefined, false, 1, initialGasPrice);

// With default config, retry should bump fees by 50%
const expectedPriorityFee = (initialGasPrice.maxPriorityFeePerGas * 150n) / 100n;
Expand All @@ -269,13 +281,13 @@ describe('GasUtils', () => {
it('respects minimum gas price bump for replacements', async () => {
const gasUtils = new L1TxUtils(publicClient, walletClient, logger, {
...defaultL1TxUtilsConfig,
priorityFeeRetryBumpPercentage: 5n, // Set lower than minimum 10%
priorityFeeRetryBumpPercentage: 5, // Set lower than minimum 10%
});

const initialGasPrice = await gasUtils['getGasPrice']();

// Get retry gas price with attempt = 1
const retryGasPrice = await gasUtils['getGasPrice'](undefined, 1, initialGasPrice);
const retryGasPrice = await gasUtils['getGasPrice'](undefined, false, 1, initialGasPrice);

// Should use 10% minimum bump even though config specified 5%
const expectedPriorityFee = (initialGasPrice.maxPriorityFeePerGas * 110n) / 100n;
Expand All @@ -300,6 +312,89 @@ describe('GasUtils', () => {
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: why is this? Does eth_estimateGas return both the execution gas and the blob gas mixed up together?

Copy link
Member Author

@spypsy spypsy Jan 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems in our estimateGas, we have to use prepareTransactionRequest to get a full estimation, which does increase a lot compared to estimateGas so I assumed that includes the blob gas

}, 20_000);

it('formats eth node errors correctly', async () => {
// Set base fee extremely high to trigger error
const extremelyHighBaseFee = WEI_CONST * 1_000_000n; // 1M gwei
await cheatCodes.setNextBlockBaseFeePerGas(extremelyHighBaseFee);
await cheatCodes.evmMine();

try {
await gasUtils.sendAndMonitorTransaction({
to: '0x1234567890123456789012345678901234567890',
data: '0x',
value: 0n,
});
fail('Should have thrown');
} catch (err: any) {
const formattedError = formatViemError(err);

// Verify the error contains actual newlines, not escaped \n
expect(formattedError).not.toContain('\\n');
expect(formattedError.split('\n').length).toBeGreaterThan(1);

// Check that we have the key error information
expect(formattedError).toContain('fee cap');

// Check request body formatting if present
if (formattedError.includes('Request body:')) {
const bodyStart = formattedError.indexOf('Request body:');
const body = formattedError.slice(bodyStart);
expect(body).toContain('eth_sendRawTransaction');
// Check params are truncated if too long
if (body.includes('0x')) {
expect(body).toContain('...');
}
}
}
}, 10_000);
it('stops trying after timeout', async () => {
await cheatCodes.setAutomine(false);
await cheatCodes.setIntervalMining(0);
Expand Down
Loading
Loading