Skip to content

Commit

Permalink
Remove Commit gossiping logic from P2P client (cosmos#717)
Browse files Browse the repository at this point in the history
Since `SignedHeaders` are gossiped, there is no use for `Commit`
gossiping. Entire related logic should be removed from `p2p.Client` and
node(s).

Closes: cosmos#686
  • Loading branch information
Manav-Aggarwal authored Jan 26, 2023
1 parent a1643e1 commit 0c7f5ef
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 59 deletions.
7 changes: 2 additions & 5 deletions block/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down
23 changes: 0 additions & 23 deletions node/full.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
31 changes: 0 additions & 31 deletions p2p/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

0 comments on commit 0c7f5ef

Please sign in to comment.