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.

Sign up for GitHub

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

Jump to bottom

feat: Allow tracking l2 fees for L2-based chains #2563

Merged
merged 46 commits into from
Aug 9, 2024
Merged

feat: Allow tracking l2 fees for L2-based chains #2563

merged 46 commits into from
Aug 9, 2024

Conversation

StanislavBreadless
Copy link
Contributor

@StanislavBreadless StanislavBreadless commented Aug 1, 2024 •
edited
Loading

What ❔

In this PR we add changes to the GasAdjuster needed to track the fees on L2-based chains. The PR aims to preserve type-safety for places that should work when L1 interface is available only, so DynClient<L1> is left in a lot of places and generally L1 is the default network to be used. However, some methods were extended to work with any network.

Overall what was added:

  • A new resource for accessing L2-specific endpoints.
  • A new field is returned from fee_history. Clients should not rely on this field. However, we added it so we could test things.
  • Some refactoring
  • While some new new config fields are added into the Rust code, they are not added to the config so that in case something has to be changed, we do not introduce legacy (esp. in protobuf)
  • Into protobuf we only added RelayedL2Calldata pubdata type, which would symbolize that the calldata is being relayed to L1 through another L2. I am pretty confident that this type will be needed anyway.

Why ❔

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.

@StanislavBreadless StanislavBreadless changed the title Allow tracking l2 fees for L2-based chains feat: Allow tracking l2 fees for L2-based chains Aug 1, 2024
Base automatically changed from sb-rename-sl-chain-id to main August 1, 2024 15:21
slowli
slowli reviewed Aug 2, 2024
View reviewed changes
core/bin/zksync_server/src/node_builder.rs Outdated Show resolved Hide resolved
core/lib/basic_types/src/lib.rs Outdated Show resolved Hide resolved
core/lib/config/src/configs/eth_sender.rs Outdated Show resolved Hide resolved
core/lib/dal/src/blocks_web3_dal.rs Show resolved Hide resolved
core/lib/eth_client/src/clients/http/decl.rs Outdated Show resolved Hide resolved
core/node/fee_model/src/l1_gas_price/gas_adjuster/mod.rs Show resolved Hide resolved
core/node/fee_model/src/l1_gas_price/gas_adjuster/mod.rs Outdated Show resolved Hide resolved
core/node/node_framework/src/implementations/layers/query_eth_client.rs Outdated Show resolved Hide resolved
core/node/node_framework/src/implementations/resources/eth_interface.rs Outdated Show resolved Hide resolved
core/node/node_framework/src/implementations/resources/eth_interface.rs Show resolved Hide resolved
tomg10
tomg10 reviewed Aug 2, 2024
View reviewed changes
Copy link
Contributor

@tomg10 tomg10 left a comment

Choose a reason for hiding this comment

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

I've left a few small comments

core/bin/external_node/src/node_builder.rs Outdated Show resolved Hide resolved
core/lib/dal/src/blocks_web3_dal.rs Show resolved Hide resolved
core/lib/web3_decl/src/client/network.rs Outdated Show resolved Hide resolved
core/node/node_framework/src/implementations/resources/eth_interface.rs Show resolved Hide resolved
slowli
slowli reviewed Aug 6, 2024
View reviewed changes
core/lib/basic_types/src/settlement.rs Outdated Show resolved Hide resolved
core/lib/eth_client/src/clients/http/query.rs Outdated Show resolved Hide resolved
core/lib/eth_client/src/clients/http/query.rs Outdated Show resolved Hide resolved
core/lib/eth_client/src/clients/mock.rs Outdated Show resolved Hide resolved
core/lib/eth_client/src/clients/mock.rs Outdated Show resolved Hide resolved
core/lib/eth_client/src/lib.rs Outdated Show resolved Hide resolved
core/lib/web3_decl/src/client/network.rs Outdated Show resolved Hide resolved
core/lib/eth_client/src/clients/mock.rs Outdated Show resolved Hide resolved
core/node/fee_model/src/l1_gas_price/gas_adjuster/mod.rs Outdated Show resolved Hide resolved
core/node/node_framework/src/implementations/resources/eth_interface.rs Show resolved Hide resolved
slowli
slowli reviewed Aug 7, 2024
View reviewed changes
Copy link
Contributor

@slowli slowli left a comment

Choose a reason for hiding this comment

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

Overall, looks pretty decent, although I'd still prefer refactoring GasAdjusterClient to use Box<dyn EthFeeInterface> for more high-level / consistent DI (can be done separately). Please remove unused EthInterface imports as pointed by clippy.

core/lib/basic_types/src/settlement.rs Outdated Show resolved Hide resolved
core/lib/types/src/api/mod.rs Outdated Show resolved Hide resolved
core/lib/eth_client/src/clients/mock.rs Show resolved Hide resolved
slowli
slowli previously approved these changes Aug 8, 2024
View reviewed changes
@StanislavBreadless StanislavBreadless added this pull request to the merge queue Aug 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2024
@StanislavBreadless StanislavBreadless added this pull request to the merge queue Aug 9, 2024
Merged via the queue into main with commit e3f7804 Aug 9, 2024
47 checks passed
@StanislavBreadless StanislavBreadless deleted the sb-track-l2-fees-for-gateway branch August 9, 2024 10:27
@zksync-era-bot zksync-era-bot mentioned this pull request Aug 8, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Aug 13, 2024
@zksync-era-bot
chore(main): release core 24.17.0 (#2625)
a0c7a03 
🤖 I have created a release *beep* *boop*
---


##
[24.17.0](core-v24.16.0...core-v24.17.0)
(2024-08-13)


### Features

* Allow tracking l2 fees for L2-based chains
([#2563](#2563))
([e3f7804](e3f7804))
* Remove old EN code
([#2595](#2595))
([8d31ebc](8d31ebc))
* **tee:** introduce get_tee_proofs RPC method for TEE proofs
([#2474](#2474))
([d40ff5f](d40ff5f))
* **vm:** Fast VM integration
([#1949](#1949))
([b752a54](b752a54))


### Bug Fixes

* query for prover API
([#2628](#2628))
([b8609eb](b8609eb))
* **vm:** Fix missing experimental VM config
([#2629](#2629))
([e07a39d](e07a39d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomg10 tomg10 tomg10 approved these changes

@slowli slowli slowli left review comments

@popzxc popzxc Awaiting requested review from popzxc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@StanislavBreadless @slowli @tomg10