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

Implement oracle price coefficient in DB for gas_adjuster #96

Closed

Conversation

thomas-nguy
Copy link
Member

@thomas-nguy thomas-nguy commented Sep 20, 2023

In complement to this proposal

matter-labs/era-contracts#15

We need to multiply the gas_adjuster by a coefficient so that the gas price can reflect the price of the base token

This PR do the following

  • Create a new value in DB called gas_token_adjust_coefficient that an external oracle can update
  • The gas adjuster will fetch this value in DB and multiple the gas price with this coefficient in estimate_effective_gas_price
  • Default value is 1 so it is a non breaking change


pub async fn get_adjust_coefficient(&mut self) -> Result<Ratio<BigUint>, SqlxError> {
let oracle = sqlx::query!("SELECT gas_token_adjust_coefficient FROM oracle")
.fetch_one(self.storage.conn())
Copy link
Collaborator

Choose a reason for hiding this comment

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

does it always fetch the latest one? Or do we assume that this table has only 1 value?

@mm-zk mm-zk requested a review from RomanBrodetski October 13, 2023 05:44
@popzxc
Copy link
Member

popzxc commented Oct 13, 2023

Hello! First, thanks for the contribution. Second, sorry for the review taking so long -- we're still getting used to being open-source.

We're currently working on improving the modularity of zkSync so that it's more suitable for different zkStack use cases.
The new shape of the code will come with the ability to replace components like GasAdjuster with custom implementations.
Once this is implemented, it will be possible to implement the proposed changes without altering the core codebase.

Given that currently there are a lot of projects with different expectations regarding zkStack node capabilities, we believe that this approach will work better at scale than trying to change the core codebase to adjust to all the possible use cases.

So, unfortunately, right now we won't be merging this change, but we promise that soon you will be able to get the desired behavior (and maybe introduce even more features) using the newly provided APIs.

@popzxc popzxc closed this Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants