From 0a1d2b82ea71729677b05f46f7a103ea643e179e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20R=C3=B3=C5=BCa=C5=84ski?= Date: Mon, 12 Aug 2024 15:18:48 +0000 Subject: [PATCH] removed redudnant contextual validation in ATX V1 handler (#6243) The removed check served no purpose. The actual contextual validation is implemented in `StoreAtx()`. --- activation/handler_v1.go | 56 +------------------------ activation/handler_v1_test.go | 79 ----------------------------------- activation/handler_v2.go | 6 +-- 3 files changed, 5 insertions(+), 136 deletions(-) diff --git a/activation/handler_v1.go b/activation/handler_v1.go index 5a568477a4..c47a73f327 100644 --- 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 index 1ef2918573..b2a9f9bd5d 100644 --- 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 index 89a1f1d4ea..2f0ad6baab 100644 --- 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),