Skip to content

Conversation

@spypsy
Copy link
Member

@spypsy spypsy commented Dec 23, 2025

small fix to return factories rather than triggered promises in fee strategies (tryTwice was not working properly)

@spypsy spypsy marked this pull request as ready for review December 23, 2025 16:50
const strategyPromisesArr = [];
for (const [key, promise] of Object.entries(strategyPromises)) {
// Get strategy promise factories for priority fee calculation
const strategyFactories = CurrentStrategy.getRequiredPromiseFactories(this.client, { isBlobTx });
Copy link
Collaborator

Choose a reason for hiding this comment

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

The way these strategies are currently used and implemented requires quite a bit of ugly non-typesafe code and back and forth between the stratgey and the caller. As I understand it, it's because the strategies don't have consistent interfaces. But in each case from what I can see, the flow is:

const strategyResults: Array<unknown> = await Promise.all(strategy.createSomePromises);
// ..... some other code goes here
const finalResult = strategy.calculate(strategyResults as any, someOtherArgs );

Ultimately I think all (both) strategies result in a feePerGas value. So why don't we just push everything to the strategy?

const feePerGas = await strategy.CalculateFee(this.client, { isBlobTx }); // This does everything required

Copy link
Collaborator

Choose a reason for hiding this comment

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

The latestBlock and blobBaseFee data are both retrieved from the instance of this.client so I'm not sure what else is even needed by the strategy.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does feel a bit overengineered but the reasoning was:

  1. define the promises separate to the strategy so that they can all get called in l1_tx_utils at the same time as the standard calls, latestBlock & blobBaseFee:
    const [latestBlockResult, blobBaseFeeResult, ...strategyResults] = await Promise.allSettled([
      latestBlockPromise,
      blobBaseFeePromise ?? Promise.resolve(0n),
      ...strategyPromisesArr,
    ]);

This way we fire all RPC requests at once and save time

  1. Using generic promises because other strategies may involve requests that are not done to an L1 RPC. e.g. we discussed about calculating profitability of the proposer which would involve fetching aztec token & ethereum prices

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants