From 0c7f5ef60e35cb96c9910808aab2e8d5439e7456 Mon Sep 17 00:00:00 2001 From: Manav Aggarwal Date: Thu, 26 Jan 2023 10:48:20 -0500 Subject: [PATCH] Remove Commit gossiping logic from P2P client (#717) Since `SignedHeaders` are gossiped, there is no use for `Commit` gossiping. Entire related logic should be removed from `p2p.Client` and node(s). Closes: #686 --- block/manager.go | 7 ++----- node/full.go | 23 ----------------------- p2p/client.go | 31 ------------------------------- 3 files changed, 2 insertions(+), 59 deletions(-) diff --git a/block/manager.go b/block/manager.go index fb73b123c97..7d4759826b4 100644 --- a/block/manager.go +++ b/block/manager.go @@ -59,7 +59,6 @@ type Manager struct { HeaderOutCh chan *types.SignedHeader HeaderInCh chan *types.SignedHeader - CommitInCh chan *types.Commit lastCommit atomic.Value FraudProofInCh chan *abci.FraudProof @@ -141,7 +140,6 @@ func NewManager( // channels are buffered to avoid blocking on input/output operations, buffer sizes are arbitrary HeaderOutCh: make(chan *types.SignedHeader, 100), HeaderInCh: make(chan *types.SignedHeader, 100), - CommitInCh: make(chan *types.Commit, 100), blockInCh: make(chan newBlockEvent, 100), FraudProofInCh: make(chan *abci.FraudProof, 100), retrieveMtx: new(sync.Mutex), @@ -227,15 +225,14 @@ func (m *Manager) SyncLoop(ctx context.Context, cancel context.CancelFunc) { atomic.StoreUint64(&m.syncTarget, newHeight) m.retrieveCond.Signal() } - m.CommitInCh <- &header.Commit - case commit := <-m.CommitInCh: + commit := &header.Commit // TODO(tzdybal): check if it's from right aggregator m.lastCommit.Store(commit) err := m.trySyncNextBlock(ctx, 0) if err != nil { m.logger.Info("failed to sync next block", "error", err) } else { - m.logger.Debug("synced using gossiped commit", "height", commit.Height) + m.logger.Debug("synced using signed header", "height", commit.Height) } case blockEvent := <-m.blockInCh: block := blockEvent.block diff --git a/node/full.go b/node/full.go index c1b4f2d99b2..d97d4772431 100644 --- a/node/full.go +++ b/node/full.go @@ -169,7 +169,6 @@ func newFullNode( node.P2P.SetTxValidator(node.newTxValidator()) node.P2P.SetHeaderValidator(node.newHeaderValidator()) - node.P2P.SetCommitValidator(node.newCommitValidator()) node.P2P.SetFraudProofValidator(node.newFraudProofValidator()) return node, nil @@ -365,28 +364,6 @@ func (n *FullNode) newHeaderValidator() p2p.GossipValidator { } } -// newCommitValidator returns a pubsub validator that runs basic checks and forwards -// the deserialized commit for further processing -func (n *FullNode) newCommitValidator() p2p.GossipValidator { - return func(commitMsg *p2p.GossipMessage) bool { - n.Logger.Debug("commit received", "from", commitMsg.From, "bytes", len(commitMsg.Data)) - var commit types.Commit - err := commit.UnmarshalBinary(commitMsg.Data) - if err != nil { - n.Logger.Error("failed to deserialize commit", "error", err) - return false - } - err = commit.ValidateBasic() - if err != nil { - n.Logger.Error("failed to validate commit", "error", err) - return false - } - n.Logger.Debug("commit received", "height", commit.Height) - n.blockManager.CommitInCh <- &commit - return true - } -} - // newFraudProofValidator returns a pubsub validator that validates a fraud proof and forwards // it to be verified func (n *FullNode) newFraudProofValidator() p2p.GossipValidator { diff --git a/p2p/client.go b/p2p/client.go index 352caf9aa3a..ebeb94e3ebc 100644 --- a/p2p/client.go +++ b/p2p/client.go @@ -41,9 +41,6 @@ const ( // headerTopicSuffix is added after namespace to create pubsub topic for block header gossiping. headerTopicSuffix = "-header" - // commitTopicSuffix is added after namespace to create pubsub topic for block commit gossiping. - commitTopicSuffix = "-commit" - fraudProofTopicSuffix = "-fraudProof" ) @@ -71,9 +68,6 @@ type Client struct { fraudProofGossiper *Gossiper fraudProofValidator GossipValidator - commitGossiper *Gossiper - commitValidator GossipValidator - // cancel is used to cancel context passed to libp2p functions // it's required because of discovery.Advertise call cancel context.CancelFunc @@ -194,17 +188,6 @@ func (c *Client) SetHeaderValidator(validator GossipValidator) { c.headerValidator = validator } -// GossipCommit sends the block commit to the P2P network. -func (c *Client) GossipCommit(ctx context.Context, commitBytes []byte) error { - c.logger.Debug("Gossiping block commit", "len", len(commitBytes)) - return c.commitGossiper.Publish(ctx, commitBytes) -} - -// SetCommitValidator sets the callback function, that will be invoked after block commit is received from P2P network. -func (c *Client) SetCommitValidator(validator GossipValidator) { - c.commitValidator = validator -} - // GossipHeader sends a fraud proof to the P2P network. func (c *Client) GossipFraudProof(ctx context.Context, fraudProof []byte) error { c.logger.Debug("Gossiping fraud proof", "len", len(fraudProof)) @@ -386,16 +369,6 @@ func (c *Client) setupGossiping(ctx context.Context) error { } go c.headerGossiper.ProcessMessages(ctx) - var opts []GossiperOption - if c.commitValidator != nil { - opts = append(opts, WithValidator(c.commitValidator)) - } - c.commitGossiper, err = NewGossiper(c.host, ps, c.getCommitTopic(), c.logger, opts...) - if err != nil { - return err - } - go c.commitGossiper.ProcessMessages(ctx) - c.fraudProofGossiper, err = NewGossiper(c.host, ps, c.getFraudProofTopic(), c.logger, WithValidator(c.fraudProofValidator)) if err != nil { @@ -445,10 +418,6 @@ func (c *Client) getHeaderTopic() string { return c.getNamespace() + headerTopicSuffix } -func (c *Client) getCommitTopic() string { - return c.getNamespace() + commitTopicSuffix -} - func (c *Client) getFraudProofTopic() string { return c.getNamespace() + fraudProofTopicSuffix }