From 1c68a7cd399ab39f0780fa0f3ed7f27b4e798554 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Thu, 8 Apr 2021 17:19:09 -0400 Subject: [PATCH] group 'external' logic in consensus --- snow/consensus/avalanche/topological.go | 8 ++--- snow/consensus/snowman/topological.go | 29 +++++++++---------- snow/consensus/snowstorm/common.go | 15 ++++------ snow/engine/snowman/bootstrap/bootstrapper.go | 2 +- 4 files changed, 23 insertions(+), 31 deletions(-) diff --git a/snow/consensus/avalanche/topological.go b/snow/consensus/avalanche/topological.go index 8b6f22a09725..6b97ce5803b9 100644 --- a/snow/consensus/avalanche/topological.go +++ b/snow/consensus/avalanche/topological.go @@ -513,19 +513,19 @@ func (ta *Topological) update(vtx Vertex) error { if err := ta.ctx.ConsensusDispatcher.Accept(ta.ctx, vtxID, vtx.Bytes()); err != nil { return err } + delete(ta.nodes, vtxID) + ta.Metrics.Accepted(vtxID) if err := vtx.Accept(); err != nil { return err } - delete(ta.nodes, vtxID) - ta.Metrics.Accepted(vtxID) case rejectable: // I'm rejectable, why not reject? ta.ctx.ConsensusDispatcher.Reject(ta.ctx, vtxID, vtx.Bytes()) + delete(ta.nodes, vtxID) + ta.Metrics.Rejected(vtxID) if err := vtx.Reject(); err != nil { return err } - delete(ta.nodes, vtxID) - ta.Metrics.Rejected(vtxID) } return nil } diff --git a/snow/consensus/snowman/topological.go b/snow/consensus/snowman/topological.go index b06a310a8e67..9d36983674ea 100644 --- a/snow/consensus/snowman/topological.go +++ b/snow/consensus/snowman/topological.go @@ -513,7 +513,8 @@ func (ts *Topological) accept(n *snowmanBlock) error { child := n.children[pref] // Notify anyone listening that this block was accepted. bytes := child.Bytes() - // Note that DecisionDispatcher.Accept / ConsensusDispatcher.Accept must be called before + ts.Metrics.Accepted(pref) + // Note that DecisionDispatcher.Accept / DecisionDispatcher.Accept must be called before // child.Accept to honor EventDispatcher.Accept's invariant. if err := ts.ctx.DecisionDispatcher.Accept(ts.ctx, pref, bytes); err != nil { return err @@ -521,19 +522,15 @@ func (ts *Topological) accept(n *snowmanBlock) error { if err := ts.ctx.ConsensusDispatcher.Accept(ts.ctx, pref, bytes); err != nil { return err } - if err := child.Accept(); err != nil { - return err - } - - ts.Metrics.Accepted(pref) - // Because this is the newest accepted block, this is the new head. ts.head = pref ts.height = child.Height() - // Remove the decided block from the set of processing IDs, as its status // now implies its preferredness. ts.preferredIDs.Remove(pref) + if err := child.Accept(); err != nil { + return err + } // Because ts.blocks contains the last accepted block, we don't delete the // block from the blocks map here. @@ -545,10 +542,6 @@ func (ts *Topological) accept(n *snowmanBlock) error { continue } - if err := child.Reject(); err != nil { - return err - } - // Notify anyone listening that this block was rejected. bytes := child.Bytes() ts.ctx.DecisionDispatcher.Reject(ts.ctx, childID, bytes) @@ -557,6 +550,10 @@ func (ts *Topological) accept(n *snowmanBlock) error { // Track which blocks have been directly rejected rejects = append(rejects, childID) + + if err := child.Reject(); err != nil { + return err + } } // reject all the descendants of the blocks we just rejected @@ -578,10 +575,6 @@ func (ts *Topological) rejectTransitively(rejected []ids.ID) error { delete(ts.blocks, rejectedID) for childID, child := range rejectedNode.children { - if err := child.Reject(); err != nil { - return err - } - // Notify anyone listening that this block was rejected. bytes := child.Bytes() ts.ctx.DecisionDispatcher.Reject(ts.ctx, childID, bytes) @@ -590,6 +583,10 @@ func (ts *Topological) rejectTransitively(rejected []ids.ID) error { // add the newly rejected block to the end of the queue rejected = append(rejected, childID) + + if err := child.Reject(); err != nil { + return err + } } } return nil diff --git a/snow/consensus/snowstorm/common.go b/snow/consensus/snowstorm/common.go index 8c73129bea98..43c6aab02564 100644 --- a/snow/consensus/snowstorm/common.go +++ b/snow/consensus/snowstorm/common.go @@ -144,12 +144,12 @@ func (c *common) shouldVote(con Consensus, tx Tx) (bool, error) { return false, err } + // Notify the metrics that this transaction was accepted. + c.Metrics.Accepted(txID) + if err := tx.Accept(); err != nil { return false, err } - - // Notify the metrics that this transaction was just accepted. - c.Metrics.Accepted(txID) return false, nil } @@ -164,12 +164,6 @@ func (c *common) acceptTx(tx Tx) error { return err } - // Accept is called before notifying the IPC so that acceptances that cause - // fatal errors aren't sent to an IPC peer. - if err := tx.Accept(); err != nil { - return err - } - // Update the metrics to account for this transaction's acceptance c.Metrics.Accepted(txID) @@ -179,7 +173,8 @@ func (c *common) acceptTx(tx Tx) error { // If there is a tx that was issued pending on this tx, the ancestor tx // doesn't need to be rejected because of this tx. c.pendingReject.Abandon(txID) - return nil + + return tx.Accept() } // reject the provided tx. diff --git a/snow/engine/snowman/bootstrap/bootstrapper.go b/snow/engine/snowman/bootstrap/bootstrapper.go index 51b208348732..96d79b992f0e 100644 --- a/snow/engine/snowman/bootstrap/bootstrapper.go +++ b/snow/engine/snowman/bootstrap/bootstrapper.go @@ -359,7 +359,7 @@ func (b *Bootstrapper) executeAll(jobs *queue.Jobs) (int, error) { for job, err := jobs.Pop(); err == nil; job, err = jobs.Pop() { jobID := job.ID() jobBytes := job.Bytes() - // Note that ConsensusDispatcher.Accept / ConsensusDispatcher.Accept must be + // Note that ConsensusDispatcher.Accept / DecisionDispatcher.Accept must be // called before job.Execute to honor EventDispatcher.Accept's invariant. if err := b.Ctx.ConsensusDispatcher.Accept(b.Ctx, jobID, jobBytes); err != nil { return numExecuted, err