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

L2 Block Source & Metadata Storage implementations #1983

Merged
merged 150 commits into from
Jul 8, 2024

Conversation

MitchTurner
Copy link
Member

@MitchTurner MitchTurner commented Jun 20, 2024

Closes: #1955

This actually implements the port adapters from the updater service to access our live databases while running.

Will follow up with another PR that replaces the current updater with the new one.

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]

}

fn name() -> &'static str {
"gas_price_service_storage"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"gas_price_service_storage"
"gas_price"

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can remove lock file for gas-price-analysis binary. It is not required binary to build

pub fn update_l2_block_data(
&mut self,
height: u32,
fullness: (u64, u64),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
fullness: (u64, u64),
used: u64, capacity: u64

[nit]s: Looks more readable=)


fn update_exec_gas_price(&mut self, used: u64, capacity: u64) {
let mut exec_gas_price = self.new_exec_price;
// TODO: Do we want to capture this error? I feel like we should assume capacity isn't 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

capacity type could be NonZeroU64 to avoid this error=)

mod tests;

#[derive(Debug, thiserror::Error, PartialEq)]
pub enum Error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where do we use it?

let inner = metadata_storage
.get_metadata()
.get_metadata(&target_block_height)
.await?
Copy link
Collaborator

Choose a reason for hiding this comment

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

The same here, I think we don't need to require usage of async it can be sync.

@@ -13,7 +13,7 @@ struct FakeL2BlockSource {

#[async_trait::async_trait]
impl L2BlockSource for FakeL2BlockSource {
async fn get_l2_block(&self, _height: BlockHeight) -> Result<BlockInfo> {
async fn get_l2_block(&mut self, _height: BlockHeight) -> Result<BlockInfo> {
let block = self.l2_block.lock().await.recv().await.unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to use lock now if it is &mut self?

Copy link
Member Author

Choose a reason for hiding this comment

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

The lock wasn't becasue this wasn't mut, it's because we need to access the value in another thread in the test.

Ok(self.inner.lock().await.clone())
}

async fn set_metadata(&self, metadata: UpdaterMetadata) -> Result<()> {
async fn set_metadata(&mut self, metadata: UpdaterMetadata) -> Result<()> {
let _ = self.inner.lock().await.replace(metadata);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The same here, do we need to use lock, if you use &mut self?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same for here.

let settings = FakeSettings::new(gas_price_factor, block_gas_limit);

let import_result = block_to_import_result(block.clone());
let blocks: Vec<Arc<dyn Deref<Target = ImportResult> + Send + Sync>> =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let blocks: Vec<Arc<dyn Deref<Target = ImportResult> + Send + Sync>> =
let blocks: Vec<SharedImportResult> =

ConsensusParameters::V1(params)
}

fn block_to_import_result(block: Block) -> Arc<ImportResult> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
fn block_to_import_result(block: Block) -> Arc<ImportResult> {
fn block_to_import_result(block: Block) -> SharedImportResult {

@MitchTurner MitchTurner requested a review from xgreenx July 2, 2024 21:37
@MitchTurner MitchTurner merged commit dded7cc into master Jul 8, 2024
33 checks passed
@MitchTurner MitchTurner deleted the feature/l2-block-source-live-adapter branch July 8, 2024 17:10
MitchTurner added a commit that referenced this pull request Jul 19, 2024
Part of #1624

The [previous PR](#1983) 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
- [ ] Breaking changes are clearly marked as such in the PR description
and changelog
- [ ] New behavior is reflected in tests
- [ ] [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?

---------

Co-authored-by: human <jamesturner@Zenobia.hsd1.wa.comcast.net>
Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
Co-authored-by: Hannes Karppila <2204863+Dentosal@users.noreply.github.com>
@MitchTurner MitchTurner mentioned this pull request Aug 2, 2024
MitchTurner added a commit that referenced this pull request Aug 6, 2024
## 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
GoldenPath1109 added a commit to GoldenPath1109/fuel-core that referenced this pull request Sep 7, 2024
Part of FuelLabs/fuel-core#1624

The [previous PR](FuelLabs/fuel-core#1983) 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
- [ ] Breaking changes are clearly marked as such in the PR description
and changelog
- [ ] New behavior is reflected in tests
- [ ] [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?

---------

Co-authored-by: human <jamesturner@Zenobia.hsd1.wa.comcast.net>
Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
Co-authored-by: Hannes Karppila <2204863+Dentosal@users.noreply.github.com>
GoldenPath1109 added a commit to GoldenPath1109/fuel-core that referenced this pull request Sep 7, 2024
## 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
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.

Implement our gas price algorithm for our GasPriceService
3 participants