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

SAM-68 better libp2p implementation #51

Conversation

JoshuaCWebDeveloper
Copy link
Collaborator

@JoshuaCWebDeveloper JoshuaCWebDeveloper commented Nov 17, 2022

Overview

This PR refactors the libp2p portion of the worker, improving logic, functionality, and the implementation of libp2p.

Currently on develop

There are several bugs and issues with what is on develop:

  • The bootstrap discovery component would only add the first address in the bootstrap list. This would prevent all other bootstrap addresses from ever being added to libp2p.
  • The addressSorter only sorts addresses for the peer being dialed. Since the peer only had a single address added, this code was effectively void.
  • The Websockets transport filter doesn't necessarily do what the name implies. It filters address for that transport, but any addresses that are filtered out are handled by the next transport down the line, in this case, the circuit relay transport (assuming it can handle them). Additionally, it was filtering a list that had already been filtered with the same logic, so it was entirely redundant.

New Logic in this PR

With regards to the above state on develop, the following things are changed in this PR:

  • The bootstrap discovery component correctly adds multiple addresses for a single peer.
  • The addressSorter is now empowered to sort multiple addresses for the peer currently being dialed as intended.
  • The built-in filters.all filter is used for the Websocket transport, since it is already being given a filtered address list.

Additional Changes

In addition to the above, this PR also heavily refactors the bootstrap list logic. Addresses are now more systemically checked, excluded, ordered, and cached.

autodial has also been disabled as it would dial every single discovered peer, whereas we only want to connect to our box. Disabling autodial reduced a lot of noise from libp2p, but it did require new server connection and retry logic. However, this logic was able to replace the existing reset logic.

Observed issues

The following issues have been observed but were not necessarily introduced in this PR:

Fixed Issues

The following issues should be fixed by this PR:

@JoshuaCWebDeveloper
Copy link
Collaborator Author

JoshuaCWebDeveloper commented Nov 17, 2022

I'm going to explore disabling autodial. As we discover more peers through connecting to relays, autodial will needlessly dial them when we are just trying to reestablish a connection to our box. This also makes the effects of samizdapp/herakles#86 worse. The libp2p clients should only ever connect to our own box, not other boxes, so having autodial enabled really makes no sense.

The tradeoff is that additional connection logic will have to be implemented to connect to and retry connections to our own box.

Update: this has been done.

@JoshuaCWebDeveloper JoshuaCWebDeveloper merged commit 2c4154b into features/refactor-service-worker Nov 17, 2022
@JoshuaCWebDeveloper JoshuaCWebDeveloper deleted the SAM-68-better-libp2p-implementation branch November 17, 2022 20:33
JoshuaCWebDeveloper added a commit that referenced this pull request Nov 18, 2022
* Improve console logs on peer:connected event (#40)

* SAM-64 high level modularization (#44)

* Rename service-worker.ts to worker-messaging.ts

* Exclude self from 'no-restricted-globals' eslint rule

* Modularize worker/index.ts and replace with new root worker/service-worker.ts file

* Add missing message handlers to new refactored code

* Add trace logging for forwarding fetch requests

* Format logger output

* Create new DevTools util accessible via 'SamizdAppDevTools'

* Return root logger from logging.getLoggers()

* Write boilerplate tests for new worker

* SAM-66 refactor p2p client (#45)

* Add localforage to SamizdAppDevTools

* Add workbox logging comment to root service-worker.ts

* Refactor p2p-client into high-level OOP architecture

* proper agentversion (#48)

* Improve worker logging (#49)

* Don't use logger namespaces as logger names (makes name matching easier)

* Prefix log with method name, not logLevel setting

* Don't log using console.trace()

* Persist loglevel using localforage and correctly set default levels

* Move logging module to separate directory

* Set prettier tabWidth to 2 for yaml and .prettierrc files

* Create new logging.yaml file that sets default levels on all matched loggers

* Handle possibility of no logging config

* Fix: set log level of worker/p2p/bootstrap to INFO

* SAM-68 better libp2p implementation (#51)

* Uninstall multiaddr package (use the same one libp2p uses instead)

* Delete old worker/index.ts file (uninstalling old package caused import errors)

* Refactor bootstrap-list to more cleanly manage addresses and configure libp2p discovery

* Refactor stream-factory with cleaner logic and more robust restart

* Adjust p2p logging

* Import workbox-precaching for types (preivously was imported in index.ts)

* Add new SamizdAppDevTools.addressBook property

* Improve p2p logging

* Disable autodial and restart() and replace with serve connection retry logic

* SAM-73 don't cache empty p2p bootstrap list (#53)

Don't cache empty p2p bootstrap list

* SAM-71 migrations (with libp2p migration) (#52)

* Make JSON parsing more defensive in bootstrap-list loadCache()

* Create new migrations layer and add first migration: libp2p.bootstrap -> p2p.bootstrap-list

* SAM-72 close stats socket (#54)

Close stats websocket after collection

* SAM-65 disable static cache when running locally (#55)

Disable static cache when running locally

Co-authored-by: Ryan Bennett <nomad.ry@gmail.com>
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.

None yet

1 participant