Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: chainstore: do not get stuck in unhappy equivocation cases #11159

Merged
merged 1 commit into from
Aug 15, 2023
Merged

fix: chainstore: do not get stuck in unhappy equivocation cases #11159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions chain/store/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,13 +446,15 @@ func (cs *ChainStore) RefreshHeaviestTipSet(ctx context.Context, newTsHeight abi
}
}

if newHeaviest == nil {
return xerrors.Errorf("failed to refresh to a new valid tipset")
}

errTake := cs.takeHeaviestTipSet(ctx, newHeaviest)
if errTake != nil {
return xerrors.Errorf("failed to take newHeaviest tipset as head: %w", err)
// if we've found something, we know it's the heaviest equivocation-free head, take it IMMEDIATELY
if newHeaviest != nil {
errTake := cs.takeHeaviestTipSet(ctx, newHeaviest)
if errTake != nil {
return xerrors.Errorf("failed to take newHeaviest tipset as head: %w", err)
}
} else {
// if we haven't found something, just stay with our equivocation-y head
newHeaviest = cs.heaviest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason not to just return nil here and move on?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- we still want to try and form a tipset at the new height we've been notified about, and take that if it's better?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm convinced.

}
}

Expand Down
5 changes: 3 additions & 2 deletions chain/store/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,9 @@ func TestEquivocations(t *testing.T) {
require.Nil(t, tryTs2)
require.True(t, tryTsWeight2.IsZero())

// now we "notify" at this height -- it should fail, because we cannot refresh our head due to equivocation and nulls
require.ErrorContains(t, cg.ChainStore().RefreshHeaviestTipSet(ctx, blkAfterNulls.Height), "failed to refresh to a new valid tipset")
// now we "notify" at this height -- it should lead to no head change because there's no formable head in near epochs
require.NoError(t, cg.ChainStore().RefreshHeaviestTipSet(ctx, blkAfterNulls.Height))
require.True(t, headAfterNulls.TipSet.TipSet().Equals(cg.ChainStore().GetHeaviestTipSet()))
}

func addBlockToTracker(t *testing.T, cs *store.ChainStore, blk *types.BlockHeader) {
Expand Down