Add new FuelGasPriceProvider

[MitchTurner]

Part of: #1624

Here is the current specs for the gas price algorithm:
https://www.notion.so/fuellabs/Gas-Price-Algo-V1-Specs-2e7a06263c4f46928b6d179e90a4ba1a

Introduce a new type to replace StaticGasPrice that will actually calculate new gas prices based on historical data.

This work is largely exploratory since we don't know exactly what the algorithm will land on. But this PR includes work defining the architecture of our FuelGasPriceProvider as well as some exploratory work defining a proof-of-concept algorithm. This is the main reason this PR is still in "Draft".

The current design separates the algorithm from the FuelBlockProvider. This helps identify what behavior is independent of the specifc algorithm we end up choosing and helps us determine the minimum set of variables the algorithm requires. Currently the algorithm looks like this:

pub trait GasPriceAlgorithm {
    fn calculate_da_gas_price(
        &self,
        previous_da_gas_prices: u64,
        total_production_reward: u64,
        total_da_recording_cost: u64,
    ) -> u64;

    fn calculate_execution_gas_price(
        &self,
        previous_execution_gas_prices: u64,
        block_fullness: BlockFullness,
    ) -> u64;

    fn maximum_next_gas_prices(&self, previous_gas_prices: GasPrices) -> GasPrices;
}

The actual impllementation of the FuelGasPriceAlgo for now is going to take inspiration from a PID control loop on the DA gas price side, but ideally as simple as possible and as deterministic as possible (we don't want to use floats in places like BlockFullness).

Architecture Diagram (WIP, I'll try to update as it changes). CLICK IF NOT SHOWING UP PROPERLY IN DARK MODE:

Checklist

• Breaking changes are clearly marked as such in the PR description and changelog
• New behavior is reflected in tests
• The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

• I have reviewed the code myself
• I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

[Add or remove entries as needed]

• Rust SDK
• Sway compiler
• Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
• Someone else?

[MitchTurner]

We might want to take into account the average number of txs per block. Or we could get a similar result with an "I" value (PID).

[MitchTurner]

We need to take into account the fact that DA recording has some lag to it. Possibly 12-24 blocks? How does that effect the gas_price calculation? The current design doesn't take that into account.

[MitchTurner]

Implemented a solution for this. The DA gas price is only updated when the da recorded height changes. Might need more testing.

[xgreenx]

You can remove this file, it is not used anymore=)

[xgreenx]

I think it is better to return anyhow::Error instead of Box<dyn ...>

[MitchTurner]

I guess so. Since our "Producer Error" is anyhow::Error.

There are other places where I might invert the error dep for the adapters as well.

I'm torn on this in Rust in general (and have been for years), because adapters necessarily have their own error type, so you have to use type erasure. And because of that, I figured we could just put the domain errors on the business side rather than expecting the adapter to return the right error type.

For example:

        let previous_gas_prices = self
            .gas_price_history
            .gas_prices(latest_block_height)
            .map_err(Error::UnableToGetGasPrices)?
            .ok_or(Error::GasPricesNotFoundForBlockHeight(latest_block_height))?;

But that's noisy, and we are already trusting the adapter code to uphold the contract of the port, so maybe we just push into the adapters instead of using this ForiegnError construct that I used.

[xgreenx]

We don't need to calculate the new gas price in this getter. We need to have a separate function that generates the next gas price without storing it in the database. We need to store it in the database when we receive a new block from BlockImporter.

Also, maybe we want to pass the size of transactions like calculate_next_gas_price(txs_size: usize) -> u64. But about this in another comment=)

[MitchTurner]

So, you're suggesting that the GasPriceProvider trait should have more methods? Like maybe a get_gas_price, a new_gas_price and a write_gas_price?

We create a new gas price, build the block, and then write it once it's built?

I don't know if the producer trait GasPriceProvider will ever need to get historical gas prices. But some of the other traits might.

[MitchTurner]

I remember why I'm storing it here. Because we only have one gas_price, but under the hood we need to record the da_gas_price and the exec_gas_price separately.

But you're right, if that block fails for some reason, then we have wrong values. I think that's fine because we are getting latest_block_height independently from requested_block_height.

That means that this should never be a getter though. So it assumes that the requested block is always the head of the chain and it can remove "bad branches".

[xgreenx]

I think the next price should be calculated without touching the database. The service itself should contain all the information in memory required to calculate the next gas price. During the service's creation, we will fetch everything that we need, and the service updates the inner state(and database as well) after each imported block. It should simplify the code and remove the return of errors from calculate_next_gas_price function.

[MitchTurner]

Okay. If we are able to move the logic for updating DA information in the non-live scenarios into the algorithm, then we can get rid of database tracking outside of the algorithm.

[xgreenx]

Since the records are always behind for some period of time, we should always go into this branch. But since self.profitablility_totals.totaled_block_height() depends on the number of self.update_totals calls, it will not happen. But because of that, it creates very unintuitive code.

What do you think if we always call self.algorithm.calculate_da_gas_price, but the self.total_reward() and self.total_cost() are different? We can achieve almost the same effect just by advancing reward and cost.

Because we always don't have live data from DA, we can predict the cost based on the previous blocks. We can calculate the da gas price per byte based on data from the past(total_cost/total_bytes). Using this price, we can calculate the cost for the upcoming block(txs_size * calculated_gas_price). The total_cost is known_cost_from_the_past + cumulutive_predicted_cost. Using total_cost, you can calculate the next gas price, which will give you a reward.

[MitchTurner]

Because we always don't have live data from DA, we can predict the cost based on the previous blocks. We can calculate the da gas price per byte based on data from the past(total_cost/total_bytes). Using this price, we can calculate the cost for the upcoming block(txs_size * calculated_gas_price). The total_cost is known_cost_from_the_past + cumulutive_predicted_cost. Using total_cost, you can calculate the next gas price, which will give you a reward.

This is what Optimism does:

The L1 Data Fee is automatically charged for any transaction that is included in an OP Mainnet block. This fee is deducted directly from the address that sent the transaction. The exact amount paid depends on the estimated size of the transaction in bytes after compression, the current Ethereum gas price and/or blob gas price, and several small parameters.

We don't have the luxury of deducting from the address. And we decided to only have one gas price. So currently we calculate the gas price before we add txs to the block, when we don't know how many bytes the txs will take up in that block. Of course, we could use dry_run to calculate the max fee, so that's still an option if we want to separate them.

[MitchTurner]

Also relevant from Optimism:

It is currently not possible to limit the maximum L1 Data Fee that a transaction is willing to pay. This is the result of limitations in existing Ethereum transaction formats and tradeoffs that the OP Stack makes to retain better EVM equivalence. Work is underway to improve this situation with the introduction of a new standardized transaction type. For now, you should be aware that the L1 Data Fee can fluctuate with the Ethereum gas price.

[xgreenx]

I think the previous block's fullness should be part of the GasProviderService. On each block imported from the block importer, the service will update its internal fullness variables and dump new values into the database.

[MitchTurner]

I'm not sure what you mean GasProviderService. Right now the producer, the txpool, and graphql each get a copy of the impl GasPriceProvider for StaticGasPrice.

That's how I imagine it will still work, but just with this new FuelGasPriceProvider instead of StaticGasPrice. I'm assuming that all the GasPriceProvider needs is a BlockHeight and then it has access to all the DBs it needs to assemble the gas price. I don't know why it needs to store block_fullness when it can just look up the block's data in its DB.

[xgreenx]

Yeah, it should be mutable. The gas price provider service should have exclusive ownership of the database(we can create a separate database for it).

[MitchTurner]

gas price provider service

I don't think of this as being a service. I just thing of it as being an adapter handed to the producer, txpool, and graphql like here:

fuel-core/crates/services/producer/src/block_producer.rs

Lines 124 to 127 in acd8d19

	let gas_price = self
	.gas_price_provider
	.gas_price(height.into())
	.ok_or(anyhow!("No gas price found for block {height:?}"))?;

[xgreenx]

Why do we need RefCell? If you need to update the internal state, then let's use &mut=)

[MitchTurner]

I agree. Kinda related to the TODO above.

[xgreenx]

Hmm, it seems that after restarting the node self.moving_average_profit and self.last_profit will be zero, which means the algorithm will work differently. We need to persist all information required for the algorithm.

Hmm, it would be nice if we used one iteration formula to avoid a persistent state. But I assume you haven't found any suitable for us=)

[xgreenx]

Or maybe

        old_da_gas_price: u64,
        old_total_production_reward: u64,
        old_total_da_recording_cost: u64,

can be stored inside of the algorithm and passed during creation. Then we should be able to calculate profit form them.

[MitchTurner]

This is a great point. Configuring of that might be a pain if the services aren't running when we start the algorithm, but it would be a solution. But how do we know what values it died on?

Another solution is we have an assoc type for the algo trait that the algo returns with arbitrary metadata. The type needs to match some data storage on the provider at compilation time, and we can have that type be versioned so the data storage can still be backwards compatible. Seems like a lot of trouble lol.

impl<FB, DA, A, GP> FuelGasPriceProvider<FB, DA, A, GP>
where
    FB: FuelBlockHistory,
    DA: DARecordingCostHistory,
    A: GasPriceAlgorithm<Metadata=<Self as GasPriceHistory>::Metadata>,
    GP: GasPriceHistory,
{

I don't know if that is compatible with what you were saying about recording the gas price after we receive the block from the importer.

Of course we can add storage to the algorithm--which is just so many traits
🐢
🐢
🐢
🐢

I'd suggest that we just merge the algo abstraction into the provider, but I think that would make versioning harder.

I feel like there is a cleaner solution here.

[MitchTurner]

This was store_gas_prices(latest_block) before and the tests were still passing. Fixing it didn't break the tests either, so the coverage needs to be updated if we want to keep this approach.