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

Use latest gas price to estimate next price for tx pool checks #2612

Merged
merged 6 commits into from
Jan 22, 2025

Conversation

MitchTurner
Copy link
Member

@MitchTurner MitchTurner commented Jan 21, 2025

Linked Issues/PRs

Description

There are 3 services that rely on the gas price: producer, tx pool, and graphql. In 0.41.0, graphql used the latest block, but the producer and tx pool use the actual next gas price.

This works great on the producer node, but for all the sentries that also have tx pools they need to be perfectly in sync with the block producer or else the expected gas prices will be different.

The new change has the tx pool behave the same as graphql, where it has access to the latest block and can calculate the (worst case) gas price for the next block.

This is already what the end users are doing to set a max fee on their tx, so it shouldn't exclude any txs that should be in there.

Sentries will reject txs if they don't have the correct parameters, this solves this problem by using the "worst case" for next gas price based on the the latest actual block.

Checklist

  • New behavior is reflected in tests

@MitchTurner MitchTurner marked this pull request as ready for review January 21, 2025 17:57
@MitchTurner MitchTurner self-assigned this Jan 21, 2025
@MitchTurner MitchTurner requested a review from netrome as a code owner January 21, 2025 18:50
AurelienFT
AurelienFT previously approved these changes Jan 21, 2025
xgreenx
xgreenx previously approved these changes Jan 21, 2025
Copy link
Collaborator

@xgreenx xgreenx left a comment

Choose a reason for hiding this comment

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

What gas price do we use during dry run as a default gas price?

@MitchTurner MitchTurner dismissed stale reviews from xgreenx and AurelienFT via f209548 January 21, 2025 23:39
@MitchTurner
Copy link
Member Author

What gas price do we use during dry run as a default gas price?

It uses the same value as the producer, unfortunately:

        let gas_price = if let Some(inner) = gas_price {
            inner
        } else {
            self.calculate_gas_price().await?
        };

Does anyone use the default value?

@xgreenx
Copy link
Collaborator

xgreenx commented Jan 21, 2025

We provide this functionality, so we need to guarantee it to work=D Looks like it uses gas price service. I think we need to change it to use value based on the previous block

@MitchTurner
Copy link
Member Author

We provide this functionality, so we need to guarantee it to work=D Looks like it uses gas price service. I think we need to change it to use value based on the previous block

K. I'll create a PR.

Copy link
Contributor

@rafal-ch rafal-ch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@MitchTurner MitchTurner merged commit 8c553d6 into master Jan 22, 2025
31 checks passed
@MitchTurner MitchTurner deleted the use-latest-block-in-tx-pool branch January 22, 2025 14:35
MitchTurner added a commit that referenced this pull request Jan 22, 2025
## Linked Issues/PRs
<!-- List of related issues/PRs -->

## Description
<!-- List of detailed changes -->
Followup to #2612

## Checklist
- [x] New behavior is reflected in tests
@MitchTurner MitchTurner mentioned this pull request Jan 22, 2025
MitchTurner added a commit that referenced this pull request Jan 22, 2025
## Version v0.41.1

* fault_proving(compression): include block_id in da compressed block
headers by @rymnc in #2551
* chore: Add myself and Andrea as codeowner for graphql API + related
crates by @netrome in #2570
* fix(integration_tests): remove flake from
produce_block__l1_committed_block_affects_gas_price by @rymnc in
#2566
* bugfix: Improve the `BlockCommitterHttpApi` client to use `url` apis
better by @MitchTurner in
#2599
* Fix version compatibility error by @AurelienFT in
#2608
* Improve error messages for responses from committer by @MitchTurner in
#2609
* Update async processor tests by @rafal-ch in
#2577
* The amount of returned dust coins is limited by factor relative to the
amount of selected big coins by @rafal-ch in
#2610
* fix(da_compression): invalid decompression of utxo id and CoinConfig
fix by @rymnc in #2593
* Use latest gas price to estimate next price for tx pool checks by
@MitchTurner in #2612
* Set Latest Recorded Height on startup by @MitchTurner in
#2603
* Use latest gas price to estimate next block gas price during dry runs
by @MitchTurner in #2615
* Check that fuel-core lib builds correctly without default features by
@rafal-ch in #2594
* Expose indexation status in `NodeInfo` endpoint by @rafal-ch in
#2595


**Full Changelog**:
v0.41.0...v0.41.1
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.

4 participants