Skip to content

Commit

Permalink
removed redudnant contextual validation in ATX V1 handler (#6243)
Browse files Browse the repository at this point in the history
The removed check served no purpose. The actual contextual validation is implemented in `StoreAtx()`.
  • Loading branch information
poszu authored and fasmat committed Aug 12, 2024
1 parent 8d594d0 commit 0a1d2b8
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 136 deletions.
56 changes: 2 additions & 54 deletions activation/handler_v1.go
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -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(

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

0 comments on commit 0a1d2b8

Please sign in to comment.