[Merged by Bors] - Cleanup code and changelog

CHANGELOG.md

@@ -15,6 +15,13 @@ such and the node will continue to scan new ATXs for their validity.
 
 ### Improvements
 
+## Release v1.6.6-hotfix1
+
+### Improvements
+
+* [#6248](https://github.com/spacemeshos/go-spacemesh/pull/6248) Fixed node not being able to handle more than 6.55M
+  ATXs per epoch.
+
 ## Release v1.6.6
 
 ### Improvements

activation/handler_v2.go

@@ -32,6 +32,8 @@
 	"github.com/spacemeshos/go-spacemesh/system"
 )
 
+var errAtxNotV2 = errors.New("ATX is not V2")
+
 type nipostValidatorV2 interface {
 	IsVerifyingFullPost() bool
 	VRFNonceV2(smesherID types.NodeID, commitment types.ATXID, vrfNonce uint64, numUnits uint32) error
@@ -666,88 +668,95 @@
 	return &result, nil
 }
 
-func (h *HandlerV2) checkMalicious(ctx context.Context, tx *sql.Tx, atx *activationTx) error {
+func (h *HandlerV2) checkMalicious(ctx context.Context, tx *sql.Tx, atx *activationTx) (bool, error) {
 	malicious, err := identities.IsMalicious(tx, atx.SmesherID)
 	if err != nil {
-		return fmt.Errorf("checking if node is malicious: %w", err)
+		return malicious, fmt.Errorf("checking if node is malicious: %w", err)
 	}
 	if malicious {
-		return nil
+		return true, nil
 	}
 
 	malicious, err = h.checkDoubleMarry(ctx, tx, atx)
 	if err != nil {
-		return fmt.Errorf("checking double marry: %w", err)
+		return malicious, fmt.Errorf("checking double marry: %w", err)
 	}
 	if malicious {
-		return nil
+		return true, nil
 	}
 
 	malicious, err = h.checkDoublePost(ctx, tx, atx)
 	if err != nil {
-		return fmt.Errorf("checking double post: %w", err)
+		return malicious, fmt.Errorf("checking double post: %w", err)
 	}
 	if malicious {
-		return nil
+		return true, nil
 	}
 
 	malicious, err = h.checkDoubleMerge(ctx, tx, atx)
 	if err != nil {
-		return fmt.Errorf("checking double merge: %w", err)
-	}
-	if malicious {
-		return nil
+		return malicious, fmt.Errorf("checking double merge: %w", err)
 	}
 
-	// TODO(mafa): contextual validation:
-	// 1. check double-publish = ID contributed post to two ATXs in the same epoch
-	// 2. check previous ATX
-	// 3  ID already married (same node ID in multiple marriage certificates)
-	// 4. two ATXs referencing the same marriage certificate in the same epoch
-	return nil
+	// TODO(mafa): missing contextual validation for invalid previous ATX
+
+	return malicious, err
+}
+
+func (h *HandlerV2) fetchWireAtx(ctx context.Context, tx *sql.Tx, id types.ATXID) (*wire.ActivationTxV2, error) {
+	var blob sql.Blob
+	v, err := atxs.LoadBlob(ctx, tx, id.Bytes(), &blob)
+	if err != nil {
+		return nil, fmt.Errorf("get atx blob %s: %w", id.ShortString(), err)
+	}
+	if v != types.AtxV2 {
+		return nil, errAtxNotV2
+	}
+	atx := &wire.ActivationTxV2{}
+	codec.MustDecode(blob.Bytes, atx)
+	return atx, nil
 }
 
 func (h *HandlerV2) checkDoubleMarry(ctx context.Context, tx *sql.Tx, atx *activationTx) (bool, error) {
 	for _, m := range atx.marriages {
-		mATX, err := identities.MarriageATX(tx, m.id)
+		mATXID, err := identities.MarriageATX(tx, m.id)
 		if err != nil {
 			return false, fmt.Errorf("checking if ID is married: %w", err)
 		}
-		if mATX != atx.ID() {
-			var blob sql.Blob
-			v, err := atxs.LoadBlob(ctx, tx, mATX.Bytes(), &blob)
-			if err != nil {
-				return true, fmt.Errorf("creating double marry proof: %w", err)
-			}
-			if v != types.AtxV2 {
-				h.logger.Fatal("Failed to create double marry malfeasance proof: ATX is not v2",
-					zap.Stringer("atx_id", mATX),
-				)
-			}
-			var otherAtx wire.ActivationTxV2
-			codec.MustDecode(blob.Bytes, &otherAtx)
+		if mATXID == atx.ID() {
+			continue
+		}
 
-			proof, err := wire.NewDoubleMarryProof(tx, atx.ActivationTxV2, &otherAtx, m.id)
-			if err != nil {
-				return true, fmt.Errorf("creating double marry proof: %w", err)
-			}
-			return true, h.malPublisher.Publish(ctx, m.id, proof)
+		otherAtx, err := h.fetchWireAtx(ctx, tx, mATXID)
+		switch {
+		case errors.Is(err, errAtxNotV2):
+			h.logger.Fatal("Failed to create double marry malfeasance proof: ATX is not v2",
+				zap.Stringer("atx_id", mATXID),
+			)
+		case err != nil:
+			return false, fmt.Errorf("fetching other ATX: %w", err)
 		}
+
+		proof, err := wire.NewDoubleMarryProof(tx, atx.ActivationTxV2, otherAtx, m.id)
+		if err != nil {
+			return true, fmt.Errorf("creating double marry proof: %w", err)
+		}
+		return true, h.malPublisher.Publish(ctx, m.id, proof)
 	}
 	return false, nil
 }
 
 func (h *HandlerV2) checkDoublePost(ctx context.Context, tx *sql.Tx, atx *activationTx) (bool, error) {
 	for id := range atx.ids {
-		atxids, err := atxs.FindDoublePublish(tx, id, atx.PublishEpoch)
+		atxIDs, err := atxs.FindDoublePublish(tx, id, atx.PublishEpoch)
 		switch {
 		case errors.Is(err, sql.ErrNotFound):
 			continue
 		case err != nil:
 			return false, fmt.Errorf("searching for double publish: %w", err)
 		}
-		otherAtxId := slices.IndexFunc(atxids, func(other types.ATXID) bool { return other != atx.ID() })
-		otherAtx := atxids[otherAtxId]
+		otherAtxId := slices.IndexFunc(atxIDs, func(other types.ATXID) bool { return other != atx.ID() })
+		otherAtx := atxIDs[otherAtxId]
 		h.logger.Debug(
 			"found ID that has already contributed its PoST in this epoch",
 			zap.Stringer("node_id", id),
@@ -821,22 +830,16 @@
 
 	atxs.AtxAdded(h.cdb, atx)
 
-	var malicious bool
+	malicious := false
 	err := h.cdb.WithTx(ctx, func(tx *sql.Tx) error {
 		// malfeasance check happens after storing the ATX because storing updates the marriage set
 		// that is needed for the malfeasance proof
 		// TODO(mafa): don't store own ATX if it would mark the node as malicious
 		//    this probably needs to be done by validating and storing own ATXs eagerly and skipping validation in
 		//    the gossip handler (not sync!)
-		err := h.checkMalicious(ctx, tx, watx)
-		if err != nil {
-			return fmt.Errorf("check malicious: %w", err)
-		}
-		malicious, err = identities.IsMalicious(tx, watx.SmesherID)
-		if err != nil {
-			return fmt.Errorf("checking if identity is malicious: %w", err)
-		}
-		return nil
+		var err error
+		malicious, err = h.checkMalicious(ctx, tx, watx)
+		return err
 	})
 	if err != nil {
 		return fmt.Errorf("check malicious: %w", err)

activation/wire/malfeasance_double_marry_test.go

@@ -21,7 +21,7 @@ func Test_DoubleMarryProof(t *testing.T) {
 	require.NoError(t, err)
 
 	t.Run("valid", func(t *testing.T) {
-		db := sql.InMemory()
+		db := sql.InMemoryTest(t)
 		otherAtx := &types.ActivationTx{}
 		otherAtx.SetID(types.RandomATXID())
 		otherAtx.SmesherID = otherSig.NodeID()
@@ -50,7 +50,7 @@ func Test_DoubleMarryProof(t *testing.T) {
 	})
 
 	t.Run("does not contain same certificate owner", func(t *testing.T) {
-		db := sql.InMemory()
+		db := sql.InMemoryTest(t)
 
 		atx1 := newActivationTxV2(
 			withMarriageCertificate(sig, types.EmptyATXID, sig.NodeID()),
@@ -79,7 +79,7 @@ func Test_DoubleMarryProof(t *testing.T) {
 		atx1 := newActivationTxV2()
 		atx1.Sign(sig)
 
-		db := sql.InMemory()
+		db := sql.InMemoryTest(t)
 		proof, err := NewDoubleMarryProof(db, atx1, atx1, sig.NodeID())
 		require.ErrorContains(t, err, "ATXs have the same ID")
 		require.Nil(t, proof)
@@ -103,7 +103,7 @@ func Test_DoubleMarryProof(t *testing.T) {
 	})
 
 	t.Run("invalid marriage proof", func(t *testing.T) {
-		db := sql.InMemory()
+		db := sql.InMemoryTest(t)
 		otherAtx := &types.ActivationTx{}
 		otherAtx.SetID(types.RandomATXID())
 		otherAtx.SmesherID = otherSig.NodeID()
@@ -150,7 +150,7 @@ func Test_DoubleMarryProof(t *testing.T) {
 	})
 
 	t.Run("invalid certificate proof", func(t *testing.T) {
-		db := sql.InMemory()
+		db := sql.InMemoryTest(t)
 		otherAtx := &types.ActivationTx{}
 		otherAtx.SetID(types.RandomATXID())
 		otherAtx.SmesherID = otherSig.NodeID()
@@ -197,7 +197,7 @@ func Test_DoubleMarryProof(t *testing.T) {
 	})
 
 	t.Run("invalid atx signature", func(t *testing.T) {
-		db := sql.InMemory()
+		db := sql.InMemoryTest(t)
 		otherAtx := &types.ActivationTx{}
 		otherAtx.SetID(types.RandomATXID())
 		otherAtx.SmesherID = otherSig.NodeID()
@@ -233,7 +233,7 @@ func Test_DoubleMarryProof(t *testing.T) {
 	})
 
 	t.Run("invalid certificate signature", func(t *testing.T) {
-		db := sql.InMemory()
+		db := sql.InMemoryTest(t)
 		otherAtx := &types.ActivationTx{}
 		otherAtx.SetID(types.RandomATXID())
 		otherAtx.SmesherID = otherSig.NodeID()
@@ -269,7 +269,7 @@ func Test_DoubleMarryProof(t *testing.T) {
 	})
 
 	t.Run("unknown reference ATX", func(t *testing.T) {
-		db := sql.InMemory()
+		db := sql.InMemoryTest(t)
 
 		atx1 := newActivationTxV2(
 			withMarriageCertificate(sig, types.EmptyATXID, sig.NodeID()),

hare3/hare.go

@@ -131,7 +131,7 @@ type Opt func(*Hare)
 
 func WithWallclock(clock clockwork.Clock) Opt {
 	return func(hr *Hare) {
-		hr.wallclock = clock
+		hr.wallClock = clock
 	}
 }
 
@@ -163,15 +163,15 @@ func WithResultsChan(c chan hare4.ConsensusOutput) Opt {
 	}
 }
 
-type nodeclock interface {
+type nodeClock interface {
 	AwaitLayer(types.LayerID) <-chan struct{}
 	CurrentLayer() types.LayerID
 	LayerToTime(types.LayerID) time.Time
 }
 
 func New(
-	nodeclock nodeclock,
-	pubsub pubsub.PublishSubsciber,
+	nodeClock nodeClock,
+	pubsub pubsub.PublishSubscriber,
 	db *sql.Database,
 	atxsdata *atxsdata.Data,
 	proposals *store.Store,
@@ -192,9 +192,9 @@ func New(
 
 		config:    DefaultConfig(),
 		log:       zap.NewNop(),
-		wallclock: clockwork.NewRealClock(),
+		wallClock: clockwork.NewRealClock(),
 
-		nodeclock: nodeclock,
+		nodeClock: nodeClock,
 		pubsub:    pubsub,
 		db:        db,
 		atxsdata:  atxsdata,
@@ -229,11 +229,11 @@ type Hare struct {
 	// options
 	config    Config
 	log       *zap.Logger
-	wallclock clockwork.Clock
+	wallClock clockwork.Clock
 
 	// dependencies
-	nodeclock nodeclock
-	pubsub    pubsub.PublishSubsciber
+	nodeClock nodeClock
+	pubsub    pubsub.PublishSubscriber
 	db        *sql.Database
 	atxsdata  *atxsdata.Data
 	proposals *store.Store
@@ -261,7 +261,7 @@ func (h *Hare) Coins() <-chan hare4.WeakCoinOutput {
 
 func (h *Hare) Start() {
 	h.pubsub.Register(h.config.ProtocolName, h.Handler, pubsub.WithValidatorInline(true))
-	current := h.nodeclock.CurrentLayer() + 1
+	current := h.nodeClock.CurrentLayer() + 1
 	enabled := max(current, h.config.EnableLayer, types.GetEffectiveGenesis()+1)
 	disabled := types.LayerID(math.MaxUint32)
 	if h.config.DisableLayer > 0 {
@@ -275,7 +275,7 @@ func (h *Hare) Start() {
 	h.eg.Go(func() error {
 		for next := enabled; next < disabled; next++ {
 			select {
-			case <-h.nodeclock.AwaitLayer(next):
+			case <-h.nodeClock.AwaitLayer(next):
 				h.log.Debug("notified", zap.Uint32("lid", next.Uint32()))
 				h.onLayer(next)
 			case <-h.ctx.Done():
@@ -349,7 +349,7 @@ func (h *Hare) Handler(ctx context.Context, peer p2p.Peer, buf []byte) error {
 		droppedMessages.Inc()
 		return errors.New("dropped by graded gossip")
 	}
-	expected := h.nodeclock.LayerToTime(msg.Layer).Add(h.config.roundStart(msg.IterRound))
+	expected := h.nodeClock.LayerToTime(msg.Layer).Add(h.config.roundStart(msg.IterRound))
 	metrics.ReportMessageLatency(h.config.ProtocolName, msg.Round.String(), time.Since(expected))
 	return nil
 }
@@ -426,12 +426,12 @@ func (h *Hare) run(session *session) error {
 	h.tracer.OnActive(session.vrfs)
 	activeLatency.Observe(time.Since(start).Seconds())
 
-	walltime := h.nodeclock.LayerToTime(session.lid).Add(h.config.PreroundDelay)
+	walltime := h.nodeClock.LayerToTime(session.lid).Add(h.config.PreroundDelay)
 	if active {
 		h.log.Debug("active in preround. waiting for preround delay", zap.Uint32("lid", session.lid.Uint32()))
 		// initial set is not needed if node is not active in preround
 		select {
-		case <-h.wallclock.After(walltime.Sub(h.wallclock.Now())):
+		case <-h.wallClock.After(walltime.Sub(h.wallClock.Now())):
 		case <-h.ctx.Done():
 			return h.ctx.Err()
 		}
@@ -459,7 +459,7 @@ func (h *Hare) run(session *session) error {
 		activeLatency.Observe(time.Since(start).Seconds())
 
 		select {
-		case <-h.wallclock.After(walltime.Sub(h.wallclock.Now())):
+		case <-h.wallClock.After(walltime.Sub(h.wallClock.Now())):
 			h.log.Debug("execute round",
 				zap.Uint32("lid", session.lid.Uint32()),
 				zap.Uint8("iter", session.proto.Iter), zap.Stringer("round", session.proto.Round),

hare3/hare_test.go

@@ -121,7 +121,7 @@ type node struct {
 	proposals  *store.Store
 
 	ctrl       *gomock.Controller
-	mpublisher *pmocks.MockPublishSubsciber
+	mpublisher *pmocks.MockPublishSubscriber
 	msyncer    *smocks.MockSyncStateProvider
 	patrol     *layerpatrol.LayerPatrol
 	tracer     *testTracer
@@ -203,7 +203,7 @@ func (n *node) withOracle() *node {
 }
 
 func (n *node) withPublisher() *node {
-	n.mpublisher = pmocks.NewMockPublishSubsciber(n.ctrl)
+	n.mpublisher = pmocks.NewMockPublishSubscriber(n.ctrl)
 	n.mpublisher.EXPECT().Register(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
 	return n
 }