swarm/network: hive bug: needed shallow peers are not sent to nodes beyond connection's proximity order #19274
Conversation
|
Would it be possible to cover this fix with a test what would fail without the fix?
|
|
@frncmx I agree we need to improve test coverage. |
|
@zelig Ideally I would say that unit tests providing "good" coverage should be present in I this specific case though, I also run tests in both mentioned packages. However either we don't have coverage or my IDE is not smart enough to backtrack the results. Anyhow - for now - this was not a blocking comment on my side. Just be aware the danger of changing stuff without tests. |
|
I'd be happy to see a |
zelig
changed the title
holisticode
force-pushed
the
suggest-close-peers
branch
from
348b548 to
cf91181
Compare
* dashboard: fix github alerts * dashboard: run go generate
* contracts/ens: update public resolver solidity code * contracts/ens: update public resolver, update go bindings * update build * fix ens.sol * contracts/ens: change contract interface * contracts/ens: implement public resolver changes * contracts/ens: added ENSRegistry contract * contracts/ens: reinstate old contract code * contracts/ens: update README.md * contracts/ens: added test coverage for fallback contract * contracts/ens: added support for fallback contract * contracts/ens: removed unused contract code * contracts/ens: add todo and decode multicodec stub * add encode * vendor: add ipfs cid libraries * contracts/ens: cid sanity tests * contracts/ens: more cid sanity checks * contracts/ens: wip integration * wip * Revert "vendor: add ipfs cid libraries" This reverts commit 29d9b6b. * contracts/ens: removed multiformats dependencies * contracts/ens: added decode tests * contracts/ens: added eip spec test, minor changes to exiting tests * contracts/ens: moved cid decoding to own file * contracts/ens: added unit test to encode hash to content hash * contracts/ens: removed unused code * contracts/ens: fix ens tests to use cid decode and encode * contracts/ens: adjust swarm multicodecs after pr merge * contracts/ens: fix linter error * constracts/ens: address PR comments * cmd, contracts: make peoples lives easier * contracts/ens: fix linter error * contracts/ens: address PR comments
* les: fixed peer id format * les: fixed peer reply error handling
holisticode
force-pushed
the
suggest-close-peers
branch
from
70e877f to
7b6422b
Compare
holisticode
requested review from
gballet,
janos,
acud,
nonsense and
zsfelfoldi
as code owners
There was a problem hiding this comment.
- not sure why you rebased on @nolash PR, this delays
- Also we said you would change tester to accept overlay so that we can generate hive connection before tester call yet already know the trigger peer overlay
- we said you make it a repeated test
registerBzzAddrcalls are still after tester call that makes test flaky
| func newDiscPeer(bzzAddr *BzzAddr, name string, hive *Hive) *Peer { | ||
| p2pPeer := p2p.NewPeer(adapters.RandomNodeConfig().ID, name, nil) | ||
| return NewPeer(&BzzPeer{ | ||
| Peer: protocols.NewPeer(p2pPeer, &dummyMsgRW{}, DiscoverySpec), |
There was a problem hiding this comment.
no, that doesn't work, on hive.Stop() this will produce a null pointer dereference
That's my fault. Sorry. |
no actually your PR will be merged today it seems |
|
obsoleted by #19326 |
The current swarm hive would in some occasions not connect to peers in shallow bins causing unhealthy connectivity
The reason is that needed shallow peers are not sent to nodes beyond connection's proximity order.
The initial response to
subPeersMsgneeds to be modified to allow enumeration of peers up to the advertised depth not the connection PO.This PR introduces a fix to this bug