exchange signed routing records in identify #747
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,21 +5,24 @@ import ( | |
| "context" | ||
| "io" | ||
| "reflect" | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/libp2p/go-libp2p-core/event" | ||
| "github.com/libp2p/go-libp2p-core/helpers" | ||
| "github.com/libp2p/go-libp2p-core/host" | ||
| "github.com/libp2p/go-libp2p-core/network" | ||
| "github.com/libp2p/go-libp2p-core/peer" | ||
| "github.com/libp2p/go-libp2p-core/peerstore" | ||
| "github.com/libp2p/go-libp2p-core/protocol" | ||
| "github.com/libp2p/go-libp2p-core/record" | ||
| "github.com/libp2p/go-libp2p-core/test" | ||
|
|
||
| "github.com/libp2p/go-eventbus" | ||
| swarmt "github.com/libp2p/go-libp2p-swarm/testing" | ||
| "github.com/libp2p/go-libp2p/p2p/protocol/identify" | ||
|
|
||
| ma "github.com/multiformats/go-multiaddr" | ||
| madns "github.com/multiformats/go-multiaddr-dns" | ||
| "github.com/stretchr/testify/require" | ||
|
|
@@ -102,16 +105,35 @@ func TestProtocolHandlerEvents(t *testing.T) { | |
| } | ||
| defer sub.Close() | ||
|
|
||
| // the identify service adds new protocol handlers shortly after the host | ||
| // starts. this helps us filter those events out, since they're unrelated | ||
| // to the test. | ||
| isIdentify := func(evt event.EvtLocalProtocolsUpdated) bool { | ||
| for _, p := range evt.Added { | ||
| if p == identify.ID || p == identify.IDPush { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| nextEvent := func() event.EvtLocalProtocolsUpdated { | ||
| for { | ||
| select { | ||
| case evt := <-sub.Out(): | ||
| next := evt.(event.EvtLocalProtocolsUpdated) | ||
| if isIdentify(next) { | ||
| continue | ||
| } | ||
| return next | ||
| case <-time.After(5 * time.Second): | ||
| t.Fatal("event not received in 5 seconds") | ||
|
There was a problem hiding this comment. Fatal must be called from the main goroutine: https://golang.org/pkg/testing/#T There was a problem hiding this comment. This code is being executed on the main goroutine, not on a separate one. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| assert := func(added, removed []protocol.ID) { | ||
| next := nextEvent() | ||
| if !reflect.DeepEqual(added, next.Added) { | ||
| t.Errorf("expected added: %v; received: %v", added, next.Added) | ||
| } | ||
|
|
@@ -443,11 +465,10 @@ func TestAddrResolution(t *testing.T) { | |
| _ = h.Connect(tctx, *pi) | ||
|
|
||
| addrs := h.Peerstore().Addrs(pi.ID) | ||
|
|
||
| require.Len(t, addrs, 2) | ||
| require.Contains(t, addrs, addr1) | ||
| require.Contains(t, addrs, addr2) | ||
| } | ||
|
|
||
| func TestAddrResolutionRecursive(t *testing.T) { | ||
|
|
@@ -498,11 +519,9 @@ func TestAddrResolutionRecursive(t *testing.T) { | |
| _ = h.Connect(tctx, *pi1) | ||
|
|
||
| addrs1 := h.Peerstore().Addrs(pi1.ID) | ||
| require.Len(t, addrs1, 2) | ||
| require.Contains(t, addrs1, addr1) | ||
| require.Contains(t, addrs1, addr2) | ||
|
|
||
| pi2, err := peer.AddrInfoFromP2pAddr(p2paddr2) | ||
| if err != nil { | ||
|
|
@@ -512,11 +531,49 @@ func TestAddrResolutionRecursive(t *testing.T) { | |
| _ = h.Connect(tctx, *pi2) | ||
|
|
||
| addrs2 := h.Peerstore().Addrs(pi2.ID) | ||
| require.Len(t, addrs2, 1) | ||
| require.Contains(t, addrs2, addr1) | ||
| } | ||
|
|
||
| func TestAddrChangeImmediatelyIfAddressNonEmpty(t *testing.T) { | ||
| ctx := context.Background() | ||
| taddrs := []ma.Multiaddr{ma.StringCast("/ip4/1.2.3.4/tcp/1234")} | ||
|
|
||
| h := New(swarmt.GenSwarm(t, ctx), AddrsFactory(func(addrs []ma.Multiaddr) []ma.Multiaddr { | ||
| return taddrs | ||
| })) | ||
| defer h.Close() | ||
|
|
||
| sub, err := h.EventBus().Subscribe(&event.EvtLocalAddressesUpdated{}) | ||
| if err != nil { | ||
| t.Error(err) | ||
| } | ||
| defer sub.Close() | ||
| // wait for the host background thread to start | ||
| time.Sleep(1 * time.Second) | ||
|
|
||
| expected := event.EvtLocalAddressesUpdated{ | ||
| Diffs: true, | ||
| Current: []event.UpdatedAddress{ | ||
| {Action: event.Added, Address: ma.StringCast("/ip4/1.2.3.4/tcp/1234")}, | ||
| }, | ||
| Removed: []event.UpdatedAddress{}} | ||
|
|
||
| // assert we get expected event | ||
| evt := waitForAddrChangeEvent(ctx, sub, t) | ||
| if !updatedAddrEventsEqual(expected, evt) { | ||
| t.Errorf("change events not equal: \n\texpected: %v \n\tactual: %v", expected, evt) | ||
| } | ||
|
|
||
| // assert it's on the signed record | ||
| rc := peerRecordFromEnvelope(t, evt.SignedPeerRecord) | ||
| require.Equal(t, taddrs, rc.Addrs) | ||
|
|
||
| // assert it's in the peerstore | ||
| ev := h.Peerstore().(peerstore.CertifiedAddrBook).GetPeerRecord(h.ID()) | ||
| require.NotNil(t, ev) | ||
| rc = peerRecordFromEnvelope(t, *ev) | ||
| require.Equal(t, taddrs, rc.Addrs) | ||
| } | ||
|
|
||
| func TestHostAddrChangeDetection(t *testing.T) { | ||
yusefnapora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
@@ -594,9 +651,18 @@ func TestHostAddrChangeDetection(t *testing.T) { | |
| h.SignalAddressChange() | ||
| evt := waitForAddrChangeEvent(ctx, sub, t) | ||
| if !updatedAddrEventsEqual(expectedEvents[i-1], evt) { | ||
| t.Errorf("change events not equal: \n\texpected: %v \n\tactual: %v", expectedEvents[i-1], evt) | ||
| } | ||
|
|
||
| // assert it's on the signed record | ||
| rc := peerRecordFromEnvelope(t, evt.SignedPeerRecord) | ||
| require.Equal(t, addrSets[i], rc.Addrs) | ||
|
|
||
| // assert it's in the peerstore | ||
| ev := h.Peerstore().(peerstore.CertifiedAddrBook).GetPeerRecord(h.ID()) | ||
| require.NotNil(t, ev) | ||
| rc = peerRecordFromEnvelope(t, *ev) | ||
| require.Equal(t, addrSets[i], rc.Addrs) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -655,10 +721,17 @@ func updatedAddrEventsEqual(a, b event.EvtLocalAddressesUpdated) bool { | |
| updatedAddrsEqual(a.Removed, b.Removed) | ||
| } | ||
|
|
||
| func peerRecordFromEnvelope(t *testing.T, ev record.Envelope) *peer.PeerRecord { | ||
| t.Helper() | ||
| rec, err := ev.Record() | ||
| if err != nil { | ||
| t.Fatalf("error getting PeerRecord from event: %v", err) | ||
| return nil | ||
| } | ||
| peerRec, ok := rec.(*peer.PeerRecord) | ||
| if !ok { | ||
| t.Fatalf("wrong type for peer record") | ||
| return nil | ||
| } | ||
| return peerRec | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this should be stateful. Are we doing this to get around the fact that we don't have some form of
GetPeerRecord()method as described in #747 (comment)? If so, we should just add that method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Stebalien We have a
GetPeerRecord(p peer.ID)on the peerstore now.However, calling that when you get a
EvtLocalAddressChangedevent could be racy as the address you see on the event could be different fro the signed record you get from the peerstore.This avoids that race.