get rid of unsigned RoutingState struct, only expose SignedRoutingState

crypto/envelope.go

@@ -78,7 +78,7 @@ func OpenEnvelope(envelopeBytes []byte, domain string) (*SignedEnvelope, error)
 // without validating its contents. Should not be used unless you have a very good reason
 // (e.g. testing)!
 func UnmarshalEnvelopeWithoutValidating(serializedEnvelope []byte) (*SignedEnvelope, error) {
-	e := pb.SignedEnvelope{}
+	var e pb.SignedEnvelope
 	if err := proto.Unmarshal(serializedEnvelope, &e); err != nil {
 		return nil, err
 	}

host/helpers.go

@@ -2,7 +2,6 @@ package host
 
 import (
 	"errors"
-	"github.com/libp2p/go-libp2p-core/crypto"
 	"github.com/libp2p/go-libp2p-core/peer"
 	"github.com/libp2p/go-libp2p-core/routing"
 )
@@ -15,19 +14,13 @@ func InfoFromHost(h Host) *peer.AddrInfo {
 	}
 }
 
-// RoutingStateFromHost returns a routing.RoutingState record that contains the Host's
-// ID and all of its listen Addrs.
-func RoutingStateFromHost(h Host) *routing.RoutingState {
-	return routing.RoutingStateFromAddrInfo(InfoFromHost(h))
-}
-
-// SignedRoutingStateFromHost wraps a routing.RoutingState record in a
-// SignedEnvelope, signed with the Host's private key.
-func SignedRoutingStateFromHost(h Host) (*crypto.SignedEnvelope, error) {
+// SignedRoutingStateFromHost returns a SignedRoutingState record containing
+// the Host's listen addresses, signed with the Host's private key.
+func SignedRoutingStateFromHost(h Host) (*routing.SignedRoutingState, error) {
 	privKey := h.Peerstore().PrivKey(h.ID())
 	if privKey == nil {
 		return nil, errors.New("unable to find host's private key in peerstore")
 	}
 
-	return RoutingStateFromHost(h).ToSignedEnvelope(privKey)
+	return routing.MakeSignedRoutingState(privKey, h.Addrs())
 }

peerstore/peerstore.go

@@ -5,6 +5,7 @@ package peerstore
 import (
 	"context"
 	"errors"
+	"github.com/libp2p/go-libp2p-core/routing"
 	"io"
 	"math"
 	"time"
@@ -96,9 +97,9 @@ type AddrBook interface {
 	// If the manager has a longer TTL, the operation is a no-op for that address
 	AddAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Duration)
 
-	// AddCertifiedAddrs adds addresses from a routing.RoutingState record
-	// contained in a serialized SignedEnvelope.
-	AddCertifiedAddrs(envelope []byte, ttl time.Duration) error
+	// AddCertifiedAddrs adds addresses from a routing.SignedRoutingState record.
+	// Certified addresses will be returned from both Addrs and CertifiedAddrs.
+	AddCertifiedAddrs(s *routing.SignedRoutingState, ttl time.Duration) error
 
 	// SetAddr calls mgr.SetAddrs(p, addr, ttl)
 	SetAddr(p peer.ID, addr ma.Multiaddr, ttl time.Duration)
@@ -111,12 +112,13 @@ type AddrBook interface {
 	// the given oldTTL to have the given newTTL.
 	UpdateAddrs(p peer.ID, oldTTL time.Duration, newTTL time.Duration)
 
-	// Addrs returns all known (and valid) addresses for a given peer
+	// Addrs returns all known (and valid) addresses for a given peer, including
+	// both certified and uncertified addresses.
 	Addrs(p peer.ID) []ma.Multiaddr
 
 	// CertifiedAddrs returns all known addresses for a peer that have
-	// been certified by that peer. CertifiedAddrs are contained in
-	// a SignedEnvelope and added to the Peerstore using AddCertifiedAddrs.
+	// been certified by that peer and added to the peerstore using
+	// AddCertifiedAddrs. Note that certified addrs are also returned.
 	CertifiedAddrs(p peer.ID) []ma.Multiaddr
 
 	// AddrStream returns a channel that gets all addresses for a given
@@ -130,17 +132,16 @@ type AddrBook interface {
 	// PeersWithAddrs returns all of the peer IDs stored in the AddrBook
 	PeersWithAddrs() peer.IDSlice
 
-	// SignedRoutingState returns a signed RoutingState record for the
-	// given peer id, if one exists in the peerstore. The record is
-	// returned as a byte slice containing a serialized SignedEnvelope.
+	// SignedRoutingState returns a SignedRoutingState record for the
+	// given peer id, if one exists in the peerstore.
 	// Returns nil if no routing state exists for the peer.
-	SignedRoutingState(p peer.ID) []byte
+	SignedRoutingState(p peer.ID) *routing.SignedRoutingState
 
-	// SignedRoutingStates returns signed RoutingState records for each of
+	// SignedRoutingStates returns SignedRoutingState records for each of
 	// the given peer ids, if one exists in the peerstore.
-	// Returns a map of peer ids to serialized SignedEnvelope messages. If
+	// Returns a map of peer ids to SignedRoutingState records. If
 	// no routing state exists for a peer, their map entry will be nil.
-	SignedRoutingStates(peers ...peer.ID) map[peer.ID][]byte
+	SignedRoutingStates(peers ...peer.ID) map[peer.ID]*routing.SignedRoutingState
 }
 
 // KeyBook tracks the keys of Peers.

routing/state.go

@@ -18,54 +18,75 @@ const StateEnvelopeDomain = "libp2p-routing-state"
 // TODO: register multicodec
 var StateEnvelopePayloadType = []byte("/libp2p/routing-state-record")
 
-// AnnotatedAddr will extend the Multiaddr type with additional metadata, as
-// extensions are added to the routing state record spec. It's defined now to
-// make refactoring simpler in the future.
-type AnnotatedAddr struct {
-	ma.Multiaddr
-}
-
-// RoutingState contains a snapshot of public, transient state (e.g. addresses, supported protocols)
-// for a peer at a given point in time, where "time" is defined by the sequence counter
-// field Seq. Greater Seq values are later in time than lesser values, but there are no
-// guarantees about the wall-clock time between any two Seq values.
-//
-// Note that Seq values are peer-specific and can only be compared for records with equal PeerIDs.
-type RoutingState struct {
+type SignedRoutingState struct {
 	// PeerID is the ID of the peer this record pertains to.
-	PeerID peer.ID
+	peerID peer.ID
 
 	// Seq is an increment-only sequence counter used to order RoutingState records in time.
-	Seq uint64
+	seq uint64
 
 	// Addresses contains the public addresses of the peer this record pertains to.
-	Addresses []*AnnotatedAddr
+	addresses []ma.Multiaddr
+
+	envelope *crypto.SignedEnvelope
 }
 
-// RoutingStateWithMultiaddrs returns a RoutingState record for the given peer id
-// that contains the given multiaddrs. It generates a timestamp-based sequence number.
-func RoutingStateWithMultiaddrs(p peer.ID, addrs []ma.Multiaddr) *RoutingState {
-	annotated := make([]*AnnotatedAddr, len(addrs))
-	for i, a := range addrs {
-		annotated[i] = &AnnotatedAddr{Multiaddr: a}
-	}
-	return &RoutingState{
-		PeerID:    p,
-		Seq:       statelessSeqNo(),
-		Addresses: annotated,
-	}
-}
-
-// RoutingStateFromAddrInfo converts a peer.AddrInfo into a RoutingState record.
+// MakeSignedRoutingState returns a SignedRoutingState record containing the given multiaddrs,
+// signed with the given private key.
 // It generates a timestamp-based sequence number.
-func RoutingStateFromAddrInfo(info *peer.AddrInfo) *RoutingState {
-	return RoutingStateWithMultiaddrs(info.ID, info.Addrs)
+func MakeSignedRoutingState(privKey crypto.PrivKey, addrs []ma.Multiaddr) (*SignedRoutingState, error) {
+	p, err := peer.IDFromPrivateKey(privKey)
+	if err != nil {
+		return nil, err
+	}
+	idBytes, err := p.MarshalBinary()
+	if err != nil {
+		return nil, err
+	}
+	seq := statelessSeqNo()
+	msg := pb.RoutingStateRecord{
+		PeerId:    idBytes,
+		Seq:       seq,
+		Addresses: addrsToProtobuf(addrs),
+	}
+	payload, err := proto.Marshal(&msg)
+	if err != nil {
+		return nil, err
+	}
+	envelope, err := crypto.MakeEnvelope(privKey, StateEnvelopeDomain, StateEnvelopePayloadType, payload)
+	if err != nil {
+		return nil, err
+	}
+	return &SignedRoutingState{
+		peerID:    p,
+		seq:       seq,
+		addresses: addrs,
+		envelope:  envelope,
+	}, nil
 }
 
-// UnmarshalRoutingState unpacks a peer RoutingState record from a serialized protobuf representation.
-func UnmarshalRoutingState(serialized []byte) (*RoutingState, error) {
-	msg := pb.RoutingStateRecord{}
-	err := proto.Unmarshal(serialized, &msg)
+// UnmarshalSignedRoutingState accepts a serialized SignedEnvelope message containing
+// a RoutingStateRecord protobuf and returns a SignedRoutingState record.
+// Fails if the signature is invalid, if the envelope has an unexpected payload type,
+// if deserialization of the envelope or its inner payload fails.
+func UnmarshalSignedRoutingState(envelopeBytes []byte) (*SignedRoutingState, error) {
+	envelope, err := crypto.OpenEnvelope(envelopeBytes, StateEnvelopeDomain)
+	if err != nil {
+		return nil, err
+	}
+	return SignedRoutingStateFromEnvelope(envelope)
+}
+
+// SignedRoutingStateFromEnvelope accepts a SignedEnvelope struct containing
+// a RoutingStateRecord protobuf and returns a SignedRoutingState record.
+// Fails if the signature is invalid, if the envelope has an unexpected payload type,
+// or if deserialization of the envelope payload fails.
+func SignedRoutingStateFromEnvelope(envelope *crypto.SignedEnvelope) (*SignedRoutingState, error) {
+	if bytes.Compare(envelope.PayloadType(), StateEnvelopePayloadType) != 0 {
+		return nil, errors.New("unexpected envelope payload type")
+	}
+	var msg pb.RoutingStateRecord
+	err := proto.Unmarshal(envelope.Payload(), &msg)
 	if err != nil {
 		return nil, err
 	}
@@ -73,107 +94,79 @@ func UnmarshalRoutingState(serialized []byte) (*RoutingState, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &RoutingState{
-		PeerID:    id,
-		Seq:       msg.Seq,
-		Addresses: addrsFromProtobuf(msg.Addresses),
+	if !id.MatchesPublicKey(envelope.PublicKey()) {
+		return nil, errors.New("peer id in routing state record does not match signing key")
+	}
+	return &SignedRoutingState{
+		peerID:    id,
+		seq:       msg.Seq,
+		addresses: addrsFromProtobuf(msg.Addresses),
+		envelope:  envelope,
 	}, nil
 }
 
-// RoutingStateFromEnvelope unwraps a peer RoutingState record from a serialized SignedEnvelope.
-// This method will fail if the signature is invalid, or if the record
-// belongs to a peer other than the one that signed the envelope.
-func RoutingStateFromEnvelope(envelopeBytes []byte) (*RoutingState, error) {
-	envelope, err := crypto.OpenEnvelope(envelopeBytes, StateEnvelopeDomain)
-	if err != nil {
-		return nil, err
-	}
-	if bytes.Compare(envelope.PayloadType(), StateEnvelopePayloadType) != 0 {
-		return nil, errors.New("unexpected envelope payload type")
-	}
-	state, err := UnmarshalRoutingState(envelope.Payload())
-	if err != nil {
-		return nil, err
-	}
-	if !state.PeerID.MatchesPublicKey(envelope.PublicKey()) {
-		return nil, errors.New("peer id in routing state record does not match signing key")
-	}
-	return state, nil
+// Marshal returns a byte slice containing the SignedRoutingState as a serialized SignedEnvelope
+// protobuf message.
+func (s *SignedRoutingState) Marshal() ([]byte, error) {
+	return s.envelope.Marshal()
 }
 
-// ToSignedEnvelope wraps a Marshal'd RoutingState record in a SignedEnvelope using the
-// given private signing key.
-func (s *RoutingState) ToSignedEnvelope(key crypto.PrivKey) (*crypto.SignedEnvelope, error) {
-	payload, err := s.Marshal()
-	if err != nil {
-		return nil, err
-	}
-	return crypto.MakeEnvelope(key, StateEnvelopeDomain, StateEnvelopePayloadType, payload)
+// PeerID is the ID of the peer this record pertains to.
+func (s *SignedRoutingState) PeerID() peer.ID {
+	return s.peerID
 }
 
-// Marshal serializes a RoutingState record to protobuf and returns its byte representation.
-func (s *RoutingState) Marshal() ([]byte, error) {
-	id, err := s.PeerID.MarshalBinary()
-	if err != nil {
-		return nil, err
-	}
-	msg := pb.RoutingStateRecord{
-		PeerId:    id,
-		Seq:       s.Seq,
-		Addresses: addrsToProtobuf(s.Addresses),
-	}
-	return proto.Marshal(&msg)
+// Seq is an increment-only sequence counter used to order RoutingState records in time.
+func (s *SignedRoutingState) Seq() uint64 {
+	return s.seq
 }
 
-// Multiaddrs returns the addresses from a RoutingState record without any metadata annotations.
-func (s *RoutingState) Multiaddrs() []ma.Multiaddr {
-	out := make([]ma.Multiaddr, len(s.Addresses))
-	for i, addr := range s.Addresses {
-		out[i] = addr.Multiaddr
-	}
-	return out
+// Multiaddrs contains the public addresses of the peer this record pertains to.
+func (s *SignedRoutingState) Multiaddrs() []ma.Multiaddr {
+	return s.addresses
 }
 
-func (s *RoutingState) Equal(other *RoutingState) bool {
-	if s.Seq != other.Seq {
+// Equal returns true if the other SignedRoutingState is identical to this one.
+func (s *SignedRoutingState) Equal(other *SignedRoutingState) bool {
+	if other == nil {
 		return false
 	}
-	if s.PeerID != other.PeerID {
+	if s.seq != other.seq {
 		return false
 	}
-	if len(s.Addresses) != len(other.Addresses) {
+	if s.peerID != other.peerID {
 		return false
 	}
-	for i, _ := range s.Addresses {
-		if !s.Addresses[i].Equal(other.Addresses[i]) {
+	if len(s.addresses) != len(other.addresses) {
+		return false
+	}
+	for i, _ := range s.addresses {
+		if !s.addresses[i].Equal(other.addresses[i]) {
 			return false
 		}
 	}
-	return true
-}
-
-func (a *AnnotatedAddr) Equal(other *AnnotatedAddr) bool {
-	return a.Multiaddr.Equal(other.Multiaddr)
+	return s.envelope.Equal(other.envelope)
 }
 
+// statelessSeqNo is a helper to generate a timestamp-based sequence number.
 func statelessSeqNo() uint64 {
 	return uint64(time.Now().UnixNano())
 }
 
-func addrsFromProtobuf(addrs []*pb.RoutingStateRecord_AddressInfo) []*AnnotatedAddr {
-	out := make([]*AnnotatedAddr, 0)
+func addrsFromProtobuf(addrs []*pb.RoutingStateRecord_AddressInfo) []ma.Multiaddr {
+	var out []ma.Multiaddr
 	for _, addr := range addrs {
 		a, err := ma.NewMultiaddrBytes(addr.Multiaddr)
 		if err != nil {
 			continue
 		}
-		out = append(out, &AnnotatedAddr{Multiaddr: a})
+		out = append(out, a)
 	}
 	return out
 }
 
-func addrsToProtobuf(addrs []*AnnotatedAddr) []*pb.RoutingStateRecord_AddressInfo {
-	out := make([]*pb.RoutingStateRecord_AddressInfo, 0)
+func addrsToProtobuf(addrs []ma.Multiaddr) []*pb.RoutingStateRecord_AddressInfo {
+	var out []*pb.RoutingStateRecord_AddressInfo
 	for _, addr := range addrs {
 		out = append(out, &pb.RoutingStateRecord_AddressInfo{Multiaddr: addr.Bytes()})
 	}

routing/state_test.go

@@ -2,60 +2,29 @@ package routing
 
 import (
 	"github.com/libp2p/go-libp2p-core/crypto"
-	"github.com/libp2p/go-libp2p-core/peer"
 	"github.com/libp2p/go-libp2p-core/test"
 	"testing"
 )
 
-func TestRoutingStateFromAddrInfo(t *testing.T) {
-	id, _ := test.RandPeerID()
-	addrs := test.GenerateTestAddrs(10)
-	info := peer.AddrInfo{
-		ID:    id,
-		Addrs: addrs,
-	}
-	state := RoutingStateFromAddrInfo(&info)
-	if state.PeerID != info.ID {
-		t.Fatalf("expected routing state to have peer id %s, got %s", id.Pretty(), state.PeerID.Pretty())
-	}
-	test.AssertAddressesEqual(t, addrs, state.Multiaddrs())
-}
-
-func TestRoutingStateFromEnvelope(t *testing.T) {
-	priv, pub, err := test.RandTestKeyPair(crypto.Ed25519, 256)
-	test.AssertNilError(t, err)
-
-	id, err := peer.IDFromPublicKey(pub)
+func TestSignedRoutingStateFromEnvelope(t *testing.T) {
+	priv, _, err := test.RandTestKeyPair(crypto.Ed25519, 256)
 	test.AssertNilError(t, err)
 
 	addrs := test.GenerateTestAddrs(10)
-	state := RoutingStateWithMultiaddrs(id, addrs)
+	state, err := MakeSignedRoutingState(priv, addrs)
+	test.AssertNilError(t, err)
 
-	t.Run("can unwrap a RoutingState from a serialized envelope", func(t *testing.T) {
-		env, err := state.ToSignedEnvelope(priv)
+	t.Run("is unaltered after round-trip serde", func(t *testing.T) {
+		envBytes, err := state.Marshal()
 		test.AssertNilError(t, err)
 
-		envBytes, err := env.Marshal()
+		state2, err := UnmarshalSignedRoutingState(envBytes)
 		test.AssertNilError(t, err)
-
-		state2, err := RoutingStateFromEnvelope(envBytes)
 		if !state.Equal(state2) {
-			t.Error("expected routing state to be unaltered after wrapping in signed envelope")
+			t.Error("expected routing state to be unaltered after round-trip serde")
 		}
 	})
 
-	t.Run("unwrapping from signed envelope fails if peer id does not match signing key", func(t *testing.T) {
-		priv2, _, err := test.RandTestKeyPair(crypto.Ed25519, 256)
-		test.AssertNilError(t, err)
-		env, err := state.ToSignedEnvelope(priv2)
-		test.AssertNilError(t, err)
-		envBytes, err := env.Marshal()
-		test.AssertNilError(t, err)
-
-		_, err = RoutingStateFromEnvelope(envBytes)
-		test.ExpectError(t, err, "unwrapping RoutingState from envelope should fail if peer id does not match key used to sign envelope")
-	})
-
 	t.Run("unwrapping from signed envelope fails if envelope has wrong domain string", func(t *testing.T) {
 		stateBytes, err := state.Marshal()
 		test.AssertNilError(t, err)
@@ -63,7 +32,7 @@ func TestRoutingStateFromEnvelope(t *testing.T) {
 		env, err := crypto.MakeEnvelope(priv, "wrong-domain", StateEnvelopePayloadType, stateBytes)
 		test.AssertNilError(t, err)
 		envBytes, err := env.Marshal()
-		_, err = RoutingStateFromEnvelope(envBytes)
+		_, err = UnmarshalSignedRoutingState(envBytes)
 		test.ExpectError(t, err, "unwrapping RoutingState from envelope should fail if envelope was created with wrong domain string")
 	})
 
@@ -74,7 +43,7 @@ func TestRoutingStateFromEnvelope(t *testing.T) {
 		env, err := crypto.MakeEnvelope(priv, StateEnvelopeDomain, payloadType, stateBytes)
 		test.AssertNilError(t, err)
 		envBytes, err := env.Marshal()
-		_, err = RoutingStateFromEnvelope(envBytes)
+		_, err = UnmarshalSignedRoutingState(envBytes)
 		test.ExpectError(t, err, "unwrapping RoutingState from envelope should fail if envelope was created with wrong payload type")
 	})
 }