Skip to content
This repository was archived by the owner on Aug 2, 2021. It is now read-only.

swarm/network: Refactor peer suggestion engine#1073

Closed
nolash wants to merge 44 commits intomasterfrom
is-there-anybody-out-there
Closed

swarm/network: Refactor peer suggestion engine#1073
nolash wants to merge 44 commits intomasterfrom
is-there-anybody-out-there

Conversation

@nolash
Copy link
Copy Markdown
Contributor

@nolash nolash commented Dec 20, 2018

This PR aims to simplify and cleanly encapsulate the component that informs which peers to attempt connections to, and which peers to request.

The implicit aim is to attain a clear and logical separation of responsbility and functionality between the files hive.go, kademlia.go and discovery.go.

merge dependencies (in order of merge to master):

@nolash nolash force-pushed the is-there-anybody-out-there branch from 8a0d503 to 2f83537 Compare December 21, 2018 20:39
Copy link
Copy Markdown
Member

@zelig zelig left a comment

Choose a reason for hiding this comment

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

just took a look. Risky PR

changes a LOT of code around without adding a fix or algorithmic improvement

Comment thread swarm/network/hive.go
}

//
lastPotentialBin++
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

put this in the for loop line

Comment thread swarm/network/kademlia.go
})
if ins && !p.BzzPeer.LightNode {
a := newEntry(p.BzzAddr)
a.conn = p
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why remove this line?

Comment thread swarm/network/kademlia.go

// knowNearestNeighbours tests if all known nearest neighbours given as arguments
// are found in the addressbook
// knowNeighbours tests if all neighbours in the peerpot
Copy link
Copy Markdown
Member

@zelig zelig Jan 3, 2019

Choose a reason for hiding this comment

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

should this not be knownNeighbours similar to connectedNeighbours?

Comment thread swarm/network/kademlia.go
return connects == len(peers), connects, culprits
}

// connectedPotential checks whether the peer is connected to a health minimum of peers it knows about in bins that are shallower than depth
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what is wrong with saturation?

@nolash
Copy link
Copy Markdown
Contributor Author

nolash commented Jan 10, 2019

On hold pending success of (along the lines of) ethereum/go-ethereum#18404

@nolash
Copy link
Copy Markdown
Contributor Author

nolash commented Jan 11, 2019

Discontinued due to ethereum/go-ethereum#18404

@nolash nolash closed this Jan 11, 2019
@frncmx frncmx deleted the is-there-anybody-out-there branch January 22, 2019 09:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants