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

Jump to bottom

Backport 6243: removed redundant contextual validation in ATX V1 handler #6245

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Backport 6243: removed redundant contextual validation in ATX V1 handler #6245

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
56 changes: 2 additions & 54 deletions activation/handler_v1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,47 +320,6 @@ func (h *HandlerV1) validateNonInitialAtx(
return nil
}

// contextuallyValidateAtx ensures that the previous ATX referenced is the last known ATX for the referenced miner ID.
// If a previous ATX is not referenced, it validates that indeed there's no previous known ATX for that miner ID.
func (h *HandlerV1) contextuallyValidateAtx(atx *wire.ActivationTxV1) error {
lastAtx, err := atxs.GetLastIDByNodeID(h.cdb, atx.SmesherID)
if err == nil && atx.PrevATXID == lastAtx {
// last atx referenced equals last ATX seen from node
return nil
}

if err == nil && atx.PrevATXID == types.EmptyATXID {
// no previous atx declared, but already seen at least one atx from node
return fmt.Errorf(
"no prev atx reported, but other atx with same node id (%v) found: %v",
atx.SmesherID,
lastAtx.ShortString(),
)
}

if err == nil && atx.PrevATXID != lastAtx {
// last atx referenced does not equal last ATX seen from node
return errors.New("last atx is not the one referenced")
}

if errors.Is(err, sql.ErrNotFound) && atx.PrevATXID == types.EmptyATXID {
// no previous atx found and none referenced
return nil
}

if err != nil && atx.PrevATXID != types.EmptyATXID {
// no previous atx found but previous atx referenced
h.logger.Error("could not fetch node last atx",
zap.Stringer("atx_id", atx.ID()),
zap.Stringer("smesher", atx.SmesherID),
zap.Error(err),
)
return fmt.Errorf("could not fetch node last atx: %w", err)
}

return err
}

// cacheAtx caches the atx in the atxsdata cache.
// Returns true if the atx was cached, false otherwise.
func (h *HandlerV1) cacheAtx(ctx context.Context, atx *types.ActivationTx) *atxsdata.ATX {
Expand Down Expand Up @@ -433,7 +392,7 @@ func (h *HandlerV1) checkDoublePublish(
return nil, fmt.Errorf("add malfeasance proof: %w", err)
}

h.logger.Warn("smesher produced more than one atx in the same epoch",
h.logger.Debug("smesher produced more than one atx in the same epoch",
log.ZContext(ctx),
zap.Stringer("smesher", atx.SmesherID),
zap.Stringer("previous", prev),
Expand Down Expand Up @@ -545,7 +504,7 @@ func (h *HandlerV1) checkWrongPrevAtx(
return nil, fmt.Errorf("add malfeasance proof: %w", err)
}

h.logger.Warn("smesher referenced the wrong previous in published ATX",
h.logger.Debug("smesher referenced the wrong previous in published ATX",
log.ZContext(ctx),
zap.Stringer("smesher", atx.SmesherID),
log.ZShortStringer("expected", prevID),
Expand Down Expand Up @@ -656,17 +615,6 @@ func (h *HandlerV1) processATX(
return proof, nil
}

if err := h.contextuallyValidateAtx(watx); err != nil {
h.logger.Warn("atx is contextually invalid ",
log.ZContext(ctx),
zap.Stringer("atx_id", watx.ID()),
zap.Stringer("smesherID", watx.SmesherID),
zap.Error(err),
)
} else {
h.logger.Debug("atx is valid", zap.Stringer("atx_id", watx.ID()))
}

var baseTickHeight uint64
if watx.PositioningATXID != h.goldenATXID {
posAtx, err := h.cdb.GetAtx(watx.PositioningATXID)
Expand Down
79 changes: 0 additions & 79 deletions activation/handler_v1_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,85 +454,6 @@ func TestHandlerV1_SyntacticallyValidateAtx(t *testing.T) {
})
}

func TestHandler_ContextuallyValidateAtx(t *testing.T) {
goldenATXID := types.ATXID{2, 3, 4}

sig, err := signing.NewEdSigner()
require.NoError(t, err)

t.Run("valid initial atx", func(t *testing.T) {
t.Parallel()

atx := newInitialATXv1(t, goldenATXID)
atx.Sign(sig)

atxHdlr := newV1TestHandler(t, goldenATXID)
require.NoError(t, atxHdlr.contextuallyValidateAtx(atx))
})

t.Run("missing prevAtx", func(t *testing.T) {
t.Parallel()

atxHdlr := newV1TestHandler(t, goldenATXID)

prevAtx := newInitialATXv1(t, goldenATXID)
atx := newChainedActivationTxV1(t, prevAtx, goldenATXID)

err = atxHdlr.contextuallyValidateAtx(atx)
require.ErrorIs(t, err, sql.ErrNotFound)
})

t.Run("wrong previous atx by same node", func(t *testing.T) {
t.Parallel()

atxHdlr := newV1TestHandler(t, goldenATXID)

atx0 := newInitialATXv1(t, goldenATXID)
atx0.Sign(sig)
atxHdlr.expectAtxV1(atx0, sig.NodeID())
_, err := atxHdlr.processATX(context.Background(), "", atx0, codec.MustEncode(atx0), time.Now())
require.NoError(t, err)

atx1 := newChainedActivationTxV1(t, atx0, goldenATXID)
atx1.Sign(sig)
atxHdlr.expectAtxV1(atx1, sig.NodeID())
atxHdlr.mockFetch.EXPECT().GetAtxs(gomock.Any(), gomock.Any(), gomock.Any())
_, err = atxHdlr.processATX(context.Background(), "", atx1, codec.MustEncode(atx1), time.Now())
require.NoError(t, err)

atxInvalidPrevious := newChainedActivationTxV1(t, atx0, goldenATXID)
atxInvalidPrevious.Sign(sig)
err = atxHdlr.contextuallyValidateAtx(atxInvalidPrevious)
require.EqualError(t, err, "last atx is not the one referenced")
})

t.Run("wrong previous atx from different node", func(t *testing.T) {
t.Parallel()

otherSig, err := signing.NewEdSigner()
require.NoError(t, err)

atxHdlr := newV1TestHandler(t, goldenATXID)

atx0 := newInitialATXv1(t, goldenATXID)
atx0.Sign(otherSig)
atxHdlr.expectAtxV1(atx0, otherSig.NodeID())
_, err = atxHdlr.processATX(context.Background(), "", atx0, codec.MustEncode(atx0), time.Now())
require.NoError(t, err)

atx1 := newInitialATXv1(t, goldenATXID)
atx1.Sign(sig)
atxHdlr.expectAtxV1(atx1, sig.NodeID())
_, err = atxHdlr.processATX(context.Background(), "", atx1, codec.MustEncode(atx1), time.Now())
require.NoError(t, err)

atxInvalidPrevious := newChainedActivationTxV1(t, atx0, goldenATXID)
atxInvalidPrevious.Sign(sig)
err = atxHdlr.contextuallyValidateAtx(atxInvalidPrevious)
require.EqualError(t, err, "last atx is not the one referenced")
})
}

func TestHandlerV1_StoreAtx(t *testing.T) {
goldenATXID := types.RandomATXID()

Expand Down
6 changes: 3 additions & 3 deletions activation/handler_v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,9 +520,9 @@
post.NumUnits,
PostSubset([]byte(h.local)),
)
var invalidIdx *verifying.ErrInvalidIndex
if errors.As(err, &invalidIdx) {
h.logger.Info(
invalidIdx := &verifying.ErrInvalidIndex{}
if errors.As(err, invalidIdx) {
h.logger.Debug(

Check warning on line 525 in activation/handler_v2.go

View check run for this annotation

Codecov / codecov/patch

activation/handler_v2.go#L525 

Added line #L525 was not covered by tests
"ATX with invalid post index",
zap.Stringer("id", atx.ID()),
zap.Int("index", invalidIdx.Index),
Expand Down
Loading