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

Upgrade fuel core to use V1 gas price algo/updater #2140

Open
MitchTurner opened this issue Aug 28, 2024 · 2 comments
Open

Upgrade fuel core to use V1 gas price algo/updater #2140

MitchTurner opened this issue Aug 28, 2024 · 2 comments
Assignees

Comments

@MitchTurner
Copy link
Member

MitchTurner commented Aug 28, 2024

This includes adding the config values once it is fully integrated.

It's not clear to me how we want to smoothly transition the values. We should probably start taking the DA costs into calculation, but configure it to always have DA costs as 0 until we get some good tests up.

@MitchTurner
Copy link
Member Author

We have some versioning going on with the algorithm:


#[derive(Debug, Clone, PartialEq)]
pub enum Algorithm {
    V0(AlgorithmV0),
    V1(AlgorithmV1),
}
impl GasPriceAlgorithm for Algorithm {
    fn next_gas_price(&self) -> u64 {
        match self {
            Algorithm::V0(v0) => v0.calculate(),
            Algorithm::V1(v1) => v1.calculate(0),
        }
    }
    fn worst_case_gas_price(&self, height: BlockHeight) -> u64 {
        match self {
            Algorithm::V0(v0) => v0.worst_case(height.into()),
            Algorithm::V1(v1) => v1.calculate(0),
        }
    }
}

I don't think we need that. I think we can just depend on a single algorithm and change it as we improve it. Shouldn't be a problem.

@rymnc rymnc self-assigned this Oct 2, 2024
@rymnc
Copy link
Member

rymnc commented Oct 2, 2024

assigning to myself

@MitchTurner MitchTurner self-assigned this Oct 21, 2024
rymnc added a commit that referenced this issue Nov 14, 2024
…#2416)

> [!NOTE]
> Some values for the tests need expert opinion from @MitchTurner. A
follow up PR will be created to define the `UninitializedTask` that
wraps over this task.

## Linked Issues/PRs
<!-- List of related issues/PRs -->
part of #2140, but doesn't
close it yet.

## Description
<!-- List of detailed changes -->
- We define a new `RunnableTask`, `GasPriceServiceV1`, which uses the da
block cost source in tandem with the l2 block source
- Tests for the same
- Casts to and from the v1 algorithm updater
- we take a direct dependency on v0's metadata so that we can migrate
between the two.

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests
- [x] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [x] 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](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?
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

No branches or pull requests

2 participants