removed redudnant contextual validation in ATX V1 handler (#6243) (#6245

) The removed check served no purpose. The actual contextual validation is implemented in `StoreAtx()`. Co-authored-by: Bartosz Różański <[email protected]>
spacemeshos · Aug 12, 2024 · e1b52c2 · e1b52c2
1 parent 8d594d0
commit e1b52c2
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 136 deletions.
diff --git a/activation/handler_v1.go b/activation/handler_v1.go
@@ -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 {
@@ -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),
@@ -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),
@@ -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)

diff --git a/activation/handler_v1_test.go b/activation/handler_v1_test.go
@@ -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()
 

diff --git a/activation/handler_v2.go b/activation/handler_v2.go
@@ -520,9 +520,9 @@ func (h *HandlerV2) syntacticallyValidateDeps(
 				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(
 					"ATX with invalid post index",
 					zap.Stringer("id", atx.ID()),
 					zap.Int("index", invalidIdx.Index),