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(en): Switch EN to use node framework #2427

Merged
merged 15 commits into from
Jul 12, 2024
Merged

Conversation

popzxc
Copy link
Member

@popzxc popzxc commented Jul 11, 2024

What ❔

This PR ports remaining parts of the EN to node framework and makes framework the default way to run it.
In more detail:

  • Config for the health check limits are now set from config.
  • EN and rust metrics are now exposed; the protocol version update task now runs.
  • ⚠️ Connection pool healthcheck was removed. It was controversial initially, its usefulness is not clear, it was supposed to be refactored a year ago but didn't, and it wasn't working well when testing. See linear issue for more context.
  • Tests were reworked to use node framework; some refactoring was also applied to reduce boilerplate.
  • Additional tests were added to check for invalid EN configurations.
  • ⚠️ Node framework was made the default way to run the EN. There is also a hook to force EN to run the old way, so that we don't have to rollback over small issues.

Why ❔

  • Part of switch to the node framework.

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.

@popzxc popzxc marked this pull request as ready for review July 11, 2024 12:34
@popzxc
Copy link
Member Author

popzxc commented Jul 12, 2024

I will fix nitpicks in a follow-up PR.

@popzxc popzxc added this pull request to the merge queue Jul 12, 2024

/// Run the node using the node framework.
#[arg(long)]
use_node_framework: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

it's incompatible, have you checked, that we don't use it in gitops ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course. EN wasn't fully ported to the framework until this PR, it wouldn't have worked with it

Merged via the queue into main with commit 0cee530 Jul 12, 2024
46 checks passed
@popzxc popzxc deleted the popzxc-finish-en-integration branch July 12, 2024 08:13
irnb pushed a commit to vianetwork/via-server that referenced this pull request Jul 12, 2024
## What ❔

This PR ports remaining parts of the EN to node framework and makes
framework the default way to run it.
In more detail:

- Config for the health check limits are now set from config.
- EN and rust metrics are now exposed; the protocol version update task
now runs.
- ⚠️ Connection pool healthcheck was removed. It was controversial
initially, its usefulness is not clear, it was supposed to be refactored
a year ago but didn't, and it wasn't working well when testing. See
[linear
issue](https://linear.app/matterlabs/issue/PLA-255/revamp-db-connection-health-check)
for more context.
- Tests were reworked to use node framework; some refactoring was also
applied to reduce boilerplate.
- Additional tests were added to check for invalid EN configurations.
- ⚠️ Node framework was made the default way to run the EN. There is
also a hook to force EN to run the old way, so that we don't have to
rollback over small issues.

## Why ❔

- Part of switch to the node framework.

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] 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`.
github-merge-queue bot pushed a commit that referenced this pull request Jul 22, 2024
🤖 I have created a release *beep* *boop*
---


##
[24.10.0](core-v24.9.0...core-v24.10.0)
(2024-07-22)


### Features

* Add blob size metrics
([#2411](#2411))
([41c535a](41c535a))
* **en:** Switch EN to use node framework
([#2427](#2427))
([0cee530](0cee530))
* **eth-sender:** add early return in sending new transactions to not
spam logs with errors
([#2425](#2425))
([192f2a3](192f2a3))
* **eth-watch:** Integrate decentralized upgrades
([#2401](#2401))
([5a48e10](5a48e10))
* L1 batch signing (BFT-474)
([#2414](#2414))
([ab699db](ab699db))
* **prover:** Make it possible to run prover out of GCP
([#2448](#2448))
([c9da549](c9da549))
* **zk_toolbox:** Small adjustment for zk toolbox
([#2424](#2424))
([ce43c42](ce43c42))


### Bug Fixes

* **eth-sender:** add bump of min 10% when resending txs to avoid
"replacement transaction underpriced"
([#2422](#2422))
([a7bcf5d](a7bcf5d))
* Set attesters in Connection::adjust_genesis (BFT-489)
([#2429](#2429))
([ca4cb3c](ca4cb3c))

---
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants