erlang-libp2p icon indicating copy to clipboard operation
erlang-libp2p copied to clipboard

standalone libp2p-peerbook integration

Open andymck opened this issue 4 years ago • 1 comments

Linked PRs: blockchain_core: https://github.com/helium/blockchain-core/pull/413 libp2p_peerbook: https://github.com/helium/libp2p-peerbook/pull/2

Test Miner PR runs which incorporates the erlang-libp2p, libp2p_peerbook and blockchain_core PRs:

https://github.com/helium/miner/pull/357

This replaces the libp2p_peerbook integrated within erlang libp2p with the new standalone libp2p_peerbook library.

Other than updating relevant calls to point to the new library, some points to note:

  1. Standalone libp2p_peerbook will now register its handle as part of its init with erlang-libp2p. As such even if the peerbook crashes and restarts, erlang-libp2p will always have the latest and correct handle

  2. The erlang-libp2p swarm TID is now just the swarm name as the ETS table is now using 'named_table'. This gives more flexibility, basically as long as a process knows the swarm name, it can access the ETS data. Whilst the code does not require a named table and will work without it, it opens up opportunities to cleanup some code which I will address later.

  3. Eligible peer capability is now mostly handled outside of libp2p_peerbook and in erlang-libp2p's libp2p_peer_add_gossip module.

  4. libp2p_peer_resolution now plays home to various helper modules which used to exist in libp2p_peer such as is_dialable, is_public_ip, is_private_ip and a few others. This module seemed the best place for these...

  5. The peerbook is under its own supervisor which itself is a child of the swarm supervisor. This gives the peerbook the ability to restart within taking down other swarm processes ( unless the peerbook restart strategy is breached )

  6. Only valid listen addrs will reach the peerbook. If the addr is not valid it will not get registered

NOTE: to get all tests passing reliably I have had to tweak various wait_until timings and also play with the notify and peer timer configs. Some of this was surely to counter generally inconsistent and time sensitive tests but I cant rule out something at the code level which I may have missed and which could be leading to a difference in updates from the peerbook.

andymck avatar Mar 20 '20 15:03 andymck

Codecov Report

Merging #265 (eeba5fe) into master (b6969bd) will decrease coverage by 0.87%. The diff coverage is 72.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #265      +/-   ##
==========================================
- Coverage   73.83%   72.95%   -0.88%     
==========================================
  Files          64       65       +1     
  Lines        3780     3476     -304     
==========================================
- Hits         2791     2536     -255     
+ Misses        989      940      -49     
Impacted Files Coverage Δ
src/group/libp2p_group_gossip.erl 75.00% <ø> (ø)
src/group/libp2p_group_worker_sup.erl 100.00% <ø> (ø)
src/proxy/libp2p_transport_proxy.erl 54.54% <0.00%> (ø)
src/relay/libp2p_relay.erl 90.90% <ø> (+0.90%) :arrow_up:
src/libp2p_transport_tcp.erl 67.63% <7.14%> (-0.81%) :arrow_down:
src/group/libp2p_peer_remove_gossip.erl 42.85% <42.85%> (ø)
src/group/libp2p_peer_resolution.erl 45.65% <45.65%> (ø)
src/group/libp2p_group_relcast_server.erl 61.25% <50.00%> (-0.45%) :arrow_down:
src/libp2p_config.erl 84.31% <61.53%> (-2.79%) :arrow_down:
src/libp2p_swarm.erl 75.00% <66.66%> (-6.04%) :arrow_down:
... and 39 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9e41e97...eeba5fe. Read the comment docs.

codecov[bot] avatar Apr 01 '20 12:04 codecov[bot]