Skip to content

Conversation

@fcecin
Copy link
Contributor

@fcecin fcecin commented Nov 29, 2025

Description

This PR removes the ENR cache and the worker loop that fed it in WakuPeerExchange, and replaces it with a direct query to the PeerStore. To avoid slowdowns, the new getEnrsFromStore doesn't allocate any memory other than the resulting sequence of ENR records to the caller.

Changes

  • remove WakuPeerExchange.enrCache
  • add forEnrPeers to support fast PeerStore search
  • add getEnrsFromStore
  • fix peer exchange tests

Issue

closes #3578

* remove WakuPeerExchange.enrCache
* add forEnrPeers to support fast PeerStore search
* add getEnrsFromStore
* fix peer exchange tests
@github-actions
Copy link

github-actions bot commented Nov 29, 2025

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:3652

Built from 4696901

Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for it! 💯
Just adding some nitpicks that I hope you find useful

@fcecin
Copy link
Contributor Author

fcecin commented Dec 1, 2025

NOTE: the ci.yml workflow is broken, likely due to the project name change.

It was blowing up on line 135 of ci.yml. I changed it to:

uses: logos-messaging/nwaku/.github/workflows/container-image.yml@master

Now it blows up on line 140, which still references waku.org.

@fryorcraken
Copy link
Collaborator

fryorcraken commented Dec 1, 2025

NOTE: the ci.yml workflow is broken, likely due to the project name change.

It was blowing up on line 135 of ci.yml. I changed it to:

uses: logos-messaging/nwaku/.github/workflows/container-image.yml@master

Now it blows up on line 140, which still references waku.org.

Can you do a dedicated PR to fixing the CI so we can get it merged asap (feel free to ping me for review)?

@fcecin
Copy link
Contributor Author

fcecin commented Dec 1, 2025

NOTE: the ci.yml workflow is broken, likely due to the project name change.
It was blowing up on line 135 of ci.yml. I changed it to:

uses: logos-messaging/nwaku/.github/workflows/container-image.yml@master

Now it blows up on line 140, which still references waku.org.

Can you do a dedicated PR to fixing the CI so we can get it merged asap (feel free to ping me for review)?

On it

Copy link
Contributor

@NagyZoltanPeter NagyZoltanPeter left a comment

Choose a reason for hiding this comment

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

Awsome, thank you! Added one recommendation for the template if you may consider to apply.

@fcecin fcecin force-pushed the fix/pex-remove-enr-cache branch from ab35cfa to 997a635 Compare December 5, 2025 17:33
@fcecin fcecin merged commit 7920368 into master Dec 8, 2025
12 checks passed
@fcecin fcecin deleted the fix/pex-remove-enr-cache branch December 8, 2025 09:35
darshankabariya pushed a commit that referenced this pull request Dec 10, 2025
* remove WakuPeerExchange.enrCache
* add forEnrPeers to support fast PeerStore search
* add getEnrsFromStore
* fix peer exchange tests
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.

fetch peers from local peerStore instead of caching them as a temporary fix

5 participants