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

swarm/network: Make MinProxBinSize constant across simulation#1074

Closed
nolash wants to merge 29 commits intomasterfrom
sim-minproxbinsize-isolationism
Closed

swarm/network: Make MinProxBinSize constant across simulation#1074
nolash wants to merge 29 commits intomasterfrom
sim-minproxbinsize-isolationism

Conversation

@nolash
Copy link
Copy Markdown
Contributor

@nolash nolash commented Dec 20, 2018

@nolash nolash force-pushed the sim-minproxbinsize-isolationism branch from 730181f to 2627104 Compare December 21, 2018 20:19
Comment thread swarm/network/kademlia.go
log.Trace(k.string())
// calculate if depth of saturation changed
depth := uint8(k.saturation(k.MinBinSize))
depth := uint8(k.saturation())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return uint8 from saturation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, because bins are int everywhere else. @nonsense raised the question whether bin should be its own type. We could consider this, but then let's do it in one PR and change it everywhere.

Comment thread swarm/network/kademlia.go Outdated
// It is used in Healthy function for testing only
func (k *Kademlia) saturation(n int) int {
// saturation iterates through all peers and
// returns the smallest po value in which the node has less than n peers
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"n peers" is no longer in the signature

Comment thread swarm/network/kademlia.go
// iterate through nearest neighbors in the peerpot map
// if we can't find the neighbor in the map we created above
// then we don't know all our neighbors
// (which sadly is all too common in modern society)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

😂 ❤️

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.

pss changes should be removed. how did they creep into this pr?

Comment thread swarm/network/kademlia.go
gots++
} else {
log.Trace(fmt.Sprintf("%08x: ExpNN: %s not found", k.BaseAddr()[:4], pk[:8]))
log.Trace(fmt.Sprintf("%08x: ExpNN: %s not found", k.base, pk))
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.

eliminate Spintf-s in logging

shutdownWG sync.WaitGroup
done chan struct{}
mu sync.RWMutex
minProxBinSize int
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.

i said my piece about renaming, now we are introducing a new field thats gonna be renamed. please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can you say your peace again, please? I can't recall what name we have agreed on using.

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.

e.g. NeighbourCount

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.

the discussion was in the depth PR

done: make(chan struct{}),
buckets: make(map[enode.ID]*sync.Map),
done: make(chan struct{}),
minProxBinSize: network.NewKadParams().MinProxBinSize,
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.

here too

@@ -0,0 +1,356 @@
package pss
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.

how did this get here?? totally out of scope

@nolash
Copy link
Copy Markdown
Contributor Author

nolash commented Jan 8, 2019

Obsoleted by ethereum/go-ethereum#18408

@nolash nolash closed this Jan 8, 2019
@frncmx frncmx deleted the sim-minproxbinsize-isolationism 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