From 1714e847954f36fdff8a7a074ad0ddb65cb9e231 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 12:58:27 -0800 Subject: [PATCH 01/59] remove accumulator from handler --- network/p2p/gossip/handler.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/network/p2p/gossip/handler.go b/network/p2p/gossip/handler.go index 38e88392636..84b14043a90 100644 --- a/network/p2p/gossip/handler.go +++ b/network/p2p/gossip/handler.go @@ -21,7 +21,6 @@ var _ p2p.Handler = (*Handler[*testTx])(nil) func NewHandler[T Gossipable]( log logging.Logger, marshaller Marshaller[T], - accumulator Accumulator[T], set Set[T], metrics Metrics, targetResponseSize int, @@ -30,7 +29,6 @@ func NewHandler[T Gossipable]( Handler: p2p.NoOpHandler{}, log: log, marshaller: marshaller, - accumulator: accumulator, set: set, metrics: metrics, targetResponseSize: targetResponseSize, @@ -40,7 +38,6 @@ func NewHandler[T Gossipable]( type Handler[T Gossipable] struct { p2p.Handler marshaller Marshaller[T] - accumulator Accumulator[T] log logging.Logger set Set[T] metrics Metrics @@ -125,14 +122,6 @@ func (h Handler[_]) AppGossip(ctx context.Context, nodeID ids.NodeID, gossipByte ) continue } - - // continue gossiping messages we have not seen to other peers - h.accumulator.Add(gossipable) - } - - if err := h.accumulator.Gossip(ctx); err != nil { - h.log.Error("failed to forward gossip", zap.Error(err)) - return } receivedCountMetric, err := h.metrics.receivedCount.GetMetricWith(pushLabels) From 9c07a02d7dd9edbb18df49e73330b112f4a33bfa Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 13:03:51 -0800 Subject: [PATCH 02/59] add more comments --- network/p2p/gossip/gossip.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 3b910216d87..223d6b633ff 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -264,6 +264,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { sentBytes := 0 gossip := make([][]byte, 0, p.pending.Len()) for sentBytes < p.targetGossipSize { + // TODO: ensure tx still in the mempool gossipable, ok := p.pending.PeekLeft() if !ok { break @@ -302,6 +303,8 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { return p.client.AppGossip(ctx, msgBytes) } +// Add should only be called when accepting transactions over RPC. Gossip between validators (of txs from the p2p layer) should +// use PullGossip. This is only for getting transactions into the hands of validators to begin with. func (p *PushGossiper[T]) Add(gossipables ...T) { p.lock.Lock() defer p.lock.Unlock() From 80fb2b4a78e07ca3f34508c431b7564acb6ad21f Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 13:13:07 -0800 Subject: [PATCH 03/59] add Has to set --- network/p2p/gossip/gossip.go | 6 +++++- network/p2p/gossip/gossipable.go | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 223d6b633ff..9779d45291a 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -231,9 +231,10 @@ func (p *PullGossiper[_]) handleResponse( } // NewPushGossiper returns an instance of PushGossiper -func NewPushGossiper[T Gossipable](marshaller Marshaller[T], client *p2p.Client, metrics Metrics, targetGossipSize int) *PushGossiper[T] { +func NewPushGossiper[T Gossipable](marshaller Marshaller[T], set Set[T], client *p2p.Client, metrics Metrics, targetGossipSize int) *PushGossiper[T] { return &PushGossiper[T]{ marshaller: marshaller, + set: set, client: client, metrics: metrics, targetGossipSize: targetGossipSize, @@ -244,6 +245,7 @@ func NewPushGossiper[T Gossipable](marshaller Marshaller[T], client *p2p.Client, // PushGossiper broadcasts gossip to peers randomly in the network type PushGossiper[T Gossipable] struct { marshaller Marshaller[T] + set Set[T] client *p2p.Client metrics Metrics targetGossipSize int @@ -265,6 +267,8 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { gossip := make([][]byte, 0, p.pending.Len()) for sentBytes < p.targetGossipSize { // TODO: ensure tx still in the mempool + if p.set.Has() { + } gossipable, ok := p.pending.PeekLeft() if !ok { break diff --git a/network/p2p/gossip/gossipable.go b/network/p2p/gossip/gossipable.go index 238c62b4641..71c67f57dba 100644 --- a/network/p2p/gossip/gossipable.go +++ b/network/p2p/gossip/gossipable.go @@ -21,6 +21,8 @@ type Set[T Gossipable] interface { // Add adds a Gossipable to the set. Returns an error if gossipable was not // added. Add(gossipable T) error + // Has returns true if the gossipable is in the set. + Has(gossipable T) bool // Iterate iterates over elements until [f] returns false Iterate(f func(gossipable T) bool) // GetFilter returns the byte representation of bloom filter and its From dee16f9416b559492dce1e890617cb7a054a09f8 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 13:15:17 -0800 Subject: [PATCH 04/59] add placeholder for has use --- network/p2p/gossip/gossip.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 9779d45291a..759da2d0a63 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -266,14 +266,17 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { sentBytes := 0 gossip := make([][]byte, 0, p.pending.Len()) for sentBytes < p.targetGossipSize { - // TODO: ensure tx still in the mempool - if p.set.Has() { - } gossipable, ok := p.pending.PeekLeft() if !ok { break } + // Ensure item is still in the set before we gossip. + if !p.set.Has(gossipable) { + _, _ = p.pending.PopLeft() + continue + } + bytes, err := p.marshaller.MarshalGossip(gossipable) if err != nil { // remove this item so we don't get stuck in a loop From e7f25bf29d88942dfce3092f56a81b2b492b8353 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:06:28 -0800 Subject: [PATCH 05/59] spike on gossip push --- network/p2p/gossip/gossip.go | 65 ++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 759da2d0a63..92eb797ffc2 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -231,14 +231,17 @@ func (p *PullGossiper[_]) handleResponse( } // NewPushGossiper returns an instance of PushGossiper -func NewPushGossiper[T Gossipable](marshaller Marshaller[T], set Set[T], client *p2p.Client, metrics Metrics, targetGossipSize int) *PushGossiper[T] { +func NewPushGossiper[T Gossipable](marshaller Marshaller[T], mempool Set[T], client *p2p.Client, metrics Metrics, targetGossipSize int) *PushGossiper[T] { return &PushGossiper[T]{ marshaller: marshaller, - set: set, + set: mempool, client: client, metrics: metrics, targetGossipSize: targetGossipSize, - pending: buffer.NewUnboundedDeque[T](0), + + tracking: make(map[ids.ID]time.Time), + pending: buffer.NewUnboundedDeque[T](0), + issued: buffer.NewUnboundedDeque[T](0), } } @@ -250,8 +253,11 @@ type PushGossiper[T Gossipable] struct { metrics Metrics targetGossipSize int - lock sync.Mutex - pending buffer.Deque[T] + lock sync.Mutex + tracking map[ids.ID]time.Time + pending buffer.Deque[T] + issued buffer.Deque[T] + // TODO: handle case of txs that keep getting added/dropped from mempool (will keep landing on pending queue) } // Gossip flushes any queued gossipables @@ -259,12 +265,14 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.lock.Lock() defer p.lock.Unlock() - if p.pending.Len() == 0 { + if p.pending.Len() == 0 && p.issued.Len() == 0 { return nil } sentBytes := 0 gossip := make([][]byte, 0, p.pending.Len()) + + // Iterate over all pending gossipables (never been sent before) for sentBytes < p.targetGossipSize { gossipable, ok := p.pending.PeekLeft() if !ok { @@ -273,6 +281,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { // Ensure item is still in the set before we gossip. if !p.set.Has(gossipable) { + delete(p.tracking, gossipable.GossipID()) _, _ = p.pending.PopLeft() continue } @@ -280,6 +289,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { bytes, err := p.marshaller.MarshalGossip(gossipable) if err != nil { // remove this item so we don't get stuck in a loop + delete(p.tracking, gossipable.GossipID()) _, _ = p.pending.PopLeft() return err } @@ -287,6 +297,43 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { gossip = append(gossip, bytes) sentBytes += len(bytes) p.pending.PopLeft() + p.issued.PushRight(gossipable) + p.tracking[gossipable.GossipID()] = time.Now() + } + + // Iterate over all issued gossipables (have been sent before) + for sentBytes < p.targetGossipSize { + gossipable, ok := p.issued.PeekLeft() + if !ok { + break + } + + // Ensure item is still in the set before we gossip. + if !p.set.Has(gossipable) { + delete(p.tracking, gossipable.GossipID()) + _, _ = p.issued.PopLeft() + continue + } + + // Ensure not gossiped too recently + lastGossipTime := p.tracking[gossipable.GossipID()] + if time.Since(lastGossipTime) < 1*time.Second { // TODO: make this a const + continue + } + + bytes, err := p.marshaller.MarshalGossip(gossipable) + if err != nil { + // Should never happen because we've already issued this once. + delete(p.tracking, gossipable.GossipID()) + _, _ = p.issued.PopLeft() + return err + } + + gossip = append(gossip, bytes) + sentBytes += len(bytes) + _, _ = p.issued.PopLeft() + p.issued.PushRight(gossipable) + p.tracking[gossipable.GossipID()] = time.Now() } msgBytes, err := MarshalAppGossip(gossip) @@ -317,6 +364,12 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { defer p.lock.Unlock() for _, gossipable := range gossipables { + gid := gossipable.GossipID() + _, contains := p.tracking[gid] + if contains { + continue + } + p.tracking[gid] = time.Time{} p.pending.PushRight(gossipable) } } From 646cadceefb43c8ca595b5875c77d0f5a76403b4 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:09:24 -0800 Subject: [PATCH 06/59] add TODO about memory use --- network/p2p/gossip/gossip.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 92eb797ffc2..051d1bfe17f 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -302,6 +302,8 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { } // Iterate over all issued gossipables (have been sent before) + // + // TODO: ensure starvation of this section doesn't allow [issued] to grow indefinitely for sentBytes < p.targetGossipSize { gossipable, ok := p.issued.PeekLeft() if !ok { From f10ed760c6281c81d3261b6850c96f1485aadf74 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:14:44 -0800 Subject: [PATCH 07/59] add logic to handle discarded tx churn --- network/p2p/gossip/gossip.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 051d1bfe17f..b14207da2d0 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -13,6 +13,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" + "github.com/ava-labs/avalanchego/cache" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/network/p2p" "github.com/ava-labs/avalanchego/utils" @@ -239,9 +240,10 @@ func NewPushGossiper[T Gossipable](marshaller Marshaller[T], mempool Set[T], cli metrics: metrics, targetGossipSize: targetGossipSize, - tracking: make(map[ids.ID]time.Time), - pending: buffer.NewUnboundedDeque[T](0), - issued: buffer.NewUnboundedDeque[T](0), + tracking: make(map[ids.ID]time.Time), + pending: buffer.NewUnboundedDeque[T](0), + issued: buffer.NewUnboundedDeque[T](0), + discarded: &cache.LRU[ids.ID, interface{}]{Size: 1024}, } } @@ -253,11 +255,11 @@ type PushGossiper[T Gossipable] struct { metrics Metrics targetGossipSize int - lock sync.Mutex - tracking map[ids.ID]time.Time - pending buffer.Deque[T] - issued buffer.Deque[T] - // TODO: handle case of txs that keep getting added/dropped from mempool (will keep landing on pending queue) + lock sync.Mutex + tracking map[ids.ID]time.Time + pending buffer.Deque[T] + issued buffer.Deque[T] + discarded *cache.LRU[ids.ID, interface{}] } // Gossip flushes any queued gossipables @@ -314,6 +316,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { if !p.set.Has(gossipable) { delete(p.tracking, gossipable.GossipID()) _, _ = p.issued.PopLeft() + p.discarded.Put(gossipable.GossipID(), nil) // only add to discarded if we sent at least once continue } @@ -367,8 +370,10 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { for _, gossipable := range gossipables { gid := gossipable.GossipID() - _, contains := p.tracking[gid] - if contains { + if _, contains := p.tracking[gid]; contains { + continue + } + if _, contains := p.discarded.Get(gid); contains { continue } p.tracking[gid] = time.Time{} From 5bb3d2eb2931e58d794e420688ba2eb5fcf08166 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:18:53 -0800 Subject: [PATCH 08/59] revert discarded cache --- network/p2p/gossip/gossip.go | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index b14207da2d0..0b52ce5161f 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -13,7 +13,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" - "github.com/ava-labs/avalanchego/cache" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/network/p2p" "github.com/ava-labs/avalanchego/utils" @@ -240,10 +239,9 @@ func NewPushGossiper[T Gossipable](marshaller Marshaller[T], mempool Set[T], cli metrics: metrics, targetGossipSize: targetGossipSize, - tracking: make(map[ids.ID]time.Time), - pending: buffer.NewUnboundedDeque[T](0), - issued: buffer.NewUnboundedDeque[T](0), - discarded: &cache.LRU[ids.ID, interface{}]{Size: 1024}, + tracking: make(map[ids.ID]time.Time), + pending: buffer.NewUnboundedDeque[T](0), + issued: buffer.NewUnboundedDeque[T](0), } } @@ -255,11 +253,12 @@ type PushGossiper[T Gossipable] struct { metrics Metrics targetGossipSize int - lock sync.Mutex - tracking map[ids.ID]time.Time - pending buffer.Deque[T] - issued buffer.Deque[T] - discarded *cache.LRU[ids.ID, interface{}] + lock sync.Mutex + tracking map[ids.ID]time.Time + pending buffer.Deque[T] + issued buffer.Deque[T] + // TODO: handle case of txs that keep getting added/dropped from mempool (will keep landing on pending queue) + // -> may not actually want to handle this here... } // Gossip flushes any queued gossipables @@ -316,7 +315,6 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { if !p.set.Has(gossipable) { delete(p.tracking, gossipable.GossipID()) _, _ = p.issued.PopLeft() - p.discarded.Put(gossipable.GossipID(), nil) // only add to discarded if we sent at least once continue } @@ -373,9 +371,6 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { if _, contains := p.tracking[gid]; contains { continue } - if _, contains := p.discarded.Get(gid); contains { - continue - } p.tracking[gid] = time.Time{} p.pending.PushRight(gossipable) } From 0d48439e309d8d86c4e2c46f8d8faaeb043456ea Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:29:40 -0800 Subject: [PATCH 09/59] periodically prune gossipables --- network/p2p/gossip/gossip.go | 49 ++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 0b52ce5161f..15ebc7f92a5 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -231,12 +231,20 @@ func (p *PullGossiper[_]) handleResponse( } // NewPushGossiper returns an instance of PushGossiper -func NewPushGossiper[T Gossipable](marshaller Marshaller[T], mempool Set[T], client *p2p.Client, metrics Metrics, targetGossipSize int) *PushGossiper[T] { +func NewPushGossiper[T Gossipable]( + marshaller Marshaller[T], + mempool Set[T], + client *p2p.Client, + metrics Metrics, + pruneSize int, + targetGossipSize int, +) *PushGossiper[T] { return &PushGossiper[T]{ marshaller: marshaller, set: mempool, client: client, metrics: metrics, + pruneSize: pruneSize, targetGossipSize: targetGossipSize, tracking: make(map[ids.ID]time.Time), @@ -251,6 +259,7 @@ type PushGossiper[T Gossipable] struct { set Set[T] client *p2p.Client metrics Metrics + pruneSize int // size at which we clean everything we are tracking (may no longer be in mempool) targetGossipSize int lock sync.Mutex @@ -266,7 +275,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.lock.Lock() defer p.lock.Unlock() - if p.pending.Len() == 0 && p.issued.Len() == 0 { + if len(p.tracking) == 0 { return nil } @@ -303,8 +312,6 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { } // Iterate over all issued gossipables (have been sent before) - // - // TODO: ensure starvation of this section doesn't allow [issued] to grow indefinitely for sentBytes < p.targetGossipSize { gossipable, ok := p.issued.PeekLeft() if !ok { @@ -357,7 +364,39 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { sentCountMetric.Add(float64(len(gossip))) sentBytesMetric.Add(float64(sentBytes)) - return p.client.AppGossip(ctx, msgBytes) + if err := p.client.AppGossip(ctx, msgBytes); err != nil { + return fmt.Errorf("failed to gossip: %w", err) + } + + // Attempt to prune gossipables that are no longer in the mempool + if len(p.tracking) < p.pruneSize { + return nil + } + for i := 0; i < p.pending.Len(); i++ { + gossipable, ok := p.pending.PopLeft() + if !ok { + // Should never happen + break + } + if !p.set.Has(gossipable) { + delete(p.tracking, gossipable.GossipID()) + } else { + p.pending.PushRight(gossipable) + } + } + for i := 0; i < p.issued.Len(); i++ { + gossipable, ok := p.issued.PopLeft() + if !ok { + // Should never happen + break + } + if !p.set.Has(gossipable) { + delete(p.tracking, gossipable.GossipID()) + } else { + p.issued.PushRight(gossipable) + } + } + return nil } // Add should only be called when accepting transactions over RPC. Gossip between validators (of txs from the p2p layer) should From 31de4f6738ee2dcb73db7fcbf6dd3b774d66d50b Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:35:31 -0800 Subject: [PATCH 10/59] add discarded cache --- network/p2p/gossip/gossip.go | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 15ebc7f92a5..2051188ebc3 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -13,6 +13,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" + "github.com/ava-labs/avalanchego/cache" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/network/p2p" "github.com/ava-labs/avalanchego/utils" @@ -247,9 +248,10 @@ func NewPushGossiper[T Gossipable]( pruneSize: pruneSize, targetGossipSize: targetGossipSize, - tracking: make(map[ids.ID]time.Time), - pending: buffer.NewUnboundedDeque[T](0), - issued: buffer.NewUnboundedDeque[T](0), + tracking: make(map[ids.ID]time.Time), + pending: buffer.NewUnboundedDeque[T](0), + issued: buffer.NewUnboundedDeque[T](0), + discarded: &cache.LRU[ids.ID, interface{}]{Size: 1024}, } } @@ -259,15 +261,14 @@ type PushGossiper[T Gossipable] struct { set Set[T] client *p2p.Client metrics Metrics - pruneSize int // size at which we clean everything we are tracking (may no longer be in mempool) + pruneSize int // size at which we check that everything we are tracking is still in the mempool targetGossipSize int - lock sync.Mutex - tracking map[ids.ID]time.Time - pending buffer.Deque[T] - issued buffer.Deque[T] - // TODO: handle case of txs that keep getting added/dropped from mempool (will keep landing on pending queue) - // -> may not actually want to handle this here... + lock sync.Mutex + tracking map[ids.ID]time.Time + pending buffer.Deque[T] + issued buffer.Deque[T] + discarded *cache.LRU[ids.ID, interface{}] // discarded ensures we don't overgossip transactions that are frequently dropped } // Gossip flushes any queued gossipables @@ -322,6 +323,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { if !p.set.Has(gossipable) { delete(p.tracking, gossipable.GossipID()) _, _ = p.issued.PopLeft() + p.discarded.Put(gossipable.GossipID(), nil) // only add to discarded if issued once continue } @@ -392,6 +394,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { } if !p.set.Has(gossipable) { delete(p.tracking, gossipable.GossipID()) + p.discarded.Put(gossipable.GossipID(), nil) // only add to discarded if issued once } else { p.issued.PushRight(gossipable) } @@ -410,8 +413,14 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { if _, contains := p.tracking[gid]; contains { continue } - p.tracking[gid] = time.Time{} - p.pending.PushRight(gossipable) + if _, contains := p.discarded.Get(gid); !contains { + p.tracking[gid] = time.Time{} + p.pending.PushRight(gossipable) + } else { + // Pretend that recently discarded transactions were just gossiped. + p.tracking[gid] = time.Now() + p.issued.PushRight(gossipable) + } } } From 8201106033833897da738d80af8039e1e65f3b97 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:39:25 -0800 Subject: [PATCH 11/59] add arg for regossip frequency --- network/p2p/gossip/gossip.go | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 2051188ebc3..42ce28aa49b 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -238,31 +238,35 @@ func NewPushGossiper[T Gossipable]( client *p2p.Client, metrics Metrics, pruneSize int, + discardedSize int, targetGossipSize int, + maxRegossipFrequency time.Duration, ) *PushGossiper[T] { return &PushGossiper[T]{ - marshaller: marshaller, - set: mempool, - client: client, - metrics: metrics, - pruneSize: pruneSize, - targetGossipSize: targetGossipSize, + marshaller: marshaller, + set: mempool, + client: client, + metrics: metrics, + pruneSize: pruneSize, + targetGossipSize: targetGossipSize, + maxRegossipFrequency: maxRegossipFrequency, tracking: make(map[ids.ID]time.Time), pending: buffer.NewUnboundedDeque[T](0), issued: buffer.NewUnboundedDeque[T](0), - discarded: &cache.LRU[ids.ID, interface{}]{Size: 1024}, + discarded: &cache.LRU[ids.ID, interface{}]{Size: discardedSize}, } } // PushGossiper broadcasts gossip to peers randomly in the network type PushGossiper[T Gossipable] struct { - marshaller Marshaller[T] - set Set[T] - client *p2p.Client - metrics Metrics - pruneSize int // size at which we check that everything we are tracking is still in the mempool - targetGossipSize int + marshaller Marshaller[T] + set Set[T] + client *p2p.Client + metrics Metrics + pruneSize int // size at which we check that everything we are tracking is still in the mempool + targetGossipSize int + maxRegossipFrequency time.Duration lock sync.Mutex tracking map[ids.ID]time.Time @@ -329,8 +333,8 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { // Ensure not gossiped too recently lastGossipTime := p.tracking[gossipable.GossipID()] - if time.Since(lastGossipTime) < 1*time.Second { // TODO: make this a const - continue + if time.Since(lastGossipTime) < p.maxRegossipFrequency { + break // items are sorted by last issuance time, so we can stop here } bytes, err := p.marshaller.MarshalGossip(gossipable) From cd7aaf6b6a86a519253e3a7d87bd61e971e95a87 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:43:13 -0800 Subject: [PATCH 12/59] add a TODO --- network/p2p/gossip/gossip.go | 1 + 1 file changed, 1 insertion(+) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 42ce28aa49b..870c50a72b8 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -419,6 +419,7 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { } if _, contains := p.discarded.Get(gid); !contains { p.tracking[gid] = time.Time{} + // TODO: sort pending by fee? p.pending.PushRight(gossipable) } else { // Pretend that recently discarded transactions were just gossiped. From 1955a67392c6bdbd1d94b3d19e67dee8e1fd5957 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:52:36 -0800 Subject: [PATCH 13/59] add metric for tracking gossipables --- network/p2p/gossip/gossip.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 870c50a72b8..6367f31ae55 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -74,6 +74,7 @@ type Metrics struct { sentBytes *prometheus.CounterVec receivedCount *prometheus.CounterVec receivedBytes *prometheus.CounterVec + tracking prometheus.Gauge } // NewMetrics returns a common set of metrics @@ -102,6 +103,11 @@ func NewMetrics( Name: "gossip_received_bytes", Help: "amount of gossip received (bytes)", }, metricLabels), + tracking: prometheus.NewGauge(prometheus.GaugeOpts{ + Namespace: namespace, + Name: "gossip_tracking", + Help: "number of gossipables being tracked", + }), } err := utils.Err( metrics.Register(m.sentCount), @@ -352,24 +358,21 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.tracking[gossipable.GossipID()] = time.Now() } + // Send gossipables to peers msgBytes, err := MarshalAppGossip(gossip) if err != nil { return err } - sentCountMetric, err := p.metrics.sentCount.GetMetricWith(pushLabels) if err != nil { return fmt.Errorf("failed to get sent count metric: %w", err) } - sentBytesMetric, err := p.metrics.sentBytes.GetMetricWith(pushLabels) if err != nil { return fmt.Errorf("failed to get sent bytes metric: %w", err) } - sentCountMetric.Add(float64(len(gossip))) sentBytesMetric.Add(float64(sentBytes)) - if err := p.client.AppGossip(ctx, msgBytes); err != nil { return fmt.Errorf("failed to gossip: %w", err) } @@ -403,6 +406,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.issued.PushRight(gossipable) } } + p.metrics.tracking.Set(float64(len(p.tracking))) return nil } @@ -427,6 +431,7 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { p.issued.PushRight(gossipable) } } + p.metrics.tracking.Set(float64(len(p.tracking))) } // Every calls [Gossip] every [frequency] amount of time. From 4d75df05976f1d7a569439cda2fed9559eb3250f Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:53:32 -0800 Subject: [PATCH 14/59] nit --- network/p2p/gossip/gossip.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 6367f31ae55..b9be621a898 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -423,7 +423,7 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { } if _, contains := p.discarded.Get(gid); !contains { p.tracking[gid] = time.Time{} - // TODO: sort pending by fee? + // TODO: sort pending by priority fee? p.pending.PushRight(gossipable) } else { // Pretend that recently discarded transactions were just gossiped. From ff965db20ac2b1c0cc1869e1dcd915b7bec2eea6 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 14:59:24 -0800 Subject: [PATCH 15/59] add TODO for x/p --- vms/avm/service.go | 2 ++ vms/platformvm/service.go | 1 + 2 files changed, 3 insertions(+) diff --git a/vms/avm/service.go b/vms/avm/service.go index 4dcc210df81..d66ee6976b8 100644 --- a/vms/avm/service.go +++ b/vms/avm/service.go @@ -214,6 +214,8 @@ func (s *Service) IssueTx(_ *http.Request, args *api.FormattedTx, reply *api.JSO } reply.TxID, err = s.vm.issueTx(tx) + + // TODO: enqueue push gossip return err } diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index 5a9c6b90081..300af16fd83 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -1418,6 +1418,7 @@ func (s *Service) IssueTx(req *http.Request, args *api.FormattedTx, response *ap if err := s.vm.issueTx(req.Context(), tx); err != nil { return fmt.Errorf("couldn't issue tx: %w", err) } + // TODO: enqueue push gossip response.TxID = tx.ID() return nil From dc695862ef6559f731696efef260024462b516b7 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 19:31:01 -0800 Subject: [PATCH 16/59] prune in add --- network/p2p/gossip/gossip.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index b9be621a898..1c0687faf75 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -377,9 +377,14 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { return fmt.Errorf("failed to gossip: %w", err) } - // Attempt to prune gossipables that are no longer in the mempool + p.prune() + return nil +} + +// prune drops gossipables that are no longer in the mempool +func (p *PushGossiper[T]) prune() { if len(p.tracking) < p.pruneSize { - return nil + return } for i := 0; i < p.pending.Len(); i++ { gossipable, ok := p.pending.PopLeft() @@ -407,7 +412,6 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { } } p.metrics.tracking.Set(float64(len(p.tracking))) - return nil } // Add should only be called when accepting transactions over RPC. Gossip between validators (of txs from the p2p layer) should @@ -432,6 +436,8 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { } } p.metrics.tracking.Set(float64(len(p.tracking))) + + p.prune() } // Every calls [Gossip] every [frequency] amount of time. From 2c199961e45a785af083101be3b20ecee370de49 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 20:40:55 -0800 Subject: [PATCH 17/59] remove unnecessary newline --- network/p2p/gossip/gossip.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 1c0687faf75..9086e4013e8 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -321,8 +321,6 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.issued.PushRight(gossipable) p.tracking[gossipable.GossipID()] = time.Now() } - - // Iterate over all issued gossipables (have been sent before) for sentBytes < p.targetGossipSize { gossipable, ok := p.issued.PeekLeft() if !ok { From 7277f63b1144d551c64814b202346bb7fb209b29 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 20:53:32 -0800 Subject: [PATCH 18/59] add more comments --- network/p2p/gossip/gossip.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 9086e4013e8..fe16811b491 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -25,6 +25,8 @@ const ( typeLabel = "type" pushType = "push" pullType = "pull" + + defaultGossipableCount = 64 ) var ( @@ -290,10 +292,9 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { return nil } - sentBytes := 0 - gossip := make([][]byte, 0, p.pending.Len()) - // Iterate over all pending gossipables (never been sent before) + sentBytes := 0 + gossip := make([][]byte, 0, defaultGossipableCount) for sentBytes < p.targetGossipSize { gossipable, ok := p.pending.PeekLeft() if !ok { @@ -375,6 +376,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { return fmt.Errorf("failed to gossip: %w", err) } + // Cleanup stale gossipables p.prune() return nil } @@ -435,6 +437,7 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { } p.metrics.tracking.Set(float64(len(p.tracking))) + // Cleanup stale gossipables p.prune() } From cfd1a3e5a44621b548daba0d15a9d91dc8e641da Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 21:10:19 -0800 Subject: [PATCH 19/59] pop first --- network/p2p/gossip/gossip.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index fe16811b491..04cc818e242 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -296,7 +296,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { sentBytes := 0 gossip := make([][]byte, 0, defaultGossipableCount) for sentBytes < p.targetGossipSize { - gossipable, ok := p.pending.PeekLeft() + gossipable, ok := p.pending.PopLeft() if !ok { break } @@ -304,7 +304,6 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { // Ensure item is still in the set before we gossip. if !p.set.Has(gossipable) { delete(p.tracking, gossipable.GossipID()) - _, _ = p.pending.PopLeft() continue } @@ -312,18 +311,16 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { if err != nil { // remove this item so we don't get stuck in a loop delete(p.tracking, gossipable.GossipID()) - _, _ = p.pending.PopLeft() return err } gossip = append(gossip, bytes) sentBytes += len(bytes) - p.pending.PopLeft() p.issued.PushRight(gossipable) p.tracking[gossipable.GossipID()] = time.Now() } for sentBytes < p.targetGossipSize { - gossipable, ok := p.issued.PeekLeft() + gossipable, ok := p.issued.PopLeft() if !ok { break } @@ -331,7 +328,6 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { // Ensure item is still in the set before we gossip. if !p.set.Has(gossipable) { delete(p.tracking, gossipable.GossipID()) - _, _ = p.issued.PopLeft() p.discarded.Put(gossipable.GossipID(), nil) // only add to discarded if issued once continue } @@ -339,6 +335,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { // Ensure not gossiped too recently lastGossipTime := p.tracking[gossipable.GossipID()] if time.Since(lastGossipTime) < p.maxRegossipFrequency { + p.issued.PushLeft(gossipable) break // items are sorted by last issuance time, so we can stop here } @@ -346,13 +343,11 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { if err != nil { // Should never happen because we've already issued this once. delete(p.tracking, gossipable.GossipID()) - _, _ = p.issued.PopLeft() return err } gossip = append(gossip, bytes) sentBytes += len(bytes) - _, _ = p.issued.PopLeft() p.issued.PushRight(gossipable) p.tracking[gossipable.GossipID()] = time.Now() } From dc2c18ea539f07e0385118518e1b128ca1db2330 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 21:23:17 -0800 Subject: [PATCH 20/59] add more TODOs --- network/p2p/gossip/gossip.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 04cc818e242..fb85ca08aaa 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -377,6 +377,10 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { } // prune drops gossipables that are no longer in the mempool +// +// TODO: should we require that `Set` removes items that are dropped +// to avoid an iteration over everything we are tracking? We would need +// to switch away from buffer, which doesn't support arbitrary deletes. func (p *PushGossiper[T]) prune() { if len(p.tracking) < p.pruneSize { return @@ -422,7 +426,7 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { } if _, contains := p.discarded.Get(gid); !contains { p.tracking[gid] = time.Time{} - // TODO: sort pending by priority fee? + // TODO: sort pending by priority fee p.pending.PushRight(gossipable) } else { // Pretend that recently discarded transactions were just gossiped. From 09abd32955e285645f1fad97bbde672b311b7ac4 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Sun, 25 Feb 2024 21:24:26 -0800 Subject: [PATCH 21/59] ensure we track size iteration separately --- network/p2p/gossip/gossip.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index fb85ca08aaa..9da95edb216 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -385,7 +385,8 @@ func (p *PushGossiper[T]) prune() { if len(p.tracking) < p.pruneSize { return } - for i := 0; i < p.pending.Len(); i++ { + pendingSize := p.pending.Len() + for i := 0; i < pendingSize; i++ { gossipable, ok := p.pending.PopLeft() if !ok { // Should never happen @@ -397,7 +398,8 @@ func (p *PushGossiper[T]) prune() { p.pending.PushRight(gossipable) } } - for i := 0; i < p.issued.Len(); i++ { + issuedSize := p.issued.Len() + for i := 0; i < issuedSize; i++ { gossipable, ok := p.issued.PopLeft() if !ok { // Should never happen From dae722ec31403f9d51c51a11ed76f99ad2b3942b Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 26 Feb 2024 13:34:35 -0800 Subject: [PATCH 22/59] remove prune --- network/p2p/gossip/gossip.go | 64 +++++++++--------------------------- 1 file changed, 16 insertions(+), 48 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 9da95edb216..28c8959827e 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -245,7 +245,7 @@ func NewPushGossiper[T Gossipable]( mempool Set[T], client *p2p.Client, metrics Metrics, - pruneSize int, + earlyGossipSize int, discardedSize int, targetGossipSize int, maxRegossipFrequency time.Duration, @@ -255,7 +255,7 @@ func NewPushGossiper[T Gossipable]( set: mempool, client: client, metrics: metrics, - pruneSize: pruneSize, + earlyGossipSize: earlyGossipSize, targetGossipSize: targetGossipSize, maxRegossipFrequency: maxRegossipFrequency, @@ -272,7 +272,7 @@ type PushGossiper[T Gossipable] struct { set Set[T] client *p2p.Client metrics Metrics - pruneSize int // size at which we check that everything we are tracking is still in the mempool + earlyGossipSize int // size at which we attempt gossip during [Add] to avoid unbounded memory use targetGossipSize int maxRegossipFrequency time.Duration @@ -288,6 +288,10 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.lock.Lock() defer p.lock.Unlock() + return p.gossip(ctx) +} + +func (p *PushGossiper[T]) gossip(ctx context.Context) error { if len(p.tracking) == 0 { return nil } @@ -371,56 +375,16 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { return fmt.Errorf("failed to gossip: %w", err) } - // Cleanup stale gossipables - p.prune() return nil } -// prune drops gossipables that are no longer in the mempool -// -// TODO: should we require that `Set` removes items that are dropped -// to avoid an iteration over everything we are tracking? We would need -// to switch away from buffer, which doesn't support arbitrary deletes. -func (p *PushGossiper[T]) prune() { - if len(p.tracking) < p.pruneSize { - return - } - pendingSize := p.pending.Len() - for i := 0; i < pendingSize; i++ { - gossipable, ok := p.pending.PopLeft() - if !ok { - // Should never happen - break - } - if !p.set.Has(gossipable) { - delete(p.tracking, gossipable.GossipID()) - } else { - p.pending.PushRight(gossipable) - } - } - issuedSize := p.issued.Len() - for i := 0; i < issuedSize; i++ { - gossipable, ok := p.issued.PopLeft() - if !ok { - // Should never happen - break - } - if !p.set.Has(gossipable) { - delete(p.tracking, gossipable.GossipID()) - p.discarded.Put(gossipable.GossipID(), nil) // only add to discarded if issued once - } else { - p.issued.PushRight(gossipable) - } - } - p.metrics.tracking.Set(float64(len(p.tracking))) -} - // Add should only be called when accepting transactions over RPC. Gossip between validators (of txs from the p2p layer) should // use PullGossip. This is only for getting transactions into the hands of validators to begin with. func (p *PushGossiper[T]) Add(gossipables ...T) { p.lock.Lock() defer p.lock.Unlock() + // Add new gossipables to the tracker for _, gossipable := range gossipables { gid := gossipable.GossipID() if _, contains := p.tracking[gid]; contains { @@ -428,7 +392,6 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { } if _, contains := p.discarded.Get(gid); !contains { p.tracking[gid] = time.Time{} - // TODO: sort pending by priority fee p.pending.PushRight(gossipable) } else { // Pretend that recently discarded transactions were just gossiped. @@ -436,10 +399,15 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { p.issued.PushRight(gossipable) } } - p.metrics.tracking.Set(float64(len(p.tracking))) - // Cleanup stale gossipables - p.prune() + // If we have too many pending gossipables, trigger gossip + if len(p.tracking) > p.earlyGossipSize { + if err := p.gossip(context.TODO()); err != nil { + // TODO: log error + } + } + + p.metrics.tracking.Set(float64(len(p.tracking))) } // Every calls [Gossip] every [frequency] amount of time. From ee0356d804874c529b0e03c016421d132a72d550 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 26 Feb 2024 13:38:04 -0800 Subject: [PATCH 23/59] add log to gossip --- network/p2p/gossip/gossip.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 28c8959827e..cbee7b1a22c 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -241,6 +241,7 @@ func (p *PullGossiper[_]) handleResponse( // NewPushGossiper returns an instance of PushGossiper func NewPushGossiper[T Gossipable]( + log logging.Logger, marshaller Marshaller[T], mempool Set[T], client *p2p.Client, @@ -251,6 +252,7 @@ func NewPushGossiper[T Gossipable]( maxRegossipFrequency time.Duration, ) *PushGossiper[T] { return &PushGossiper[T]{ + log: log, marshaller: marshaller, set: mempool, client: client, @@ -268,10 +270,12 @@ func NewPushGossiper[T Gossipable]( // PushGossiper broadcasts gossip to peers randomly in the network type PushGossiper[T Gossipable] struct { - marshaller Marshaller[T] - set Set[T] - client *p2p.Client - metrics Metrics + log logging.Logger + marshaller Marshaller[T] + set Set[T] + client *p2p.Client + metrics Metrics + earlyGossipSize int // size at which we attempt gossip during [Add] to avoid unbounded memory use targetGossipSize int maxRegossipFrequency time.Duration @@ -378,7 +382,7 @@ func (p *PushGossiper[T]) gossip(ctx context.Context) error { return nil } -// Add should only be called when accepting transactions over RPC. Gossip between validators (of txs from the p2p layer) should +// TODO: wording cleanup Add should only be called when accepting transactions over RPC. Gossip between validators (of txs from the p2p layer) should // use PullGossip. This is only for getting transactions into the hands of validators to begin with. func (p *PushGossiper[T]) Add(gossipables ...T) { p.lock.Lock() @@ -400,13 +404,13 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { } } - // If we have too many pending gossipables, trigger gossip + // If we have too many gossipables, trigger gossip to evict + // stale entries. if len(p.tracking) > p.earlyGossipSize { if err := p.gossip(context.TODO()); err != nil { - // TODO: log error + p.log.Error("failed to gossip", zap.Error(err)) } } - p.metrics.tracking.Set(float64(len(p.tracking))) } From 55c65fea3fb498e8bd72429cebc6faf668d9b857 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 26 Feb 2024 15:49:52 -0800 Subject: [PATCH 24/59] Gossip cleanup suggestions (#2773) Co-authored-by: Stephen Buttolph --- network/p2p/gossip/gossip.go | 146 +++++++++++--------- network/p2p/gossip/gossip_test.go | 216 ++++++++++-------------------- network/p2p/gossip/gossipable.go | 2 +- network/p2p/gossip/handler.go | 2 +- network/p2p/gossip/test_gossip.go | 5 + vms/platformvm/network/gossip.go | 5 + vms/platformvm/network/network.go | 108 ++++++--------- vms/platformvm/vm.go | 3 +- 8 files changed, 210 insertions(+), 277 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index cbee7b1a22c..636976fd007 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -17,6 +17,7 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/network/p2p" "github.com/ava-labs/avalanchego/utils" + "github.com/ava-labs/avalanchego/utils/bloom" "github.com/ava-labs/avalanchego/utils/buffer" "github.com/ava-labs/avalanchego/utils/logging" ) @@ -33,11 +34,9 @@ var ( _ Gossiper = (*ValidatorGossiper)(nil) _ Gossiper = (*PullGossiper[*testTx])(nil) _ Gossiper = (*NoOpGossiper)(nil) - _ Gossiper = (*TestGossiper)(nil) - _ Accumulator[*testTx] = (*PushGossiper[*testTx])(nil) - _ Accumulator[*testTx] = (*NoOpAccumulator[*testTx])(nil) - _ Accumulator[*testTx] = (*TestAccumulator[*testTx])(nil) + _ Set[*testTx] = (*EmptySet[*testTx])(nil) + _ Set[*testTx] = (*FullSet[*testTx])(nil) metricLabels = []string{typeLabel} pushLabels = prometheus.Labels{ @@ -46,6 +45,8 @@ var ( pullLabels = prometheus.Labels{ typeLabel: pullType, } + + errEmptySetCantAdd = errors.New("empty set can not add") ) // Gossiper gossips Gossipables to other nodes @@ -54,13 +55,6 @@ type Gossiper interface { Gossip(ctx context.Context) error } -// Accumulator allows a caller to accumulate gossipables to be gossiped -type Accumulator[T Gossipable] interface { - Gossiper - // Add queues gossipables to be gossiped - Add(gossipables ...T) -} - // ValidatorGossiper only calls [Gossip] if the given node is a validator type ValidatorGossiper struct { Gossiper @@ -206,17 +200,17 @@ func (p *PullGossiper[_]) handleResponse( continue } - hash := gossipable.GossipID() + gossipID := gossipable.GossipID() p.log.Debug( "received gossip", zap.Stringer("nodeID", nodeID), - zap.Stringer("id", hash), + zap.Stringer("id", gossipID), ) if err := p.set.Add(gossipable); err != nil { p.log.Debug( "failed to add gossip to the known set", zap.Stringer("nodeID", nodeID), - zap.Stringer("id", hash), + zap.Stringer("id", gossipID), zap.Error(err), ) continue @@ -287,7 +281,7 @@ type PushGossiper[T Gossipable] struct { discarded *cache.LRU[ids.ID, interface{}] // discarded ensures we don't overgossip transactions that are frequently dropped } -// Gossip flushes any queued gossipables +// Gossip flushes any queued gossipables. func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.lock.Lock() defer p.lock.Unlock() @@ -300,9 +294,13 @@ func (p *PushGossiper[T]) gossip(ctx context.Context) error { return nil } - // Iterate over all pending gossipables (never been sent before) - sentBytes := 0 - gossip := make([][]byte, 0, defaultGossipableCount) + var ( + sentBytes = 0 + gossip = make([][]byte, 0, defaultGossipableCount) + now = time.Now() + ) + + // Iterate over all pending gossipables (never been sent before). for sentBytes < p.targetGossipSize { gossipable, ok := p.pending.PopLeft() if !ok { @@ -310,23 +308,26 @@ func (p *PushGossiper[T]) gossip(ctx context.Context) error { } // Ensure item is still in the set before we gossip. - if !p.set.Has(gossipable) { - delete(p.tracking, gossipable.GossipID()) + gossipID := gossipable.GossipID() + if !p.set.Has(gossipID) { + delete(p.tracking, gossipID) continue } bytes, err := p.marshaller.MarshalGossip(gossipable) if err != nil { - // remove this item so we don't get stuck in a loop - delete(p.tracking, gossipable.GossipID()) + delete(p.tracking, gossipID) return err } gossip = append(gossip, bytes) sentBytes += len(bytes) p.issued.PushRight(gossipable) - p.tracking[gossipable.GossipID()] = time.Now() + p.tracking[gossipID] = now } + + // Iterate over all issued gossipables (have been sent before) to fill + // any remaining space in gossip batch. for sentBytes < p.targetGossipSize { gossipable, ok := p.issued.PopLeft() if !ok { @@ -334,31 +335,35 @@ func (p *PushGossiper[T]) gossip(ctx context.Context) error { } // Ensure item is still in the set before we gossip. - if !p.set.Has(gossipable) { - delete(p.tracking, gossipable.GossipID()) - p.discarded.Put(gossipable.GossipID(), nil) // only add to discarded if issued once + gossipID := gossipable.GossipID() + if !p.set.Has(gossipID) { + delete(p.tracking, gossipID) + p.discarded.Put(gossipID, nil) // only add to discarded if issued once continue } - // Ensure not gossiped too recently - lastGossipTime := p.tracking[gossipable.GossipID()] - if time.Since(lastGossipTime) < p.maxRegossipFrequency { + // Ensure we don't attempt to send a gossipable too frequently. + lastGossipTime := p.tracking[gossipID] + if now.Sub(lastGossipTime) < p.maxRegossipFrequency { + // Put the gossipable on the front of the queue to keep items sorted + // by last issuance time. p.issued.PushLeft(gossipable) - break // items are sorted by last issuance time, so we can stop here + break } bytes, err := p.marshaller.MarshalGossip(gossipable) if err != nil { // Should never happen because we've already issued this once. - delete(p.tracking, gossipable.GossipID()) + delete(p.tracking, gossipID) return err } gossip = append(gossip, bytes) sentBytes += len(bytes) p.issued.PushRight(gossipable) - p.tracking[gossipable.GossipID()] = time.Now() + p.tracking[gossipID] = now } + p.metrics.tracking.Set(float64(len(p.tracking))) // Send gossipables to peers msgBytes, err := MarshalAppGossip(gossip) @@ -378,34 +383,38 @@ func (p *PushGossiper[T]) gossip(ctx context.Context) error { if err := p.client.AppGossip(ctx, msgBytes); err != nil { return fmt.Errorf("failed to gossip: %w", err) } - return nil } -// TODO: wording cleanup Add should only be called when accepting transactions over RPC. Gossip between validators (of txs from the p2p layer) should -// use PullGossip. This is only for getting transactions into the hands of validators to begin with. +// Add enqueues new gossipables to be pushed. If a gossiable is already tracked, +// it is not added again. func (p *PushGossiper[T]) Add(gossipables ...T) { p.lock.Lock() defer p.lock.Unlock() - // Add new gossipables to the tracker + // Add new gossipables to the pending queue. + now := time.Now() for _, gossipable := range gossipables { - gid := gossipable.GossipID() - if _, contains := p.tracking[gid]; contains { + gossipID := gossipable.GossipID() + if _, contains := p.tracking[gossipID]; contains { continue } - if _, contains := p.discarded.Get(gid); !contains { - p.tracking[gid] = time.Time{} + if _, contains := p.discarded.Get(gossipID); !contains { + p.tracking[gossipID] = time.Time{} p.pending.PushRight(gossipable) } else { // Pretend that recently discarded transactions were just gossiped. - p.tracking[gid] = time.Now() + p.tracking[gossipID] = now p.issued.PushRight(gossipable) } } - // If we have too many gossipables, trigger gossip to evict - // stale entries. + // If we have too many gossipables, trigger gossip to push pending gossipables + // and/or evict stale entries. + // + // This invocation of [gossip] may not clear any stale entries (if there are + // a bunch of pending, valid gossipables), however, successive calls eventually + // will. if len(p.tracking) > p.earlyGossipSize { if err := p.gossip(context.TODO()); err != nil { p.log.Error("failed to gossip", zap.Error(err)) @@ -438,39 +447,50 @@ func (NoOpGossiper) Gossip(context.Context) error { return nil } -type NoOpAccumulator[T Gossipable] struct{} +type TestGossiper struct { + GossipF func(ctx context.Context) error +} + +func (t *TestGossiper) Gossip(ctx context.Context) error { + return t.GossipF(ctx) +} + +type EmptySet[T Gossipable] struct{} -func (NoOpAccumulator[_]) Gossip(context.Context) error { +func (EmptySet[_]) Gossip(context.Context) error { return nil } -func (NoOpAccumulator[T]) Add(...T) {} +func (EmptySet[T]) Add(T) error { + return errEmptySetCantAdd +} -type TestGossiper struct { - GossipF func(ctx context.Context) error +func (EmptySet[T]) Has(ids.ID) bool { + return false } -func (t *TestGossiper) Gossip(ctx context.Context) error { - return t.GossipF(ctx) +func (EmptySet[T]) Iterate(func(gossipable T) bool) {} + +func (EmptySet[_]) GetFilter() ([]byte, []byte) { + return bloom.EmptyFilter.Marshal(), ids.Empty[:] } -type TestAccumulator[T Gossipable] struct { - GossipF func(ctx context.Context) error - AddF func(...T) +type FullSet[T Gossipable] struct{} + +func (FullSet[_]) Gossip(context.Context) error { + return nil } -func (t TestAccumulator[T]) Gossip(ctx context.Context) error { - if t.GossipF == nil { - return nil - } +func (FullSet[T]) Add(T) error { + return nil +} - return t.GossipF(ctx) +func (FullSet[T]) Has(ids.ID) bool { + return true } -func (t TestAccumulator[T]) Add(gossipables ...T) { - if t.AddF == nil { - return - } +func (FullSet[T]) Iterate(func(gossipable T) bool) {} - t.AddF(gossipables...) +func (FullSet[_]) GetFilter() ([]byte, []byte) { + return bloom.FullFilter.Marshal(), ids.Empty[:] } diff --git a/network/p2p/gossip/gossip_test.go b/network/p2p/gossip/gossip_test.go index 5d6fe9914d4..6f2eb062c97 100644 --- a/network/p2p/gossip/gossip_test.go +++ b/network/p2p/gossip/gossip_test.go @@ -124,7 +124,6 @@ func TestGossiperGossip(t *testing.T) { handler := NewHandler[*testTx]( logging.NoLog{}, marshaller, - NoOpAccumulator[*testTx]{}, responseSet, metrics, tt.targetResponseSize, @@ -235,71 +234,96 @@ func TestValidatorGossiper(t *testing.T) { // Tests that the outgoing gossip is equivalent to what was accumulated func TestPushGossiper(t *testing.T) { + type cycle struct { + toAdd []*testTx + expected []*testTx + } tests := []struct { name string - cycles [][]*testTx + cycles []cycle }{ { name: "single cycle", - cycles: [][]*testTx{ + cycles: []cycle{ { - &testTx{ - id: ids.ID{0}, - }, - &testTx{ - id: ids.ID{1}, + toAdd: []*testTx{ + { + id: ids.ID{0}, + }, + { + id: ids.ID{1}, + }, + { + id: ids.ID{2}, + }, }, - &testTx{ - id: ids.ID{2}, + expected: []*testTx{ + { + id: ids.ID{0}, + }, + { + id: ids.ID{1}, + }, + { + id: ids.ID{2}, + }, }, }, }, }, { name: "multiple cycles", - cycles: [][]*testTx{ - { - &testTx{ - id: ids.ID{0}, - }, - }, + cycles: []cycle{ { - &testTx{ - id: ids.ID{1}, + toAdd: []*testTx{ + { + id: ids.ID{0}, + }, }, - &testTx{ - id: ids.ID{2}, + expected: []*testTx{ + { + id: ids.ID{0}, + }, }, }, { - &testTx{ - id: ids.ID{3}, + toAdd: []*testTx{ + { + id: ids.ID{1}, + }, }, - &testTx{ - id: ids.ID{4}, - }, - &testTx{ - id: ids.ID{5}, + expected: []*testTx{ + { + id: ids.ID{1}, + }, + { + id: ids.ID{0}, + }, }, }, { - &testTx{ - id: ids.ID{6}, - }, - &testTx{ - id: ids.ID{7}, + toAdd: []*testTx{ + { + id: ids.ID{2}, + }, }, - &testTx{ - id: ids.ID{8}, - }, - &testTx{ - id: ids.ID{9}, + expected: []*testTx{ + { + id: ids.ID{2}, + }, + { + id: ids.ID{1}, + }, + { + id: ids.ID{0}, + }, }, }, }, }, } + const regossipTime = time.Nanosecond for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { require := require.New(t) @@ -321,20 +345,24 @@ func TestPushGossiper(t *testing.T) { marshaller := testMarshaller{} gossiper := NewPushGossiper[*testTx]( marshaller, + FullSet[*testTx]{}, client, metrics, + 0, + 0, units.MiB, + regossipTime, ) - for _, gossipables := range tt.cycles { - gossiper.Add(gossipables...) + for _, cycle := range tt.cycles { + gossiper.Add(cycle.toAdd...) require.NoError(gossiper.Gossip(ctx)) want := &sdk.PushGossip{ - Gossip: make([][]byte, 0, len(tt.cycles)), + Gossip: make([][]byte, 0, len(cycle.expected)), } - for _, gossipable := range gossipables { + for _, gossipable := range cycle.expected { bytes, err := marshaller.MarshalGossip(gossipable) require.NoError(err) @@ -347,113 +375,15 @@ func TestPushGossiper(t *testing.T) { require.NoError(proto.Unmarshal(sentMsg[1:], got)) require.Equal(want.Gossip, got.Gossip) + + // Ensure that subsequent calls to `time.Now()` are sufficient + // for regossip. + time.Sleep(regossipTime) } }) } } -// Tests that gossip to a peer should forward the gossip if it was not -// previously known -func TestPushGossipE2E(t *testing.T) { - require := require.New(t) - - // tx known by both the sender and the receiver which should not be - // forwarded - knownTx := &testTx{id: ids.GenerateTestID()} - - log := logging.NoLog{} - bloom, err := NewBloomFilter(prometheus.NewRegistry(), "", 100, 0.01, 0.05) - require.NoError(err) - set := &testSet{ - txs: make(map[ids.ID]*testTx), - bloom: bloom, - } - require.NoError(set.Add(knownTx)) - - forwarder := &common.FakeSender{ - SentAppGossip: make(chan []byte, 1), - } - forwarderNetwork, err := p2p.NewNetwork(log, forwarder, prometheus.NewRegistry(), "") - require.NoError(err) - handlerID := uint64(123) - client := forwarderNetwork.NewClient(handlerID) - - metrics, err := NewMetrics(prometheus.NewRegistry(), "") - require.NoError(err) - marshaller := testMarshaller{} - forwarderGossiper := NewPushGossiper[*testTx]( - marshaller, - client, - metrics, - units.MiB, - ) - - handler := NewHandler[*testTx]( - log, - marshaller, - forwarderGossiper, - set, - metrics, - 0, - ) - require.NoError(err) - require.NoError(forwarderNetwork.AddHandler(handlerID, handler)) - - issuer := &common.FakeSender{ - SentAppGossip: make(chan []byte, 1), - } - issuerNetwork, err := p2p.NewNetwork(log, issuer, prometheus.NewRegistry(), "") - require.NoError(err) - issuerClient := issuerNetwork.NewClient(handlerID) - require.NoError(err) - issuerGossiper := NewPushGossiper[*testTx]( - marshaller, - issuerClient, - metrics, - units.MiB, - ) - - want := []*testTx{ - {id: ids.GenerateTestID()}, - {id: ids.GenerateTestID()}, - {id: ids.GenerateTestID()}, - } - - // gossip both some unseen txs and one the receiver already knows about - var gossiped []*testTx - gossiped = append(gossiped, want...) - gossiped = append(gossiped, knownTx) - - issuerGossiper.Add(gossiped...) - addedToSet := make([]*testTx, 0, len(want)) - set.onAdd = func(tx *testTx) { - addedToSet = append(addedToSet, tx) - } - - ctx := context.Background() - require.NoError(issuerGossiper.Gossip(ctx)) - - // make sure that we only add new txs someone gossips to us - require.NoError(forwarderNetwork.AppGossip(ctx, ids.EmptyNodeID, <-issuer.SentAppGossip)) - require.Equal(want, addedToSet) - - // make sure that we only forward txs we have not already seen before - forwardedBytes := <-forwarder.SentAppGossip - forwardedMsg := &sdk.PushGossip{} - require.NoError(proto.Unmarshal(forwardedBytes[1:], forwardedMsg)) - require.Len(forwardedMsg.Gossip, len(want)) - - gotForwarded := make([]*testTx, 0, len(addedToSet)) - - for _, bytes := range forwardedMsg.Gossip { - tx, err := marshaller.UnmarshalGossip(bytes) - require.NoError(err) - gotForwarded = append(gotForwarded, tx) - } - - require.Equal(want, gotForwarded) -} - type testValidatorSet struct { validators set.Set[ids.NodeID] } diff --git a/network/p2p/gossip/gossipable.go b/network/p2p/gossip/gossipable.go index 71c67f57dba..6af60d666bb 100644 --- a/network/p2p/gossip/gossipable.go +++ b/network/p2p/gossip/gossipable.go @@ -22,7 +22,7 @@ type Set[T Gossipable] interface { // added. Add(gossipable T) error // Has returns true if the gossipable is in the set. - Has(gossipable T) bool + Has(gossipID ids.ID) bool // Iterate iterates over elements until [f] returns false Iterate(f func(gossipable T) bool) // GetFilter returns the byte representation of bloom filter and its diff --git a/network/p2p/gossip/handler.go b/network/p2p/gossip/handler.go index 84b14043a90..c91f435badd 100644 --- a/network/p2p/gossip/handler.go +++ b/network/p2p/gossip/handler.go @@ -94,7 +94,7 @@ func (h Handler[T]) AppRequest(_ context.Context, _ ids.NodeID, _ time.Time, req return MarshalAppResponse(gossipBytes) } -func (h Handler[_]) AppGossip(ctx context.Context, nodeID ids.NodeID, gossipBytes []byte) { +func (h Handler[_]) AppGossip(_ context.Context, nodeID ids.NodeID, gossipBytes []byte) { gossip, err := ParseAppGossip(gossipBytes) if err != nil { h.log.Debug("failed to unmarshal gossip", zap.Error(err)) diff --git a/network/p2p/gossip/test_gossip.go b/network/p2p/gossip/test_gossip.go index 03098399462..7f8782b6591 100644 --- a/network/p2p/gossip/test_gossip.go +++ b/network/p2p/gossip/test_gossip.go @@ -56,6 +56,11 @@ func (t *testSet) Add(gossipable *testTx) error { return nil } +func (t *testSet) Has(gossipID ids.ID) bool { + _, ok := t.txs[gossipID] + return ok +} + func (t *testSet) Iterate(f func(gossipable *testTx) bool) { for _, tx := range t.txs { if !f(tx) { diff --git a/vms/platformvm/network/gossip.go b/vms/platformvm/network/gossip.go index 5259a80ee54..4ef98de351b 100644 --- a/vms/platformvm/network/gossip.go +++ b/vms/platformvm/network/gossip.go @@ -135,6 +135,11 @@ func (g *gossipMempool) Add(tx *txs.Tx) error { return nil } +func (g *gossipMempool) Has(txID ids.ID) bool { + _, ok := g.Mempool.Get(txID) + return ok +} + func (g *gossipMempool) GetFilter() (bloom []byte, salt []byte) { g.lock.RLock() defer g.lock.RUnlock() diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index af51c4755f4..7d8c558f015 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -5,13 +5,11 @@ package network import ( "context" - "sync" "time" "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" - "github.com/ava-labs/avalanchego/cache" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/network/p2p" "github.com/ava-labs/avalanchego/network/p2p/gossip" @@ -28,10 +26,12 @@ const TxGossipHandlerID = 0 type Network interface { common.AppHandler - // Gossip starts gossiping transactions and blocks until it completes. - Gossip(ctx context.Context) - // IssueTx verifies the transaction at the currently preferred state, adds - // it to the mempool, and gossips it to the network. + // PushGossip starts push gossiping transactions and blocks until it completes. + PushGossip(ctx context.Context) + // PullGossip starts pull gossiping transactions and blocks until it completes. + PullGossip(ctx context.Context) + // IssueTx verifies the transaction at the currently preferred state and adds + // it to the mempool. IssueTx(context.Context, *txs.Tx) error } @@ -44,13 +44,10 @@ type network struct { partialSyncPrimaryNetwork bool appSender common.AppSender - txPushGossiper gossip.Accumulator[*txs.Tx] - txPullGossiper gossip.Gossiper - txGossipFrequency time.Duration - - // gossip related attributes - recentTxsLock sync.Mutex - recentTxs *cache.LRU[ids.ID, struct{}] + txPushGossiper *gossip.PushGossiper[*txs.Tx] + txPushGossipFrequency time.Duration + txPullGossiper gossip.Gossiper + txPullGossipFrequency time.Duration } func New( @@ -87,13 +84,6 @@ func New( return nil, err } - txPushGossiper := gossip.NewPushGossiper[*txs.Tx]( - marshaller, - txGossipClient, - txGossipMetrics, - config.TargetGossipSize, - ) - gossipMempool, err := newGossipMempool( mempool, registerer, @@ -107,6 +97,18 @@ func New( return nil, err } + txPushGossiper := gossip.NewPushGossiper[*txs.Tx]( + log, + marshaller, + gossipMempool, + txGossipClient, + txGossipMetrics, + 0, // earlyGossipSize (IF THIS IS SMALLER THAN MAX SIZE, COULD LEAD TO GOSSIP EACH ADD) + 1024, // discardedSize + config.TargetGossipSize, + time.Second, // maxRegossipFrequency + ) + var txPullGossiper gossip.Gossiper txPullGossiper = gossip.NewPullGossiper[*txs.Tx]( log, @@ -127,7 +129,6 @@ func New( handler := gossip.NewHandler[*txs.Tx]( log, marshaller, - txPushGossiper, gossipMempool, txGossipMetrics, config.TargetGossipSize, @@ -165,20 +166,30 @@ func New( partialSyncPrimaryNetwork: partialSyncPrimaryNetwork, appSender: appSender, txPushGossiper: txPushGossiper, + txPushGossipFrequency: config.PushGossipFrequency, txPullGossiper: txPullGossiper, - txGossipFrequency: config.PullGossipFrequency, - recentTxs: &cache.LRU[ids.ID, struct{}]{Size: config.LegacyPushGossipCacheSize}, + txPullGossipFrequency: config.PullGossipFrequency, }, nil } -func (n *network) Gossip(ctx context.Context) { +func (n *network) PushGossip(ctx context.Context) { // If the node is running partial sync, we should not perform any pull // gossip. if n.partialSyncPrimaryNetwork { return } - gossip.Every(ctx, n.log, n.txPullGossiper, n.txGossipFrequency) + gossip.Every(ctx, n.log, n.txPushGossiper, n.txPushGossipFrequency) +} + +func (n *network) PullGossip(ctx context.Context) { + // If the node is running partial sync, we should not perform any pull + // gossip. + if n.partialSyncPrimaryNetwork { + return + } + + gossip.Every(ctx, n.log, n.txPullGossiper, n.txPullGossipFrequency) } func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []byte) error { @@ -220,35 +231,19 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b ) return nil } - txID := tx.ID() - - if err := n.issueTx(tx); err == nil { - n.legacyGossipTx(ctx, txID, msgBytes) - n.txPushGossiper.Add(tx) - return n.txPushGossiper.Gossip(ctx) - } - return nil + return n.issueTx(tx) } +// TODO: is this only invoked by user submissions? +// TODO: naming here is confusing (should be clearer about which functions invoke push gossip if mempool addition is successful) func (n *network) IssueTx(ctx context.Context, tx *txs.Tx) error { if err := n.issueTx(tx); err != nil { return err } - txBytes := tx.Bytes() - msg := &message.Tx{ - Tx: txBytes, - } - msgBytes, err := message.Build(msg) - if err != nil { - return err - } - - txID := tx.ID() - n.legacyGossipTx(ctx, txID, msgBytes) n.txPushGossiper.Add(tx) - return n.txPushGossiper.Gossip(ctx) + return nil } // returns nil if the tx is in the mempool @@ -270,26 +265,3 @@ func (n *network) issueTx(tx *txs.Tx) error { return nil } - -func (n *network) legacyGossipTx(ctx context.Context, txID ids.ID, msgBytes []byte) { - n.recentTxsLock.Lock() - _, has := n.recentTxs.Get(txID) - n.recentTxs.Put(txID, struct{}{}) - n.recentTxsLock.Unlock() - - // Don't gossip a transaction if it has been recently gossiped. - if has { - return - } - - n.log.Debug("gossiping tx", - zap.Stringer("txID", txID), - ) - - if err := n.appSender.SendAppGossip(ctx, msgBytes); err != nil { - n.log.Error("failed to gossip tx", - zap.Stringer("txID", txID), - zap.Error(err), - ) - } -} diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go index 8c4801e06ea..37903743ed1 100644 --- a/vms/platformvm/vm.go +++ b/vms/platformvm/vm.go @@ -219,7 +219,8 @@ func (vm *VM) Initialize( vm.onShutdownCtx, vm.onShutdownCtxCancel = context.WithCancel(context.Background()) // TODO: Wait for this goroutine to exit during Shutdown once the platformvm // has better control of the context lock. - go vm.Network.Gossip(vm.onShutdownCtx) + go vm.Network.PushGossip(vm.onShutdownCtx) + go vm.Network.PullGossip(vm.onShutdownCtx) vm.Builder = blockbuilder.New( mempool, From a7e8bf701b01d28ddb8635c8b815e37e5264d06e Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Mon, 26 Feb 2024 19:01:44 -0500 Subject: [PATCH 25/59] cleanup some compilation issues --- network/p2p/gossip/gossip.go | 25 ++++++++++++------------- network/p2p/gossip/gossip_test.go | 5 +++-- vms/platformvm/network/config.go | 4 ++++ vms/platformvm/network/network.go | 5 +---- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 636976fd007..6c1d9c31a29 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -235,7 +235,6 @@ func (p *PullGossiper[_]) handleResponse( // NewPushGossiper returns an instance of PushGossiper func NewPushGossiper[T Gossipable]( - log logging.Logger, marshaller Marshaller[T], mempool Set[T], client *p2p.Client, @@ -246,7 +245,6 @@ func NewPushGossiper[T Gossipable]( maxRegossipFrequency time.Duration, ) *PushGossiper[T] { return &PushGossiper[T]{ - log: log, marshaller: marshaller, set: mempool, client: client, @@ -264,7 +262,6 @@ func NewPushGossiper[T Gossipable]( // PushGossiper broadcasts gossip to peers randomly in the network type PushGossiper[T Gossipable] struct { - log logging.Logger marshaller Marshaller[T] set Set[T] client *p2p.Client @@ -344,6 +341,8 @@ func (p *PushGossiper[T]) gossip(ctx context.Context) error { // Ensure we don't attempt to send a gossipable too frequently. lastGossipTime := p.tracking[gossipID] + // Invariant: maxRegossipFrequency must be greater than 0 to prevent + // gossiping the same transaction multiple times in the same message. if now.Sub(lastGossipTime) < p.maxRegossipFrequency { // Put the gossipable on the front of the queue to keep items sorted // by last issuance time. @@ -388,7 +387,7 @@ func (p *PushGossiper[T]) gossip(ctx context.Context) error { // Add enqueues new gossipables to be pushed. If a gossiable is already tracked, // it is not added again. -func (p *PushGossiper[T]) Add(gossipables ...T) { +func (p *PushGossiper[T]) Add(ctx context.Context, gossipables ...T) error { p.lock.Lock() defer p.lock.Unlock() @@ -408,19 +407,19 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { p.issued.PushRight(gossipable) } } + p.metrics.tracking.Set(float64(len(p.tracking))) - // If we have too many gossipables, trigger gossip to push pending gossipables - // and/or evict stale entries. + // If we have too many gossipables, trigger gossip to issue pending + // gossipables and/or evict stale entries. // // This invocation of [gossip] may not clear any stale entries (if there are - // a bunch of pending, valid gossipables), however, successive calls eventually - // will. - if len(p.tracking) > p.earlyGossipSize { - if err := p.gossip(context.TODO()); err != nil { - p.log.Error("failed to gossip", zap.Error(err)) - } + // a bunch of pending, valid gossipables), however, successive calls + // eventually will. + if p.pending.Len() < p.earlyGossipSize { + return nil } - p.metrics.tracking.Set(float64(len(p.tracking))) + + return p.gossip(ctx) } // Every calls [Gossip] every [frequency] amount of time. diff --git a/network/p2p/gossip/gossip_test.go b/network/p2p/gossip/gossip_test.go index 6f2eb062c97..b85bba91cab 100644 --- a/network/p2p/gossip/gossip_test.go +++ b/network/p2p/gossip/gossip_test.go @@ -5,6 +5,7 @@ package gossip import ( "context" + "math" "sync" "testing" "time" @@ -348,14 +349,14 @@ func TestPushGossiper(t *testing.T) { FullSet[*testTx]{}, client, metrics, - 0, + math.MaxInt, 0, units.MiB, regossipTime, ) for _, cycle := range tt.cycles { - gossiper.Add(cycle.toAdd...) + require.NoError(gossiper.Add(context.Background(), cycle.toAdd...)) require.NoError(gossiper.Gossip(ctx)) want := &sdk.PushGossip{ diff --git a/vms/platformvm/network/config.go b/vms/platformvm/network/config.go index 8536504d838..d31b37e47ad 100644 --- a/vms/platformvm/network/config.go +++ b/vms/platformvm/network/config.go @@ -12,6 +12,7 @@ import ( var DefaultConfig = Config{ MaxValidatorSetStaleness: time.Minute, TargetGossipSize: 20 * units.KiB, + PushGossipFrequency: 1500 * time.Millisecond, PullGossipPollSize: 1, PullGossipFrequency: 1500 * time.Millisecond, PullGossipThrottlingPeriod: 10 * time.Second, @@ -30,6 +31,9 @@ type Config struct { // sent when pushing transactions and when responded to transaction pull // requests. TargetGossipSize int `json:"target-gossip-size"` + // PushGossipFrequency is how frequently rounds of push gossip are + // performed. + PushGossipFrequency time.Duration `json:"push-gossip-frequency"` // PullGossipPollSize is the number of validators to sample when performing // a round of pull gossip. PullGossipPollSize int `json:"pull-gossip-poll-size"` diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index 7d8c558f015..fb840d815cf 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -98,7 +98,6 @@ func New( } txPushGossiper := gossip.NewPushGossiper[*txs.Tx]( - log, marshaller, gossipMempool, txGossipClient, @@ -241,9 +240,7 @@ func (n *network) IssueTx(ctx context.Context, tx *txs.Tx) error { if err := n.issueTx(tx); err != nil { return err } - - n.txPushGossiper.Add(tx) - return nil + return n.txPushGossiper.Add(ctx, tx) } // returns nil if the tx is in the mempool From 06971d7590e9e029c7035d3c35f897a74a139270 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Mon, 26 Feb 2024 19:11:06 -0500 Subject: [PATCH 26/59] comment nits --- network/p2p/gossip/gossip.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 6c1d9c31a29..f927e15176c 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -267,7 +267,7 @@ type PushGossiper[T Gossipable] struct { client *p2p.Client metrics Metrics - earlyGossipSize int // size at which we attempt gossip during [Add] to avoid unbounded memory use + earlyGossipSize int // size at which we attempt gossip during [Add] to reduce memory use targetGossipSize int maxRegossipFrequency time.Duration @@ -275,7 +275,7 @@ type PushGossiper[T Gossipable] struct { tracking map[ids.ID]time.Time pending buffer.Deque[T] issued buffer.Deque[T] - discarded *cache.LRU[ids.ID, interface{}] // discarded ensures we don't overgossip transactions that are frequently dropped + discarded *cache.LRU[ids.ID, interface{}] // discarded attempts to avoid overgossiping transactions that are frequently dropped } // Gossip flushes any queued gossipables. From 0ef143290b294a3b74940c9217bf09ff72e2d95b Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Mon, 26 Feb 2024 19:14:10 -0500 Subject: [PATCH 27/59] comment nits --- network/p2p/gossip/gossip_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/network/p2p/gossip/gossip_test.go b/network/p2p/gossip/gossip_test.go index b85bba91cab..8202f41ecf4 100644 --- a/network/p2p/gossip/gossip_test.go +++ b/network/p2p/gossip/gossip_test.go @@ -349,8 +349,8 @@ func TestPushGossiper(t *testing.T) { FullSet[*testTx]{}, client, metrics, - math.MaxInt, - 0, + math.MaxInt, // never gossip during Add + 0, // the discarded cache size doesn't matter for this test units.MiB, regossipTime, ) From cd91a1d05ede97d3a525c864925022751c7c5aff Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Mon, 26 Feb 2024 19:14:51 -0500 Subject: [PATCH 28/59] nit remove useless continue --- network/p2p/gossip/handler.go | 1 - 1 file changed, 1 deletion(-) diff --git a/network/p2p/gossip/handler.go b/network/p2p/gossip/handler.go index c91f435badd..5c125864cc6 100644 --- a/network/p2p/gossip/handler.go +++ b/network/p2p/gossip/handler.go @@ -120,7 +120,6 @@ func (h Handler[_]) AppGossip(_ context.Context, nodeID ids.NodeID, gossipBytes zap.Stringer("id", gossipable.GossipID()), zap.Error(err), ) - continue } } From bcfe2cf65e085da5e1c36d09760de1478e9abe62 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Mon, 26 Feb 2024 19:22:02 -0500 Subject: [PATCH 29/59] implement Has in AVM --- vms/avm/network/gossip.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vms/avm/network/gossip.go b/vms/avm/network/gossip.go index 0876f122c66..1a13eb58b6f 100644 --- a/vms/avm/network/gossip.go +++ b/vms/avm/network/gossip.go @@ -121,6 +121,10 @@ func (g *gossipMempool) Add(tx *txs.Tx) error { return g.AddVerified(tx) } +func (g *gossipMempool) Has(txID ids.ID) bool { + _, ok := g.Mempool.Get(txID) + return ok +} func (g *gossipMempool) AddVerified(tx *txs.Tx) error { if err := g.Mempool.Add(tx); err != nil { From 9e40b645a0dd2dc5bb7d402dd092312840ff59c8 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Mon, 26 Feb 2024 19:30:30 -0500 Subject: [PATCH 30/59] push gossip with partial sync --- vms/platformvm/network/network.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index fb840d815cf..2809fccd2e3 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -172,12 +172,6 @@ func New( } func (n *network) PushGossip(ctx context.Context) { - // If the node is running partial sync, we should not perform any pull - // gossip. - if n.partialSyncPrimaryNetwork { - return - } - gossip.Every(ctx, n.log, n.txPushGossiper, n.txPushGossipFrequency) } From a3b824f44bfdc56428a8d0bf64e79cc75c9be384 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 13:04:01 -0500 Subject: [PATCH 31/59] remove early gossip --- network/p2p/gossip/gossip.go | 17 +----- network/p2p/gossip/gossip_test.go | 6 +- vms/platformvm/block/builder/builder_test.go | 8 +-- vms/platformvm/network/config.go | 10 +++- vms/platformvm/network/network.go | 24 ++++---- vms/platformvm/network/network_test.go | 38 +----------- vms/platformvm/service.go | 3 +- vms/platformvm/service_test.go | 4 +- vms/platformvm/validator_set_property_test.go | 4 +- vms/platformvm/vm.go | 4 +- vms/platformvm/vm_regression_test.go | 60 +++++++++---------- vms/platformvm/vm_test.go | 34 +++++------ 12 files changed, 84 insertions(+), 128 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index f927e15176c..44ddb3b3143 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -239,7 +239,6 @@ func NewPushGossiper[T Gossipable]( mempool Set[T], client *p2p.Client, metrics Metrics, - earlyGossipSize int, discardedSize int, targetGossipSize int, maxRegossipFrequency time.Duration, @@ -249,7 +248,6 @@ func NewPushGossiper[T Gossipable]( set: mempool, client: client, metrics: metrics, - earlyGossipSize: earlyGossipSize, targetGossipSize: targetGossipSize, maxRegossipFrequency: maxRegossipFrequency, @@ -267,7 +265,6 @@ type PushGossiper[T Gossipable] struct { client *p2p.Client metrics Metrics - earlyGossipSize int // size at which we attempt gossip during [Add] to reduce memory use targetGossipSize int maxRegossipFrequency time.Duration @@ -387,7 +384,7 @@ func (p *PushGossiper[T]) gossip(ctx context.Context) error { // Add enqueues new gossipables to be pushed. If a gossiable is already tracked, // it is not added again. -func (p *PushGossiper[T]) Add(ctx context.Context, gossipables ...T) error { +func (p *PushGossiper[T]) Add(gossipables ...T) { p.lock.Lock() defer p.lock.Unlock() @@ -408,18 +405,6 @@ func (p *PushGossiper[T]) Add(ctx context.Context, gossipables ...T) error { } } p.metrics.tracking.Set(float64(len(p.tracking))) - - // If we have too many gossipables, trigger gossip to issue pending - // gossipables and/or evict stale entries. - // - // This invocation of [gossip] may not clear any stale entries (if there are - // a bunch of pending, valid gossipables), however, successive calls - // eventually will. - if p.pending.Len() < p.earlyGossipSize { - return nil - } - - return p.gossip(ctx) } // Every calls [Gossip] every [frequency] amount of time. diff --git a/network/p2p/gossip/gossip_test.go b/network/p2p/gossip/gossip_test.go index 8202f41ecf4..777948ed883 100644 --- a/network/p2p/gossip/gossip_test.go +++ b/network/p2p/gossip/gossip_test.go @@ -5,7 +5,6 @@ package gossip import ( "context" - "math" "sync" "testing" "time" @@ -349,14 +348,13 @@ func TestPushGossiper(t *testing.T) { FullSet[*testTx]{}, client, metrics, - math.MaxInt, // never gossip during Add - 0, // the discarded cache size doesn't matter for this test + 0, // the discarded cache size doesn't matter for this test units.MiB, regossipTime, ) for _, cycle := range tt.cycles { - require.NoError(gossiper.Add(context.Background(), cycle.toAdd...)) + gossiper.Add(cycle.toAdd...) require.NoError(gossiper.Gossip(ctx)) want := &sdk.PushGossip{ diff --git a/vms/platformvm/block/builder/builder_test.go b/vms/platformvm/block/builder/builder_test.go index e3486f96dca..8471b4fbed1 100644 --- a/vms/platformvm/block/builder/builder_test.go +++ b/vms/platformvm/block/builder/builder_test.go @@ -52,7 +52,7 @@ func TestBuildBlockBasic(t *testing.T) { // Issue the transaction env.ctx.Lock.Unlock() - require.NoError(env.network.IssueTx(context.Background(), tx)) + require.NoError(env.network.IssueTx(tx)) env.ctx.Lock.Lock() _, ok := env.mempool.Get(txID) require.True(ok) @@ -126,7 +126,7 @@ func TestBuildBlockShouldReward(t *testing.T) { // Issue the transaction env.ctx.Lock.Unlock() - require.NoError(env.network.IssueTx(context.Background(), tx)) + require.NoError(env.network.IssueTx(tx)) env.ctx.Lock.Lock() _, ok := env.mempool.Get(txID) require.True(ok) @@ -247,7 +247,7 @@ func TestBuildBlockForceAdvanceTime(t *testing.T) { // Issue the transaction env.ctx.Lock.Unlock() - require.NoError(env.network.IssueTx(context.Background(), tx)) + require.NoError(env.network.IssueTx(tx)) env.ctx.Lock.Lock() _, ok := env.mempool.Get(txID) require.True(ok) @@ -504,7 +504,7 @@ func TestPreviouslyDroppedTxsCannotBeReAddedToMempool(t *testing.T) { // Issue the transaction env.ctx.Lock.Unlock() - err = env.network.IssueTx(context.Background(), tx) + err = env.network.IssueTx(tx) require.ErrorIs(err, errTestingDropped) env.ctx.Lock.Lock() _, ok := env.mempool.Get(txID) diff --git a/vms/platformvm/network/config.go b/vms/platformvm/network/config.go index d31b37e47ad..2fe878c64a8 100644 --- a/vms/platformvm/network/config.go +++ b/vms/platformvm/network/config.go @@ -12,7 +12,9 @@ import ( var DefaultConfig = Config{ MaxValidatorSetStaleness: time.Minute, TargetGossipSize: 20 * units.KiB, - PushGossipFrequency: 1500 * time.Millisecond, + PushGossipDiscardedCacheSize: 1024, + PushGossipMaxRegossipFrequency: 2000 * time.Millisecond, + PushGossipFrequency: 500 * time.Millisecond, PullGossipPollSize: 1, PullGossipFrequency: 1500 * time.Millisecond, PullGossipThrottlingPeriod: 10 * time.Second, @@ -33,6 +35,12 @@ type Config struct { TargetGossipSize int `json:"target-gossip-size"` // PushGossipFrequency is how frequently rounds of push gossip are // performed. + PushGossipDiscardedCacheSize int `json:"push-gossip-discarded-cache-size"` + // PushGossipMaxRegossipFrequency is the limit for how frequently a + // transaction will be push gossiped. + PushGossipMaxRegossipFrequency time.Duration `json:"push-gossip-max-regossip-frequency"` + // PushGossipFrequency is how frequently rounds of push gossip are + // performed. PushGossipFrequency time.Duration `json:"push-gossip-frequency"` // PullGossipPollSize is the number of validators to sample when performing // a round of pull gossip. diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index 2809fccd2e3..7ddbc5f5634 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -26,13 +26,15 @@ const TxGossipHandlerID = 0 type Network interface { common.AppHandler - // PushGossip starts push gossiping transactions and blocks until it completes. + // PushGossip starts push gossiping transactions and blocks until it + // completes. PushGossip(ctx context.Context) - // PullGossip starts pull gossiping transactions and blocks until it completes. + // PullGossip starts pull gossiping transactions and blocks until it + // completes. PullGossip(ctx context.Context) - // IssueTx verifies the transaction at the currently preferred state and adds - // it to the mempool. - IssueTx(context.Context, *txs.Tx) error + // IssueTx verifies the transaction at the currently preferred state and + // adds it to the mempool. + IssueTx(*txs.Tx) error } type network struct { @@ -102,10 +104,9 @@ func New( gossipMempool, txGossipClient, txGossipMetrics, - 0, // earlyGossipSize (IF THIS IS SMALLER THAN MAX SIZE, COULD LEAD TO GOSSIP EACH ADD) - 1024, // discardedSize + config.PushGossipDiscardedCacheSize, config.TargetGossipSize, - time.Second, // maxRegossipFrequency + config.PushGossipMaxRegossipFrequency, ) var txPullGossiper gossip.Gossiper @@ -228,13 +229,14 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b return n.issueTx(tx) } -// TODO: is this only invoked by user submissions? // TODO: naming here is confusing (should be clearer about which functions invoke push gossip if mempool addition is successful) -func (n *network) IssueTx(ctx context.Context, tx *txs.Tx) error { +// TODO: issueTx doesn't add the tx to the mempool if partial sync is enabled, so the push gossiper will always drop the tx before broadcasting it +func (n *network) IssueTx(tx *txs.Tx) error { if err := n.issueTx(tx); err != nil { return err } - return n.txPushGossiper.Add(ctx, tx) + n.txPushGossiper.Add(tx) + return nil } // returns nil if the tx is in the mempool diff --git a/vms/platformvm/network/network_test.go b/vms/platformvm/network/network_test.go index 56957b0007c..dbe39afa3c2 100644 --- a/vms/platformvm/network/network_test.go +++ b/vms/platformvm/network/network_test.go @@ -329,44 +329,8 @@ func TestNetworkIssueTx(t *testing.T) { ) require.NoError(err) - err = n.IssueTx(context.Background(), tx) + err = n.IssueTx(tx) require.ErrorIs(err, tt.expectedErr) }) } } - -func TestNetworkGossipTx(t *testing.T) { - require := require.New(t) - ctrl := gomock.NewController(t) - - appSender := common.NewMockSender(ctrl) - - snowCtx := snowtest.Context(t, ids.Empty) - nIntf, err := New( - snowCtx.Log, - snowCtx.NodeID, - snowCtx.SubnetID, - snowCtx.ValidatorState, - testTxVerifier{}, - mempool.NewMockMempool(ctrl), - false, - appSender, - prometheus.NewRegistry(), - testConfig, - ) - require.NoError(err) - require.IsType(&network{}, nIntf) - n := nIntf.(*network) - - // Case: Tx was recently gossiped - txID := ids.GenerateTestID() - n.recentTxs.Put(txID, struct{}{}) - n.legacyGossipTx(context.Background(), txID, []byte{}) - // Didn't make a call to SendAppGossip - - // Case: Tx was not recently gossiped - msgBytes := []byte{1, 2, 3} - appSender.EXPECT().SendAppGossip(gomock.Any(), msgBytes).Return(nil) - n.legacyGossipTx(context.Background(), ids.GenerateTestID(), msgBytes) - // Did make a call to SendAppGossip -} diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index 300af16fd83..fff7e4a2348 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -1415,10 +1415,9 @@ func (s *Service) IssueTx(req *http.Request, args *api.FormattedTx, response *ap return fmt.Errorf("couldn't parse tx: %w", err) } - if err := s.vm.issueTx(req.Context(), tx); err != nil { + if err := s.vm.issueTx(tx); err != nil { return fmt.Errorf("couldn't issue tx: %w", err) } - // TODO: enqueue push gossip response.TxID = tx.ID() return nil diff --git a/vms/platformvm/service_test.go b/vms/platformvm/service_test.go index ad5409d313b..41a9e8b2674 100644 --- a/vms/platformvm/service_test.go +++ b/vms/platformvm/service_test.go @@ -184,7 +184,7 @@ func TestGetTxStatus(t *testing.T) { require.Zero(resp.Reason) // put the chain in existing chain list - require.NoError(service.vm.Network.IssueTx(context.Background(), tx)) + require.NoError(service.vm.Network.IssueTx(tx)) service.vm.ctx.Lock.Lock() block, err := service.vm.BuildBlock(context.Background()) @@ -285,7 +285,7 @@ func TestGetTx(t *testing.T) { err = service.GetTx(nil, arg, &response) require.ErrorIs(err, database.ErrNotFound) // We haven't issued the tx yet - require.NoError(service.vm.Network.IssueTx(context.Background(), tx)) + require.NoError(service.vm.Network.IssueTx(tx)) service.vm.ctx.Lock.Lock() blk, err := service.vm.BuildBlock(context.Background()) diff --git a/vms/platformvm/validator_set_property_test.go b/vms/platformvm/validator_set_property_test.go index cdac03ca53d..1b23c72bf40 100644 --- a/vms/platformvm/validator_set_property_test.go +++ b/vms/platformvm/validator_set_property_test.go @@ -298,7 +298,7 @@ func addPrimaryValidatorWithBLSKey(vm *VM, data *validatorInputData) (*state.Sta func internalAddValidator(vm *VM, signedTx *txs.Tx) (*state.Staker, error) { vm.ctx.Lock.Unlock() - err := vm.issueTx(context.Background(), signedTx) + err := vm.issueTx(signedTx) vm.ctx.Lock.Lock() if err != nil { @@ -689,7 +689,7 @@ func buildVM(t *testing.T) (*VM, ids.ID, error) { return nil, ids.Empty, err } vm.ctx.Lock.Unlock() - err = vm.issueTx(context.Background(), testSubnet1) + err = vm.issueTx(testSubnet1) vm.ctx.Lock.Lock() if err != nil { return nil, ids.Empty, err diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go index 37903743ed1..d72e1cba25d 100644 --- a/vms/platformvm/vm.go +++ b/vms/platformvm/vm.go @@ -542,8 +542,8 @@ func (vm *VM) GetBlockIDAtHeight(_ context.Context, height uint64) (ids.ID, erro return vm.state.GetBlockIDAtHeight(height) } -func (vm *VM) issueTx(ctx context.Context, tx *txs.Tx) error { - err := vm.Network.IssueTx(ctx, tx) +func (vm *VM) issueTx(tx *txs.Tx) error { + err := vm.Network.IssueTx(tx) if err != nil && !errors.Is(err, mempool.ErrDuplicateTx) { vm.ctx.Log.Debug("failed to add tx to mempool", zap.Stringer("txID", tx.ID()), diff --git a/vms/platformvm/vm_regression_test.go b/vms/platformvm/vm_regression_test.go index e612340546f..6909ee6952e 100644 --- a/vms/platformvm/vm_regression_test.go +++ b/vms/platformvm/vm_regression_test.go @@ -77,7 +77,7 @@ func TestAddDelegatorTxOverDelegatedRegression(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addValidatorTx)) + require.NoError(vm.issueTx(addValidatorTx)) vm.ctx.Lock.Lock() addValidatorBlock, err := vm.Builder.BuildBlock(context.Background()) @@ -112,7 +112,7 @@ func TestAddDelegatorTxOverDelegatedRegression(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addFirstDelegatorTx)) + require.NoError(vm.issueTx(addFirstDelegatorTx)) vm.ctx.Lock.Lock() addFirstDelegatorBlock, err := vm.Builder.BuildBlock(context.Background()) @@ -149,7 +149,7 @@ func TestAddDelegatorTxOverDelegatedRegression(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addSecondDelegatorTx)) + require.NoError(vm.issueTx(addSecondDelegatorTx)) vm.ctx.Lock.Lock() addSecondDelegatorBlock, err := vm.Builder.BuildBlock(context.Background()) @@ -176,7 +176,7 @@ func TestAddDelegatorTxOverDelegatedRegression(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - err = vm.issueTx(context.Background(), addThirdDelegatorTx) + err = vm.issueTx(addThirdDelegatorTx) require.ErrorIs(err, executor.ErrOverDelegated) vm.ctx.Lock.Lock() } @@ -249,7 +249,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the add validator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addValidatorTx)) + require.NoError(vm.issueTx(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -274,7 +274,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the first add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addFirstDelegatorTx)) + require.NoError(vm.issueTx(addFirstDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the first add delegator tx @@ -299,7 +299,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the second add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addSecondDelegatorTx)) + require.NoError(vm.issueTx(addSecondDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the second add delegator tx @@ -324,7 +324,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the third add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addThirdDelegatorTx)) + require.NoError(vm.issueTx(addThirdDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the third add delegator tx @@ -349,7 +349,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the fourth add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addFourthDelegatorTx)) + require.NoError(vm.issueTx(addFourthDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the fourth add delegator tx @@ -1179,7 +1179,7 @@ func TestAddDelegatorTxAddBeforeRemove(t *testing.T) { // issue the add validator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addValidatorTx)) + require.NoError(vm.issueTx(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1204,7 +1204,7 @@ func TestAddDelegatorTxAddBeforeRemove(t *testing.T) { // issue the first add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addFirstDelegatorTx)) + require.NoError(vm.issueTx(addFirstDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the first add delegator tx @@ -1230,7 +1230,7 @@ func TestAddDelegatorTxAddBeforeRemove(t *testing.T) { // attempting to issue the second add delegator tx should fail because the // total stake weight would go over the limit. vm.ctx.Lock.Unlock() - err = vm.issueTx(context.Background(), addSecondDelegatorTx) + err = vm.issueTx(addSecondDelegatorTx) require.ErrorIs(err, executor.ErrOverDelegated) vm.ctx.Lock.Lock() } @@ -1266,7 +1266,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionNotTracked(t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addValidatorTx)) + require.NoError(vm.issueTx(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1286,7 +1286,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionNotTracked(t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), createSubnetTx)) + require.NoError(vm.issueTx(createSubnetTx)) vm.ctx.Lock.Lock() // trigger block creation for the subnet tx @@ -1309,7 +1309,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionNotTracked(t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addSubnetValidatorTx)) + require.NoError(vm.issueTx(addSubnetValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1341,7 +1341,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionNotTracked(t vm.clock.Set(validatorStartTime) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), removeSubnetValidatorTx)) + require.NoError(vm.issueTx(removeSubnetValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1391,7 +1391,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addValidatorTx)) + require.NoError(vm.issueTx(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1411,7 +1411,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), createSubnetTx)) + require.NoError(vm.issueTx(createSubnetTx)) vm.ctx.Lock.Lock() // trigger block creation for the subnet tx @@ -1434,7 +1434,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addSubnetValidatorTx)) + require.NoError(vm.issueTx(addSubnetValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1458,7 +1458,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t vm.clock.Set(validatorStartTime) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), removeSubnetValidatorTx)) + require.NoError(vm.issueTx(removeSubnetValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1520,7 +1520,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { uPrimaryTx := primaryTx.Unsigned.(*txs.AddPermissionlessValidatorTx) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), primaryTx)) + require.NoError(vm.issueTx(primaryTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1550,7 +1550,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), subnetTx)) + require.NoError(vm.issueTx(subnetTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1626,7 +1626,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { uPrimaryRestartTx := primaryRestartTx.Unsigned.(*txs.AddPermissionlessValidatorTx) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), primaryRestartTx)) + require.NoError(vm.issueTx(primaryRestartTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1733,7 +1733,7 @@ func TestPrimaryNetworkValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), primaryTx1)) + require.NoError(vm.issueTx(primaryTx1)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1795,7 +1795,7 @@ func TestPrimaryNetworkValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), primaryRestartTx)) + require.NoError(vm.issueTx(primaryRestartTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1866,7 +1866,7 @@ func TestSubnetValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), primaryTx1)) + require.NoError(vm.issueTx(primaryTx1)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1896,7 +1896,7 @@ func TestSubnetValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), subnetTx)) + require.NoError(vm.issueTx(subnetTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1970,7 +1970,7 @@ func TestSubnetValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), primaryRestartTx)) + require.NoError(vm.issueTx(primaryRestartTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -2048,7 +2048,7 @@ func TestSubnetValidatorSetAfterPrimaryNetworkValidatorRemoval(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), primaryTx1)) + require.NoError(vm.issueTx(primaryTx1)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -2075,7 +2075,7 @@ func TestSubnetValidatorSetAfterPrimaryNetworkValidatorRemoval(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), subnetTx)) + require.NoError(vm.issueTx(subnetTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) diff --git a/vms/platformvm/vm_test.go b/vms/platformvm/vm_test.go index 1b16e72bf3c..423ba9e8d0c 100644 --- a/vms/platformvm/vm_test.go +++ b/vms/platformvm/vm_test.go @@ -310,7 +310,7 @@ func defaultVM(t *testing.T, f fork) (*VM, database.Database, *mutableSharedMemo ) require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), testSubnet1)) + require.NoError(vm.issueTx(testSubnet1)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) require.NoError(err) @@ -415,7 +415,7 @@ func TestAddValidatorCommit(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), tx)) + require.NoError(vm.issueTx(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -514,7 +514,7 @@ func TestAddValidatorReject(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), tx)) + require.NoError(vm.issueTx(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -563,7 +563,7 @@ func TestAddValidatorInvalidNotReissued(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - err = vm.issueTx(context.Background(), tx) + err = vm.issueTx(tx) vm.ctx.Lock.Lock() require.ErrorIs(err, txexecutor.ErrDuplicateValidator) } @@ -598,7 +598,7 @@ func TestAddSubnetValidatorAccept(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), tx)) + require.NoError(vm.issueTx(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -646,7 +646,7 @@ func TestAddSubnetValidatorReject(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), tx)) + require.NoError(vm.issueTx(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -832,7 +832,7 @@ func TestCreateChain(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), tx)) + require.NoError(vm.issueTx(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -883,7 +883,7 @@ func TestCreateSubnet(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), createSubnetTx)) + require.NoError(vm.issueTx(createSubnetTx)) vm.ctx.Lock.Lock() // should contain the CreateSubnetTx @@ -927,7 +927,7 @@ func TestCreateSubnet(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addValidatorTx)) + require.NoError(vm.issueTx(addValidatorTx)) vm.ctx.Lock.Lock() blk, err = vm.Builder.BuildBlock(context.Background()) // should add validator to the new subnet @@ -1031,7 +1031,7 @@ func TestAtomicImport(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), tx)) + require.NoError(vm.issueTx(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -2030,7 +2030,7 @@ func TestRemovePermissionedValidatorDuringAddPending(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addValidatorTx)) + require.NoError(vm.issueTx(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -2050,7 +2050,7 @@ func TestRemovePermissionedValidatorDuringAddPending(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), createSubnetTx)) + require.NoError(vm.issueTx(createSubnetTx)) vm.ctx.Lock.Lock() // trigger block creation for the subnet tx @@ -2121,7 +2121,7 @@ func TestTransferSubnetOwnershipTx(t *testing.T) { subnetID := createSubnetTx.ID() vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), createSubnetTx)) + require.NoError(vm.issueTx(createSubnetTx)) vm.ctx.Lock.Lock() createSubnetBlock, err := vm.Builder.BuildBlock(context.Background()) require.NoError(err) @@ -2156,7 +2156,7 @@ func TestTransferSubnetOwnershipTx(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), transferSubnetOwnershipTx)) + require.NoError(vm.issueTx(transferSubnetOwnershipTx)) vm.ctx.Lock.Lock() transferSubnetOwnershipBlock, err := vm.Builder.BuildBlock(context.Background()) require.NoError(err) @@ -2242,7 +2242,7 @@ func TestBaseTx(t *testing.T) { require.Equal(sendAmt, key1OutputAmt) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), baseTx)) + require.NoError(vm.issueTx(baseTx)) vm.ctx.Lock.Lock() baseTxBlock, err := vm.Builder.BuildBlock(context.Background()) require.NoError(err) @@ -2281,7 +2281,7 @@ func TestPruneMempool(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), baseTx)) + require.NoError(vm.issueTx(baseTx)) vm.ctx.Lock.Lock() // [baseTx] should be in the mempool. @@ -2313,7 +2313,7 @@ func TestPruneMempool(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(context.Background(), addValidatorTx)) + require.NoError(vm.issueTx(addValidatorTx)) vm.ctx.Lock.Lock() // Advance clock to [endTime], making [addValidatorTx] invalid. From 5031a6e72f550c681d1de20a70b9ed7dfb4c67fb Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 13:05:57 -0500 Subject: [PATCH 32/59] nit --- network/p2p/gossip/gossip.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 44ddb3b3143..0d36b7e0693 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -280,10 +280,6 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.lock.Lock() defer p.lock.Unlock() - return p.gossip(ctx) -} - -func (p *PushGossiper[T]) gossip(ctx context.Context) error { if len(p.tracking) == 0 { return nil } From 927cff2afa8966580668163ad66368a658d7e6c6 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 13:38:24 -0500 Subject: [PATCH 33/59] nit move time logic out of the loop --- network/p2p/gossip/gossip.go | 12 +++++++----- network/p2p/gossip/gossip_test.go | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 0d36b7e0693..657d4442658 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -316,8 +316,12 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.tracking[gossipID] = now } - // Iterate over all issued gossipables (have been sent before) to fill - // any remaining space in gossip batch. + // Invariant: maxRegossipFrequency must be non-negative to prevent gossiping + // the same transaction multiple times in the same message. + maxLastGossipTimeToRegossip := now.Add(-p.maxRegossipFrequency) + + // Iterate over all issued gossipables (have been sent before) to fill any + // remaining space in gossip batch. for sentBytes < p.targetGossipSize { gossipable, ok := p.issued.PopLeft() if !ok { @@ -334,9 +338,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { // Ensure we don't attempt to send a gossipable too frequently. lastGossipTime := p.tracking[gossipID] - // Invariant: maxRegossipFrequency must be greater than 0 to prevent - // gossiping the same transaction multiple times in the same message. - if now.Sub(lastGossipTime) < p.maxRegossipFrequency { + if maxLastGossipTimeToRegossip.Before(lastGossipTime) { // Put the gossipable on the front of the queue to keep items sorted // by last issuance time. p.issued.PushLeft(gossipable) diff --git a/network/p2p/gossip/gossip_test.go b/network/p2p/gossip/gossip_test.go index 777948ed883..e8701b3f908 100644 --- a/network/p2p/gossip/gossip_test.go +++ b/network/p2p/gossip/gossip_test.go @@ -377,7 +377,7 @@ func TestPushGossiper(t *testing.T) { // Ensure that subsequent calls to `time.Now()` are sufficient // for regossip. - time.Sleep(regossipTime) + time.Sleep(regossipTime + time.Nanosecond) } }) } From 35fc54b9c0ef888f053c8477ce2652238e7f197a Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 13:46:30 -0500 Subject: [PATCH 34/59] fix comment --- vms/platformvm/network/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vms/platformvm/network/config.go b/vms/platformvm/network/config.go index 2fe878c64a8..af22c89f52a 100644 --- a/vms/platformvm/network/config.go +++ b/vms/platformvm/network/config.go @@ -33,8 +33,8 @@ type Config struct { // sent when pushing transactions and when responded to transaction pull // requests. TargetGossipSize int `json:"target-gossip-size"` - // PushGossipFrequency is how frequently rounds of push gossip are - // performed. + // PushGossipDiscardedCacheSize is the number of txIDs to cache to avoid + // pushing transactions that were recently dropped from the mempool. PushGossipDiscardedCacheSize int `json:"push-gossip-discarded-cache-size"` // PushGossipMaxRegossipFrequency is the limit for how frequently a // transaction will be push gossiped. From 6cbd5b4be2dda55b07add2f65d801e8bad4f9692 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 14:04:28 -0500 Subject: [PATCH 35/59] finalize P-chain push gossip --- vms/platformvm/block/builder/builder_test.go | 8 ++--- vms/platformvm/network/network.go | 35 +++++++++++--------- vms/platformvm/network/network_test.go | 2 +- vms/platformvm/service_test.go | 4 +-- vms/platformvm/vm.go | 2 +- 5 files changed, 27 insertions(+), 24 deletions(-) diff --git a/vms/platformvm/block/builder/builder_test.go b/vms/platformvm/block/builder/builder_test.go index 8471b4fbed1..001f326ce20 100644 --- a/vms/platformvm/block/builder/builder_test.go +++ b/vms/platformvm/block/builder/builder_test.go @@ -52,7 +52,7 @@ func TestBuildBlockBasic(t *testing.T) { // Issue the transaction env.ctx.Lock.Unlock() - require.NoError(env.network.IssueTx(tx)) + require.NoError(env.network.IssueTxFromRPC(tx)) env.ctx.Lock.Lock() _, ok := env.mempool.Get(txID) require.True(ok) @@ -126,7 +126,7 @@ func TestBuildBlockShouldReward(t *testing.T) { // Issue the transaction env.ctx.Lock.Unlock() - require.NoError(env.network.IssueTx(tx)) + require.NoError(env.network.IssueTxFromRPC(tx)) env.ctx.Lock.Lock() _, ok := env.mempool.Get(txID) require.True(ok) @@ -247,7 +247,7 @@ func TestBuildBlockForceAdvanceTime(t *testing.T) { // Issue the transaction env.ctx.Lock.Unlock() - require.NoError(env.network.IssueTx(tx)) + require.NoError(env.network.IssueTxFromRPC(tx)) env.ctx.Lock.Lock() _, ok := env.mempool.Get(txID) require.True(ok) @@ -504,7 +504,7 @@ func TestPreviouslyDroppedTxsCannotBeReAddedToMempool(t *testing.T) { // Issue the transaction env.ctx.Lock.Unlock() - err = env.network.IssueTx(tx) + err = env.network.IssueTxFromRPC(tx) require.ErrorIs(err, errTestingDropped) env.ctx.Lock.Lock() _, ok := env.mempool.Get(txID) diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index 7ddbc5f5634..bb80980bfbf 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -5,6 +5,7 @@ package network import ( "context" + "errors" "time" "github.com/prometheus/client_golang/prometheus" @@ -23,6 +24,8 @@ import ( const TxGossipHandlerID = 0 +var errMempoolDisabledWithPartialSync = errors.New("mempool is disabled partial syncing") + type Network interface { common.AppHandler @@ -32,9 +35,11 @@ type Network interface { // PullGossip starts pull gossiping transactions and blocks until it // completes. PullGossip(ctx context.Context) - // IssueTx verifies the transaction at the currently preferred state and - // adds it to the mempool. - IssueTx(*txs.Tx) error + // IssueTxFromRPC issues a transaction that was received from the local RPC. + // The transaction will be verified against the currently preferred state. + // If transaction verification passes, it will be added to the mempool and + // periodically pushed to validators until it is removed from the mempool. + IssueTxFromRPC(*txs.Tx) error } type network struct { @@ -226,35 +231,33 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b return nil } - return n.issueTx(tx) + _ = n.addTxToMempool(tx) + return nil } -// TODO: naming here is confusing (should be clearer about which functions invoke push gossip if mempool addition is successful) -// TODO: issueTx doesn't add the tx to the mempool if partial sync is enabled, so the push gossiper will always drop the tx before broadcasting it -func (n *network) IssueTx(tx *txs.Tx) error { - if err := n.issueTx(tx); err != nil { +func (n *network) IssueTxFromRPC(tx *txs.Tx) error { + // TODO: We should still push the transaction to some peers when partial + // syncing. + if err := n.addTxToMempool(tx); err != nil { return err } n.txPushGossiper.Add(tx) return nil } -// returns nil if the tx is in the mempool -func (n *network) issueTx(tx *txs.Tx) error { +func (n *network) addTxToMempool(tx *txs.Tx) error { // If we are partially syncing the Primary Network, we should not be // maintaining the transaction mempool locally. if n.partialSyncPrimaryNetwork { - return nil + return errMempoolDisabledWithPartialSync } - if err := n.mempool.Add(tx); err != nil { + err := n.mempool.Add(tx) + if err != nil { n.log.Debug("tx failed to be added to the mempool", zap.Stringer("txID", tx.ID()), zap.Error(err), ) - - return err } - - return nil + return err } diff --git a/vms/platformvm/network/network_test.go b/vms/platformvm/network/network_test.go index dbe39afa3c2..ac298e5c42f 100644 --- a/vms/platformvm/network/network_test.go +++ b/vms/platformvm/network/network_test.go @@ -329,7 +329,7 @@ func TestNetworkIssueTx(t *testing.T) { ) require.NoError(err) - err = n.IssueTx(tx) + err = n.IssueTxFromRPC(tx) require.ErrorIs(err, tt.expectedErr) }) } diff --git a/vms/platformvm/service_test.go b/vms/platformvm/service_test.go index 41a9e8b2674..84ef2c60422 100644 --- a/vms/platformvm/service_test.go +++ b/vms/platformvm/service_test.go @@ -184,7 +184,7 @@ func TestGetTxStatus(t *testing.T) { require.Zero(resp.Reason) // put the chain in existing chain list - require.NoError(service.vm.Network.IssueTx(tx)) + require.NoError(service.vm.Network.IssueTxFromRPC(tx)) service.vm.ctx.Lock.Lock() block, err := service.vm.BuildBlock(context.Background()) @@ -285,7 +285,7 @@ func TestGetTx(t *testing.T) { err = service.GetTx(nil, arg, &response) require.ErrorIs(err, database.ErrNotFound) // We haven't issued the tx yet - require.NoError(service.vm.Network.IssueTx(tx)) + require.NoError(service.vm.Network.IssueTxFromRPC(tx)) service.vm.ctx.Lock.Lock() blk, err := service.vm.BuildBlock(context.Background()) diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go index d72e1cba25d..e08cf4f8509 100644 --- a/vms/platformvm/vm.go +++ b/vms/platformvm/vm.go @@ -543,7 +543,7 @@ func (vm *VM) GetBlockIDAtHeight(_ context.Context, height uint64) (ids.ID, erro } func (vm *VM) issueTx(tx *txs.Tx) error { - err := vm.Network.IssueTx(tx) + err := vm.Network.IssueTxFromRPC(tx) if err != nil && !errors.Is(err, mempool.ErrDuplicateTx) { vm.ctx.Log.Debug("failed to add tx to mempool", zap.Stringer("txID", tx.ID()), From e1da49fa3c04432da4faaff0162f09e8934e0791 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 14:05:30 -0500 Subject: [PATCH 36/59] nit default --- vms/platformvm/network/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vms/platformvm/network/config.go b/vms/platformvm/network/config.go index af22c89f52a..e20bc520c95 100644 --- a/vms/platformvm/network/config.go +++ b/vms/platformvm/network/config.go @@ -13,7 +13,7 @@ var DefaultConfig = Config{ MaxValidatorSetStaleness: time.Minute, TargetGossipSize: 20 * units.KiB, PushGossipDiscardedCacheSize: 1024, - PushGossipMaxRegossipFrequency: 2000 * time.Millisecond, + PushGossipMaxRegossipFrequency: 10 * time.Second, PushGossipFrequency: 500 * time.Millisecond, PullGossipPollSize: 1, PullGossipFrequency: 1500 * time.Millisecond, From b6167142114e5c1e770c1fe056361dd8649e3411 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 14:07:20 -0500 Subject: [PATCH 37/59] nit --- vms/platformvm/network/network.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index bb80980bfbf..a8374551e02 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -178,6 +178,12 @@ func New( } func (n *network) PushGossip(ctx context.Context) { + // TODO: Even though the node is running partial sync, we should support + // issuing transactions from the RPC. + if n.partialSyncPrimaryNetwork { + return + } + gossip.Every(ctx, n.log, n.txPushGossiper, n.txPushGossipFrequency) } From 92fdc35018c79eecfe28974123679c6dba4d0f2a Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 14:09:07 -0500 Subject: [PATCH 38/59] add comment --- vms/platformvm/network/network.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index a8374551e02..ff9130d53a0 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -237,6 +237,9 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b return nil } + // Returning an error here would result in shutting down the P-chain. + // Logging is already included inside addTxToMempool, so there's nothing to + // do with the returned error here. _ = n.addTxToMempool(tx) return nil } From c9620b9dd4bc98a56854d30b8c984ce7a8bd9568 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 14:12:37 -0500 Subject: [PATCH 39/59] rename method --- vms/platformvm/service.go | 2 +- vms/platformvm/validator_set_property_test.go | 4 +- vms/platformvm/vm.go | 2 +- vms/platformvm/vm_regression_test.go | 60 +++++++++---------- vms/platformvm/vm_test.go | 34 +++++------ 5 files changed, 51 insertions(+), 51 deletions(-) diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index fff7e4a2348..d0379bc2268 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -1415,7 +1415,7 @@ func (s *Service) IssueTx(req *http.Request, args *api.FormattedTx, response *ap return fmt.Errorf("couldn't parse tx: %w", err) } - if err := s.vm.issueTx(tx); err != nil { + if err := s.vm.issueTxFromRPC(tx); err != nil { return fmt.Errorf("couldn't issue tx: %w", err) } diff --git a/vms/platformvm/validator_set_property_test.go b/vms/platformvm/validator_set_property_test.go index 1b23c72bf40..abd08bc1866 100644 --- a/vms/platformvm/validator_set_property_test.go +++ b/vms/platformvm/validator_set_property_test.go @@ -298,7 +298,7 @@ func addPrimaryValidatorWithBLSKey(vm *VM, data *validatorInputData) (*state.Sta func internalAddValidator(vm *VM, signedTx *txs.Tx) (*state.Staker, error) { vm.ctx.Lock.Unlock() - err := vm.issueTx(signedTx) + err := vm.issueTxFromRPC(signedTx) vm.ctx.Lock.Lock() if err != nil { @@ -689,7 +689,7 @@ func buildVM(t *testing.T) (*VM, ids.ID, error) { return nil, ids.Empty, err } vm.ctx.Lock.Unlock() - err = vm.issueTx(testSubnet1) + err = vm.issueTxFromRPC(testSubnet1) vm.ctx.Lock.Lock() if err != nil { return nil, ids.Empty, err diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go index e08cf4f8509..7907cd6d881 100644 --- a/vms/platformvm/vm.go +++ b/vms/platformvm/vm.go @@ -542,7 +542,7 @@ func (vm *VM) GetBlockIDAtHeight(_ context.Context, height uint64) (ids.ID, erro return vm.state.GetBlockIDAtHeight(height) } -func (vm *VM) issueTx(tx *txs.Tx) error { +func (vm *VM) issueTxFromRPC(tx *txs.Tx) error { err := vm.Network.IssueTxFromRPC(tx) if err != nil && !errors.Is(err, mempool.ErrDuplicateTx) { vm.ctx.Log.Debug("failed to add tx to mempool", diff --git a/vms/platformvm/vm_regression_test.go b/vms/platformvm/vm_regression_test.go index 6909ee6952e..cae74f60293 100644 --- a/vms/platformvm/vm_regression_test.go +++ b/vms/platformvm/vm_regression_test.go @@ -77,7 +77,7 @@ func TestAddDelegatorTxOverDelegatedRegression(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addValidatorTx)) + require.NoError(vm.issueTxFromRPC(addValidatorTx)) vm.ctx.Lock.Lock() addValidatorBlock, err := vm.Builder.BuildBlock(context.Background()) @@ -112,7 +112,7 @@ func TestAddDelegatorTxOverDelegatedRegression(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addFirstDelegatorTx)) + require.NoError(vm.issueTxFromRPC(addFirstDelegatorTx)) vm.ctx.Lock.Lock() addFirstDelegatorBlock, err := vm.Builder.BuildBlock(context.Background()) @@ -149,7 +149,7 @@ func TestAddDelegatorTxOverDelegatedRegression(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addSecondDelegatorTx)) + require.NoError(vm.issueTxFromRPC(addSecondDelegatorTx)) vm.ctx.Lock.Lock() addSecondDelegatorBlock, err := vm.Builder.BuildBlock(context.Background()) @@ -176,7 +176,7 @@ func TestAddDelegatorTxOverDelegatedRegression(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - err = vm.issueTx(addThirdDelegatorTx) + err = vm.issueTxFromRPC(addThirdDelegatorTx) require.ErrorIs(err, executor.ErrOverDelegated) vm.ctx.Lock.Lock() } @@ -249,7 +249,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the add validator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addValidatorTx)) + require.NoError(vm.issueTxFromRPC(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -274,7 +274,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the first add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addFirstDelegatorTx)) + require.NoError(vm.issueTxFromRPC(addFirstDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the first add delegator tx @@ -299,7 +299,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the second add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addSecondDelegatorTx)) + require.NoError(vm.issueTxFromRPC(addSecondDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the second add delegator tx @@ -324,7 +324,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the third add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addThirdDelegatorTx)) + require.NoError(vm.issueTxFromRPC(addThirdDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the third add delegator tx @@ -349,7 +349,7 @@ func TestAddDelegatorTxHeapCorruption(t *testing.T) { // issue the fourth add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addFourthDelegatorTx)) + require.NoError(vm.issueTxFromRPC(addFourthDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the fourth add delegator tx @@ -1179,7 +1179,7 @@ func TestAddDelegatorTxAddBeforeRemove(t *testing.T) { // issue the add validator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addValidatorTx)) + require.NoError(vm.issueTxFromRPC(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1204,7 +1204,7 @@ func TestAddDelegatorTxAddBeforeRemove(t *testing.T) { // issue the first add delegator tx vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addFirstDelegatorTx)) + require.NoError(vm.issueTxFromRPC(addFirstDelegatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the first add delegator tx @@ -1230,7 +1230,7 @@ func TestAddDelegatorTxAddBeforeRemove(t *testing.T) { // attempting to issue the second add delegator tx should fail because the // total stake weight would go over the limit. vm.ctx.Lock.Unlock() - err = vm.issueTx(addSecondDelegatorTx) + err = vm.issueTxFromRPC(addSecondDelegatorTx) require.ErrorIs(err, executor.ErrOverDelegated) vm.ctx.Lock.Lock() } @@ -1266,7 +1266,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionNotTracked(t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addValidatorTx)) + require.NoError(vm.issueTxFromRPC(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1286,7 +1286,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionNotTracked(t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(createSubnetTx)) + require.NoError(vm.issueTxFromRPC(createSubnetTx)) vm.ctx.Lock.Lock() // trigger block creation for the subnet tx @@ -1309,7 +1309,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionNotTracked(t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addSubnetValidatorTx)) + require.NoError(vm.issueTxFromRPC(addSubnetValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1341,7 +1341,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionNotTracked(t vm.clock.Set(validatorStartTime) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(removeSubnetValidatorTx)) + require.NoError(vm.issueTxFromRPC(removeSubnetValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1391,7 +1391,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addValidatorTx)) + require.NoError(vm.issueTxFromRPC(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1411,7 +1411,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(createSubnetTx)) + require.NoError(vm.issueTxFromRPC(createSubnetTx)) vm.ctx.Lock.Lock() // trigger block creation for the subnet tx @@ -1434,7 +1434,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addSubnetValidatorTx)) + require.NoError(vm.issueTxFromRPC(addSubnetValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1458,7 +1458,7 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t vm.clock.Set(validatorStartTime) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(removeSubnetValidatorTx)) + require.NoError(vm.issueTxFromRPC(removeSubnetValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -1520,7 +1520,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { uPrimaryTx := primaryTx.Unsigned.(*txs.AddPermissionlessValidatorTx) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(primaryTx)) + require.NoError(vm.issueTxFromRPC(primaryTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1550,7 +1550,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(subnetTx)) + require.NoError(vm.issueTxFromRPC(subnetTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1626,7 +1626,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { uPrimaryRestartTx := primaryRestartTx.Unsigned.(*txs.AddPermissionlessValidatorTx) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(primaryRestartTx)) + require.NoError(vm.issueTxFromRPC(primaryRestartTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1733,7 +1733,7 @@ func TestPrimaryNetworkValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(primaryTx1)) + require.NoError(vm.issueTxFromRPC(primaryTx1)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1795,7 +1795,7 @@ func TestPrimaryNetworkValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(primaryRestartTx)) + require.NoError(vm.issueTxFromRPC(primaryRestartTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1866,7 +1866,7 @@ func TestSubnetValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(primaryTx1)) + require.NoError(vm.issueTxFromRPC(primaryTx1)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1896,7 +1896,7 @@ func TestSubnetValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(subnetTx)) + require.NoError(vm.issueTxFromRPC(subnetTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -1970,7 +1970,7 @@ func TestSubnetValidatorPopulatedToEmptyBLSKeyDiff(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(primaryRestartTx)) + require.NoError(vm.issueTxFromRPC(primaryRestartTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -2048,7 +2048,7 @@ func TestSubnetValidatorSetAfterPrimaryNetworkValidatorRemoval(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(primaryTx1)) + require.NoError(vm.issueTxFromRPC(primaryTx1)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) @@ -2075,7 +2075,7 @@ func TestSubnetValidatorSetAfterPrimaryNetworkValidatorRemoval(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(subnetTx)) + require.NoError(vm.issueTxFromRPC(subnetTx)) vm.ctx.Lock.Lock() require.NoError(buildAndAcceptStandardBlock(vm)) diff --git a/vms/platformvm/vm_test.go b/vms/platformvm/vm_test.go index 423ba9e8d0c..58bb2f92910 100644 --- a/vms/platformvm/vm_test.go +++ b/vms/platformvm/vm_test.go @@ -310,7 +310,7 @@ func defaultVM(t *testing.T, f fork) (*VM, database.Database, *mutableSharedMemo ) require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(testSubnet1)) + require.NoError(vm.issueTxFromRPC(testSubnet1)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) require.NoError(err) @@ -415,7 +415,7 @@ func TestAddValidatorCommit(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(tx)) + require.NoError(vm.issueTxFromRPC(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -514,7 +514,7 @@ func TestAddValidatorReject(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(tx)) + require.NoError(vm.issueTxFromRPC(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -563,7 +563,7 @@ func TestAddValidatorInvalidNotReissued(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - err = vm.issueTx(tx) + err = vm.issueTxFromRPC(tx) vm.ctx.Lock.Lock() require.ErrorIs(err, txexecutor.ErrDuplicateValidator) } @@ -598,7 +598,7 @@ func TestAddSubnetValidatorAccept(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(tx)) + require.NoError(vm.issueTxFromRPC(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -646,7 +646,7 @@ func TestAddSubnetValidatorReject(t *testing.T) { // trigger block creation vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(tx)) + require.NoError(vm.issueTxFromRPC(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -832,7 +832,7 @@ func TestCreateChain(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(tx)) + require.NoError(vm.issueTxFromRPC(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -883,7 +883,7 @@ func TestCreateSubnet(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(createSubnetTx)) + require.NoError(vm.issueTxFromRPC(createSubnetTx)) vm.ctx.Lock.Lock() // should contain the CreateSubnetTx @@ -927,7 +927,7 @@ func TestCreateSubnet(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addValidatorTx)) + require.NoError(vm.issueTxFromRPC(addValidatorTx)) vm.ctx.Lock.Lock() blk, err = vm.Builder.BuildBlock(context.Background()) // should add validator to the new subnet @@ -1031,7 +1031,7 @@ func TestAtomicImport(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(tx)) + require.NoError(vm.issueTxFromRPC(tx)) vm.ctx.Lock.Lock() blk, err := vm.Builder.BuildBlock(context.Background()) @@ -2030,7 +2030,7 @@ func TestRemovePermissionedValidatorDuringAddPending(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addValidatorTx)) + require.NoError(vm.issueTxFromRPC(addValidatorTx)) vm.ctx.Lock.Lock() // trigger block creation for the validator tx @@ -2050,7 +2050,7 @@ func TestRemovePermissionedValidatorDuringAddPending(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(createSubnetTx)) + require.NoError(vm.issueTxFromRPC(createSubnetTx)) vm.ctx.Lock.Lock() // trigger block creation for the subnet tx @@ -2121,7 +2121,7 @@ func TestTransferSubnetOwnershipTx(t *testing.T) { subnetID := createSubnetTx.ID() vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(createSubnetTx)) + require.NoError(vm.issueTxFromRPC(createSubnetTx)) vm.ctx.Lock.Lock() createSubnetBlock, err := vm.Builder.BuildBlock(context.Background()) require.NoError(err) @@ -2156,7 +2156,7 @@ func TestTransferSubnetOwnershipTx(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(transferSubnetOwnershipTx)) + require.NoError(vm.issueTxFromRPC(transferSubnetOwnershipTx)) vm.ctx.Lock.Lock() transferSubnetOwnershipBlock, err := vm.Builder.BuildBlock(context.Background()) require.NoError(err) @@ -2242,7 +2242,7 @@ func TestBaseTx(t *testing.T) { require.Equal(sendAmt, key1OutputAmt) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(baseTx)) + require.NoError(vm.issueTxFromRPC(baseTx)) vm.ctx.Lock.Lock() baseTxBlock, err := vm.Builder.BuildBlock(context.Background()) require.NoError(err) @@ -2281,7 +2281,7 @@ func TestPruneMempool(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(baseTx)) + require.NoError(vm.issueTxFromRPC(baseTx)) vm.ctx.Lock.Lock() // [baseTx] should be in the mempool. @@ -2313,7 +2313,7 @@ func TestPruneMempool(t *testing.T) { require.NoError(err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTx(addValidatorTx)) + require.NoError(vm.issueTxFromRPC(addValidatorTx)) vm.ctx.Lock.Lock() // Advance clock to [endTime], making [addValidatorTx] invalid. From 6ce907bca397721720ac7bb1caac2d3ddcbcfeb7 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 14:57:43 -0500 Subject: [PATCH 40/59] implement avm --- network/p2p/gossip/gossip.go | 5 + vms/avm/config_test.go | 4 +- vms/avm/environment_test.go | 2 +- vms/avm/network/config.go | 21 ++-- vms/avm/network/gossip.go | 5 +- vms/avm/network/gossip_test.go | 2 +- vms/avm/network/network.go | 113 +++++------------- vms/avm/network/network_test.go | 77 +++--------- vms/avm/service.go | 18 +-- vms/avm/vm.go | 18 ++- vms/avm/wallet_service.go | 5 +- .../config/execution_config_test.go | 5 +- vms/platformvm/network/config.go | 7 -- vms/platformvm/network/network.go | 9 +- vms/platformvm/network/network_test.go | 58 +++------ 15 files changed, 119 insertions(+), 230 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 657d4442658..e1ce6101723 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -359,6 +359,11 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { } p.metrics.tracking.Set(float64(len(p.tracking))) + // If there is nothing to gossip, we can exit early. + if len(gossip) == 0 { + return nil + } + // Send gossipables to peers msgBytes, err := MarshalAppGossip(gossip) if err != nil { diff --git a/vms/avm/config_test.go b/vms/avm/config_test.go index 27481d78b90..748ab8ae325 100644 --- a/vms/avm/config_test.go +++ b/vms/avm/config_test.go @@ -40,6 +40,9 @@ func TestParseConfig(t *testing.T) { Network: network.Config{ MaxValidatorSetStaleness: time.Nanosecond, TargetGossipSize: network.DefaultConfig.TargetGossipSize, + PushGossipDiscardedCacheSize: network.DefaultConfig.PushGossipDiscardedCacheSize, + PushGossipMaxRegossipFrequency: network.DefaultConfig.PushGossipMaxRegossipFrequency, + PushGossipFrequency: network.DefaultConfig.PushGossipFrequency, PullGossipPollSize: network.DefaultConfig.PullGossipPollSize, PullGossipFrequency: network.DefaultConfig.PullGossipFrequency, PullGossipThrottlingPeriod: network.DefaultConfig.PullGossipThrottlingPeriod, @@ -47,7 +50,6 @@ func TestParseConfig(t *testing.T) { ExpectedBloomFilterElements: network.DefaultConfig.ExpectedBloomFilterElements, ExpectedBloomFilterFalsePositiveProbability: network.DefaultConfig.ExpectedBloomFilterFalsePositiveProbability, MaxBloomFilterFalsePositiveProbability: network.DefaultConfig.MaxBloomFilterFalsePositiveProbability, - LegacyPushGossipCacheSize: network.DefaultConfig.LegacyPushGossipCacheSize, }, IndexTransactions: DefaultConfig.IndexTransactions, IndexAllowIncomplete: DefaultConfig.IndexAllowIncomplete, diff --git a/vms/avm/environment_test.go b/vms/avm/environment_test.go index 7b8ec890207..92675ef96de 100644 --- a/vms/avm/environment_test.go +++ b/vms/avm/environment_test.go @@ -489,7 +489,7 @@ func issueAndAccept( issuer <-chan common.Message, tx *txs.Tx, ) { - txID, err := vm.issueTx(tx) + txID, err := vm.issueTxFromRPC(tx) require.NoError(err) require.Equal(tx.ID(), txID) diff --git a/vms/avm/network/config.go b/vms/avm/network/config.go index 8536504d838..3e8c6ef6673 100644 --- a/vms/avm/network/config.go +++ b/vms/avm/network/config.go @@ -12,6 +12,9 @@ import ( var DefaultConfig = Config{ MaxValidatorSetStaleness: time.Minute, TargetGossipSize: 20 * units.KiB, + PushGossipDiscardedCacheSize: 1024, + PushGossipMaxRegossipFrequency: 10 * time.Second, + PushGossipFrequency: 500 * time.Millisecond, PullGossipPollSize: 1, PullGossipFrequency: 1500 * time.Millisecond, PullGossipThrottlingPeriod: 10 * time.Second, @@ -19,7 +22,6 @@ var DefaultConfig = Config{ ExpectedBloomFilterElements: 8 * 1024, ExpectedBloomFilterFalsePositiveProbability: .01, MaxBloomFilterFalsePositiveProbability: .05, - LegacyPushGossipCacheSize: 512, } type Config struct { @@ -30,6 +32,17 @@ type Config struct { // sent when pushing transactions and when responded to transaction pull // requests. TargetGossipSize int `json:"target-gossip-size"` + // PushGossipDiscardedCacheSize is the number of txIDs to cache to avoid + // pushing transactions that were recently dropped from the mempool. + PushGossipDiscardedCacheSize int `json:"push-gossip-discarded-cache-size"` + // PushGossipMaxRegossipFrequency is the limit for how frequently a + // transaction will be push gossiped. + PushGossipMaxRegossipFrequency time.Duration `json:"push-gossip-max-regossip-frequency"` + // PushGossipFrequency is how frequently rounds of push gossip are + // performed. + PushGossipFrequency time.Duration `json:"push-gossip-frequency"` + // PullGossipPollSize is the number of validators to sample when performing + // a round of pull gossip. // PullGossipPollSize is the number of validators to sample when performing // a round of pull gossip. PullGossipPollSize int `json:"pull-gossip-poll-size"` @@ -57,10 +70,4 @@ type Config struct { // The smaller this number is, the more frequently that the bloom filter // will be regenerated. MaxBloomFilterFalsePositiveProbability float64 `json:"max-bloom-filter-false-positive-probability"` - // LegacyPushGossipCacheSize tracks the most recently received transactions - // and ensures to only gossip them once. - // - // Deprecated: The legacy push gossip mechanism is deprecated in favor of - // the p2p SDK's push gossip mechanism. - LegacyPushGossipCacheSize int `json:"legacy-push-gossip-cache-size"` } diff --git a/vms/avm/network/gossip.go b/vms/avm/network/gossip.go index 1a13eb58b6f..2d3ab40bf7b 100644 --- a/vms/avm/network/gossip.go +++ b/vms/avm/network/gossip.go @@ -119,14 +119,15 @@ func (g *gossipMempool) Add(tx *txs.Tx) error { return err } - return g.AddVerified(tx) + return g.AddWithoutVerification(tx) } + func (g *gossipMempool) Has(txID ids.ID) bool { _, ok := g.Mempool.Get(txID) return ok } -func (g *gossipMempool) AddVerified(tx *txs.Tx) error { +func (g *gossipMempool) AddWithoutVerification(tx *txs.Tx) error { if err := g.Mempool.Add(tx); err != nil { g.Mempool.MarkDropped(tx.ID(), err) return err diff --git a/vms/avm/network/gossip_test.go b/vms/avm/network/gossip_test.go index 0a19dccc1d7..afd08920f1c 100644 --- a/vms/avm/network/gossip_test.go +++ b/vms/avm/network/gossip_test.go @@ -128,6 +128,6 @@ func TestGossipMempoolAddVerified(t *testing.T) { TxID: ids.GenerateTestID(), } - require.NoError(mempool.AddVerified(tx)) + require.NoError(mempool.AddWithoutVerification(tx)) require.True(mempool.bloom.Has(tx)) } diff --git a/vms/avm/network/network.go b/vms/avm/network/network.go index 9cad3cb9aa6..fbba1051479 100644 --- a/vms/avm/network/network.go +++ b/vms/avm/network/network.go @@ -5,13 +5,11 @@ package network import ( "context" - "sync" "time" "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" - "github.com/ava-labs/avalanchego/cache" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/network/p2p" "github.com/ava-labs/avalanchego/network/p2p/gossip" @@ -33,7 +31,8 @@ var ( type Network struct { *p2p.Network - txPushGossiper gossip.Accumulator[*txs.Tx] + txPushGossiper *gossip.PushGossiper[*txs.Tx] + txPushGossipFrequency time.Duration txPullGossiper gossip.Gossiper txPullGossipFrequency time.Duration @@ -41,10 +40,6 @@ type Network struct { parser txs.Parser mempool *gossipMempool appSender common.AppSender - - // gossip related attributes - recentTxsLock sync.Mutex - recentTxs *cache.LRU[ids.ID, struct{}] } func New( @@ -80,13 +75,6 @@ func New( return nil, err } - txPushGossiper := gossip.NewPushGossiper[*txs.Tx]( - marshaller, - txGossipClient, - txGossipMetrics, - config.TargetGossipSize, - ) - gossipMempool, err := newGossipMempool( mempool, registerer, @@ -101,8 +89,17 @@ func New( return nil, err } - var txPullGossiper gossip.Gossiper - txPullGossiper = gossip.NewPullGossiper[*txs.Tx]( + txPushGossiper := gossip.NewPushGossiper[*txs.Tx]( + marshaller, + gossipMempool, + txGossipClient, + txGossipMetrics, + config.PushGossipDiscardedCacheSize, + config.TargetGossipSize, + config.PushGossipMaxRegossipFrequency, + ) + + var txPullGossiper gossip.Gossiper = gossip.NewPullGossiper[*txs.Tx]( ctx.Log, marshaller, gossipMempool, @@ -121,7 +118,6 @@ func New( handler := gossip.NewHandler[*txs.Tx]( ctx.Log, marshaller, - txPushGossiper, gossipMempool, txGossipMetrics, config.TargetGossipSize, @@ -154,20 +150,21 @@ func New( return &Network{ Network: p2pNetwork, txPushGossiper: txPushGossiper, + txPushGossipFrequency: config.PushGossipFrequency, txPullGossiper: txPullGossiper, txPullGossipFrequency: config.PullGossipFrequency, ctx: ctx, parser: parser, mempool: gossipMempool, appSender: appSender, - - recentTxs: &cache.LRU[ids.ID, struct{}]{ - Size: config.LegacyPushGossipCacheSize, - }, }, nil } -func (n *Network) Gossip(ctx context.Context) { +func (n *Network) PushGossip(ctx context.Context) { + gossip.Every(ctx, n.ctx.Log, n.txPushGossiper, n.txPushGossipFrequency) +} + +func (n *Network) PullGossip(ctx context.Context) { gossip.Every(ctx, n.ctx.Log, n.txPullGossiper, n.txPullGossipFrequency) } @@ -204,16 +201,11 @@ func (n *Network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b return nil } - if err := n.mempool.Add(tx); err == nil { - txID := tx.ID() - n.txPushGossiper.Add(tx) - if err := n.txPushGossiper.Gossip(ctx); err != nil { - n.ctx.Log.Error("failed to gossip tx", - zap.Stringer("txID", tx.ID()), - zap.Error(err), - ) - } - n.gossipTxMessage(ctx, txID, msgBytes) + if err := n.mempool.Add(tx); err != nil { + n.ctx.Log.Debug("tx failed to be added to the mempool", + zap.Stringer("txID", tx.ID()), + zap.Error(err), + ) } return nil } @@ -225,11 +217,12 @@ func (n *Network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b // If the tx is already in the mempool, mempool.ErrDuplicateTx will be // returned. // If the tx is not added to the mempool, an error will be returned. -func (n *Network) IssueTx(ctx context.Context, tx *txs.Tx) error { +func (n *Network) IssueTxFromRPC(tx *txs.Tx) error { if err := n.mempool.Add(tx); err != nil { return err } - return n.gossipTx(ctx, tx) + n.txPushGossiper.Add(tx) + return nil } // IssueVerifiedTx attempts to add a tx to the mempool, without first verifying @@ -239,58 +232,10 @@ func (n *Network) IssueTx(ctx context.Context, tx *txs.Tx) error { // If the tx is already in the mempool, mempool.ErrDuplicateTx will be // returned. // If the tx is not added to the mempool, an error will be returned. -func (n *Network) IssueVerifiedTx(ctx context.Context, tx *txs.Tx) error { - if err := n.mempool.AddVerified(tx); err != nil { +func (n *Network) IssueTxFromRPCWithoutVerification(tx *txs.Tx) error { + if err := n.mempool.AddWithoutVerification(tx); err != nil { return err } - return n.gossipTx(ctx, tx) -} - -// gossipTx pushes the tx to peers using both the legacy and p2p SDK. -func (n *Network) gossipTx(ctx context.Context, tx *txs.Tx) error { n.txPushGossiper.Add(tx) - if err := n.txPushGossiper.Gossip(ctx); err != nil { - n.ctx.Log.Error("failed to gossip tx", - zap.Stringer("txID", tx.ID()), - zap.Error(err), - ) - } - - txBytes := tx.Bytes() - msg := &message.Tx{ - Tx: txBytes, - } - msgBytes, err := message.Build(msg) - if err != nil { - return err - } - - txID := tx.ID() - n.gossipTxMessage(ctx, txID, msgBytes) return nil } - -// gossipTxMessage pushes the tx message to peers using the legacy format. -// If the tx was recently gossiped, this function does nothing. -func (n *Network) gossipTxMessage(ctx context.Context, txID ids.ID, msgBytes []byte) { - n.recentTxsLock.Lock() - _, has := n.recentTxs.Get(txID) - n.recentTxs.Put(txID, struct{}{}) - n.recentTxsLock.Unlock() - - // Don't gossip a transaction if it has been recently gossiped. - if has { - return - } - - n.ctx.Log.Debug("gossiping tx", - zap.Stringer("txID", txID), - ) - - if err := n.appSender.SendAppGossip(ctx, msgBytes); err != nil { - n.ctx.Log.Error("failed to gossip tx", - zap.Stringer("txID", txID), - zap.Error(err), - ) - } -} diff --git a/vms/avm/network/network_test.go b/vms/avm/network/network_test.go index 0e4ff2990b6..7eacad534c0 100644 --- a/vms/avm/network/network_test.go +++ b/vms/avm/network/network_test.go @@ -32,6 +32,9 @@ var ( testConfig = Config{ MaxValidatorSetStaleness: time.Second, TargetGossipSize: 1, + PushGossipDiscardedCacheSize: 1, + PushGossipMaxRegossipFrequency: time.Second, + PushGossipFrequency: time.Second, PullGossipPollSize: 1, PullGossipFrequency: time.Second, PullGossipThrottlingPeriod: time.Second, @@ -39,7 +42,6 @@ var ( ExpectedBloomFilterElements: 10, ExpectedBloomFilterFalsePositiveProbability: .1, MaxBloomFilterFalsePositiveProbability: .5, - LegacyPushGossipCacheSize: 512, } errTest = errors.New("test error") @@ -71,7 +73,6 @@ func TestNetworkAppGossip(t *testing.T) { msgBytesFunc func() []byte mempoolFunc func(*gomock.Controller) mempool.Mempool txVerifierFunc func(*gomock.Controller) TxVerifier - appSenderFunc func(*gomock.Controller) common.AppSender } tests := []test{ @@ -172,11 +173,6 @@ func TestNetworkAppGossip(t *testing.T) { txVerifier.EXPECT().VerifyTx(gomock.Any()).Return(nil) return txVerifier }, - appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { - appSender := common.NewMockSender(ctrl) - appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Return(nil).Times(2) - return appSender - }, }, } @@ -209,13 +205,6 @@ func TestNetworkAppGossip(t *testing.T) { txVerifierFunc = tt.txVerifierFunc } - appSenderFunc := func(ctrl *gomock.Controller) common.AppSender { - return common.NewMockSender(ctrl) - } - if tt.appSenderFunc != nil { - appSenderFunc = tt.appSenderFunc - } - n, err := New( &snow.Context{ Log: logging.NoLog{}, @@ -223,7 +212,7 @@ func TestNetworkAppGossip(t *testing.T) { parser, txVerifierFunc(ctrl), mempoolFunc(ctrl), - appSenderFunc(ctrl), + common.NewMockSender(ctrl), prometheus.NewRegistry(), testConfig, ) @@ -233,7 +222,7 @@ func TestNetworkAppGossip(t *testing.T) { } } -func TestNetworkIssueTx(t *testing.T) { +func TestNetworkIssueTxFromRPC(t *testing.T) { type test struct { name string mempoolFunc func(*gomock.Controller) mempool.Mempool @@ -304,6 +293,7 @@ func TestNetworkIssueTx(t *testing.T) { mempool.EXPECT().Add(gomock.Any()).Return(nil) mempool.EXPECT().Len().Return(0) mempool.EXPECT().RequestBuildBlock() + mempool.EXPECT().Get(gomock.Any()).Return(nil, true).Times(2) return mempool }, txVerifierFunc: func(ctrl *gomock.Controller) TxVerifier { @@ -313,7 +303,7 @@ func TestNetworkIssueTx(t *testing.T) { }, appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { appSender := common.NewMockSender(ctrl) - appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Return(nil).Times(2) + appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Return(nil) return appSender }, expectedErr: nil, @@ -368,13 +358,15 @@ func TestNetworkIssueTx(t *testing.T) { testConfig, ) require.NoError(err) - err = n.IssueTx(context.Background(), &txs.Tx{}) + err = n.IssueTxFromRPC(&txs.Tx{}) require.ErrorIs(err, tt.expectedErr) + + require.NoError(n.txPushGossiper.Gossip(context.Background())) }) } } -func TestNetworkIssueVerifiedTx(t *testing.T) { +func TestNetworkIssueTxFromRPCWithoutVerification(t *testing.T) { type test struct { name string mempoolFunc func(*gomock.Controller) mempool.Mempool @@ -397,6 +389,7 @@ func TestNetworkIssueVerifiedTx(t *testing.T) { name: "happy path", mempoolFunc: func(ctrl *gomock.Controller) mempool.Mempool { mempool := mempool.NewMockMempool(ctrl) + mempool.EXPECT().Get(gomock.Any()).Return(nil, true).Times(2) mempool.EXPECT().Add(gomock.Any()).Return(nil) mempool.EXPECT().Len().Return(0) mempool.EXPECT().RequestBuildBlock() @@ -404,7 +397,7 @@ func TestNetworkIssueVerifiedTx(t *testing.T) { }, appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { appSender := common.NewMockSender(ctrl) - appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Return(nil).Times(2) + appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Return(nil) return appSender }, expectedErr: nil, @@ -452,48 +445,10 @@ func TestNetworkIssueVerifiedTx(t *testing.T) { testConfig, ) require.NoError(err) - err = n.IssueVerifiedTx(context.Background(), &txs.Tx{}) + err = n.IssueTxFromRPCWithoutVerification(&txs.Tx{}) require.ErrorIs(err, tt.expectedErr) + + require.NoError(n.txPushGossiper.Gossip(context.Background())) }) } } - -func TestNetworkGossipTx(t *testing.T) { - require := require.New(t) - ctrl := gomock.NewController(t) - - parser, err := txs.NewParser( - time.Time{}, - []fxs.Fx{ - &secp256k1fx.Fx{}, - }, - ) - require.NoError(err) - - appSender := common.NewMockSender(ctrl) - - n, err := New( - &snow.Context{ - Log: logging.NoLog{}, - }, - parser, - executor.NewMockManager(ctrl), - mempool.NewMockMempool(ctrl), - appSender, - prometheus.NewRegistry(), - testConfig, - ) - require.NoError(err) - - // Case: Tx was recently gossiped - txID := ids.GenerateTestID() - n.recentTxs.Put(txID, struct{}{}) - n.gossipTxMessage(context.Background(), txID, []byte{}) - // Didn't make a call to SendAppGossip - - // Case: Tx was not recently gossiped - msgBytes := []byte{1, 2, 3} - appSender.EXPECT().SendAppGossip(gomock.Any(), msgBytes).Return(nil) - n.gossipTxMessage(context.Background(), ids.GenerateTestID(), msgBytes) - // Did make a call to SendAppGossip -} diff --git a/vms/avm/service.go b/vms/avm/service.go index d66ee6976b8..8d6cbb0ce72 100644 --- a/vms/avm/service.go +++ b/vms/avm/service.go @@ -213,7 +213,7 @@ func (s *Service) IssueTx(_ *http.Request, args *api.FormattedTx, reply *api.JSO return err } - reply.TxID, err = s.vm.issueTx(tx) + reply.TxID, err = s.vm.issueTxFromRPC(tx) // TODO: enqueue push gossip return err @@ -716,7 +716,7 @@ func (s *Service) CreateAsset(_ *http.Request, args *CreateAssetArgs, reply *Ass return err } - assetID, err := s.vm.issueTx(tx) + assetID, err := s.vm.issueTxFromRPC(tx) if err != nil { return fmt.Errorf("problem issuing transaction: %w", err) } @@ -881,7 +881,7 @@ func (s *Service) CreateNFTAsset(_ *http.Request, args *CreateNFTAssetArgs, repl return err } - assetID, err := s.vm.issueTx(tx) + assetID, err := s.vm.issueTxFromRPC(tx) if err != nil { return fmt.Errorf("problem issuing transaction: %w", err) } @@ -1201,7 +1201,7 @@ func (s *Service) SendMultiple(_ *http.Request, args *SendMultipleArgs, reply *a return err } - txID, err := s.vm.issueTx(tx) + txID, err := s.vm.issueTxFromRPC(tx) if err != nil { return fmt.Errorf("problem issuing transaction: %w", err) } @@ -1363,7 +1363,7 @@ func (s *Service) Mint(_ *http.Request, args *MintArgs, reply *api.JSONTxIDChang return err } - txID, err := s.vm.issueTx(tx) + txID, err := s.vm.issueTxFromRPC(tx) if err != nil { return fmt.Errorf("problem issuing transaction: %w", err) } @@ -1496,7 +1496,7 @@ func (s *Service) SendNFT(_ *http.Request, args *SendNFTArgs, reply *api.JSONTxI return err } - txID, err := s.vm.issueTx(tx) + txID, err := s.vm.issueTxFromRPC(tx) if err != nil { return fmt.Errorf("problem issuing transaction: %w", err) } @@ -1619,7 +1619,7 @@ func (s *Service) MintNFT(_ *http.Request, args *MintNFTArgs, reply *api.JSONTxI return err } - txID, err := s.vm.issueTx(tx) + txID, err := s.vm.issueTxFromRPC(tx) if err != nil { return fmt.Errorf("problem issuing transaction: %w", err) } @@ -1756,7 +1756,7 @@ func (s *Service) Import(_ *http.Request, args *ImportArgs, reply *api.JSONTxID) return err } - txID, err := s.vm.issueTx(tx) + txID, err := s.vm.issueTxFromRPC(tx) if err != nil { return fmt.Errorf("problem issuing transaction: %w", err) } @@ -1887,7 +1887,7 @@ func (s *Service) Export(_ *http.Request, args *ExportArgs, reply *api.JSONTxIDC return err } - txID, err := s.vm.issueTx(tx) + txID, err := s.vm.issueTxFromRPC(tx) if err != nil { return fmt.Errorf("problem issuing transaction: %w", err) } diff --git a/vms/avm/vm.go b/vms/avm/vm.go index 833bd6f79a4..f401398f0a2 100644 --- a/vms/avm/vm.go +++ b/vms/avm/vm.go @@ -459,12 +459,18 @@ func (vm *VM) Linearize(ctx context.Context, stopVertexID ids.ID, toEngine chan< // handled asynchronously. vm.Atomic.Set(vm.network) - vm.awaitShutdown.Add(1) + vm.awaitShutdown.Add(2) go func() { defer vm.awaitShutdown.Done() - // Invariant: Gossip must never grab the context lock. - vm.network.Gossip(vm.onShutdownCtx) + // Invariant: PushGossip must never grab the context lock. + vm.network.PushGossip(vm.onShutdownCtx) + }() + go func() { + defer vm.awaitShutdown.Done() + + // Invariant: PullGossip must never grab the context lock. + vm.network.PullGossip(vm.onShutdownCtx) }() go func() { @@ -507,13 +513,13 @@ func (vm *VM) ParseTx(_ context.Context, bytes []byte) (snowstorm.Tx, error) { ****************************************************************************** */ -// issueTx attempts to send a transaction to consensus. +// issueTxFromRPC attempts to send a transaction to consensus. // // Invariant: The context lock is not held // Invariant: This function is only called after Linearize has been called. -func (vm *VM) issueTx(tx *txs.Tx) (ids.ID, error) { +func (vm *VM) issueTxFromRPC(tx *txs.Tx) (ids.ID, error) { txID := tx.ID() - err := vm.network.IssueTx(context.TODO(), tx) + err := vm.network.IssueTxFromRPC(tx) if err != nil && !errors.Is(err, mempool.ErrDuplicateTx) { vm.ctx.Log.Debug("failed to add tx to mempool", zap.Stringer("txID", txID), diff --git a/vms/avm/wallet_service.go b/vms/avm/wallet_service.go index 321bf9e57eb..96b4cd40548 100644 --- a/vms/avm/wallet_service.go +++ b/vms/avm/wallet_service.go @@ -4,7 +4,6 @@ package avm import ( - "context" "errors" "fmt" "net/http" @@ -45,7 +44,7 @@ func (w *WalletService) decided(txID ids.ID) { return } - err := w.vm.network.IssueVerifiedTx(context.TODO(), tx) + err := w.vm.network.IssueTxFromRPCWithoutVerification(tx) if err == nil { w.vm.ctx.Log.Info("issued tx to mempool over wallet API", zap.Stringer("txID", txID), @@ -78,7 +77,7 @@ func (w *WalletService) issue(tx *txs.Tx) (ids.ID, error) { } if w.pendingTxs.Len() == 0 { - if err := w.vm.network.IssueVerifiedTx(context.TODO(), tx); err == nil { + if err := w.vm.network.IssueTxFromRPCWithoutVerification(tx); err == nil { w.vm.ctx.Log.Info("issued tx to mempool over wallet API", zap.Stringer("txID", txID), ) diff --git a/vms/platformvm/config/execution_config_test.go b/vms/platformvm/config/execution_config_test.go index 89fd5cd55b0..d80f2c67c82 100644 --- a/vms/platformvm/config/execution_config_test.go +++ b/vms/platformvm/config/execution_config_test.go @@ -78,7 +78,6 @@ func TestExecutionConfigUnmarshal(t *testing.T) { ExpectedBloomFilterElements: 7, ExpectedBloomFilterFalsePositiveProbability: 8, MaxBloomFilterFalsePositiveProbability: 9, - LegacyPushGossipCacheSize: 10, }, BlockCacheSize: 1, TxCacheSize: 2, @@ -120,6 +119,9 @@ func TestExecutionConfigUnmarshal(t *testing.T) { Network: network.Config{ MaxValidatorSetStaleness: 1, TargetGossipSize: 2, + PushGossipDiscardedCacheSize: DefaultExecutionConfig.Network.PushGossipDiscardedCacheSize, + PushGossipMaxRegossipFrequency: DefaultExecutionConfig.Network.PushGossipMaxRegossipFrequency, + PushGossipFrequency: DefaultExecutionConfig.Network.PushGossipFrequency, PullGossipPollSize: 3, PullGossipFrequency: 4, PullGossipThrottlingPeriod: 5, @@ -127,7 +129,6 @@ func TestExecutionConfigUnmarshal(t *testing.T) { ExpectedBloomFilterElements: DefaultExecutionConfig.Network.ExpectedBloomFilterElements, ExpectedBloomFilterFalsePositiveProbability: DefaultExecutionConfig.Network.ExpectedBloomFilterFalsePositiveProbability, MaxBloomFilterFalsePositiveProbability: DefaultExecutionConfig.Network.MaxBloomFilterFalsePositiveProbability, - LegacyPushGossipCacheSize: DefaultExecutionConfig.Network.LegacyPushGossipCacheSize, }, BlockCacheSize: 1, TxCacheSize: 2, diff --git a/vms/platformvm/network/config.go b/vms/platformvm/network/config.go index e20bc520c95..599d7a96263 100644 --- a/vms/platformvm/network/config.go +++ b/vms/platformvm/network/config.go @@ -22,7 +22,6 @@ var DefaultConfig = Config{ ExpectedBloomFilterElements: 8 * 1024, ExpectedBloomFilterFalsePositiveProbability: .01, MaxBloomFilterFalsePositiveProbability: .05, - LegacyPushGossipCacheSize: 512, } type Config struct { @@ -69,10 +68,4 @@ type Config struct { // The smaller this number is, the more frequently that the bloom filter // will be regenerated. MaxBloomFilterFalsePositiveProbability float64 `json:"max-bloom-filter-false-positive-probability"` - // LegacyPushGossipCacheSize tracks the most recently received transactions - // and ensures to only gossip them once. - // - // Deprecated: The legacy push gossip mechanism is deprecated in favor of - // the p2p SDK's push gossip mechanism. - LegacyPushGossipCacheSize int `json:"legacy-push-gossip-cache-size"` } diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index ff9130d53a0..c4d0268787d 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -114,8 +114,7 @@ func New( config.PushGossipMaxRegossipFrequency, ) - var txPullGossiper gossip.Gossiper - txPullGossiper = gossip.NewPullGossiper[*txs.Tx]( + var txPullGossiper gossip.Gossiper = gossip.NewPullGossiper[*txs.Tx]( log, marshaller, gossipMempool, @@ -237,9 +236,9 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b return nil } - // Returning an error here would result in shutting down the P-chain. - // Logging is already included inside addTxToMempool, so there's nothing to - // do with the returned error here. + // Returning an error here would result in shutting down the chain. Logging + // is already included inside addTxToMempool, so there's nothing to do with + // the returned error here. _ = n.addTxToMempool(tx) return nil } diff --git a/vms/platformvm/network/network_test.go b/vms/platformvm/network/network_test.go index ac298e5c42f..223a929f368 100644 --- a/vms/platformvm/network/network_test.go +++ b/vms/platformvm/network/network_test.go @@ -29,6 +29,9 @@ var ( testConfig = Config{ MaxValidatorSetStaleness: time.Second, TargetGossipSize: 1, + PushGossipDiscardedCacheSize: 1, + PushGossipMaxRegossipFrequency: time.Second, + PushGossipFrequency: time.Second, PullGossipPollSize: 1, PullGossipFrequency: time.Second, PullGossipThrottlingPeriod: time.Second, @@ -36,7 +39,6 @@ var ( ExpectedBloomFilterElements: 10, ExpectedBloomFilterFalsePositiveProbability: .1, MaxBloomFilterFalsePositiveProbability: .5, - LegacyPushGossipCacheSize: 512, } ) @@ -68,7 +70,6 @@ func TestNetworkAppGossip(t *testing.T) { msgBytesFunc func() []byte mempoolFunc func(*gomock.Controller) mempool.Mempool partialSyncPrimaryNetwork bool - appSenderFunc func(*gomock.Controller) common.AppSender } tests := []test{ @@ -81,9 +82,6 @@ func TestNetworkAppGossip(t *testing.T) { mempoolFunc: func(*gomock.Controller) mempool.Mempool { return nil }, - appSenderFunc: func(*gomock.Controller) common.AppSender { - return nil - }, }, { // Shouldn't attempt to issue or gossip the tx @@ -99,9 +97,6 @@ func TestNetworkAppGossip(t *testing.T) { mempoolFunc: func(ctrl *gomock.Controller) mempool.Mempool { return mempool.NewMockMempool(ctrl) }, - appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { - return common.NewMockSender(ctrl) - }, }, { name: "issuance succeeds", @@ -122,13 +117,6 @@ func TestNetworkAppGossip(t *testing.T) { mempool.EXPECT().RequestBuildBlock(false) return mempool }, - appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { - // we should gossip the tx twice because sdk and legacy gossip - // currently runs together - appSender := common.NewMockSender(ctrl) - appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Times(2) - return appSender - }, }, { // Issue returns error because tx was dropped. We shouldn't gossip the tx. @@ -147,12 +135,9 @@ func TestNetworkAppGossip(t *testing.T) { mempool.EXPECT().GetDropReason(gomock.Any()).Return(errTest) return mempool }, - appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { - return common.NewMockSender(ctrl) - }, }, { - name: "should AppGossip if primary network is not being fully synced", + name: "shouldn't AppGossip if primary network is not being fully synced", msgBytesFunc: func() []byte { msg := message.Tx{ Tx: testTx.Bytes(), @@ -162,16 +147,9 @@ func TestNetworkAppGossip(t *testing.T) { return msgBytes }, mempoolFunc: func(ctrl *gomock.Controller) mempool.Mempool { - mempool := mempool.NewMockMempool(ctrl) - // mempool.EXPECT().Has(gomock.Any()).Return(true) - return mempool + return mempool.NewMockMempool(ctrl) }, partialSyncPrimaryNetwork: true, - appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { - appSender := common.NewMockSender(ctrl) - // appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()) - return appSender - }, }, } @@ -190,7 +168,7 @@ func TestNetworkAppGossip(t *testing.T) { testTxVerifier{}, tt.mempoolFunc(ctrl), tt.partialSyncPrimaryNetwork, - tt.appSenderFunc(ctrl), + common.NewMockSender(ctrl), prometheus.NewRegistry(), DefaultConfig, ) @@ -201,7 +179,7 @@ func TestNetworkAppGossip(t *testing.T) { } } -func TestNetworkIssueTx(t *testing.T) { +func TestNetworkIssueTxFromRPC(t *testing.T) { tx := &txs.Tx{} type test struct { @@ -273,19 +251,15 @@ func TestNetworkIssueTx(t *testing.T) { expectedErr: errTest, }, { - name: "AppGossip tx but do not add to mempool if primary network is not being fully synced", + name: "mempool is disalbed if primary network is not being fully synced", mempoolFunc: func(ctrl *gomock.Controller) mempool.Mempool { return mempool.NewMockMempool(ctrl) }, partialSyncPrimaryNetwork: true, appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { - // we should gossip the tx twice because sdk and legacy gossip - // currently runs together - appSender := common.NewMockSender(ctrl) - appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Return(nil).Times(2) - return appSender + return common.NewMockSender(ctrl) }, - expectedErr: nil, + expectedErr: errMempoolDisabledWithPartialSync, }, { name: "happy path", @@ -296,13 +270,12 @@ func TestNetworkIssueTx(t *testing.T) { mempool.EXPECT().Add(gomock.Any()).Return(nil) mempool.EXPECT().Len().Return(0) mempool.EXPECT().RequestBuildBlock(false) + mempool.EXPECT().Get(gomock.Any()).Return(nil, true).Times(2) return mempool }, appSenderFunc: func(ctrl *gomock.Controller) common.AppSender { - // we should gossip the tx twice because sdk and legacy gossip - // currently runs together appSender := common.NewMockSender(ctrl) - appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Return(nil).Times(2) + appSender.EXPECT().SendAppGossip(gomock.Any(), gomock.Any()).Return(nil) return appSender }, expectedErr: nil, @@ -315,7 +288,7 @@ func TestNetworkIssueTx(t *testing.T) { ctrl := gomock.NewController(t) snowCtx := snowtest.Context(t, ids.Empty) - n, err := New( + nIntf, err := New( snowCtx.Log, snowCtx.NodeID, snowCtx.SubnetID, @@ -329,8 +302,11 @@ func TestNetworkIssueTx(t *testing.T) { ) require.NoError(err) - err = n.IssueTxFromRPC(tx) + err = nIntf.IssueTxFromRPC(tx) require.ErrorIs(err, tt.expectedErr) + + n := nIntf.(*network) + require.NoError(n.txPushGossiper.Gossip(context.Background())) }) } } From b603ee646ce9ac358a764073262988e452ba0a81 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:00:01 -0500 Subject: [PATCH 41/59] move param validation Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- network/p2p/gossip/gossip.go | 15 ++++++++++----- network/p2p/gossip/gossip_test.go | 8 +++++++- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index e1ce6101723..fdfc1523429 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -46,7 +46,8 @@ var ( typeLabel: pullType, } - errEmptySetCantAdd = errors.New("empty set can not add") + errEmptySetCantAdd = errors.New("empty set can not add") + ErrInvalidRegossipFrequency = errors.New("re-gossip frequency cannot be negative") ) // Gossiper gossips Gossipables to other nodes @@ -242,7 +243,13 @@ func NewPushGossiper[T Gossipable]( discardedSize int, targetGossipSize int, maxRegossipFrequency time.Duration, -) *PushGossiper[T] { +) (*PushGossiper[T], error) { + // maxRegossipFrequency must be non-negative to prevent gossiping + // the same transaction multiple times in the same message. + if maxRegossipFrequency < 0 { + return nil, ErrInvalidRegossipFrequency + } + return &PushGossiper[T]{ marshaller: marshaller, set: mempool, @@ -255,7 +262,7 @@ func NewPushGossiper[T Gossipable]( pending: buffer.NewUnboundedDeque[T](0), issued: buffer.NewUnboundedDeque[T](0), discarded: &cache.LRU[ids.ID, interface{}]{Size: discardedSize}, - } + }, nil } // PushGossiper broadcasts gossip to peers randomly in the network @@ -316,8 +323,6 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { p.tracking[gossipID] = now } - // Invariant: maxRegossipFrequency must be non-negative to prevent gossiping - // the same transaction multiple times in the same message. maxLastGossipTimeToRegossip := now.Add(-p.maxRegossipFrequency) // Iterate over all issued gossipables (have been sent before) to fill any diff --git a/network/p2p/gossip/gossip_test.go b/network/p2p/gossip/gossip_test.go index e8701b3f908..4ec146d5413 100644 --- a/network/p2p/gossip/gossip_test.go +++ b/network/p2p/gossip/gossip_test.go @@ -232,6 +232,11 @@ func TestValidatorGossiper(t *testing.T) { require.Equal(2, calls) } +func TestPushGossiperNew(t *testing.T) { + _, err := NewPushGossiper[*testTx](nil, nil, nil, Metrics{}, 0, 0, -1) + require.ErrorIs(t, err, ErrInvalidRegossipFrequency) +} + // Tests that the outgoing gossip is equivalent to what was accumulated func TestPushGossiper(t *testing.T) { type cycle struct { @@ -343,7 +348,7 @@ func TestPushGossiper(t *testing.T) { metrics, err := NewMetrics(prometheus.NewRegistry(), "") require.NoError(err) marshaller := testMarshaller{} - gossiper := NewPushGossiper[*testTx]( + gossiper, err := NewPushGossiper[*testTx]( marshaller, FullSet[*testTx]{}, client, @@ -352,6 +357,7 @@ func TestPushGossiper(t *testing.T) { units.MiB, regossipTime, ) + require.NoError(err) for _, cycle := range tt.cycles { gossiper.Add(cycle.toAdd...) From ad847dc5a5fcc756afa8221ac9dc4090bb374d00 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:07:01 -0500 Subject: [PATCH 42/59] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- network/p2p/gossip/gossip.go | 12 ++++++++-- network/p2p/gossip/gossip_test.go | 38 +++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index fdfc1523429..b835d5e1eae 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -47,6 +47,8 @@ var ( } errEmptySetCantAdd = errors.New("empty set can not add") + ErrInvalidDiscardedSize = errors.New("discarded size cannot be negative") + ErrInvalidTargetGossipSize = errors.New("target gossip size cannot be negative") ErrInvalidRegossipFrequency = errors.New("re-gossip frequency cannot be negative") ) @@ -244,8 +246,14 @@ func NewPushGossiper[T Gossipable]( targetGossipSize int, maxRegossipFrequency time.Duration, ) (*PushGossiper[T], error) { - // maxRegossipFrequency must be non-negative to prevent gossiping - // the same transaction multiple times in the same message. + if discardedSize < 0 { + return nil, ErrInvalidDiscardedSize + } + + if targetGossipSize < 0 { + return nil, ErrInvalidTargetGossipSize + } + if maxRegossipFrequency < 0 { return nil, ErrInvalidRegossipFrequency } diff --git a/network/p2p/gossip/gossip_test.go b/network/p2p/gossip/gossip_test.go index 4ec146d5413..c95f7e984a9 100644 --- a/network/p2p/gossip/gossip_test.go +++ b/network/p2p/gossip/gossip_test.go @@ -233,8 +233,42 @@ func TestValidatorGossiper(t *testing.T) { } func TestPushGossiperNew(t *testing.T) { - _, err := NewPushGossiper[*testTx](nil, nil, nil, Metrics{}, 0, 0, -1) - require.ErrorIs(t, err, ErrInvalidRegossipFrequency) + tests := []struct { + name string + discardedSize int + targetGossipSize int + maxRegossipFrequency time.Duration + expected error + }{ + { + name: "invalid discarded size", + discardedSize: -1, + expected: ErrInvalidDiscardedSize, + }, + { + name: "invalid target gossip size", + targetGossipSize: -1, + expected: ErrInvalidTargetGossipSize, + }, + { + name: "invalid max re-gossip frequency", + maxRegossipFrequency: -1, + expected: ErrInvalidRegossipFrequency, + }, + } + + for _, tt := range tests { + _, err := NewPushGossiper[*testTx]( + nil, + nil, + nil, + Metrics{}, + tt.discardedSize, + tt.targetGossipSize, + tt.maxRegossipFrequency, + ) + require.ErrorIs(t, err, tt.expected) + } } // Tests that the outgoing gossip is equivalent to what was accumulated From fe14e10f8fb4e4a4b08abeb1601212054cc7f101 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:08:31 -0500 Subject: [PATCH 43/59] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- vms/avm/network/network.go | 5 ++++- vms/platformvm/network/network.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/vms/avm/network/network.go b/vms/avm/network/network.go index fbba1051479..3ed5524e68e 100644 --- a/vms/avm/network/network.go +++ b/vms/avm/network/network.go @@ -89,7 +89,7 @@ func New( return nil, err } - txPushGossiper := gossip.NewPushGossiper[*txs.Tx]( + txPushGossiper, err := gossip.NewPushGossiper[*txs.Tx]( marshaller, gossipMempool, txGossipClient, @@ -98,6 +98,9 @@ func New( config.TargetGossipSize, config.PushGossipMaxRegossipFrequency, ) + if err != nil { + return nil, err + } var txPullGossiper gossip.Gossiper = gossip.NewPullGossiper[*txs.Tx]( ctx.Log, diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index c4d0268787d..11209c2ab67 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -104,7 +104,7 @@ func New( return nil, err } - txPushGossiper := gossip.NewPushGossiper[*txs.Tx]( + txPushGossiper, err := gossip.NewPushGossiper[*txs.Tx]( marshaller, gossipMempool, txGossipClient, @@ -113,6 +113,9 @@ func New( config.TargetGossipSize, config.PushGossipMaxRegossipFrequency, ) + if err != nil { + return nil, err + } var txPullGossiper gossip.Gossiper = gossip.NewPullGossiper[*txs.Tx]( log, From 5de194a571442da6960c6da2ea1dce291249291c Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:18:48 -0500 Subject: [PATCH 44/59] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- network/p2p/gossip/gossip.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index b835d5e1eae..8f5920db056 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -408,10 +408,10 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { now := time.Now() for _, gossipable := range gossipables { gossipID := gossipable.GossipID() - if _, contains := p.tracking[gossipID]; contains { + if _, ok := p.tracking[gossipID]; ok { continue } - if _, contains := p.discarded.Get(gossipID); !contains { + if _, ok := p.discarded.Get(gossipID); !ok { p.tracking[gossipID] = time.Time{} p.pending.PushRight(gossipable) } else { From b4d4d1dbe50d9b97cfdb9f718c37d1094079046c Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:20:46 -0500 Subject: [PATCH 45/59] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- network/p2p/gossip/gossip.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 8f5920db056..54e8df539da 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -411,13 +411,13 @@ func (p *PushGossiper[T]) Add(gossipables ...T) { if _, ok := p.tracking[gossipID]; ok { continue } - if _, ok := p.discarded.Get(gossipID); !ok { - p.tracking[gossipID] = time.Time{} - p.pending.PushRight(gossipable) - } else { + if _, ok := p.discarded.Get(gossipID); ok { // Pretend that recently discarded transactions were just gossiped. p.tracking[gossipID] = now p.issued.PushRight(gossipable) + } else { + p.tracking[gossipID] = time.Time{} + p.pending.PushRight(gossipable) } } p.metrics.tracking.Set(float64(len(p.tracking))) From 2b74cfd0bb3913caaa5f4566db63538c490a6a45 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:41:06 -0500 Subject: [PATCH 46/59] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- .../config/execution_config_test.go | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/vms/platformvm/config/execution_config_test.go b/vms/platformvm/config/execution_config_test.go index d80f2c67c82..8c28208cb49 100644 --- a/vms/platformvm/config/execution_config_test.go +++ b/vms/platformvm/config/execution_config_test.go @@ -45,14 +45,16 @@ func TestExecutionConfigUnmarshal(t *testing.T) { "network": { "max-validator-set-staleness": 1, "target-gossip-size": 2, - "pull-gossip-poll-size": 3, - "pull-gossip-frequency": 4, - "pull-gossip-throttling-period": 5, - "pull-gossip-throttling-limit": 6, - "expected-bloom-filter-elements":7, - "expected-bloom-filter-false-positive-probability": 8, - "max-bloom-filter-false-positive-probability": 9, - "legacy-push-gossip-cache-size": 10 + "push-gossip-discarded-cache-size": 3, + "push-gossip-max-regossip-frequency": 4, + "push-gossip-frequency": 5, + "pull-gossip-poll-size": 6, + "pull-gossip-frequency": 7, + "pull-gossip-throttling-period": 8, + "pull-gossip-throttling-limit": 9, + "expected-bloom-filter-elements": 10, + "expected-bloom-filter-false-positive-probability": 11, + "max-bloom-filter-false-positive-probability": 12 }, "block-cache-size": 1, "tx-cache-size": 2, @@ -71,13 +73,16 @@ func TestExecutionConfigUnmarshal(t *testing.T) { Network: network.Config{ MaxValidatorSetStaleness: 1, TargetGossipSize: 2, - PullGossipPollSize: 3, - PullGossipFrequency: 4, - PullGossipThrottlingPeriod: 5, - PullGossipThrottlingLimit: 6, - ExpectedBloomFilterElements: 7, - ExpectedBloomFilterFalsePositiveProbability: 8, - MaxBloomFilterFalsePositiveProbability: 9, + PushGossipDiscardedCacheSize: 3, + PushGossipMaxRegossipFrequency: 4, + PushGossipFrequency: 5, + PullGossipPollSize: 6, + PullGossipFrequency: 7, + PullGossipThrottlingPeriod: 8, + PullGossipThrottlingLimit: 9, + ExpectedBloomFilterElements: 10, + ExpectedBloomFilterFalsePositiveProbability: 11, + MaxBloomFilterFalsePositiveProbability: 12, }, BlockCacheSize: 1, TxCacheSize: 2, @@ -99,6 +104,8 @@ func TestExecutionConfigUnmarshal(t *testing.T) { "network": { "max-validator-set-staleness": 1, "target-gossip-size": 2, + "push-gossip-discarded-cache-size": 1024, + "push-gossip-max-regossip-frequency": 10000000000, "pull-gossip-poll-size": 3, "pull-gossip-frequency": 4, "pull-gossip-throttling-period": 5 From 28f16646ff8bbfcafcf72476cbe0fabe45c1188a Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:41:39 -0500 Subject: [PATCH 47/59] remove network interface Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- vms/platformvm/block/builder/helpers_test.go | 2 +- vms/platformvm/network/network.go | 32 +++++--------------- vms/platformvm/network/network_test.go | 5 ++- vms/platformvm/vm.go | 2 +- 4 files changed, 12 insertions(+), 29 deletions(-) diff --git a/vms/platformvm/block/builder/helpers_test.go b/vms/platformvm/block/builder/helpers_test.go index 9190f01e2c6..c95844b842b 100644 --- a/vms/platformvm/block/builder/helpers_test.go +++ b/vms/platformvm/block/builder/helpers_test.go @@ -103,7 +103,7 @@ type environment struct { Builder blkManager blockexecutor.Manager mempool mempool.Mempool - network network.Network + network *network.Network sender *common.SenderTest isBootstrapped *utils.Atomic[bool] diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go index 11209c2ab67..8116524e019 100644 --- a/vms/platformvm/network/network.go +++ b/vms/platformvm/network/network.go @@ -26,23 +26,7 @@ const TxGossipHandlerID = 0 var errMempoolDisabledWithPartialSync = errors.New("mempool is disabled partial syncing") -type Network interface { - common.AppHandler - - // PushGossip starts push gossiping transactions and blocks until it - // completes. - PushGossip(ctx context.Context) - // PullGossip starts pull gossiping transactions and blocks until it - // completes. - PullGossip(ctx context.Context) - // IssueTxFromRPC issues a transaction that was received from the local RPC. - // The transaction will be verified against the currently preferred state. - // If transaction verification passes, it will be added to the mempool and - // periodically pushed to validators until it is removed from the mempool. - IssueTxFromRPC(*txs.Tx) error -} - -type network struct { +type Network struct { *p2p.Network log logging.Logger @@ -68,7 +52,7 @@ func New( appSender common.AppSender, registerer prometheus.Registerer, config Config, -) (Network, error) { +) (*Network, error) { p2pNetwork, err := p2p.NewNetwork(log, appSender, registerer, "p2p") if err != nil { return nil, err @@ -165,7 +149,7 @@ func New( return nil, err } - return &network{ + return &Network{ Network: p2pNetwork, log: log, txVerifier: txVerifier, @@ -179,7 +163,7 @@ func New( }, nil } -func (n *network) PushGossip(ctx context.Context) { +func (n *Network) PushGossip(ctx context.Context) { // TODO: Even though the node is running partial sync, we should support // issuing transactions from the RPC. if n.partialSyncPrimaryNetwork { @@ -189,7 +173,7 @@ func (n *network) PushGossip(ctx context.Context) { gossip.Every(ctx, n.log, n.txPushGossiper, n.txPushGossipFrequency) } -func (n *network) PullGossip(ctx context.Context) { +func (n *Network) PullGossip(ctx context.Context) { // If the node is running partial sync, we should not perform any pull // gossip. if n.partialSyncPrimaryNetwork { @@ -199,7 +183,7 @@ func (n *network) PullGossip(ctx context.Context) { gossip.Every(ctx, n.log, n.txPullGossiper, n.txPullGossipFrequency) } -func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []byte) error { +func (n *Network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []byte) error { n.log.Debug("called AppGossip message handler", zap.Stringer("nodeID", nodeID), zap.Int("messageLen", len(msgBytes)), @@ -246,7 +230,7 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b return nil } -func (n *network) IssueTxFromRPC(tx *txs.Tx) error { +func (n *Network) IssueTxFromRPC(tx *txs.Tx) error { // TODO: We should still push the transaction to some peers when partial // syncing. if err := n.addTxToMempool(tx); err != nil { @@ -256,7 +240,7 @@ func (n *network) IssueTxFromRPC(tx *txs.Tx) error { return nil } -func (n *network) addTxToMempool(tx *txs.Tx) error { +func (n *Network) addTxToMempool(tx *txs.Tx) error { // If we are partially syncing the Primary Network, we should not be // maintaining the transaction mempool locally. if n.partialSyncPrimaryNetwork { diff --git a/vms/platformvm/network/network_test.go b/vms/platformvm/network/network_test.go index 223a929f368..b2eb1b36c6d 100644 --- a/vms/platformvm/network/network_test.go +++ b/vms/platformvm/network/network_test.go @@ -288,7 +288,7 @@ func TestNetworkIssueTxFromRPC(t *testing.T) { ctrl := gomock.NewController(t) snowCtx := snowtest.Context(t, ids.Empty) - nIntf, err := New( + n, err := New( snowCtx.Log, snowCtx.NodeID, snowCtx.SubnetID, @@ -302,10 +302,9 @@ func TestNetworkIssueTxFromRPC(t *testing.T) { ) require.NoError(err) - err = nIntf.IssueTxFromRPC(tx) + err = n.IssueTxFromRPC(tx) require.ErrorIs(err, tt.expectedErr) - n := nIntf.(*network) require.NoError(n.txPushGossiper.Gossip(context.Background())) }) } diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go index 7907cd6d881..30e1b8c3d63 100644 --- a/vms/platformvm/vm.go +++ b/vms/platformvm/vm.go @@ -62,7 +62,7 @@ var ( type VM struct { config.Config blockbuilder.Builder - network.Network + *network.Network validators.State metrics metrics.Metrics From 876fc3f3608e02ed110494afc31efa6f8dfeb006 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:42:47 -0500 Subject: [PATCH 48/59] fix typo Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- vms/platformvm/network/network_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vms/platformvm/network/network_test.go b/vms/platformvm/network/network_test.go index b2eb1b36c6d..463f5c0cdc2 100644 --- a/vms/platformvm/network/network_test.go +++ b/vms/platformvm/network/network_test.go @@ -251,7 +251,7 @@ func TestNetworkIssueTxFromRPC(t *testing.T) { expectedErr: errTest, }, { - name: "mempool is disalbed if primary network is not being fully synced", + name: "mempool is disabled if primary network is not being fully synced", mempoolFunc: func(ctrl *gomock.Controller) mempool.Mempool { return mempool.NewMockMempool(ctrl) }, From 795f6d58ba4f9c0ed0cf1809b7a22fb8a92435b3 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:43:21 -0500 Subject: [PATCH 49/59] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- vms/avm/service.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/vms/avm/service.go b/vms/avm/service.go index 8d6cbb0ce72..5392308480a 100644 --- a/vms/avm/service.go +++ b/vms/avm/service.go @@ -214,8 +214,6 @@ func (s *Service) IssueTx(_ *http.Request, args *api.FormattedTx, reply *api.JSO } reply.TxID, err = s.vm.issueTxFromRPC(tx) - - // TODO: enqueue push gossip return err } From e76f252b22cc62f5401689fb308e3238cc6c21d2 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:44:01 -0500 Subject: [PATCH 50/59] duplicate line Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- vms/avm/network/config.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/vms/avm/network/config.go b/vms/avm/network/config.go index 3e8c6ef6673..599d7a96263 100644 --- a/vms/avm/network/config.go +++ b/vms/avm/network/config.go @@ -43,8 +43,6 @@ type Config struct { PushGossipFrequency time.Duration `json:"push-gossip-frequency"` // PullGossipPollSize is the number of validators to sample when performing // a round of pull gossip. - // PullGossipPollSize is the number of validators to sample when performing - // a round of pull gossip. PullGossipPollSize int `json:"pull-gossip-poll-size"` // PullGossipFrequency is how frequently rounds of pull gossip are // performed. From 6d3dfafae7fd39457bcd2f9700832abf650ce19b Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 16:08:58 -0500 Subject: [PATCH 51/59] nits --- network/p2p/gossip/gossip.go | 20 +++++++++----------- vms/avm/network/network.go | 12 ++++++------ 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 54e8df539da..4e5a36b1778 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -46,10 +46,11 @@ var ( typeLabel: pullType, } - errEmptySetCantAdd = errors.New("empty set can not add") ErrInvalidDiscardedSize = errors.New("discarded size cannot be negative") ErrInvalidTargetGossipSize = errors.New("target gossip size cannot be negative") ErrInvalidRegossipFrequency = errors.New("re-gossip frequency cannot be negative") + + errEmptySetCantAdd = errors.New("empty set can not add") ) // Gossiper gossips Gossipables to other nodes @@ -246,15 +247,12 @@ func NewPushGossiper[T Gossipable]( targetGossipSize int, maxRegossipFrequency time.Duration, ) (*PushGossiper[T], error) { - if discardedSize < 0 { + switch { + case discardedSize < 0: return nil, ErrInvalidDiscardedSize - } - - if targetGossipSize < 0 { + case targetGossipSize < 0: return nil, ErrInvalidTargetGossipSize - } - - if maxRegossipFrequency < 0 { + case maxRegossipFrequency < 0: return nil, ErrInvalidRegossipFrequency } @@ -269,7 +267,7 @@ func NewPushGossiper[T Gossipable]( tracking: make(map[ids.ID]time.Time), pending: buffer.NewUnboundedDeque[T](0), issued: buffer.NewUnboundedDeque[T](0), - discarded: &cache.LRU[ids.ID, interface{}]{Size: discardedSize}, + discarded: &cache.LRU[ids.ID, struct{}]{Size: discardedSize}, }, nil } @@ -287,7 +285,7 @@ type PushGossiper[T Gossipable] struct { tracking map[ids.ID]time.Time pending buffer.Deque[T] issued buffer.Deque[T] - discarded *cache.LRU[ids.ID, interface{}] // discarded attempts to avoid overgossiping transactions that are frequently dropped + discarded *cache.LRU[ids.ID, struct{}] // discarded attempts to avoid overgossiping transactions that are frequently dropped } // Gossip flushes any queued gossipables. @@ -345,7 +343,7 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { gossipID := gossipable.GossipID() if !p.set.Has(gossipID) { delete(p.tracking, gossipID) - p.discarded.Put(gossipID, nil) // only add to discarded if issued once + p.discarded.Put(gossipID, struct{}{}) // only add to discarded if issued once continue } diff --git a/vms/avm/network/network.go b/vms/avm/network/network.go index 3ed5524e68e..63785b2b66d 100644 --- a/vms/avm/network/network.go +++ b/vms/avm/network/network.go @@ -213,9 +213,9 @@ func (n *Network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b return nil } -// IssueTx attempts to add a tx to the mempool, after verifying it. If the tx is -// added to the mempool, it will attempt to push gossip the tx to random peers -// in the network using both the legacy and p2p SDK. +// IssueTxFromRPC attempts to add a tx to the mempool, after verifying it. If +// the tx is added to the mempool, it will attempt to push gossip the tx to +// random peers in the network. // // If the tx is already in the mempool, mempool.ErrDuplicateTx will be // returned. @@ -228,9 +228,9 @@ func (n *Network) IssueTxFromRPC(tx *txs.Tx) error { return nil } -// IssueVerifiedTx attempts to add a tx to the mempool, without first verifying -// it. If the tx is added to the mempool, it will attempt to push gossip the tx -// to random peers in the network using both the legacy and p2p SDK. +// IssueTxFromRPCWithoutVerification attempts to add a tx to the mempool, +// without first verifying it. If the tx is added to the mempool, it will +// attempt to push gossip the tx to random peers in the network. // // If the tx is already in the mempool, mempool.ErrDuplicateTx will be // returned. From 445056779be2ee40fae44c41690c2c8cd77f2670 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 16:18:22 -0500 Subject: [PATCH 52/59] Add test for empty gossip --- network/p2p/gossip/gossip_test.go | 68 ++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/network/p2p/gossip/gossip_test.go b/network/p2p/gossip/gossip_test.go index c95f7e984a9..ce612be1e55 100644 --- a/network/p2p/gossip/gossip_test.go +++ b/network/p2p/gossip/gossip_test.go @@ -278,11 +278,12 @@ func TestPushGossiper(t *testing.T) { expected []*testTx } tests := []struct { - name string - cycles []cycle + name string + cycles []cycle + shouldRegossip bool }{ { - name: "single cycle", + name: "single cycle with regossip", cycles: []cycle{ { toAdd: []*testTx{ @@ -309,9 +310,10 @@ func TestPushGossiper(t *testing.T) { }, }, }, + shouldRegossip: true, }, { - name: "multiple cycles", + name: "multiple cycles with regossip", cycles: []cycle{ { toAdd: []*testTx{ @@ -359,10 +361,32 @@ func TestPushGossiper(t *testing.T) { }, }, }, + shouldRegossip: true, + }, + { + name: "verify that we don't gossip empty messages", + cycles: []cycle{ + { + toAdd: []*testTx{ + { + id: ids.ID{0}, + }, + }, + expected: []*testTx{ + { + id: ids.ID{0}, + }, + }, + }, + { + toAdd: []*testTx{}, + expected: []*testTx{}, + }, + }, + shouldRegossip: false, }, } - const regossipTime = time.Nanosecond for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { require := require.New(t) @@ -382,6 +406,12 @@ func TestPushGossiper(t *testing.T) { metrics, err := NewMetrics(prometheus.NewRegistry(), "") require.NoError(err) marshaller := testMarshaller{} + + regossipTime := time.Hour + if tt.shouldRegossip { + regossipTime = time.Nanosecond + } + gossiper, err := NewPushGossiper[*testTx]( marshaller, FullSet[*testTx]{}, @@ -408,16 +438,26 @@ func TestPushGossiper(t *testing.T) { want.Gossip = append(want.Gossip, bytes) } - // remove the handler prefix - sentMsg := <-sender.SentAppGossip - got := &sdk.PushGossip{} - require.NoError(proto.Unmarshal(sentMsg[1:], got)) - - require.Equal(want.Gossip, got.Gossip) + if len(want.Gossip) > 0 { + // remove the handler prefix + sentMsg := <-sender.SentAppGossip + got := &sdk.PushGossip{} + require.NoError(proto.Unmarshal(sentMsg[1:], got)) + + require.Equal(want.Gossip, got.Gossip) + } else { + select { + case <-sender.SentAppGossip: + require.FailNow("unexpectedly sent gossip message") + default: + } + } - // Ensure that subsequent calls to `time.Now()` are sufficient - // for regossip. - time.Sleep(regossipTime + time.Nanosecond) + if tt.shouldRegossip { + // Ensure that subsequent calls to `time.Now()` are + // sufficient for regossip. + time.Sleep(regossipTime + time.Nanosecond) + } } }) } From 63269d7d73557b7595c50deca58e48ddcc5696af Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 16:38:33 -0500 Subject: [PATCH 53/59] gomod Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 1d77176690d..7b66290e56d 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ go 1.21 require ( github.com/DataDog/zstd v1.5.2 github.com/NYTimes/gziphandler v1.1.1 - github.com/ava-labs/coreth v0.13.0-rc.0 + github.com/ava-labs/coreth v0.13.0-rc.0.0.20240227212508-bfcae19e9865 github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34 github.com/btcsuite/btcd/btcutil v1.1.3 github.com/cockroachdb/pebble v0.0.0-20230209160836-829675f94811 diff --git a/go.sum b/go.sum index 160a06369f8..a846c3fc98a 100644 --- a/go.sum +++ b/go.sum @@ -63,8 +63,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah4HI848JfFxHt+iPb26b4zyfspmqY0/8= github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= -github.com/ava-labs/coreth v0.13.0-rc.0 h1:V2l3qj2ek3geKDJAnF2M94mYJK8kg2kePixujfJ0bmk= -github.com/ava-labs/coreth v0.13.0-rc.0/go.mod h1:eUMbBLDhlZASJjcbf0gIcD2GMn2rRRCUxC8MXLt5QQk= +github.com/ava-labs/coreth v0.13.0-rc.0.0.20240227212508-bfcae19e9865 h1:o6MEsdCwfAUxkVTrJmteuHn/NVL3336JQDnCn5hBNtE= +github.com/ava-labs/coreth v0.13.0-rc.0.0.20240227212508-bfcae19e9865/go.mod h1:zKQzkv4RLLWxpDPwAGh3woL4xoXCftF4rQKAjllX/Nc= github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34 h1:mg9Uw6oZFJKytJxgxnl3uxZOs/SB8CVHg6Io4Tf99Zc= github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34/go.mod h1:pJxaT9bUgeRNVmNRgtCHb7sFDIRKy7CzTQVi8gGNT6g= github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g= From f2f0e112d5e78215e0e089c9a140dfa142139205 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 27 Feb 2024 16:42:47 -0500 Subject: [PATCH 54/59] lint Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- vms/platformvm/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index d0379bc2268..6e48f198fb9 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -1400,7 +1400,7 @@ func (s *Service) GetBlockchains(_ *http.Request, _ *struct{}, response *GetBloc return nil } -func (s *Service) IssueTx(req *http.Request, args *api.FormattedTx, response *api.JSONTxID) error { +func (s *Service) IssueTx(_ *http.Request, args *api.FormattedTx, response *api.JSONTxID) error { s.vm.ctx.Log.Debug("API called", zap.String("service", "platform"), zap.String("method", "issueTx"), From 7257cc4af6f8433400d74336c374b77118a3cb73 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 Feb 2024 23:16:38 -0500 Subject: [PATCH 55/59] Update coreth to v0.13.1-rc.0 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 7b66290e56d..471f19e9824 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ go 1.21 require ( github.com/DataDog/zstd v1.5.2 github.com/NYTimes/gziphandler v1.1.1 - github.com/ava-labs/coreth v0.13.0-rc.0.0.20240227212508-bfcae19e9865 + github.com/ava-labs/coreth v0.13.1-rc.0 github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34 github.com/btcsuite/btcd/btcutil v1.1.3 github.com/cockroachdb/pebble v0.0.0-20230209160836-829675f94811 diff --git a/go.sum b/go.sum index a846c3fc98a..bdbf832b380 100644 --- a/go.sum +++ b/go.sum @@ -63,8 +63,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah4HI848JfFxHt+iPb26b4zyfspmqY0/8= github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= -github.com/ava-labs/coreth v0.13.0-rc.0.0.20240227212508-bfcae19e9865 h1:o6MEsdCwfAUxkVTrJmteuHn/NVL3336JQDnCn5hBNtE= -github.com/ava-labs/coreth v0.13.0-rc.0.0.20240227212508-bfcae19e9865/go.mod h1:zKQzkv4RLLWxpDPwAGh3woL4xoXCftF4rQKAjllX/Nc= +github.com/ava-labs/coreth v0.13.1-rc.0 h1:T4qXxaHF3NIom3pCLQufalja0vEE/Tve7ANzERaTviQ= +github.com/ava-labs/coreth v0.13.1-rc.0/go.mod h1:lSFCGNYXM2+RFMxpoNSsGxtIObfeG5oNpTOi7mmEPng= github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34 h1:mg9Uw6oZFJKytJxgxnl3uxZOs/SB8CVHg6Io4Tf99Zc= github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34/go.mod h1:pJxaT9bUgeRNVmNRgtCHb7sFDIRKy7CzTQVi8gGNT6g= github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g= From 5b04b7a8e04ab481e6494f8e35af60576cb26b4d Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Wed, 28 Feb 2024 00:11:41 -0700 Subject: [PATCH 56/59] register tracking --- network/p2p/gossip/gossip.go | 1 + 1 file changed, 1 insertion(+) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 4e5a36b1778..dcfa6d37f93 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -114,6 +114,7 @@ func NewMetrics( metrics.Register(m.sentBytes), metrics.Register(m.receivedCount), metrics.Register(m.receivedBytes), + metrics.Register(m.tracking), ) return m, err } From 648915ce941a7cc2d1900e841ce94c0519b9edd3 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Wed, 28 Feb 2024 10:58:09 -0500 Subject: [PATCH 57/59] Register metric --- network/p2p/gossip/gossip.go | 1 + 1 file changed, 1 insertion(+) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index 4e5a36b1778..dcfa6d37f93 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -114,6 +114,7 @@ func NewMetrics( metrics.Register(m.sentBytes), metrics.Register(m.receivedCount), metrics.Register(m.receivedBytes), + metrics.Register(m.tracking), ) return m, err } From 1cbea42c3feef0987af5dce3b5f4342c16d7835f Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Wed, 28 Feb 2024 13:26:05 -0500 Subject: [PATCH 58/59] Make metrics more fine-grained (#2778) --- network/p2p/gossip/gossip.go | 145 ++++++++++++++++++++++++----------- 1 file changed, 102 insertions(+), 43 deletions(-) diff --git a/network/p2p/gossip/gossip.go b/network/p2p/gossip/gossip.go index dcfa6d37f93..5e3f4d3cf77 100644 --- a/network/p2p/gossip/gossip.go +++ b/network/p2p/gossip/gossip.go @@ -23,9 +23,11 @@ import ( ) const ( - typeLabel = "type" - pushType = "push" - pullType = "pull" + typeLabel = "type" + pushType = "push" + pullType = "pull" + unsentType = "unsent" + sentType = "sent" defaultGossipableCount = 64 ) @@ -45,6 +47,12 @@ var ( pullLabels = prometheus.Labels{ typeLabel: pullType, } + unsentLabels = prometheus.Labels{ + typeLabel: unsentType, + } + sentLabels = prometheus.Labels{ + typeLabel: sentType, + } ErrInvalidDiscardedSize = errors.New("discarded size cannot be negative") ErrInvalidTargetGossipSize = errors.New("target gossip size cannot be negative") @@ -70,11 +78,12 @@ type ValidatorGossiper struct { // Metrics that are tracked across a gossip protocol. A given protocol should // only use a single instance of Metrics. type Metrics struct { - sentCount *prometheus.CounterVec - sentBytes *prometheus.CounterVec - receivedCount *prometheus.CounterVec - receivedBytes *prometheus.CounterVec - tracking prometheus.Gauge + sentCount *prometheus.CounterVec + sentBytes *prometheus.CounterVec + receivedCount *prometheus.CounterVec + receivedBytes *prometheus.CounterVec + tracking *prometheus.GaugeVec + trackingLifetimeAverage prometheus.Gauge } // NewMetrics returns a common set of metrics @@ -103,10 +112,15 @@ func NewMetrics( Name: "gossip_received_bytes", Help: "amount of gossip received (bytes)", }, metricLabels), - tracking: prometheus.NewGauge(prometheus.GaugeOpts{ + tracking: prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: namespace, Name: "gossip_tracking", Help: "number of gossipables being tracked", + }, metricLabels), + trackingLifetimeAverage: prometheus.NewGauge(prometheus.GaugeOpts{ + Namespace: namespace, + Name: "gossip_tracking_lifetime_average", + Help: "average duration a gossipable has been tracked (ns)", }), } err := utils.Err( @@ -115,6 +129,7 @@ func NewMetrics( metrics.Register(m.receivedCount), metrics.Register(m.receivedBytes), metrics.Register(m.tracking), + metrics.Register(m.trackingLifetimeAverage), ) return m, err } @@ -265,10 +280,10 @@ func NewPushGossiper[T Gossipable]( targetGossipSize: targetGossipSize, maxRegossipFrequency: maxRegossipFrequency, - tracking: make(map[ids.ID]time.Time), - pending: buffer.NewUnboundedDeque[T](0), - issued: buffer.NewUnboundedDeque[T](0), - discarded: &cache.LRU[ids.ID, struct{}]{Size: discardedSize}, + tracking: make(map[ids.ID]*tracking), + toGossip: buffer.NewUnboundedDeque[T](0), + toRegossip: buffer.NewUnboundedDeque[T](0), + discarded: &cache.LRU[ids.ID, struct{}]{Size: discardedSize}, }, nil } @@ -282,17 +297,31 @@ type PushGossiper[T Gossipable] struct { targetGossipSize int maxRegossipFrequency time.Duration - lock sync.Mutex - tracking map[ids.ID]time.Time - pending buffer.Deque[T] - issued buffer.Deque[T] - discarded *cache.LRU[ids.ID, struct{}] // discarded attempts to avoid overgossiping transactions that are frequently dropped + lock sync.Mutex + tracking map[ids.ID]*tracking + addedTimeSum float64 // unix nanoseconds + toGossip buffer.Deque[T] + toRegossip buffer.Deque[T] + discarded *cache.LRU[ids.ID, struct{}] // discarded attempts to avoid overgossiping transactions that are frequently dropped +} + +type tracking struct { + addedTime float64 // unix nanoseconds + lastGossiped time.Time } // Gossip flushes any queued gossipables. func (p *PushGossiper[T]) Gossip(ctx context.Context) error { + var ( + now = time.Now() + nowUnixNano = float64(now.UnixNano()) + ) + p.lock.Lock() - defer p.lock.Unlock() + defer func() { + p.updateMetrics(nowUnixNano) + p.lock.Unlock() + }() if len(p.tracking) == 0 { return nil @@ -301,75 +330,78 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { var ( sentBytes = 0 gossip = make([][]byte, 0, defaultGossipableCount) - now = time.Now() ) - // Iterate over all pending gossipables (never been sent before). + // Iterate over all unsent gossipables. for sentBytes < p.targetGossipSize { - gossipable, ok := p.pending.PopLeft() + gossipable, ok := p.toGossip.PopLeft() if !ok { break } // Ensure item is still in the set before we gossip. gossipID := gossipable.GossipID() + tracking := p.tracking[gossipID] if !p.set.Has(gossipID) { delete(p.tracking, gossipID) + p.addedTimeSum -= tracking.addedTime continue } bytes, err := p.marshaller.MarshalGossip(gossipable) if err != nil { delete(p.tracking, gossipID) + p.addedTimeSum -= tracking.addedTime return err } gossip = append(gossip, bytes) sentBytes += len(bytes) - p.issued.PushRight(gossipable) - p.tracking[gossipID] = now + p.toRegossip.PushRight(gossipable) + tracking.lastGossiped = now } maxLastGossipTimeToRegossip := now.Add(-p.maxRegossipFrequency) - // Iterate over all issued gossipables (have been sent before) to fill any - // remaining space in gossip batch. + // Iterate over all previously sent gossipables to fill any remaining space + // in the gossip batch. for sentBytes < p.targetGossipSize { - gossipable, ok := p.issued.PopLeft() + gossipable, ok := p.toRegossip.PopLeft() if !ok { break } // Ensure item is still in the set before we gossip. gossipID := gossipable.GossipID() + tracking := p.tracking[gossipID] if !p.set.Has(gossipID) { delete(p.tracking, gossipID) - p.discarded.Put(gossipID, struct{}{}) // only add to discarded if issued once + p.addedTimeSum -= tracking.addedTime + p.discarded.Put(gossipID, struct{}{}) // only add to discarded if previously sent continue } // Ensure we don't attempt to send a gossipable too frequently. - lastGossipTime := p.tracking[gossipID] - if maxLastGossipTimeToRegossip.Before(lastGossipTime) { + if maxLastGossipTimeToRegossip.Before(tracking.lastGossiped) { // Put the gossipable on the front of the queue to keep items sorted // by last issuance time. - p.issued.PushLeft(gossipable) + p.toRegossip.PushLeft(gossipable) break } bytes, err := p.marshaller.MarshalGossip(gossipable) if err != nil { - // Should never happen because we've already issued this once. + // Should never happen because we've already sent this once. delete(p.tracking, gossipID) + p.addedTimeSum -= tracking.addedTime return err } gossip = append(gossip, bytes) sentBytes += len(bytes) - p.issued.PushRight(gossipable) - p.tracking[gossipID] = now + p.toRegossip.PushRight(gossipable) + tracking.lastGossiped = now } - p.metrics.tracking.Set(float64(len(p.tracking))) // If there is nothing to gossip, we can exit early. if len(gossip) == 0 { @@ -400,26 +432,53 @@ func (p *PushGossiper[T]) Gossip(ctx context.Context) error { // Add enqueues new gossipables to be pushed. If a gossiable is already tracked, // it is not added again. func (p *PushGossiper[T]) Add(gossipables ...T) { + var ( + now = time.Now() + nowUnixNano = float64(now.UnixNano()) + ) + p.lock.Lock() - defer p.lock.Unlock() + defer func() { + p.updateMetrics(nowUnixNano) + p.lock.Unlock() + }() - // Add new gossipables to the pending queue. - now := time.Now() + // Add new gossipables to be sent. for _, gossipable := range gossipables { gossipID := gossipable.GossipID() if _, ok := p.tracking[gossipID]; ok { continue } + + tracking := &tracking{ + addedTime: nowUnixNano, + } if _, ok := p.discarded.Get(gossipID); ok { // Pretend that recently discarded transactions were just gossiped. - p.tracking[gossipID] = now - p.issued.PushRight(gossipable) + tracking.lastGossiped = now + p.toRegossip.PushRight(gossipable) } else { - p.tracking[gossipID] = time.Time{} - p.pending.PushRight(gossipable) + p.toGossip.PushRight(gossipable) } + p.tracking[gossipID] = tracking + p.addedTimeSum += nowUnixNano + } +} + +func (p *PushGossiper[_]) updateMetrics(nowUnixNano float64) { + var ( + numUnsent = float64(p.toGossip.Len()) + numSent = float64(p.toRegossip.Len()) + numTracking = numUnsent + numSent + averageLifetime float64 + ) + if numTracking != 0 { + averageLifetime = nowUnixNano - p.addedTimeSum/numTracking } - p.metrics.tracking.Set(float64(len(p.tracking))) + + p.metrics.tracking.With(unsentLabels).Set(numUnsent) + p.metrics.tracking.With(sentLabels).Set(numSent) + p.metrics.trackingLifetimeAverage.Set(averageLifetime) } // Every calls [Gossip] every [frequency] amount of time. From 18faf4539beb804a8a02a12890044d3e16510d0b Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Wed, 28 Feb 2024 21:07:48 -0500 Subject: [PATCH 59/59] update coreth --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 471f19e9824..555b0655a9a 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ go 1.21 require ( github.com/DataDog/zstd v1.5.2 github.com/NYTimes/gziphandler v1.1.1 - github.com/ava-labs/coreth v0.13.1-rc.0 + github.com/ava-labs/coreth v0.13.1-rc.1 github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34 github.com/btcsuite/btcd/btcutil v1.1.3 github.com/cockroachdb/pebble v0.0.0-20230209160836-829675f94811 diff --git a/go.sum b/go.sum index bdbf832b380..1cb67722efe 100644 --- a/go.sum +++ b/go.sum @@ -63,8 +63,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah4HI848JfFxHt+iPb26b4zyfspmqY0/8= github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= -github.com/ava-labs/coreth v0.13.1-rc.0 h1:T4qXxaHF3NIom3pCLQufalja0vEE/Tve7ANzERaTviQ= -github.com/ava-labs/coreth v0.13.1-rc.0/go.mod h1:lSFCGNYXM2+RFMxpoNSsGxtIObfeG5oNpTOi7mmEPng= +github.com/ava-labs/coreth v0.13.1-rc.1 h1:T838dWZicYXrajXBeLbJTat5rtSXkZisOZ0qcHTEVjM= +github.com/ava-labs/coreth v0.13.1-rc.1/go.mod h1:LzD4pI5AjkPS3r86e36Xpt6PZB2pZ/wUS4Vn8kJwWmY= github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34 h1:mg9Uw6oZFJKytJxgxnl3uxZOs/SB8CVHg6Io4Tf99Zc= github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34/go.mod h1:pJxaT9bUgeRNVmNRgtCHb7sFDIRKy7CzTQVi8gGNT6g= github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g=