Skip to content

Commit

Permalink
bitswap: network interface changed
Browse files Browse the repository at this point in the history
Had to change the network interface from DialPeer(peer.ID) to
DialPeer(peer.PeerInfo), so that addresses of a provider are
handed to the network.

@maybebtc and I are discussing whether this should go all the
way down to the network, or whether the network _should always
work_ with just an ID (which means the network needs to be
able to resolve ID -> Addresses, using the routing system.
This latter point might mean that "routing" might need to
break down into subcomponents. It's a bit sketchy that
the Network would become smarter than just dial/listen and
I/O, but maybe there's a distinction between net.Network,
and something like a peernet.Network that has routing
built in...)
  • Loading branch information
jbenet committed Dec 23, 2014
1 parent eff726d commit bf88f1a
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 11 deletions.
12 changes: 7 additions & 5 deletions exchange/bitswap/bitswap.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,16 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.PeerInf
message.AddEntry(wanted.Key, wanted.Priority)
}
wg := sync.WaitGroup{}
for peerToQuery := range peers {
log.Event(ctx, "PeerToQuery", peerToQuery.ID)
for pi := range peers {
log.Debugf("bitswap.sendWantListTo: %s %s", pi.ID, pi.Addrs)
log.Event(ctx, "PeerToQuery", pi.ID)
wg.Add(1)
go func(p peer.ID) {
go func(pi peer.PeerInfo) {
defer wg.Done()
p := pi.ID

log.Event(ctx, "DialPeer", p)
err := bs.sender.DialPeer(ctx, p)
err := bs.sender.DialPeer(ctx, pi)
if err != nil {
log.Errorf("Error sender.DialPeer(%s): %s", p, err)
return
Expand All @@ -198,7 +200,7 @@ func (bs *bitswap) sendWantListTo(ctx context.Context, peers <-chan peer.PeerInf
// communication fails. May require slightly different API to
// get better guarantees. May need shared sequence numbers.
bs.engine.MessageSent(p, message)
}(peerToQuery.ID)
}(pi)
}
wg.Wait()
return nil
Expand Down
2 changes: 1 addition & 1 deletion exchange/bitswap/network/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
type BitSwapNetwork interface {

// DialPeer ensures there is a connection to peer.
DialPeer(context.Context, peer.ID) error
DialPeer(context.Context, peer.PeerInfo) error

// SendMessage sends a BitSwap message to a peer.
SendMessage(
Expand Down
5 changes: 3 additions & 2 deletions exchange/bitswap/network/ipfs_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ func (bsnet *impl) handleNewStream(s inet.Stream) {

}

func (bsnet *impl) DialPeer(ctx context.Context, p peer.ID) error {
return bsnet.network.DialPeer(ctx, p)
func (bsnet *impl) DialPeer(ctx context.Context, p peer.PeerInfo) error {
bsnet.network.Peerstore().AddAddresses(p.ID, p.Addrs)
return bsnet.network.DialPeer(ctx, p.ID)
}

func (bsnet *impl) SendMessage(
Expand Down
6 changes: 3 additions & 3 deletions exchange/bitswap/testnet/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,10 @@ func (nc *networkClient) SendRequest(
return nc.network.SendRequest(ctx, nc.local, to, message)
}

func (nc *networkClient) DialPeer(ctx context.Context, p peer.ID) error {
func (nc *networkClient) DialPeer(ctx context.Context, p peer.PeerInfo) error {
// no need to do anything because dialing isn't a thing in this test net.
if !nc.network.HasPeer(p) {
return fmt.Errorf("Peer not in network: %s", p)
if !nc.network.HasPeer(p.ID) {
return fmt.Errorf("Peer not in network: %s", p.ID)
}
return nil
}
Expand Down

2 comments on commit bf88f1a

@btc
Copy link
Contributor

@btc btc commented on bf88f1a Dec 23, 2014

Choose a reason for hiding this comment

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

If bitswap provides a peer info with just one address, should the network ignore other addresses it has on file?

If not, perhaps bitswap should provide this extra address by adding it to the network peerstore.

@jbenet
Copy link
Member Author

@jbenet jbenet commented on bf88f1a Dec 23, 2014

Choose a reason for hiding this comment

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

If not, perhaps bitswap should provide this extra address by adding it to the network peerstore.

(in the case of going with "bitswap sees + gives addresses to the network") yeah, i agree with this.

Please sign in to comment.