Skip to content

Commit

Permalink
sync: make sure data is available before checking votes (#3256)
Browse files Browse the repository at this point in the history 
## Motivation
<!-- Please mention the issue fixed by this PR or detailed motivation -->
Closes #3255
<!-- `Closes #XXXX, closes #XXXX, ...` links mentioned issues to this PR and automatically closes them when this it's merged -->

## Changes
<!-- Please describe in detail the changes made -->
make sure handler downloads the ballot data before checking for votes consistency

## Test Plan
<!-- Please specify how these changes were tested 
(e.g. unit tests, manual testing, etc.) -->
unit test

## DevOps Notes
<!-- Please uncheck these items as applicable to make DevOps aware of changes that may affect releases -->
- [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
- [x] This PR does not affect public APIs
- [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
- [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)
  • Loading branch information
@countvonzero
countvonzero committed Jun 13, 2022
1 parent 3613b9f commit 35ab2c2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 20 deletions.
15 changes: 7 additions & 8 deletions proposals/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@ func (h *Handler) checkBallotSyntacticValidity(ctx context.Context, b *types.Bal
return err
}

if err := h.checkVotesConsistency(ctx, b); err != nil {
h.logger.WithContext(ctx).With().Warning("ballot votes consistency check failed", log.Err(err))
return err
}

if eligible, err := h.validator.CheckEligibility(ctx, b); err != nil || !eligible {
h.logger.WithContext(ctx).With().Warning("ballot eligibility check failed", log.Err(err))
return errNotEligible
Expand Down Expand Up @@ -271,14 +276,8 @@ func (h *Handler) checkBallotDataIntegrity(ctx context.Context, b *types.Ballot)
}
set[atx] = struct{}{}
}
} else {
if b.EpochData != nil {
return errUnexpectedEpochData
}
}

if err := h.checkVotesConsistency(ctx, b); err != nil {
return err
} else if b.EpochData != nil {
return errUnexpectedEpochData
}
return nil
}
Expand Down
36 changes: 24 additions & 12 deletions proposals/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ func TestBallot_BallotDoubleVotedWithinHdist(t *testing.T) {
require.GreaterOrEqual(t, 2, len(b.Votes.Support))
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), b.Votes.Support).Return(nil).Times(1)
cutoff := b.LayerIndex.Sub(th.cfg.Hdist)
th.mm.EXPECT().GetBlockLayer(b.Votes.Support[0]).Return(cutoff.Add(1), nil)
th.mm.EXPECT().GetBlockLayer(b.Votes.Support[1]).Return(cutoff.Add(1), nil)
Expand All @@ -245,6 +248,9 @@ func TestBallot_BallotDoubleVotedWithinHdist_LyrBfrHdist(t *testing.T) {
require.GreaterOrEqual(t, 2, len(b.Votes.Support))
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), b.Votes.Support).Return(nil).Times(1)
th.mm.EXPECT().GetBlockLayer(b.Votes.Support[0]).Return(b.LayerIndex.Sub(1), nil)
th.mm.EXPECT().GetBlockLayer(b.Votes.Support[1]).Return(b.LayerIndex.Sub(1), nil)
th.mm.EXPECT().SetIdentityMalicious(b.SmesherID()).Return(nil)
Expand All @@ -257,6 +263,9 @@ func TestBallot_BallotDoubleVotedWithinHdist_SetMaliciousError(t *testing.T) {
require.GreaterOrEqual(t, 2, len(b.Votes.Support))
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), b.Votes.Support).Return(nil).Times(1)
cutoff := b.LayerIndex.Sub(th.cfg.Hdist)
th.mm.EXPECT().GetBlockLayer(b.Votes.Support[0]).Return(cutoff, nil)
th.mm.EXPECT().GetBlockLayer(b.Votes.Support[1]).Return(cutoff, nil)
Expand All @@ -270,13 +279,13 @@ func TestBallot_BallotDoubleVotedOutsideHdist(t *testing.T) {
b := createBallot(t)
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), b.Votes.Support).Return(nil).Times(1)
cutoff := b.LayerIndex.Sub(th.cfg.Hdist)
for _, bid := range b.Votes.Support {
th.mm.EXPECT().GetBlockLayer(bid).Return(cutoff.Sub(1), nil)
}
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), b.Votes.Support).Return(nil).Times(1)
th.mv.EXPECT().CheckEligibility(gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, ballot *types.Ballot) (bool, error) {
require.Equal(t, b.ID(), ballot.ID())
Expand All @@ -297,6 +306,9 @@ func TestBallot_ConflictingForAndAgainst(t *testing.T) {
b = signAndInit(t, b)
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), append(b.Votes.Support, b.Votes.Against...)).Return(nil).Times(1)
for i, bid := range b.Votes.Support {
th.mm.EXPECT().GetBlockLayer(bid).Return(b.LayerIndex.Sub(uint32(i+1)), nil)
}
Expand All @@ -310,6 +322,9 @@ func TestBallot_ConflictingForAndAbstain(t *testing.T) {
b = signAndInit(t, b)
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), b.Votes.Support).Return(nil).Times(1)
for i, bid := range b.Votes.Support {
th.mm.EXPECT().GetBlockLayer(bid).Return(b.LayerIndex.Sub(uint32(i+1)), nil)
}
Expand All @@ -326,6 +341,9 @@ func TestBallot_ConflictingAgainstAndAbstain(t *testing.T) {
b = signAndInit(t, b)
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), append(b.Votes.Support, b.Votes.Against...)).Return(nil).Times(1)
for i, bid := range b.Votes.Against {
th.mm.EXPECT().GetBlockLayer(bid).Return(b.LayerIndex.Sub(uint32(i+1)), nil)
}
Expand All @@ -340,6 +358,9 @@ func TestBallot_ExceedMaxExceptions(t *testing.T) {
b = signAndInit(t, b)
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
th.mf.EXPECT().GetBlocks(gomock.Any(), b.Votes.Support).Return(nil).Times(1)
for i, bid := range b.Votes.Support {
th.mm.EXPECT().GetBlockLayer(bid).Return(b.LayerIndex.Sub(uint32(i+1)), nil)
}
Expand All @@ -351,9 +372,6 @@ func TestBallot_BallotsNotAvailable(t *testing.T) {
b := createBallot(t)
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
for i, bid := range b.Votes.Support {
th.mm.EXPECT().GetBlockLayer(bid).Return(b.LayerIndex.Sub(uint32(i+1)), nil)
}
errUnknown := errors.New("unknown")
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(errUnknown).Times(1)
require.ErrorIs(t, th.HandleBallotData(context.TODO(), data), errUnknown)
Expand All @@ -364,9 +382,6 @@ func TestBallot_ATXsNotAvailable(t *testing.T) {
b := createBallot(t)
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
for i, bid := range b.Votes.Support {
th.mm.EXPECT().GetBlockLayer(bid).Return(b.LayerIndex.Sub(uint32(i+1)), nil)
}
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
errUnknown := errors.New("unknown")
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(errUnknown).Times(1)
Expand All @@ -378,9 +393,6 @@ func TestBallot_BlocksNotAvailable(t *testing.T) {
b := createBallot(t)
data := encodeBallot(t, b)
th.mm.EXPECT().HasBallot(b.ID()).Return(false).Times(1)
for i, bid := range b.Votes.Support {
th.mm.EXPECT().GetBlockLayer(bid).Return(b.LayerIndex.Sub(uint32(i+1)), nil)
}
th.mf.EXPECT().GetBallots(gomock.Any(), []types.BallotID{b.Votes.Base, b.RefBallot}).Return(nil).Times(1)
th.mf.EXPECT().GetAtxs(gomock.Any(), types.ATXIDList{b.AtxID}).Return(nil).Times(1)
errUnknown := errors.New("unknown")
Expand Down

0 comments on commit 35ab2c2

Please sign in to comment.