-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add V0 algorithm to actual services #2025
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
Conversation
…ce for previous block
| .expect("Should be able do decode the Transaction from json representation"); | ||
|
|
||
| let bytes = dry_run_tx_from_json.to_bytes(); | ||
| dbg!(hex::encode(&bytes)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to keep this dbg?
| #[arg(long = "native-executor-version", env)] | ||
| pub native_executor_version: Option<StateTransitionBytecodeVersion>, | ||
|
|
||
| /// The starting gas price for the network |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking into account that we also need some configuration for DA as well, maybe it makes sense to move all price-related fields into their config. What do yu think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you thinking that we'd submit all the fields for the config in a single CLI argument? I'm not sure how that affects this specific piece of code.
bin/fuel-core/src/cli/run.rs
Outdated
| #[arg(long = "starting-gas-price", default_value = "1000", env)] | ||
| pub starting_gas_price: u64, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be backward compatible the default starting price also should be zero and gas-price-change-percent should be zero as well.
|
|
||
| /// Mutable getter for consensus portion of header | ||
| fn consensus_mut(&mut self) -> &mut ConsensusHeader<GeneratedConsensusFields> { | ||
| pub fn consensus_mut(&mut self) -> &mut ConsensusHeader<GeneratedConsensusFields> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pub fn worst_case(&self, height: u32) -> u64 { | ||
| let mut price = self.new_exec_price; | ||
| for _ in self.for_height..height { | ||
| let change = price.saturating_mul(self.percentage).saturating_div(100); | ||
| price = price.saturating_add(change); | ||
| } | ||
| price | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is off-chain logic, so floats are not a problem. The only requirement is that the final gas price should not be less than if we calculated it via iterations. Maybe we could add a test to verify this behavior.
| let percentage = self.percentage as f64; | ||
| let percentage_as_decimal = percentage / 100.0; | ||
| let multiple = (1.0f64 + percentage_as_decimal).powf(blocks); | ||
| let mut approx = price * multiple; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure that this function will not overflow during any of the operations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prop test should ensure that it will never happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. We don't check that (because the test iterates over that entire size and I don't want to do u32 max in a test).
I can add another test that makes sure it never overflows :)
## What's Changed * L2 Block Source & Metadata Storage implementations by @MitchTurner in #1983 * Weekly `cargo update` by @github-actions in #2029 * Add V0 algorithm to actual services by @MitchTurner in #2025 * Weekly `cargo update` by @github-actions in #2039 * Add syncronization for Gas Price Database and On Chain Database on startup by @MitchTurner in #2041 * Ignore the message receipt if transaction is reverted by @xgreenx in #2045 * feat: add chain config to Docker images by @mchristopher in #2052 * Blob tx support and fuel-vm 0.56.0 by @Dentosal in #1988 * Disable SMT for `ContractsAssets` and `ContractsState` by @xgreenx in #2048 ## New Contributors * @mchristopher made their first contribution in #2052 **Full Changelog**: v0.31.0...v0.32.0
## What's Changed * L2 Block Source & Metadata Storage implementations by @MitchTurner in FuelLabs/fuel-core#1983 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#2029 * Add V0 algorithm to actual services by @MitchTurner in FuelLabs/fuel-core#2025 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#2039 * Add syncronization for Gas Price Database and On Chain Database on startup by @MitchTurner in FuelLabs/fuel-core#2041 * Ignore the message receipt if transaction is reverted by @xgreenx in FuelLabs/fuel-core#2045 * feat: add chain config to Docker images by @mchristopher in FuelLabs/fuel-core#2052 * Blob tx support and fuel-vm 0.56.0 by @Dentosal in FuelLabs/fuel-core#1988 * Disable SMT for `ContractsAssets` and `ContractsState` by @xgreenx in FuelLabs/fuel-core#2048 ## New Contributors * @mchristopher made their first contribution in FuelLabs/fuel-core#2052 **Full Changelog**: FuelLabs/fuel-core@v0.31.0...v0.32.0
## What's Changed * L2 Block Source & Metadata Storage implementations by @MitchTurner in FuelLabs/fuel-core#1983 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#2029 * Add V0 algorithm to actual services by @MitchTurner in FuelLabs/fuel-core#2025 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#2039 * Add syncronization for Gas Price Database and On Chain Database on startup by @MitchTurner in FuelLabs/fuel-core#2041 * Ignore the message receipt if transaction is reverted by @xgreenx in FuelLabs/fuel-core#2045 * feat: add chain config to Docker images by @mchristopher in FuelLabs/fuel-core#2052 * Blob tx support and fuel-vm 0.56.0 by @Dentosal in FuelLabs/fuel-core#1988 * Disable SMT for `ContractsAssets` and `ContractsState` by @xgreenx in FuelLabs/fuel-core#2048 ## New Contributors * @mchristopher made their first contribution in FuelLabs/fuel-core#2052 **Full Changelog**: FuelLabs/fuel-core@v0.31.0...v0.32.0
## What's Changed * L2 Block Source & Metadata Storage implementations by @MitchTurner in FuelLabs/fuel-core#1983 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#2029 * Add V0 algorithm to actual services by @MitchTurner in FuelLabs/fuel-core#2025 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#2039 * Add syncronization for Gas Price Database and On Chain Database on startup by @MitchTurner in FuelLabs/fuel-core#2041 * Ignore the message receipt if transaction is reverted by @xgreenx in FuelLabs/fuel-core#2045 * feat: add chain config to Docker images by @mchristopher in FuelLabs/fuel-core#2052 * Blob tx support and fuel-vm 0.56.0 by @Dentosal in FuelLabs/fuel-core#1988 * Disable SMT for `ContractsAssets` and `ContractsState` by @xgreenx in FuelLabs/fuel-core#2048 ## New Contributors * @mchristopher made their first contribution in FuelLabs/fuel-core#2052 **Full Changelog**: FuelLabs/fuel-core@v0.31.0...v0.32.0
## What's Changed * L2 Block Source & Metadata Storage implementations by @MitchTurner in FuelLabs/fuel-core#1983 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#2029 * Add V0 algorithm to actual services by @MitchTurner in FuelLabs/fuel-core#2025 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#2039 * Add syncronization for Gas Price Database and On Chain Database on startup by @MitchTurner in FuelLabs/fuel-core#2041 * Ignore the message receipt if transaction is reverted by @xgreenx in FuelLabs/fuel-core#2045 * feat: add chain config to Docker images by @mchristopher in FuelLabs/fuel-core#2052 * Blob tx support and fuel-vm 0.56.0 by @Dentosal in FuelLabs/fuel-core#1988 * Disable SMT for `ContractsAssets` and `ContractsState` by @xgreenx in FuelLabs/fuel-core#2048 ## New Contributors * @mchristopher made their first contribution in FuelLabs/fuel-core#2052 **Full Changelog**: FuelLabs/fuel-core@v0.31.0...v0.32.0


Part of #1624
The previous PR added all the code for working with the dynamic gas price, but didn't plug it into our services. This PR does the actual dep injection. It also adds gas price estimation to AlgoirthmV0.
This change includes new flags for the CLI:
- "starting-gas-price" - the starting gas price for the gas price algorithm
- "gas-price-change-percent" - the percent change for each gas price update
- "gas-price-threshold-percent" - the threshold percent for determining if the gas price will be increase or decreased
And the following CLI flags are serving a new purpose
- "min-gas-price" - the minimum gas price that the gas price algorithm will return
Checklist
Before requesting review
After merging, notify other teams
[Add or remove entries as needed]