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

perf(hubble): improve libp2p sync #1912

Closed
wants to merge 9 commits into from

Conversation

Wazzymandias
Copy link
Contributor

@Wazzymandias Wazzymandias commented Apr 15, 2024

Motivation

  • Update libp2p to latest version for bug fixes, features, and performance
    • Some specific examples include (but not limited to): batch publishing, tunable message concurrency, exported types for Message Cache to allow extension (e.g with Bloom Filter)
    • The latest release conforms to newest spec of libp2p, which has subtle but important improvements to gossipsub

Change Summary

  • Updated libp2p from 6.1.0 to 13.0.0
  • Enabled batch publishing for significant reduction in Network I/O

Merge Checklist

Choose all relevant options below by adding an x now or at any time before submitting for review

Additional Context

Before

image

After

image


PR-Codex overview

This PR updates dependencies, refactors PeerId imports, and improves IP family handling.

Detailed summary

  • Updated Node.js version to 21 in CI workflow
  • Refactored PeerId imports to use @libp2p/interface
  • Improved IP family handling in ipFamilyFromString function
  • Updated dependencies related to libp2p, libp2p-gossipsub, libp2p-noise, and more

The following files were skipped due to too many changes: apps/hubble/src/test/e2e/gossipNetwork.test.ts, apps/hubble/src/test/e2e/gossipNetworkBundle.test.ts, apps/hubble/src/hubble.ts, apps/hubble/src/network/p2p/gossipNode.test.ts, apps/hubble/src/network/p2p/gossipNode.ts, apps/hubble/src/network/p2p/gossipNodeWorker.ts, yarn.lock

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@Wazzymandias Wazzymandias added the t-perf Improve performance label Apr 15, 2024
Copy link

changeset-bot bot commented Apr 15, 2024

⚠️ No Changeset found

Latest commit: 68d26aa

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Apr 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hub-monorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 16, 2024 2:52am

Copy link

socket-security bot commented Apr 15, 2024

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSource
Install scripts npm/[email protected]
  • Install script: postinstall
  • Source: node scripts/postinstall
Install scripts npm/[email protected]
  • Install script: install
  • Source: npm run rebuild || echo "Couldn't build bindings. Non-native version used."
Install scripts npm/[email protected]
  • Install script: postinstall
  • Source: node ./scripts/postinstall.js
Install scripts npm/[email protected]
  • Install script: postinstall
  • Source: node scripts/postinstall

View full report↗︎

Next steps

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

runs_on: 'buildjet-8vcpu-ubuntu-2204'
- node_version: 20
- node_version: 21
Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep both 20 and 21 (but can remove 18) from the matrix

// Hashes the raw message data
return noSignMsgId(msg.data);
const id = noSignMsgId(msg.data);
if (id instanceof Promise) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we can just "await id" right, and javascript will handle both the promise and non-promise cases

@@ -1253,7 +1253,7 @@ class SyncEngine extends TypedEmitter<SyncEvents> {
let fetchMessagesThreshold = HASHES_PER_FETCH;
// If we have more messages but the hashes still mismatch, we need to find the exact message that's missing.
if (ourNode && ourNode.numMessages >= 1) {
fetchMessagesThreshold = 1;
fetchMessagesThreshold = HASHES_PER_FETCH / 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this line from this PR. We'll remove this whole concept from the DiffSync v2 PR

@adityapk00
Copy link
Contributor

Pretty cool. I thought upgrading will be very messy, but looks quite managable.

everything and needs a major overhaul, and now it does not properly
handle peer discovery or connection management, and the connection
manager is a nonsensical dumpster fire of spaghetti code
Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@babel/[email protected] None +1 275 kB nicolo-ribaudo
npm/@ethersproject/[email protected] None 0 498 kB ricmoo
npm/@ethersproject/[email protected] None 0 61 kB ricmoo
npm/@ethersproject/[email protected] None 0 82.2 kB ricmoo
npm/@ethersproject/[email protected] None 0 33.1 kB ricmoo
npm/@ethersproject/[email protected] None 0 11.3 kB ricmoo
npm/@ethersproject/[email protected] None 0 30.1 kB ricmoo
npm/@ethersproject/[email protected] None 0 143 kB ricmoo
npm/@ethersproject/[email protected] None 0 80.9 kB ricmoo
npm/@ethersproject/[email protected] None 0 18.7 kB ricmoo
npm/@ethersproject/[email protected] None 0 257 kB ricmoo
npm/@ethersproject/[email protected] None 0 80.7 kB ricmoo
npm/@ethersproject/[email protected] None +1 2.56 MB ricmoo
npm/@ethersproject/[email protected] None +1 59 kB ricmoo
npm/@ethersproject/[email protected] None 0 69.5 kB ricmoo
npm/@ethersproject/[email protected] None 0 47.5 kB ricmoo
npm/@ethersproject/[email protected] None 0 17.4 kB ricmoo
npm/@ethersproject/[email protected] None 0 31 kB ricmoo
npm/@ethersproject/[email protected] network +2 1.33 MB ricmoo
npm/@ethersproject/[email protected] None 0 17.3 kB ricmoo
npm/@ethersproject/[email protected] None 0 29.6 kB ricmoo
npm/@ethersproject/[email protected] None 0 21.7 kB ricmoo
npm/@ethersproject/[email protected] None 0 244 kB ricmoo
npm/@ethersproject/[email protected] None 0 119 kB ricmoo
npm/@ethersproject/[email protected] None 0 89.6 kB ricmoo
npm/@ethersproject/[email protected] network 0 128 kB ricmoo
npm/@ethersproject/[email protected] None 0 390 kB ricmoo
npm/@farcaster/[email protected] eval +1 10.9 MB sanjayprabhu
npm/@farcaster/[email protected] eval Transitive: environment, filesystem, network +34 20.4 MB ecm_merkle
npm/@farcaster/[email protected] eval Transitive: environment, network +17 57.2 MB sanjayprabhu
npm/@lit-labs/[email protected] environment 0 33.7 kB lit-robot
npm/@lit/[email protected] None 0 788 kB lit-robot
npm/@metamask/[email protected] None 0 7.65 kB whymarrh
npm/@motionone/[email protected] None +2 135 kB popmotion
npm/@motionone/[email protected] None +2 533 kB popmotion
npm/@motionone/[email protected] None 0 11 kB popmotion
npm/@motionone/[email protected] None +1 41.9 kB popmotion
npm/@noble/[email protected] None 0 1.39 MB paulmillr
npm/@noble/[email protected] None 0 737 kB paulmillr
npm/@noble/[email protected] None 0 761 kB paulmillr
npm/@noble/[email protected] None 0 111 kB paulmillr
npm/@protobufjs/[email protected] None 0 9.05 kB dcode
npm/@scure/[email protected] None 0 79.4 kB paulmillr
npm/@stablelib/[email protected] None +3 157 kB dchest
npm/@types/[email protected] None 0 3.62 MB types
npm/@types/[email protected] None +1 4.01 MB types
npm/@walletconnect/[email protected] network +5 683 kB gancho_walletconnect
npm/@walletconnect/[email protected] None 0 103 kB gancho_walletconnect
npm/@walletconnect/[email protected] None +1 188 kB gancho_walletconnect
npm/@walletconnect/[email protected] None +1 345 kB gancho_walletconnect
npm/@walletconnect/[email protected] None 0 215 kB gancho_walletconnect
npm/[email protected] None +3 555 kB vweevers
npm/[email protected] None 0 99 kB fanatid
npm/[email protected] None +1 13.9 kB dcousens
npm/[email protected] None +1 39.9 kB ljharb
npm/[email protected] None 0 5 kB vweevers
npm/[email protected] environment, filesystem +9 239 kB paulmillr
npm/[email protected] None 0 27 kB damonoehlman
npm/[email protected] None +4 247 kB indutny
npm/[email protected] None +1 91.3 kB rekmarks
npm/[email protected] None +14 2.35 MB holgerd77
npm/[email protected] None +2 244 kB silentcicero
npm/[email protected] None 0 2 MB gcanti
npm/[email protected] None 0 4.72 kB stefanpenner
npm/[email protected] eval +3 74.7 kB ljharb
npm/[email protected] filesystem Transitive: environment +3 75.6 kB isaacs
npm/[email protected] environment, filesystem 0 32.5 kB isaacs
npm/[email protected] environment, filesystem, network, shell Transitive: eval, unsafe +179 75.3 MB fvictorio
npm/[email protected] None 0 20.6 kB ljharb
npm/[email protected] None 0 4.41 kB hughsk
npm/[email protected] None 0 47.4 kB rekmarks
npm/[email protected] None 0 2.12 kB kumavis
npm/[email protected] None +3 57.8 kB isaacs
npm/[email protected] None 0 7.28 kB vweevers
npm/[email protected] environment, filesystem 0 13.4 kB mafintosh
npm/[email protected] None 0 9.22 kB jonschlinkert
npm/[email protected] None 0 90 kB mrmlnc
npm/[email protected] None 0 1.27 MB jdecroock
npm/[email protected] environment, filesystem, unsafe 0 11.2 MB prettier-bot
npm/[email protected] filesystem Transitive: environment +23 1.63 MB soldair
npm/[email protected] None 0 8.37 kB feross
npm/[email protected] None +1 15.9 kB dcousens
npm/[email protected] None 0 226 kB ricmoo
npm/[email protected] None +2 58.4 kB sindresorhus
npm/[email protected] None 0 34 kB typescript-bot
npm/[email protected] Transitive: environment, eval, filesystem, network, shell, unsafe +28 201 MB hirokiosame
npm/[email protected] None 0 389 kB hirokiosame
npm/[email protected] None 0 39.2 MB typescript-bot
npm/[email protected] None 0 32 MB typescript-bot
npm/[email protected] None +1 624 kB achingbrain
npm/[email protected] None 0 72.7 kB matteo.collina
npm/[email protected] environment +3 375 kB gnoff
npm/[email protected] network +1 5.3 MB jmoxey
npm/[email protected] network +1 6.26 MB jmoxey
npm/[email protected] filesystem, network, shell, unsafe Transitive: environment, eval +84 249 MB brc-dd
npm/[email protected] Transitive: environment, eval, filesystem, network, shell, unsafe +167 62.4 MB awkweb
npm/[email protected] None +5 108 kB ljharb
npm/[email protected] environment, filesystem 0 120 kB oss-bot

🚮 Removed packages: npm/@altafonte/[email protected], npm/@altafonte/[email protected], npm/@altafonte/[email protected], npm/@angular-devkit/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@angular/[email protected], npm/@betterer/[email protected], npm/@betterer/[email protected], npm/@betterer/[email protected], npm/@capacitor/[email protected], npm/@farcaster/[email protected], npm/@ionic/[email protected], npm/@material/[email protected], npm/@material/[email protected], npm/@material/[email protected], npm/@material/[email protected], npm/@material/[email protected], npm/@sentry/[email protected], npm/@stencil/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@typescript-eslint/[email protected], npm/@typescript-eslint/[email protected], npm/@vaadin/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]

View full report↗︎

@sds
Copy link
Member

sds commented Sep 26, 2024

Closing due to age, and not actively being pursued. Thanks!

@sds sds closed this Sep 26, 2024
@sds sds deleted the wazzymandias/perf-improve-sync branch September 26, 2024 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-perf Improve performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants