Skip to content

Commit 7365a96

Browse files
poszuposzu
authored
Merge pull request #6705 from spacemeshos/node-split-poc-avoid-self-ban
Avoid self-banning when smeshing-service publishes invalid ATX
2 parents 6712378 + a21cfea commit 7365a96

File tree

4 files changed

+50
-56
lines changed

4 files changed

+50
-56
lines changed

‎activation/handler.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ func NewHandler(
128128
tortoise: tortoise,
129129
malPublisher: legacyMalPublisher,
130130
malPublisher2: malPublisher,
131-
signers: make(map[types.NodeID]*signing.EdSigner),
132131
},
133132

134133
v2: &HandlerV2{
@@ -163,10 +162,6 @@ func NewHandler(
163162
return h
164163
}
165164

166-
func (h *Handler) Register(sig *signing.EdSigner) {
167-
h.v1.Register(sig)
168-
}
169-
170165
// HandleSyncedAtx handles atxs received by sync.
171166
func (h *Handler) HandleSyncedAtx(ctx context.Context, expHash types.Hash32, peer p2p.Peer, data []byte) error {
172167
err := h.handleAtx(ctx, expHash, peer, data)

‎activation/handler_v1.go‎

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import (
55
"errors"
66
"fmt"
77
"math/bits"
8-
"sync"
98
"time"
109

10+
"github.com/libp2p/go-libp2p/core/peer"
1111
"github.com/spacemeshos/post/shared"
1212
"github.com/spacemeshos/post/verifying"
1313
"go.uber.org/zap"
@@ -78,21 +78,6 @@ type HandlerV1 struct {
7878
fetcher system.Fetcher
7979
malPublisher legacyMalfeasancePublisher
8080
malPublisher2 atxMalfeasancePublisher
81-
82-
signerMtx sync.Mutex
83-
signers map[types.NodeID]*signing.EdSigner
84-
}
85-
86-
func (h *HandlerV1) Register(sig *signing.EdSigner) {
87-
h.signerMtx.Lock()
88-
defer h.signerMtx.Unlock()
89-
if _, exists := h.signers[sig.NodeID()]; exists {
90-
h.logger.Error("signing key already registered", log.ZShortStringer("id", sig.NodeID()))
91-
return
92-
}
93-
94-
h.logger.Info("registered signing key", log.ZShortStringer("id", sig.NodeID()))
95-
h.signers[sig.NodeID()] = sig
9681
}
9782

9883
func (h *HandlerV1) syntacticallyValidate(ctx context.Context, atx *wire.ActivationTxV1) error {
@@ -337,7 +322,12 @@ func (h *HandlerV1) cacheAtx(ctx context.Context, atx *types.ActivationTx, malic
337322
}
338323

339324
// checkDoublePublish verifies if a node has already published an ATX in the same epoch.
340-
func (h *HandlerV1) checkDoublePublish(ctx context.Context, tx sql.Executor, atx *wire.ActivationTxV1) (bool, error) {
325+
func (h *HandlerV1) checkDoublePublish(
326+
ctx context.Context,
327+
tx sql.Executor,
328+
atx *wire.ActivationTxV1,
329+
peer peer.ID,
330+
) (bool, error) {
341331
prev, err := atxs.GetByEpochAndNodeID(tx, atx.PublishEpoch, atx.SmesherID)
342332
if err != nil && !errors.Is(err, sql.ErrNotFound) {
343333
return false, err
@@ -347,7 +337,7 @@ func (h *HandlerV1) checkDoublePublish(ctx context.Context, tx sql.Executor, atx
347337
return false, nil
348338
}
349339

350-
if _, ok := h.signers[atx.SmesherID]; ok {
340+
if peer == h.local {
351341
// if we land here we tried to publish 2 ATXs in the same epoch
352342
// don't punish ourselves but fail validation and thereby the handling of the incoming ATX
353343
return false, fmt.Errorf(
@@ -396,7 +386,12 @@ func (h *HandlerV1) checkDoublePublish(ctx context.Context, tx sql.Executor, atx
396386
}
397387

398388
// checkWrongPrevAtx verifies if the previous ATX referenced in the ATX is correct.
399-
func (h *HandlerV1) checkWrongPrevAtx(ctx context.Context, tx sql.Executor, atx *wire.ActivationTxV1) (bool, error) {
389+
func (h *HandlerV1) checkWrongPrevAtx(
390+
ctx context.Context,
391+
tx sql.Executor,
392+
atx *wire.ActivationTxV1,
393+
peer peer.ID,
394+
) (bool, error) {
400395
expectedPrevID, err := atxs.PrevIDByNodeID(tx, atx.SmesherID, atx.PublishEpoch)
401396
if err != nil && !errors.Is(err, sql.ErrNotFound) {
402397
return false, fmt.Errorf("get last atx by node id: %w", err)
@@ -405,7 +400,7 @@ func (h *HandlerV1) checkWrongPrevAtx(ctx context.Context, tx sql.Executor, atx
405400
return false, nil
406401
}
407402

408-
if _, ok := h.signers[atx.SmesherID]; ok {
403+
if peer == h.local {
409404
// if we land here we tried to publish an ATX with a wrong prevATX
410405
h.logger.Warn(
411406
"Node produced an ATX with a wrong prevATX. This can happened when the node wasn't synced when "+
@@ -473,23 +468,33 @@ func (h *HandlerV1) checkWrongPrevAtx(ctx context.Context, tx sql.Executor, atx
473468
return true, h.malPublisher.PublishProof(ctx, atx.SmesherID, proof)
474469
}
475470

476-
func (h *HandlerV1) checkMalicious(ctx context.Context, tx sql.Transaction, watx *wire.ActivationTxV1) (bool, error) {
477-
malicious, err := h.checkDoublePublish(ctx, tx, watx)
471+
func (h *HandlerV1) checkMalicious(
472+
ctx context.Context,
473+
tx sql.Transaction,
474+
watx *wire.ActivationTxV1,
475+
peer peer.ID,
476+
) (bool, error) {
477+
malicious, err := h.checkDoublePublish(ctx, tx, watx, peer)
478478
if err != nil {
479479
return malicious, fmt.Errorf("check double publish: %w", err)
480480
}
481481
if malicious {
482482
return true, nil
483483
}
484-
malicious, err = h.checkWrongPrevAtx(ctx, tx, watx)
484+
malicious, err = h.checkWrongPrevAtx(ctx, tx, watx, peer)
485485
if err != nil {
486486
return malicious, fmt.Errorf("check wrong prev atx: %w", err)
487487
}
488488
return malicious, nil
489489
}
490490

491491
// storeAtx stores an ATX and notifies subscribers of the ATXID.
492-
func (h *HandlerV1) storeAtx(ctx context.Context, atx *types.ActivationTx, watx *wire.ActivationTxV1) error {
492+
func (h *HandlerV1) storeAtx(
493+
ctx context.Context,
494+
atx *types.ActivationTx,
495+
watx *wire.ActivationTxV1,
496+
peer peer.ID,
497+
) error {
493498
var malicious bool
494499
if err := h.cdb.WithTxImmediate(ctx, func(tx sql.Transaction) error {
495500
var err error
@@ -503,7 +508,7 @@ func (h *HandlerV1) storeAtx(ctx context.Context, atx *types.ActivationTx, watx
503508
}
504509
malicious = malicious || malicious2
505510
if !malicious {
506-
malicious, err = h.checkMalicious(ctx, tx, watx)
511+
malicious, err = h.checkMalicious(ctx, tx, watx, peer)
507512
if err != nil {
508513
return fmt.Errorf("check malicious: %w", err)
509514
}
@@ -576,7 +581,7 @@ func (h *HandlerV1) processATX(
576581
return fmt.Errorf("%w: validating atx %s (deps): %w", pubsub.ErrValidationReject, watx.ID(), err)
577582
}
578583

579-
if err := h.storeAtx(ctx, atx, watx); err != nil {
584+
if err := h.storeAtx(ctx, atx, watx, peer); err != nil {
580585
return fmt.Errorf("cannot store atx %s: %w", atx.ShortString(), err)
581586
}
582587

‎activation/handler_v1_test.go‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ func newV1TestHandler(tb testing.TB, goldenATXID types.ATXID) *v1TestHandler {
5454
beacon: mocks.mBeacon,
5555
tortoise: mocks.mTortoise,
5656
malPublisher: mocks.mLegacyMalPublish,
57-
signers: make(map[types.NodeID]*signing.EdSigner),
5857
},
5958
handlerMocks: mocks,
6059
}
@@ -583,7 +582,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
583582
return atx.ID() == watx.ID()
584583
}))
585584
atxHdlr.mTortoise.EXPECT().OnAtx(watx.PublishEpoch+1, watx.ID(), gomock.Any())
586-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx, watx))
585+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx, watx, p2p.Peer("other")))
587586

588587
atxFromDb, err := atxs.Get(atxHdlr.cdb, atx.ID())
589588
require.NoError(t, err)
@@ -602,13 +601,13 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
602601
return atx.ID() == watx.ID()
603602
}))
604603
atxHdlr.mTortoise.EXPECT().OnAtx(watx.PublishEpoch+1, watx.ID(), gomock.Any())
605-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx, watx))
604+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx, watx, p2p.Peer("other")))
606605

607606
atxHdlr.mBeacon.EXPECT().OnAtx(gomock.Cond(func(atx *types.ActivationTx) bool {
608607
return atx.ID() == watx.ID()
609608
}))
610609
// Note: tortoise is not informed about the same ATX again
611-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx, watx))
610+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx, watx, p2p.Peer("other")))
612611
})
613612

614613
t.Run("stores ATX of malicious identity", func(t *testing.T) {
@@ -626,7 +625,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
626625
return atx.ID() == watx.ID()
627626
}))
628627
atxHdlr.mTortoise.EXPECT().OnAtx(watx.PublishEpoch+1, watx.ID(), gomock.Any())
629-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx, watx))
628+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx, watx, p2p.Peer("other")))
630629

631630
atxFromDb, err := atxs.Get(atxHdlr.cdb, atx.ID())
632631
require.NoError(t, err)
@@ -645,7 +644,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
645644
return atx.ID() == watx0.ID()
646645
}))
647646
atxHdlr.mTortoise.EXPECT().OnAtx(watx0.PublishEpoch+1, watx0.ID(), gomock.Any())
648-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx0, watx0))
647+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx0, watx0, p2p.Peer("other")))
649648

650649
watx1 := newInitialATXv1(t, goldenATXID)
651650
watx1.Coinbase = types.GenerateAddress([]byte("aaaa"))
@@ -668,12 +667,11 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
668667
return nil
669668
},
670669
)
671-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx1, watx1))
670+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx1, watx1, p2p.Peer("other")))
672671
})
673672

674673
t.Run("another atx for the same epoch for registered ID doesn't create a malfeasance proof", func(t *testing.T) {
675674
atxHdlr := newV1TestHandler(t, goldenATXID)
676-
atxHdlr.Register(sig)
677675

678676
watx0 := newInitialATXv1(t, goldenATXID)
679677
watx0.Sign(sig)
@@ -683,15 +681,15 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
683681
return atx.ID() == watx0.ID()
684682
}))
685683
atxHdlr.mTortoise.EXPECT().OnAtx(watx0.PublishEpoch+1, watx0.ID(), gomock.Any())
686-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx0, watx0))
684+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx0, watx0, atxHdlr.local))
687685

688686
watx1 := newInitialATXv1(t, goldenATXID)
689687
watx1.Coinbase = types.GenerateAddress([]byte("aaaa"))
690688
watx1.Sign(sig)
691689
atx1 := toAtx(t, watx1)
692690

693691
require.ErrorContains(t,
694-
atxHdlr.storeAtx(context.Background(), atx1, watx1),
692+
atxHdlr.storeAtx(context.Background(), atx1, watx1, atxHdlr.local),
695693
fmt.Sprintf("%s already published an ATX", sig.NodeID().ShortString()),
696694
)
697695
})
@@ -707,7 +705,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
707705
return atx.ID() == initialATX.ID()
708706
}))
709707
atxHdlr.mTortoise.EXPECT().OnAtx(initialATX.PublishEpoch+1, initialATX.ID(), gomock.Any())
710-
require.NoError(t, atxHdlr.storeAtx(context.Background(), wInitialATX, initialATX))
708+
require.NoError(t, atxHdlr.storeAtx(context.Background(), wInitialATX, initialATX, p2p.Peer("other")))
711709

712710
// valid first non-initial ATX
713711
watx1 := newChainedActivationTxV1(t, initialATX, goldenATXID)
@@ -718,7 +716,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
718716
return atx.ID() == watx1.ID()
719717
}))
720718
atxHdlr.mTortoise.EXPECT().OnAtx(watx1.PublishEpoch+1, watx1.ID(), gomock.Any())
721-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx1, watx1))
719+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx1, watx1, p2p.Peer("other")))
722720

723721
watx2 := newChainedActivationTxV1(t, watx1, goldenATXID)
724722
watx2.Sign(sig)
@@ -728,7 +726,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
728726
return atx.ID() == watx2.ID()
729727
}))
730728
atxHdlr.mTortoise.EXPECT().OnAtx(watx2.PublishEpoch+1, watx2.ID(), gomock.Any())
731-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx2, watx2))
729+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx2, watx2, p2p.Peer("other")))
732730

733731
// third non-initial ATX references initial ATX as prevATX
734732
watx3 := newChainedActivationTxV1(t, initialATX, goldenATXID)
@@ -753,7 +751,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
753751
},
754752
)
755753

756-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx3, watx3))
754+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx3, watx3, p2p.Peer("other")))
757755
})
758756

759757
t.Run("another atx of v2 with the same prevatx is considered malicious", func(t *testing.T) {
@@ -767,7 +765,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
767765
return atx.ID() == initialATX.ID()
768766
}))
769767
atxHdlr.mTortoise.EXPECT().OnAtx(initialATX.PublishEpoch+1, initialATX.ID(), gomock.Any())
770-
require.NoError(t, atxHdlr.v1.storeAtx(context.Background(), wInitialATX, initialATX))
768+
require.NoError(t, atxHdlr.v1.storeAtx(context.Background(), wInitialATX, initialATX, p2p.Peer("other")))
771769

772770
// valid first non-initial ATX
773771
watx1 := newChainedActivationTxV1(t, initialATX, goldenATXID)
@@ -778,7 +776,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
778776
return atx.ID() == watx1.ID()
779777
}))
780778
atxHdlr.mTortoise.EXPECT().OnAtx(watx1.PublishEpoch+1, watx1.ID(), gomock.Any())
781-
require.NoError(t, atxHdlr.v1.storeAtx(context.Background(), atx1, watx1))
779+
require.NoError(t, atxHdlr.v1.storeAtx(context.Background(), atx1, watx1, p2p.Peer("other")))
782780

783781
watx2 := newSoloATXv2(t, watx1.PublishEpoch+1, watx1.ID(), watx1.ID())
784782
watx2.Sign(sig)
@@ -814,12 +812,11 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
814812
},
815813
)
816814

817-
require.NoError(t, atxHdlr.v1.storeAtx(context.Background(), atx3, watx3))
815+
require.NoError(t, atxHdlr.v1.storeAtx(context.Background(), atx3, watx3, p2p.Peer("other")))
818816
})
819817

820-
t.Run("another atx with the same prevatx for registered ID doesn't create a malfeasance proof", func(t *testing.T) {
818+
t.Run("another atx with the same prevatx when publishing doesn't create a malfeasance proof", func(t *testing.T) {
821819
atxHdlr := newV1TestHandler(t, goldenATXID)
822-
atxHdlr.Register(sig)
823820

824821
// Act & Assert
825822
wInitialATX := newInitialATXv1(t, goldenATXID)
@@ -830,7 +827,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
830827
return atx.ID() == wInitialATX.ID()
831828
}))
832829
atxHdlr.mTortoise.EXPECT().OnAtx(wInitialATX.PublishEpoch+1, wInitialATX.ID(), gomock.Any())
833-
require.NoError(t, atxHdlr.storeAtx(context.Background(), initialAtx, wInitialATX))
830+
require.NoError(t, atxHdlr.storeAtx(context.Background(), initialAtx, wInitialATX, atxHdlr.local))
834831

835832
// valid first non-initial ATX
836833
watx1 := newChainedActivationTxV1(t, wInitialATX, goldenATXID)
@@ -841,7 +838,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
841838
return atx.ID() == watx1.ID()
842839
}))
843840
atxHdlr.mTortoise.EXPECT().OnAtx(watx1.PublishEpoch+1, watx1.ID(), gomock.Any())
844-
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx1, watx1))
841+
require.NoError(t, atxHdlr.storeAtx(context.Background(), atx1, watx1, atxHdlr.local))
845842

846843
// second non-initial ATX references empty as prevATX
847844
watx2 := newInitialATXv1(t, goldenATXID)
@@ -850,7 +847,7 @@ func TestHandlerV1_StoreAtx(t *testing.T) {
850847
atx2 := toAtx(t, watx2)
851848

852849
require.ErrorContains(t,
853-
atxHdlr.storeAtx(context.Background(), atx2, watx2),
850+
atxHdlr.storeAtx(context.Background(), atx2, watx2, atxHdlr.local),
854851
fmt.Sprintf("%s referenced incorrect previous ATX", sig.NodeID().ShortString()),
855852
)
856853
})

‎node/node.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,9 +1260,6 @@ func (app *App) initServices(ctx context.Context) error {
12601260
activation.WithTickSize(app.Config.TickSize),
12611261
activation.WithAtxVersions(app.Config.AtxVersions),
12621262
)
1263-
for _, sig := range app.signers {
1264-
atxHandler.Register(sig)
1265-
}
12661263
malHandler2.RegisterHandler(malfeasance2.InvalidActivation, atxMalHandler)
12671264

12681265
fetcher.SetMalfeasanceProvider(malfeasancePublisher)

0 commit comments

Comments
 (0)