Skip to content

Commit

Permalink
fix dht commands when pubsub routing is enabled
Browse files Browse the repository at this point in the history 
Instead of checking if Routing is a DHT (because it can now be a tiered
router and still contain a DHT), stash the DHT in a separate field in the IPFS
node (same as we do with the PSRouter).

fixes #5197

License: MIT
Signed-off-by: Steven Allen <[email protected]>
  • Loading branch information
@Stebalien
Stebalien committed Jul 9, 2018
1 parent cb0ad0d commit 7df7d11
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 114 deletions.
57 changes: 27 additions & 30 deletions core/commands/dht.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
dag "github.com/ipfs/go-ipfs/merkledag"
path "github.com/ipfs/go-ipfs/path"

ipdht "gx/ipfs/QmNg6M98bwS97SL9ArvrRxKujFps3eV6XvmKgduiYga8Bn/go-libp2p-kad-dht"
routing "gx/ipfs/QmPpdpS9fknTBM3qHDcpayU6nYPZQeVjia2fbNrD8YWDe6/go-libp2p-routing"
notif "gx/ipfs/QmPpdpS9fknTBM3qHDcpayU6nYPZQeVjia2fbNrD8YWDe6/go-libp2p-routing/notifications"
b58 "gx/ipfs/QmWFAMPqsEyUX7gDUsRVmMWz59FxSpJ1b2v6bJ1yYzo7jY/go-base58-fast/base58"
Expand All @@ -26,6 +25,9 @@ import (

var ErrNotDHT = errors.New("routing service is not a DHT")

// TODO: Factor into `ipfs dht` and `ipfs routing`.
// Everything *except `query` goes into `ipfs routing`.

var DhtCmd = &cmds.Command{
Helptext: cmdkit.HelpText{
Tagline: "Issue commands directly through the DHT.",
Expand Down Expand Up @@ -61,8 +63,7 @@ var queryDhtCmd = &cmds.Command{
return
}

dht, ok := n.Routing.(*ipdht.IpfsDHT)
if !ok {
if n.DHT == nil {
res.SetError(ErrNotDHT, cmdkit.ErrNormal)
return
}
Expand All @@ -76,7 +77,7 @@ var queryDhtCmd = &cmds.Command{
return
}

closestPeers, err := dht.GetClosestPeers(ctx, string(id))
closestPeers, err := n.DHT.GetClosestPeers(ctx, string(id))
if err != nil {
res.SetError(err, cmdkit.ErrNormal)
return
Expand Down Expand Up @@ -140,7 +141,7 @@ var queryDhtCmd = &cmds.Command{

var findProvidersDhtCmd = &cmds.Command{
Helptext: cmdkit.HelpText{
Tagline: "Find peers in the DHT that can provide a specific value, given a key.",
Tagline: "Find peers that can provide a specific value, given a key.",
ShortDescription: "Outputs a list of newline-delimited provider Peer IDs.",
},

Expand All @@ -158,9 +159,8 @@ var findProvidersDhtCmd = &cmds.Command{
return
}

dht, ok := n.Routing.(*ipdht.IpfsDHT)
if !ok {
res.SetError(ErrNotDHT, cmdkit.ErrNormal)
if n.Routing == nil {
res.SetError(errNotOnline, cmdkit.ErrNormal)
return
}

Expand All @@ -186,7 +186,7 @@ var findProvidersDhtCmd = &cmds.Command{
outChan := make(chan interface{})
res.SetOutput((<-chan interface{})(outChan))

pchan := dht.FindProvidersAsync(ctx, c, numProviders)
pchan := n.Routing.FindProvidersAsync(ctx, c, numProviders)
go func() {
defer close(outChan)
for e := range events {
Expand Down Expand Up @@ -406,7 +406,7 @@ func provideKeysRec(ctx context.Context, r routing.IpfsRouting, dserv ipld.DAGSe

var findPeerDhtCmd = &cmds.Command{
Helptext: cmdkit.HelpText{
Tagline: "Query the DHT for all of the multiaddresses associated with a Peer ID.",
Tagline: "Find the multiaddresses associated with a Peer ID.",
ShortDescription: "Outputs a list of newline-delimited multiaddresses.",
},

Expand All @@ -423,9 +423,8 @@ var findPeerDhtCmd = &cmds.Command{
return
}

dht, ok := n.Routing.(*ipdht.IpfsDHT)
if !ok {
res.SetError(ErrNotDHT, cmdkit.ErrNormal)
if n.Routing == nil {
res.SetError(errNotOnline, cmdkit.ErrNormal)
return
}

Expand Down Expand Up @@ -454,7 +453,7 @@ var findPeerDhtCmd = &cmds.Command{

go func() {
defer close(events)
pi, err := dht.FindPeer(ctx, pid)
pi, err := n.Routing.FindPeer(ctx, pid)
if err != nil {
notif.PublishQueryEvent(ctx, &notif.QueryEvent{
Type: notif.QueryError,
Expand Down Expand Up @@ -504,14 +503,14 @@ var findPeerDhtCmd = &cmds.Command{

var getValueDhtCmd = &cmds.Command{
Helptext: cmdkit.HelpText{
Tagline: "Given a key, query the DHT for its best value.",
Tagline: "Given a key, query the routing system for its best value.",
ShortDescription: `
Outputs the best value for the given key.
There may be several different values for a given key stored in the DHT; in
this context 'best' means the record that is most desirable. There is no one
metric for 'best': it depends entirely on the key type. For IPNS, 'best' is
the record that is both valid and has the highest sequence number (freshest).
There may be several different values for a given key stored in the routing
system; in this context 'best' means the record that is most desirable. There is
no one metric for 'best': it depends entirely on the key type. For IPNS, 'best'
is the record that is both valid and has the highest sequence number (freshest).
Different key types can specify other 'best' rules.
`,
},
Expand All @@ -529,9 +528,8 @@ Different key types can specify other 'best' rules.
return
}

dht, ok := n.Routing.(*ipdht.IpfsDHT)
if !ok {
res.SetError(ErrNotDHT, cmdkit.ErrNormal)
if n.Routing == nil {
res.SetError(errNotOnline, cmdkit.ErrNormal)
return
}

Expand Down Expand Up @@ -559,7 +557,7 @@ Different key types can specify other 'best' rules.

go func() {
defer close(events)
val, err := dht.GetValue(ctx, dhtkey)
val, err := n.Routing.GetValue(ctx, dhtkey)
if err != nil {
notif.PublishQueryEvent(ctx, &notif.QueryEvent{
Type: notif.QueryError,
Expand Down Expand Up @@ -610,18 +608,18 @@ Different key types can specify other 'best' rules.

var putValueDhtCmd = &cmds.Command{
Helptext: cmdkit.HelpText{
Tagline: "Write a key/value pair to the DHT.",
Tagline: "Write a key/value pair to the routing system.",
ShortDescription: `
Given a key of the form /foo/bar and a value of any form, this will write that
value to the DHT with that key.
value to the routing system with that key.
Keys have two parts: a keytype (foo) and the key name (bar). IPNS uses the
/ipns keytype, and expects the key name to be a Peer ID. IPNS entries are
specifically formatted (protocol buffer).
You may only use keytypes that are supported in your ipfs binary: currently
this is only /ipns. Unless you have a relatively deep understanding of the
go-ipfs DHT internals, you likely want to be using 'ipfs name publish' instead
go-ipfs routing internals, you likely want to be using 'ipfs name publish' instead
of this.
Value is arbitrary text. Standard input can be used to provide value.
Expand All @@ -644,9 +642,8 @@ NOTE: A value may not exceed 2048 bytes.
return
}

dht, ok := n.Routing.(*ipdht.IpfsDHT)
if !ok {
res.SetError(ErrNotDHT, cmdkit.ErrNormal)
if n.Routing == nil {
res.SetError(errNotOnline, cmdkit.ErrNormal)
return
}

Expand Down Expand Up @@ -677,7 +674,7 @@ NOTE: A value may not exceed 2048 bytes.

go func() {
defer close(events)
err := dht.PutValue(ctx, key, []byte(data))
err := n.Routing.PutValue(ctx, key, []byte(data))
if err != nil {
notif.PublishQueryEvent(ctx, &notif.QueryEvent{
Type: notif.QueryError,
Expand Down
22 changes: 20 additions & 2 deletions core/core.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ type IpfsNode struct {

Floodsub *floodsub.PubSub
PSRouter *psrouter.PubsubValueStore
DHT *dht.IpfsDHT
P2P *p2p.P2P

proc goprocess.Process
Expand Down Expand Up @@ -465,6 +466,23 @@ func (n *IpfsNode) startOnlineServicesWithHost(ctx context.Context, host p2phost
}
n.Routing = r

// TODO: I'm not a fan of type assertions like this but the
// `RoutingOption` system doesn't currently provide access to the
// IpfsNode.
//
// Ideally, we'd do something like:
//
// 1. Add some fancy method to introspect into tiered routers to extract
// things like the pubsub router or the DHT (complicated, messy,
// probably not worth it).
// 2. Pass the IpfsNode into the RoutingOption (would also remove the
// PSRouter case below.
// 3. Introduce some kind of service manager? (my personal favorite but
// that requires a fair amount of work).
if dht, ok := r.(*dht.IpfsDHT); ok {
n.DHT = dht
}

if ipnsps {
n.PSRouter = psrouter.NewPubsubValueStore(
ctx,
Expand Down Expand Up @@ -601,8 +619,8 @@ func (n *IpfsNode) teardown() error {
closers = append(closers, mount.Closer(n.Mounts.Ipns))
}

if dht, ok := n.Routing.(*dht.IpfsDHT); ok {
closers = append(closers, dht.Process())
if n.DHT != nil {
closers = append(closers, n.DHT.Process())
}

if n.Blocks != nil {
Expand Down
170 changes: 88 additions & 82 deletions test/sharness/t0170-dht.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,87 +7,93 @@ test_description="Test dht command"
TEST_DHT_VALUE="foobar"
TEST_DHT_PATH="/pk/QmbWTwYGcmdyK9CYfNBcfs9nhZs17a6FQ4Y8oea278xx41"

# start iptb + wait for peering
NUM_NODES=5
test_expect_success 'init iptb' '
iptb init -n $NUM_NODES --bootstrap=none --port=0
'

startup_cluster $NUM_NODES

test_expect_success 'peer ids' '
PEERID_0=$(iptb get id 0) &&
PEERID_2=$(iptb get id 2)
'

# ipfs dht findpeer <peerID>
test_expect_success 'findpeer' '
ipfsi 1 dht findpeer $PEERID_0 | sort >actual &&
ipfsi 0 id -f "<addrs>" | cut -d / -f 1-5 | sort >expected &&
test_cmp actual expected
'

# ipfs dht put <key> <value>
test_expect_success 'put with good keys' '
ipfsi 0 dht put "$TEST_DHT_PATH" "$TEST_DHT_VALUE" | sort >putted &&
[ -s putted ] ||
test_fsh cat putted
'

# ipfs dht get <key>
test_expect_success 'get with good keys' '
HASH="$(echo "hello world" | ipfsi 2 add -q)" &&
ipfsi 2 name publish "/ipfs/$HASH" &&
ipfsi 1 dht get "/ipns/$PEERID_2" | grep -aq "/ipfs/$HASH"
'

test_expect_success 'put with bad keys fails (issue #5113)' '
ipfsi 0 dht put "foo" "bar" >putted
ipfsi 0 dht put "/pk/foo" "bar" >>putted
ipfsi 0 dht put "/ipns/foo" "bar" >>putted
[ ! -s putted ] ||
test_fsh cat putted
'

test_expect_failure 'put with bad keys returns error (issue #4611)' '
! ipfsi 0 dht put "foo" "bar" &&
! ipfsi 0 dht put "/pk/foo" "bar" &&
! ipfsi 0 dht put "/ipns/foo" "bar"
'

test_expect_failure 'get with bad keys (issue #4611)' '
! ipfsi 0 dht get "foo" &&
! ipfsi 0 dht get "/pk/foo"
'

test_expect_success "add a ref so we can find providers for it" '
echo "some stuff" > afile &&
HASH=$(ipfsi 3 add -q afile)
'

# ipfs dht findprovs <key>
test_expect_success 'findprovs' '
ipfsi 4 dht findprovs $HASH > provs &&
iptb get id 3 > expected &&
test_cmp provs expected
'


# ipfs dht query <peerID>
## We query 3 different keys, to statisically lower the chance that the queryer
## turns out to be the closest to what a key hashes to.
# TODO: flaky. tracked by https://github.com/ipfs/go-ipfs/issues/2620
test_expect_success 'query' '
ipfsi 3 dht query "$(echo banana | ipfsi 3 add -q)" >actual &&
ipfsi 3 dht query "$(echo apple | ipfsi 3 add -q)" >>actual &&
ipfsi 3 dht query "$(echo pear | ipfsi 3 add -q)" >>actual &&
PEERS=$(wc -l actual | cut -d '"'"' '"'"' -f 1) &&
[ -s actual ] ||
test_might_fail test_fsh cat actual
'

test_expect_success 'stop iptb' '
iptb stop
'
test_dht() {
NUM_NODES=5

test_expect_success 'init iptb' '
rm -rf .iptb/ &&
iptb init -n $NUM_NODES --bootstrap=none --port=0
'

startup_cluster $NUM_NODES "$@"

test_expect_success 'peer ids' '
PEERID_0=$(iptb get id 0) &&
PEERID_2=$(iptb get id 2)
'

# ipfs dht findpeer <peerID>
test_expect_success 'findpeer' '
ipfsi 1 dht findpeer $PEERID_0 | sort >actual &&
ipfsi 0 id -f "<addrs>" | cut -d / -f 1-5 | sort >expected &&
test_cmp actual expected
'

# ipfs dht put <key> <value>
test_expect_success 'put with good keys' '
ipfsi 0 dht put "$TEST_DHT_PATH" "$TEST_DHT_VALUE" | sort >putted &&
[ -s putted ] ||
test_fsh cat putted
'

# ipfs dht get <key>
test_expect_success 'get with good keys' '
HASH="$(echo "hello world" | ipfsi 2 add -q)" &&
ipfsi 2 name publish "/ipfs/$HASH" &&
ipfsi 1 dht get "/ipns/$PEERID_2" | grep -aq "/ipfs/$HASH"
'

test_expect_success 'put with bad keys fails (issue #5113)' '
ipfsi 0 dht put "foo" "bar" >putted
ipfsi 0 dht put "/pk/foo" "bar" >>putted
ipfsi 0 dht put "/ipns/foo" "bar" >>putted
[ ! -s putted ] ||
test_fsh cat putted
'

test_expect_failure 'put with bad keys returns error (issue #4611)' '
! ipfsi 0 dht put "foo" "bar" &&
! ipfsi 0 dht put "/pk/foo" "bar" &&
! ipfsi 0 dht put "/ipns/foo" "bar"
'

test_expect_failure 'get with bad keys (issue #4611)' '
! ipfsi 0 dht get "foo" &&
! ipfsi 0 dht get "/pk/foo"
'

test_expect_success "add a ref so we can find providers for it" '
echo "some stuff" > afile &&
HASH=$(ipfsi 3 add -q afile)
'

# ipfs dht findprovs <key>
test_expect_success 'findprovs' '
ipfsi 4 dht findprovs $HASH > provs &&
iptb get id 3 > expected &&
test_cmp provs expected
'


# ipfs dht query <peerID>
## We query 3 different keys, to statisically lower the chance that the queryer
## turns out to be the closest to what a key hashes to.
# TODO: flaky. tracked by https://github.com/ipfs/go-ipfs/issues/2620
test_expect_success 'query' '
ipfsi 3 dht query "$(echo banana | ipfsi 3 add -q)" >actual &&
ipfsi 3 dht query "$(echo apple | ipfsi 3 add -q)" >>actual &&
ipfsi 3 dht query "$(echo pear | ipfsi 3 add -q)" >>actual &&
PEERS=$(wc -l actual | cut -d '"'"' '"'"' -f 1) &&
[ -s actual ] ||
test_might_fail test_fsh cat actual
'

test_expect_success 'stop iptb' '
iptb stop
'
}

test_dht
test_dht --enable-pubsub-experiment --enable-namesys-pubsub

test_done

0 comments on commit 7df7d11

Please sign in to comment.