dht find connected peers #428
Conversation
this is the type of assumption we shouldn't violate.
Uses an inet.Dialer
jbenet
force-pushed
the
dht-connected-peers
branch
from
ab1d12d
to
e0f11df
Compare
|
@whyrusleeping @maybebtc RFCR |
|
|
||
| const ( | ||
| // NotConnected means no connection to peer, and no extra information (default) | ||
| NotConnected Connectedness = 0 |
There was a problem hiding this comment.
minor thing but you can use iota to enumerate
const (
Not Connectedness = iota
Connected
Can
Cannot
)There was a problem hiding this comment.
yeah, i wanted to be explicit here to make sure it matches Message_ConnectionType_value. This may not be the best idea, perhaps i should import those symbols.
There was a problem hiding this comment.
may not need it, given the functions: https://github.com/jbenet/go-ipfs/pull/428/files#diff-d7b3509b9fcd5fce42069cf169ede81eR142
do you care much either way?
There was a problem hiding this comment.
yeah. functions like those are kosher 👍
given the existence of those functions, i don't see a reason to rely on the enum value to provide a mapping between the types. iota seems like an all-around better deal.
but it's really not a big deal. i don't forsee a bug being introduced due to the decision. 🌲 ✊ : 🌲
Looking at the big picture, where would you want me to focus CR attention? Is there anything particular you're concerned about?
(It's easy to get lost in syntax)
There was a problem hiding this comment.
but it's really not a big deal. i don't forsee a bug being introduced due to the decision.
agreed. same here, i think this was a vestige from before those funcs. addressed in 93872a5
Looking at the big picture, where would you want me to focus CR attention? Is there anything particular you're concerned about?
Mostly the notion of "connectedness" and DHT logic. the added dht query https://github.com/jbenet/go-ipfs/pull/428/files#diff-b4153ad02ca247a808e7a06fd1e2f133R272 -- it's mostly the same as the others, with small tweaks
| // FindPeersConnectedToPeer searches for peers directly connected to a given peer. | ||
| func (dht *IpfsDHT) FindPeersConnectedToPeer(ctx context.Context, id peer.ID) (<-chan peer.Peer, error) { | ||
|
|
||
| peerchan := make(chan peer.Peer, 10) |
There was a problem hiding this comment.
minor but
Perhaps we should start extracting these chanbuffer constants so we preserve information that went into the decision to choose the particular value.
0 and 1 are obvious to the reader, but arbitrary c, I can never be sure.
There was a problem hiding this comment.
Good point. addressed in: 26a44fc
|
LGTM |
Refresh cpl's in dht
This PR extends the DHT to send information re: peer "connectedness".
This means that --upon resolving dht queries -- connected peers will
signal whether or not they are directly connected to the peer. this is
in preparation for mesh messaging, and our "STUN" for NAT traversal.