Skip to content

feat(l1): improved lookup interval - #5518

Merged
ElFantasma merged 18 commits into
mainfrom
lookup_function
Dec 12, 2025
Merged

ElFantasma merged 18 commits into
mainfrom
lookup_function

Conversation

@ElFantasma

Copy link
Copy Markdown
Contributor

Motivation

Node lookup interval is used to send FindNode and ENRRequest messages for discovery and to try initiate new RLPx connections and currently it uses only two values.

Description

  • This PR changes it for a variable interval according to the peer target completion.
  • Also it provides a CLI argument to set up the initial interval, to be able to set the node more or less eager on startup.

Closes #5149 (also addresses #5148, already closed)

@ElFantasma
ElFantasma requested a review from a team as a code owner December 4, 2025 14:58
@github-actions github-actions Bot added the L1 Ethereum client label Dec 4, 2025
@ElFantasma ElFantasma changed the title feat(l1): Improved lookup interval feat(l1): improved lookup interval Dec 4, 2025
@github-actions

github-actions Bot commented Dec 4, 2025 •

Copy link
Copy Markdown

Lines of code report

Total lines added: 46
Total lines removed: 6
Total lines changed: 52

Detailed view
+---------------------------------------------------+-------+------+
| File                                              | Lines | Diff |
+---------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/cli.rs                          | 792   | +11  |
+---------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/initializers.rs                 | 456   | +1   |
+---------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/initializers.rs              | 334   | +1   |
+---------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/discv4/peer_table.rs | 1068  | +11  |
+---------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/discv4/server.rs     | 678   | +17  |
+---------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/network.rs           | 367   | +4   |
+---------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/initiator.rs    | 105   | -6   |
+---------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/test_utils.rs        | 298   | +1   |
+---------------------------------------------------+-------+------+

@Oppen

Oppen commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

LGTM, I'll test it at home and approve if able to sync.

@Oppen Oppen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Didn't complete the sync due to local network speed limits, but it downloaded all accounts and a significant number of storages, and peers were stable. Found 7 quickly and didn't lose any in an hour.

@github-project-automation github-project-automation Bot moved this to In Review in ethrex_l1 Dec 12, 2025
@ElFantasma
ElFantasma added this pull request to the merge queue Dec 12, 2025
Merged via the queue into main with commit 13f93e8 Dec 12, 2025
48 checks passed
@ElFantasma
ElFantasma deleted the lookup_function branch December 12, 2025 23:30
@github-project-automation github-project-automation Bot moved this from In Review to Done in ethrex_l1 Dec 12, 2025
MysticRyuujin pushed a commit to MysticRyuujin/ethrex that referenced this pull request Aug 24, 2026
…aclass#7163)

**Motivation**

`DiscoveryConfig::initial_lookup_interval` is dead data. lambdaclass#6511 (kademlia
k-bucket routing table v2) replaced the configurable bound in
`DiscoveryServer::get_lookup_interval` with its own constants, because
each iterative lookup emits ~16 FindNode messages instead of 1 and
needed a rescaled schedule:

```diff
         lookup_interval_function(
             peer_completion,
-            self.config.initial_lookup_interval,
-            super::LOOKUP_INTERVAL_MS,
+            ITERATIVE_LOOKUP_INITIAL_MS,   // 500.0
+            ITERATIVE_LOOKUP_INTERVAL_MS,  // 10_000.0
         )
```

The field itself survived, still threaded from `--p2p.lookup-interval`
through `P2PContext` and `start_network` into a `DiscoveryServer` that
never reads it. The only live consumer of the flag is the RLPx
initiator, so the help text was also misleading.

While tracing it, a second problem turned up: `impl Default for Options`
sets `lookup_interval: Default::default()`, which is `0.0` for `f64`
rather than the `INITIAL_LOOKUP_INTERVAL_MS` that clap uses for the
flag. `default_l1()`, `default_l2()` and the L2 node options all build
on that impl, so they hand the RLPx initiator a 0ms lower bound. With no
peers connected the easing curve returns `Duration::ZERO`, and
`handle_look_for_peer` re-arms its timer with no delay at all. Present
since lambdaclass#5518 added the flag.

**Description**

- Remove `initial_lookup_interval` from `DiscoveryConfig` and stop
  overriding it in `start_network`. `DiscoveryConfig`'s `Default` impl
  goes with it: both remaining fields are set explicitly at the single
  construction site, and clap already defaults them to `true`.
- Reword `--p2p.lookup-interval`'s help to describe what it actually
  controls, and refresh the now-stale doc comments on
  `INITIAL_LOOKUP_INTERVAL_MS` / `LOOKUP_INTERVAL_MS` /
  `lookup_interval_function`.
- Default `Options::lookup_interval` to `INITIAL_LOOKUP_INTERVAL_MS`.

No behavior change for the discovery server, which was already running
on its own constants. The second commit does change behavior for the
dev/L2 paths that go through `Options::default()`, restoring the 100ms
floor they were meant to have.

**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Make Discovery and Connection attempt Lookup interval automatically adjustable.

4 participants