Skip to content

Commit

Permalink
Remove Decided from the Consensus interface (#3123)
Browse files Browse the repository at this point in the history
  • Loading branch information
StephenButtolph authored Jun 18, 2024
1 parent 576b392 commit 5d5b9cf
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 25 deletions.
3 changes: 0 additions & 3 deletions snow/consensus/snowman/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ type Consensus interface {
// Returns if a critical error has occurred.
Add(Block) error

// Decided returns true if the block has been decided.
Decided(Block) bool

// Processing returns true if the block ID is currently processing.
Processing(ids.ID) bool

Expand Down
4 changes: 0 additions & 4 deletions snow/consensus/snowman/consensus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ func StatusOrProcessingPreviouslyAcceptedTest(t *testing.T, factory Factory) {

require.Equal(choices.Accepted, snowmantest.Genesis.Status())
require.False(sm.Processing(snowmantest.Genesis.ID()))
require.True(sm.Decided(snowmantest.Genesis))
require.True(sm.IsPreferred(snowmantest.Genesis.ID()))

pref, ok := sm.PreferenceAtHeight(snowmantest.Genesis.Height())
Expand Down Expand Up @@ -329,7 +328,6 @@ func StatusOrProcessingPreviouslyRejectedTest(t *testing.T, factory Factory) {

require.Equal(choices.Rejected, block.Status())
require.False(sm.Processing(block.ID()))
require.True(sm.Decided(block))
require.False(sm.IsPreferred(block.ID()))

_, ok := sm.PreferenceAtHeight(block.Height())
Expand Down Expand Up @@ -365,7 +363,6 @@ func StatusOrProcessingUnissuedTest(t *testing.T, factory Factory) {

require.Equal(choices.Processing, block.Status())
require.False(sm.Processing(block.ID()))
require.False(sm.Decided(block))
require.False(sm.IsPreferred(block.ID()))

_, ok := sm.PreferenceAtHeight(block.Height())
Expand Down Expand Up @@ -402,7 +399,6 @@ func StatusOrProcessingIssuedTest(t *testing.T, factory Factory) {
require.NoError(sm.Add(block))
require.Equal(choices.Processing, block.Status())
require.True(sm.Processing(block.ID()))
require.False(sm.Decided(block))
require.True(sm.IsPreferred(block.ID()))

pref, ok := sm.PreferenceAtHeight(block.Height())
Expand Down
11 changes: 0 additions & 11 deletions snow/consensus/snowman/topological.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow"
"github.com/ava-labs/avalanchego/snow/choices"
"github.com/ava-labs/avalanchego/snow/consensus/snowball"
"github.com/ava-labs/avalanchego/utils/bag"
"github.com/ava-labs/avalanchego/utils/set"
Expand Down Expand Up @@ -182,16 +181,6 @@ func (ts *Topological) Add(blk Block) error {
return nil
}

func (ts *Topological) Decided(blk Block) bool {
// If the block is decided, then it must have been previously issued.
if blk.Status().Decided() {
return true
}
// If the block is marked as fetched, we can check if it has been
// transitively rejected.
return blk.Status() == choices.Processing && blk.Height() <= ts.lastAcceptedHeight
}

func (ts *Topological) Processing(blkID ids.ID) bool {
// The last accepted block is in the blocks map, so we first must ensure the
// requested block isn't the last accepted block.
Expand Down
26 changes: 19 additions & 7 deletions snow/engine/snowman/transitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ func (t *Transitive) issueFrom(
delete(t.blkReqSourceMetric, req)
}

issued := t.Consensus.Decided(blk) || t.Consensus.Processing(blkID)
issued := t.isDecided(blk) || t.Consensus.Processing(blkID)
if issued {
// A dependency should never be waiting on a decided or processing
// block. However, if the block was marked as rejected by the VM, the
Expand Down Expand Up @@ -794,7 +794,7 @@ func (t *Transitive) issueWithAncestors(
}

// The block was issued into consensus. This is the happy path.
if status != choices.Unknown && (t.Consensus.Decided(blk) || t.Consensus.Processing(blkID)) {
if status != choices.Unknown && (t.isDecided(blk) || t.Consensus.Processing(blkID)) {
return true, nil
}

Expand All @@ -815,7 +815,7 @@ func (t *Transitive) issueWithAncestors(
// If the block is queued to be added to consensus, then it was issued.
func (t *Transitive) wasIssued(blk snowman.Block) bool {
blkID := blk.ID()
return t.Consensus.Decided(blk) || t.Consensus.Processing(blkID) || t.pendingContains(blkID)
return t.isDecided(blk) || t.Consensus.Processing(blkID) || t.pendingContains(blkID)
}

// Issue [blk] to consensus once its ancestors have been issued.
Expand Down Expand Up @@ -849,7 +849,7 @@ func (t *Transitive) issue(

// block on the parent if needed
parentID := blk.Parent()
if parent, err := t.getBlock(ctx, parentID); err != nil || !(t.Consensus.Decided(parent) || t.Consensus.Processing(parentID)) {
if parent, err := t.getBlock(ctx, parentID); err != nil || !(t.isDecided(parent) || t.Consensus.Processing(parentID)) {
t.Ctx.Log.Verbo("block waiting for parent to be issued",
zap.Stringer("blkID", blkID),
zap.Stringer("parentID", parentID),
Expand Down Expand Up @@ -958,7 +958,7 @@ func (t *Transitive) deliver(
t.removeFromPending(blk)

blkID := blk.ID()
if t.Consensus.Decided(blk) || t.Consensus.Processing(blkID) {
if t.isDecided(blk) || t.Consensus.Processing(blkID) {
// If [blk] is decided, then it shouldn't be added to consensus.
// Similarly, if [blkID] is already in the processing set, it shouldn't
// be added to consensus again.
Expand Down Expand Up @@ -1073,7 +1073,7 @@ func (t *Transitive) removeFromPending(blk snowman.Block) {
func (t *Transitive) addToNonVerifieds(blk snowman.Block) {
// don't add this blk if it's decided or processing.
blkID := blk.ID()
if t.Consensus.Decided(blk) || t.Consensus.Processing(blkID) {
if t.isDecided(blk) || t.Consensus.Processing(blkID) {
return
}
parentID := blk.Parent()
Expand Down Expand Up @@ -1163,7 +1163,7 @@ func (t *Transitive) getProcessingAncestor(ctx context.Context, initialVote ids.
return ids.Empty, false
}

if t.Consensus.Decided(blk) {
if t.isDecided(blk) {
t.Ctx.Log.Debug("dropping vote",
zap.String("reason", "bubbled vote already decided"),
zap.Stringer("initialVoteID", initialVote),
Expand All @@ -1178,3 +1178,15 @@ func (t *Transitive) getProcessingAncestor(ctx context.Context, initialVote ids.
bubbledVote = blk.Parent()
}
}

// isDecided reports true if the provided block's status is Accepted, Rejected,
// or if the block's height implies that the block is either Accepted or
// Rejected.
func (t *Transitive) isDecided(blk snowman.Block) bool {
if blk.Status().Decided() {
return true
}

_, lastAcceptedHeight := t.Consensus.LastAccepted()
return blk.Height() <= lastAcceptedHeight
}

0 comments on commit 5d5b9cf

Please sign in to comment.