Skip to content

Fix the network unreachable test panic - #3869

Open
ii-cruz wants to merge 2 commits into
albatrossfrom
iicruz/fix-local-tests
Open

Fix the network unreachable test panic#3869
ii-cruz wants to merge 2 commits into
albatrossfrom
iicruz/fix-local-tests

Conversation

@ii-cruz

@ii-cruz ii-cruz commented Jul 13, 2026

Copy link
Copy Markdown
Member

What's in this pull request?

Some two-peer sync tests used hardcoded libp2p memory addresses. Since the memory transport registry is shared by tests running in the same binary, parallel tests could attempt to listen on the same address. This caused Swarm::listen_on to fail and led to subsequent network cancellation or unreachable panics before the sync assertions were reached.

This PR adds an asynchronous Network::listen_on_address API that waits for libp2p to report the resolved listen address. Test networks can now listen on /memory/0, allowing libp2p to allocate an available address, and return that address to callers for connecting peers.

The full, history, light, and pico sync tests now use these allocated addresses instead of deriving or hardcoding memory addresses. A regression test verifies that concurrent /memory/0 listeners receive distinct, nonzero addresses.

The PR also fixes a separate deadlock exposed when history-sync tests initialize genesis data in parallel. The temporary genesis block is kept alive until the permanent decompressed BLS keys are cached, preventing its lazy public keys from being dropped while another thread is initializing the same key cache.

Pull request checklist

  • All tests pass. The project builds and runs.
  • I have resolved any merge conflicts.
  • I have resolved all clippy and rustfmt warnings.

@ii-cruz ii-cruz self-assigned this Jul 13, 2026
@ii-cruz ii-cruz added bug Something isn't working enhancement New feature or request and removed enhancement New feature or request labels Jul 13, 2026
@ii-cruz ii-cruz added this to the Next release milestone Jul 13, 2026
@ii-cruz
ii-cruz force-pushed the iicruz/fix-local-tests branch from 309394d to 29dce28 Compare July 13, 2026 18:35
@ii-cruz
ii-cruz marked this pull request as ready for review July 13, 2026 18:36
@ii-cruz
ii-cruz requested review from Copilot and jsdanielh July 13, 2026 18:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to fix flaky/panicking network-related tests by avoiding reuse of hardcoded libp2p in-memory listen addresses (/memory/<id>) across parallel test runs.

Changes:

  • Updated consensus sync test utilities to derive peer “memory addresses” from a higher, parameter-based base value rather than fixed small constants.
  • Changed Tendermint validator tests to use different hardcoded peer IDs for TestNetwork::build_network.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
validator/tests/tendermint.rs Adjusts the in-memory libp2p address IDs used by Tendermint protocol upgrade tests.
consensus/tests/sync_utils.rs Computes peer IDs from a high base derived from num_upgrades and uses that consistently for network setup/connection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread validator/tests/tendermint.rs Outdated
Comment thread validator/tests/tendermint.rs Outdated

@jsdanielh jsdanielh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd prefer a random number per test. We should display this generated number though

@jsdanielh
jsdanielh force-pushed the iicruz/web-client-flaky-tests branch from 46636f9 to 93f0d49 Compare July 14, 2026 04:19
Base automatically changed from iicruz/web-client-flaky-tests to albatross July 14, 2026 05:28
@ii-cruz
ii-cruz force-pushed the iicruz/fix-local-tests branch from 29dce28 to cbe4044 Compare July 14, 2026 13:46
@pkg-pr-new

pkg-pr-new Bot commented Jul 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@nimiq/core@3869

commit: 4ee1ce9

@ii-cruz
ii-cruz force-pushed the iicruz/fix-local-tests branch from cbe4044 to 854e03d Compare July 14, 2026 14:07
@ii-cruz
ii-cruz force-pushed the iicruz/fix-local-tests branch from 4ee1ce9 to 7e76786 Compare July 14, 2026 17:31
@ii-cruz
ii-cruz requested a review from jsdanielh July 14, 2026 17:38

@viquezclaudio viquezclaudio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants