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

feat: Custom Base Token Gas Price Oracle #996

Merged
merged 71 commits into from
Apr 9, 2024

Conversation

jrchatruc
Copy link
Contributor

@jrchatruc jrchatruc commented Feb 1, 2024

What ❔

This PR adds a new BaseTokenFetcher component to be used by custom base token stacks. It's in charge of caching the latest conversion rate between the base token and eth so gas prices can be converted accordingly (both l1_gas_price and l1_pubdata_price).

A new config toml file is added holding the configuration for it, currently just a host and poll_interval variables to get the host from which the conversion rate is queried and how often the conversion rate is refreshed. The component is initialized by adding base_token_fetcher as a server --component.

There is also an example server component that acts like a sample conversion rate api, it returns a fixed conversion rate of value 42. This component can instantiated by adding the dev_conversion_rate_api argument to the the server components at initialization.
To see a complete the implementations as a whole, run:

zk server --components "api,tree,eth,state_keeper,housekeeper,commitment_generator,base_token_fetcher,dev_conversion_rate_api"

to quickly test out the fetcher component. This will be deleted once proper tests are written.

Disclaimer

As explained here, current implementation is not overflow safe, this is left for a pending continuation of this PR, as of this one, it's stated that there's no u64 safety.

Why ❔

Gas Prices in a custom base token setting need to be in the native currency of the chain. These prices are partially determined by l1 gas prices in L1, which are in eth, so a conversion is required.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.
  • Spellcheck has been run via zk spellcheck.
  • Linkcheck has been run via zk linkcheck.

@jrchatruc jrchatruc marked this pull request as ready for review February 16, 2024 13:00
@jrchatruc jrchatruc requested review from kelemeno and mm-zk February 20, 2024 20:59
core/lib/zksync_core/src/native_erc20_fetcher/mod.rs Outdated Show resolved Hide resolved
core/lib/zksync_core/src/native_erc20_fetcher/mod.rs Outdated Show resolved Hide resolved
core/lib/zksync_core/src/lib.rs Outdated Show resolved Hide resolved
core/lib/zksync_core/src/native_erc20_fetcher/mod.rs Outdated Show resolved Hide resolved
core/lib/zksync_core/src/native_erc20_fetcher/mod.rs Outdated Show resolved Hide resolved
core/lib/zksync_core/src/l1_gas_price/singleton.rs Outdated Show resolved Hide resolved
core/lib/zksync_core/src/lib.rs Outdated Show resolved Hide resolved
core/lib/zksync_core/src/native_erc20_fetcher/mod.rs Outdated Show resolved Hide resolved
core/lib/zksync_core/src/native_erc20_fetcher/mod.rs Outdated Show resolved Hide resolved
@juan518munoz
Copy link
Contributor

@popzxc how would you recommend we test correct functionality of the added components? Any example we can use as a template?

@popzxc
Copy link
Member

popzxc commented Mar 28, 2024

@juan518munoz I don't think that there exists a good template that would help testing this feature, though we do have some infra that may help with it.

For instance, we already run tests in a matrix in CI. You can add a value for custom base token there, and when tests are ran for custom token, you may add the relevant config and spawn the server with the dev conversion rate fetcher. An example of such integration in CI could be found in this PR.

That said, I believe that it should be done as a separate PR to not overly extend the scope for this PR.

@fkrause98 fkrause98 mentioned this pull request Apr 3, 2024
6 tasks
@fkrause98
Copy link
Contributor

Thanks for the answer @popzxc, we'll be working on those tests on this draft.

fkrause98 and others added 4 commits April 8, 2024 19:20
Implements a way to get the resource from the context

Used at core/node/node_framework/src/implementations/layers/fee_input.rs
@lferrigno lferrigno merged commit 2a9b9bd into matter-labs:kl-factory Apr 9, 2024
2 checks passed
@fkrause98 fkrause98 deleted the gas-oracle branch April 10, 2024 18:28
ly0va added a commit that referenced this pull request Apr 11, 2024
This reverts commit 2a9b9bd, reversing
changes made to 20574f7.
fkrause98 added a commit to lambdaclass/zksync-era that referenced this pull request Apr 11, 2024
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.

8 participants