From 7981f9bfbd893c176c3dd544323b71d71545db43 Mon Sep 17 00:00:00 2001 From: protolambda Date: Mon, 16 Dec 2019 03:38:28 +0100 Subject: [PATCH 1/2] fixes #247: implement msg id function as pubsub option --- gossipsub.go | 2 ++ mcache.go | 8 +++++++- mcache_test.go | 1 + pubsub.go | 26 ++++++++++++++++++++++---- trace.go | 19 ++++++++++--------- validation.go | 2 +- 6 files changed, 43 insertions(+), 15 deletions(-) diff --git a/gossipsub.go b/gossipsub.go index 6327dfa0..d2d46f78 100644 --- a/gossipsub.go +++ b/gossipsub.go @@ -75,6 +75,8 @@ func (gs *GossipSubRouter) Protocols() []protocol.ID { func (gs *GossipSubRouter) Attach(p *PubSub) { gs.p = p gs.tracer = p.tracer + // start using the same msg ID function as PubSub for caching messages. + gs.mcache.ChangeMsgIdFn(p.msgID) go gs.heartbeatTimer() } diff --git a/mcache.go b/mcache.go index 9f5e5dcd..cbb9a953 100644 --- a/mcache.go +++ b/mcache.go @@ -28,6 +28,7 @@ func NewMessageCache(gossip, history int) *MessageCache { msgs: make(map[string]*pb.Message), history: make([][]CacheEntry, history), gossip: gossip, + msgID: DefaultMsgIdFn, } } @@ -35,6 +36,11 @@ type MessageCache struct { msgs map[string]*pb.Message history [][]CacheEntry gossip int + msgID MsgIdFunction +} + +func (mc *MessageCache) ChangeMsgIdFn(msgID MsgIdFunction) { + mc.msgID = msgID } type CacheEntry struct { @@ -43,7 +49,7 @@ type CacheEntry struct { } func (mc *MessageCache) Put(msg *pb.Message) { - mid := msgID(msg) + mid := mc.msgID(msg) mc.msgs[mid] = msg mc.history[0] = append(mc.history[0], CacheEntry{mid: mid, topics: msg.GetTopicIDs()}) } diff --git a/mcache_test.go b/mcache_test.go index 5616c218..e8a50f38 100644 --- a/mcache_test.go +++ b/mcache_test.go @@ -10,6 +10,7 @@ import ( func TestMessageCache(t *testing.T) { mcache := NewMessageCache(3, 5) + msgID := DefaultMsgIdFn msgs := make([]*pb.Message, 60) for i := range msgs { diff --git a/pubsub.go b/pubsub.go index 530c47a4..b1d872d9 100644 --- a/pubsub.go +++ b/pubsub.go @@ -117,6 +117,9 @@ type PubSub struct { seenMessagesMx sync.Mutex seenMessages *timecache.TimeCache + // function used to compute the ID for a message + msgID MsgIdFunction + // key for signing messages; nil when signing is disabled (default for now) signKey crypto.PrivKey // source ID for signed messages; corresponds to signKey @@ -208,6 +211,7 @@ func NewPubSub(ctx context.Context, h host.Host, rt PubSubRouter, opts ...Option blacklist: NewMapBlacklist(), blacklistPeer: make(chan peer.ID), seenMessages: timecache.NewTimeCache(TimeCacheDuration), + msgID: DefaultMsgIdFn, counter: uint64(time.Now().UnixNano()), } @@ -240,6 +244,20 @@ func NewPubSub(ctx context.Context, h host.Host, rt PubSubRouter, opts ...Option return ps, nil } +// MsgIdFunction returns a unique ID for the passed Message, and PubSub can be customized to use any +// implementation of this function by configuring it with the Option from WithMessageIdFn. +type MsgIdFunction func(pmsg *pb.Message) string + +// WithMessageIdFn is an option to customize the way a message ID is computed for a pubsub message. +// The default ID function is DefaultMsgIdFn (concatenate source and seq nr.), +// but it can be customized to e.g. the hash of the message. +func WithMessageIdFn(fn MsgIdFunction) Option { + return func(p *PubSub) error { + p.msgID = fn + return nil + } +} + // WithPeerOutboundQueueSize is an option to set the buffer size for outbound messages to a peer // We start dropping messages to a peer if the outbound queue if full func WithPeerOutboundQueueSize(size int) Option { @@ -326,7 +344,7 @@ func WithDiscovery(d discovery.Discovery, opts ...DiscoverOpt) Option { // WithEventTracer provides a tracer for the pubsub system func WithEventTracer(tracer EventTracer) Option { return func(p *PubSub) error { - p.tracer = &pubsubTracer{tracer: tracer, pid: p.host.ID()} + p.tracer = &pubsubTracer{tracer: tracer, pid: p.host.ID(), msgID: p.msgID} return nil } } @@ -730,8 +748,8 @@ func (p *PubSub) handleIncomingRPC(rpc *RPC) { p.rt.HandleRPC(rpc) } -// msgID returns a unique ID of the passed Message -func msgID(pmsg *pb.Message) string { +// DefaultMsgIdFn returns a unique ID of the passed Message +func DefaultMsgIdFn(pmsg *pb.Message) string { return string(pmsg.GetFrom()) + string(pmsg.GetSeqno()) } @@ -760,7 +778,7 @@ func (p *PubSub) pushMsg(msg *Message) { } // have we already seen and validated this message? - id := msgID(msg.Message) + id := p.msgID(msg.Message) if p.seenMessage(id) { p.tracer.DuplicateMessage(msg) return diff --git a/trace.go b/trace.go index 293f1660..1f980536 100644 --- a/trace.go +++ b/trace.go @@ -18,6 +18,7 @@ type EventTracer interface { type pubsubTracer struct { tracer EventTracer pid peer.ID + msgID MsgIdFunction } func (t *pubsubTracer) PublishMessage(msg *Message) { @@ -31,7 +32,7 @@ func (t *pubsubTracer) PublishMessage(msg *Message) { PeerID: []byte(t.pid), Timestamp: &now, PublishMessage: &pb.TraceEvent_PublishMessage{ - MessageID: []byte(msgID(msg.Message)), + MessageID: []byte(t.msgID(msg.Message)), Topics: msg.Message.TopicIDs, }, } @@ -50,7 +51,7 @@ func (t *pubsubTracer) RejectMessage(msg *Message, reason string) { PeerID: []byte(t.pid), Timestamp: &now, RejectMessage: &pb.TraceEvent_RejectMessage{ - MessageID: []byte(msgID(msg.Message)), + MessageID: []byte(t.msgID(msg.Message)), ReceivedFrom: []byte(msg.ReceivedFrom), Reason: &reason, }, @@ -70,7 +71,7 @@ func (t *pubsubTracer) DuplicateMessage(msg *Message) { PeerID: []byte(t.pid), Timestamp: &now, DuplicateMessage: &pb.TraceEvent_DuplicateMessage{ - MessageID: []byte(msgID(msg.Message)), + MessageID: []byte(t.msgID(msg.Message)), ReceivedFrom: []byte(msg.ReceivedFrom), }, } @@ -89,7 +90,7 @@ func (t *pubsubTracer) DeliverMessage(msg *Message) { PeerID: []byte(t.pid), Timestamp: &now, DeliverMessage: &pb.TraceEvent_DeliverMessage{ - MessageID: []byte(msgID(msg.Message)), + MessageID: []byte(t.msgID(msg.Message)), }, } @@ -146,7 +147,7 @@ func (t *pubsubTracer) RecvRPC(rpc *RPC) { Timestamp: &now, RecvRPC: &pb.TraceEvent_RecvRPC{ ReceivedFrom: []byte(rpc.from), - Meta: traceRPCMeta(rpc), + Meta: t.traceRPCMeta(rpc), }, } @@ -165,7 +166,7 @@ func (t *pubsubTracer) SendRPC(rpc *RPC, p peer.ID) { Timestamp: &now, SendRPC: &pb.TraceEvent_SendRPC{ SendTo: []byte(rpc.from), - Meta: traceRPCMeta(rpc), + Meta: t.traceRPCMeta(rpc), }, } @@ -184,20 +185,20 @@ func (t *pubsubTracer) DropRPC(rpc *RPC, p peer.ID) { Timestamp: &now, DropRPC: &pb.TraceEvent_DropRPC{ SendTo: []byte(rpc.from), - Meta: traceRPCMeta(rpc), + Meta: t.traceRPCMeta(rpc), }, } t.tracer.Trace(evt) } -func traceRPCMeta(rpc *RPC) *pb.TraceEvent_RPCMeta { +func (t *pubsubTracer) traceRPCMeta(rpc *RPC) *pb.TraceEvent_RPCMeta { rpcMeta := new(pb.TraceEvent_RPCMeta) var msgs []*pb.TraceEvent_MessageMeta for _, m := range rpc.Publish { msgs = append(msgs, &pb.TraceEvent_MessageMeta{ - MessageID: []byte(msgID(m)), + MessageID: []byte(t.msgID(m)), Topics: m.TopicIDs, }) } diff --git a/validation.go b/validation.go index bef86da3..70b4cde2 100644 --- a/validation.go +++ b/validation.go @@ -201,7 +201,7 @@ func (v *validation) validate(vals []*topicVal, src peer.ID, msg *Message) { // we can mark the message as seen now that we have verified the signature // and avoid invoking user validators more than once - id := msgID(msg.Message) + id := v.p.msgID(msg.Message) if !v.p.markSeen(id) { v.tracer.DuplicateMessage(msg) return From 6bd07a71fcb02b3537520d83455b38f1997930d7 Mon Sep 17 00:00:00 2001 From: protolambda Date: Mon, 16 Dec 2019 12:46:30 +0100 Subject: [PATCH 2/2] PR feedback: name change and account for different options order --- gossipsub.go | 2 +- mcache.go | 2 +- pubsub.go | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/gossipsub.go b/gossipsub.go index d2d46f78..31c57243 100644 --- a/gossipsub.go +++ b/gossipsub.go @@ -76,7 +76,7 @@ func (gs *GossipSubRouter) Attach(p *PubSub) { gs.p = p gs.tracer = p.tracer // start using the same msg ID function as PubSub for caching messages. - gs.mcache.ChangeMsgIdFn(p.msgID) + gs.mcache.SetMsgIdFn(p.msgID) go gs.heartbeatTimer() } diff --git a/mcache.go b/mcache.go index cbb9a953..e0852978 100644 --- a/mcache.go +++ b/mcache.go @@ -39,7 +39,7 @@ type MessageCache struct { msgID MsgIdFunction } -func (mc *MessageCache) ChangeMsgIdFn(msgID MsgIdFunction) { +func (mc *MessageCache) SetMsgIdFn(msgID MsgIdFunction) { mc.msgID = msgID } diff --git a/pubsub.go b/pubsub.go index b1d872d9..b8bd579c 100644 --- a/pubsub.go +++ b/pubsub.go @@ -254,6 +254,10 @@ type MsgIdFunction func(pmsg *pb.Message) string func WithMessageIdFn(fn MsgIdFunction) Option { return func(p *PubSub) error { p.msgID = fn + // the tracer Option may already be set. Update its message ID function to make options order-independent. + if p.tracer != nil { + p.tracer.msgID = fn + } return nil } }