fix: use fee strategy promises properly with tryTwice

yarn-project/ethereum/src/l1_tx_utils/fee-strategies/fee-strategies.test.ts

@@ -0,0 +1,177 @@
+import { createLogger } from '@aztec/foundation/log';
+
+import { jest } from '@jest/globals';
+
+import type { ViemClient } from '../../types.js';
+import type { L1TxUtilsConfig } from '../config.js';
+import { P75AllTxsPriorityFeeStrategy } from './p75_competitive.js';
+import type { PriorityFeeStrategy, PriorityFeeStrategyContext } from './types.js';
+
+const logger = createLogger('ethereum:test:fee-strategies');
+
+describe('PriorityFeeStrategy', () => {
+  describe('execute runs all RPC calls internally', () => {
+    it('P75AllTxsPriorityFeeStrategy executes and returns priority fee with block data', async () => {
+      const mockClient = {
+        getBlock: jest.fn().mockImplementation((args: any) => {
+          if (args.blockTag === 'latest') {
+            return Promise.resolve({
+              number: 100n,
+              baseFeePerGas: 1000000000n,
+              transactions: [],
+            });
+          }
+          // pending block
+          return Promise.resolve({ transactions: [] });
+        }),
+        getBlobBaseFee: jest.fn<() => Promise<bigint>>().mockResolvedValue(100000000n),
+        estimateMaxPriorityFeePerGas: jest.fn<() => Promise<bigint>>().mockResolvedValue(1000000000n),
+        getFeeHistory: jest.fn<() => Promise<{ reward: bigint[][] }>>().mockResolvedValue({ reward: [[1000000000n]] }),
+      } as unknown as ViemClient;
+
+      const context: PriorityFeeStrategyContext = {
+        gasConfig: {} as L1TxUtilsConfig,
+        isBlobTx: false,
+        logger,
+      };
+
+      const result = await P75AllTxsPriorityFeeStrategy.execute(mockClient, context);
+
+      // Should return priority fee
+      expect(result.priorityFee).toBeGreaterThanOrEqual(0n);
+
+      // Should return latest block
+      expect(result.latestBlock).toBeDefined();
+      expect(result.latestBlock.number).toBe(100n);
+
+      // Should not return blob base fee for non-blob tx
+      expect(result.blobBaseFee).toBeUndefined();
+
+      // RPC methods should have been called
+      expect(mockClient.getBlock).toHaveBeenCalled();
+      expect(mockClient.estimateMaxPriorityFeePerGas).toHaveBeenCalled();
+      expect(mockClient.getFeeHistory).toHaveBeenCalled();
+    });
+
+    it('returns blob base fee for blob transactions', async () => {
+      const mockClient = {
+        getBlock: jest.fn().mockImplementation((args: any) => {
+          if (args.blockTag === 'latest') {
+            return Promise.resolve({
+              number: 100n,
+              baseFeePerGas: 1000000000n,
+              transactions: [],
+            });
+          }
+          return Promise.resolve({ transactions: [] });
+        }),
+        getBlobBaseFee: jest.fn<() => Promise<bigint>>().mockResolvedValue(500000000n),
+        estimateMaxPriorityFeePerGas: jest.fn<() => Promise<bigint>>().mockResolvedValue(1000000000n),
+        getFeeHistory: jest.fn<() => Promise<{ reward: bigint[][] }>>().mockResolvedValue({ reward: [[1000000000n]] }),
+      } as unknown as ViemClient;
+
+      const context: PriorityFeeStrategyContext = {
+        gasConfig: {} as L1TxUtilsConfig,
+        isBlobTx: true,
+        logger,
+      };
+
+      const result = await P75AllTxsPriorityFeeStrategy.execute(mockClient, context);
+
+      // Should return blob base fee for blob tx
+      expect(result.blobBaseFee).toBe(500000000n);
+      expect(mockClient.getBlobBaseFee).toHaveBeenCalled();
+    });
+
+    it('handles RPC failures gracefully', async () => {
+      const mockClient = {
+        getBlock: jest.fn().mockImplementation((args: any) => {
+          if (args.blockTag === 'latest') {
+            return Promise.resolve({
+              number: 100n,
+              baseFeePerGas: 1000000000n,
+              transactions: [],
+            });
+          }
+          return Promise.reject(new Error('RPC error'));
+        }),
+        getBlobBaseFee: jest.fn<() => Promise<bigint>>().mockRejectedValue(new Error('RPC error')),
+        estimateMaxPriorityFeePerGas: jest.fn<() => Promise<bigint>>().mockRejectedValue(new Error('RPC error')),
+        getFeeHistory: jest.fn<() => Promise<unknown>>().mockRejectedValue(new Error('RPC error')),
+      } as unknown as ViemClient;
+
+      const context: PriorityFeeStrategyContext = {
+        gasConfig: {} as L1TxUtilsConfig,
+        isBlobTx: false,
+        logger,
+      };
+
+      // Should not throw, should return a result (with fallback values)
+      const result = await P75AllTxsPriorityFeeStrategy.execute(mockClient, context);
+
+      // Strategy should handle failures gracefully (falls back to 0n)
+      expect(result.priorityFee).toBe(0n);
+      expect(result.latestBlock).toBeDefined();
+    });
+
+    it('throws if latest block fetch fails', async () => {
+      const mockClient = {
+        getBlock: jest.fn<() => Promise<any>>().mockRejectedValue(new Error('RPC error')),
+        getBlobBaseFee: jest.fn<() => Promise<bigint>>().mockResolvedValue(100000000n),
+        estimateMaxPriorityFeePerGas: jest.fn<() => Promise<bigint>>().mockResolvedValue(1000000000n),
+        getFeeHistory: jest.fn<() => Promise<{ reward: bigint[][] }>>().mockResolvedValue({ reward: [[1000000000n]] }),
+      } as unknown as ViemClient;
+
+      const context: PriorityFeeStrategyContext = {
+        gasConfig: {} as L1TxUtilsConfig,
+        isBlobTx: false,
+        logger,
+      };
+
+      // Should throw because latest block is required
+      await expect(P75AllTxsPriorityFeeStrategy.execute(mockClient, context)).rejects.toThrow(
+        'Failed to get latest block',
+      );
+    });
+  });
+
+  describe('custom strategy', () => {
+    it('allows custom strategies with execute function', async () => {
+      let executeCalled = false;
+
+      const customStrategy: PriorityFeeStrategy = {
+        id: 'test-custom',
+        name: 'Test Custom Strategy',
+        execute: async (client, _context) => {
+          executeCalled = true;
+          const latestBlock = await client.getBlock({ blockTag: 'latest' });
+          return {
+            priorityFee: 5000000000n,
+            latestBlock,
+            debugInfo: { custom: 'test' },
+          };
+        },
+      };
+
+      const mockClient = {
+        getBlock: jest.fn<() => Promise<any>>().mockResolvedValue({
+          number: 100n,
+          baseFeePerGas: 1000000000n,
+        }),
+      } as unknown as ViemClient;
+
+      const context: PriorityFeeStrategyContext = {
+        gasConfig: {} as L1TxUtilsConfig,
+        isBlobTx: false,
+        logger,
+      };
+
+      const result = await customStrategy.execute(mockClient, context);
+
+      expect(executeCalled).toBe(true);
+      expect(result.priorityFee).toBe(5000000000n);
+      expect(result.latestBlock.number).toBe(100n);
+      expect(result.debugInfo).toEqual({ custom: 'test' });
+    });
+  });
+});

yarn-project/ethereum/src/l1_tx_utils/fee-strategies/index.ts

@@ -4,13 +4,13 @@ import type { PriorityFeeStrategy } from './types.js';
 
 export {
   HISTORICAL_BLOCK_COUNT,
-  executeStrategy,
   type PriorityFeeStrategy,
   type PriorityFeeStrategyContext,
   type PriorityFeeStrategyResult,
 } from './types.js';
 
 export { P75AllTxsPriorityFeeStrategy } from './p75_competitive.js';
+export { P75BlobTxsOnlyPriorityFeeStrategy } from './p75_competitive_blob_txs_only.js';
 
 /**
  * Default list of priority fee strategies to analyze.

yarn-project/ethereum/src/l1_tx_utils/fee-strategies/p75_competitive.ts

@@ -12,59 +12,63 @@ import {
   type PriorityFeeStrategyResult,
 } from './types.js';
 
-/**
- * Type for the promises required by the competitive strategy
- */
-type P75AllTxsStrategyPromises = {
-  networkEstimate: Promise<bigint>;
-  pendingBlock: Promise<Awaited<ReturnType<ViemClient['getBlock']>> | null>;
-  feeHistory: Promise<Awaited<ReturnType<ViemClient['getFeeHistory']>> | null>;
-};
-
 /**
  * Our current competitive priority fee strategy.
  * Analyzes p75 of pending transactions and 5-block fee history to determine a competitive priority fee.
  * Falls back to network estimate if data is unavailable.
  */
-export const P75AllTxsPriorityFeeStrategy: PriorityFeeStrategy<P75AllTxsStrategyPromises> = {
+export const P75AllTxsPriorityFeeStrategy: PriorityFeeStrategy = {
   name: 'Competitive (P75 + History) - CURRENT',
   id: 'p75_pending_txs_and_history_all_txs',
 
-  getRequiredPromises(client: ViemClient): P75AllTxsStrategyPromises {
-    return {
-      networkEstimate: client.estimateMaxPriorityFeePerGas().catch(() => 0n),
-      pendingBlock: client.getBlock({ blockTag: 'pending', includeTransactions: true }).catch(() => null),
-      feeHistory: client
-        .getFeeHistory({
-          blockCount: HISTORICAL_BLOCK_COUNT,
-          rewardPercentiles: [75],
-          blockTag: 'latest',
-        })
-        .catch(() => null),
-    };
-  },
-
-  calculate(
-    results: {
-      [K in keyof P75AllTxsStrategyPromises]: PromiseSettledResult<Awaited<P75AllTxsStrategyPromises[K]>>;
-    },
-    context: PriorityFeeStrategyContext,
-  ): PriorityFeeStrategyResult {
-    const { logger } = context;
+  async execute(client: ViemClient, context: PriorityFeeStrategyContext): Promise<PriorityFeeStrategyResult> {
+    const { isBlobTx, logger } = context;
+
+    // Fire all RPC calls in parallel
+    const [latestBlockResult, blobBaseFeeResult, networkEstimateResult, pendingBlockResult, feeHistoryResult] =
+      await Promise.allSettled([
+        client.getBlock({ blockTag: 'latest' }),
+        isBlobTx ? client.getBlobBaseFee() : Promise.resolve(undefined),
+        client.estimateMaxPriorityFeePerGas().catch(() => 0n),
+        client.getBlock({ blockTag: 'pending', includeTransactions: true }).catch(() => null),
+        client
+          .getFeeHistory({
+            blockCount: HISTORICAL_BLOCK_COUNT,
+            rewardPercentiles: [75],
+            blockTag: 'latest',
+          })
+          .catch(() => null),
+      ]);
+
+    // Extract latest block
+    if (latestBlockResult.status === 'rejected') {
+      throw new Error(`Failed to get latest block: ${latestBlockResult.reason}`);
+    }
+    const latestBlock = latestBlockResult.value;
+
+    // Extract blob base fee (only for blob txs)
+    let blobBaseFee: bigint | undefined;
+    if (isBlobTx) {
+      if (blobBaseFeeResult.status === 'fulfilled' && typeof blobBaseFeeResult.value === 'bigint') {
+        blobBaseFee = blobBaseFeeResult.value;
+      } else {
+        logger?.warn('Failed to get L1 blob base fee');
+      }
+    }
 
-    // Extract network estimate from settled result
+    // Extract network estimate
     const networkEstimate =
-      results.networkEstimate.status === 'fulfilled' && typeof results.networkEstimate.value === 'bigint'
-        ? results.networkEstimate.value
+      networkEstimateResult.status === 'fulfilled' && typeof networkEstimateResult.value === 'bigint'
+        ? networkEstimateResult.value
         : 0n;
 
     let competitiveFee = networkEstimate;
     const debugInfo: Record<string, string | number> = {
       networkEstimateGwei: formatGwei(networkEstimate),
     };
 
-    // Extract pending block from settled result
-    const pendingBlock = results.pendingBlock.status === 'fulfilled' ? results.pendingBlock.value : null;
+    // Extract pending block
+    const pendingBlock = pendingBlockResult.status === 'fulfilled' ? pendingBlockResult.value : null;
 
     // Analyze pending block transactions
     if (pendingBlock?.transactions && pendingBlock.transactions.length > 0) {
@@ -73,9 +77,7 @@ export const P75AllTxsPriorityFeeStrategy: PriorityFeeStrategy<P75AllTxsStrategy
           if (typeof tx === 'string') {
             return 0n;
           }
-          const fee = tx.maxPriorityFeePerGas || 0n;
-
-          return fee;
+          return tx.maxPriorityFeePerGas || 0n;
         })
         .filter((fee: bigint) => fee > 0n);
 
@@ -97,8 +99,8 @@ export const P75AllTxsPriorityFeeStrategy: PriorityFeeStrategy<P75AllTxsStrategy
       }
     }
 
-    // Extract fee history from settled result
-    const feeHistory = results.feeHistory.status === 'fulfilled' ? results.feeHistory.value : null;
+    // Extract fee history
+    const feeHistory = feeHistoryResult.status === 'fulfilled' ? feeHistoryResult.value : null;
 
     // Analyze fee history
     if (feeHistory?.reward && feeHistory.reward.length > 0) {
@@ -153,6 +155,8 @@ export const P75AllTxsPriorityFeeStrategy: PriorityFeeStrategy<P75AllTxsStrategy
 
     return {
       priorityFee: competitiveFee,
+      latestBlock,
+      blobBaseFee,
       debugInfo,
     };
   },