proposal signed data was used in id (#4992)

> {"L":"ERROR","T":"2023-09-08T15:46:18.944Z","N":"9aa91.blockGenerator","M":"failed to process hare output","node_id":"9aa91374168177589adb11c04a9984f33684e3e3cee72b4298c4d5d56d03dcdb","module":"blockGenerator","layer_id":12,"errmsg":"preprocess fetch layer 12 proposals: hint: proposalDB, hash: 0x1d286d884f4a8699182bde631972d1ad649549aa000000000000000000000000, err: validation reject: incorrect hash: proposal want 1d286d884f, got c2986cf657\nhint: proposalDB, hash: 0xab3ef72044915bb64ef3c7eacfdd98f923a72195000000000000000000000000, err: validation reject: incorrect hash: proposal want ab3ef72044, got 9eafb3985e\nhint: proposalDB, hash: 0x5a0cbb754637b44d5df63ca6938be1fad001ac4b000000000000000000000000, err: validation reject: incorrect hash: proposal want 5a0cbb7546, got ab4994459c","name":"blockGenerator"}
spacemeshos · Sep 11, 2023 · ea75e24 · ea75e24
1 parent 0cb8203
commit ea75e24
Show file tree

Hide file tree

Showing 2 changed files with 105 additions and 2 deletions.
diff --git a/proposals/handler.go b/proposals/handler.go
@@ -272,25 +272,27 @@ func (h *Handler) handleProposal(ctx context.Context, expHash types.Hash32, peer
 	if p.Layer >= h.cfg.AllowEmptyActiveSet {
 		p.ActiveSet = nil
 	}
+
 	if !h.edVerifier.Verify(signing.PROPOSAL, p.SmesherID, p.SignedBytes(), p.Signature) {
 		badSigProposal.Inc()
 		return fmt.Errorf("failed to verify proposal signature")
 	}
-	p.ActiveSet = set
 	if !h.edVerifier.Verify(signing.BALLOT, p.Ballot.SmesherID, p.Ballot.SignedBytes(), p.Ballot.Signature) {
 		badSigBallot.Inc()
 		return fmt.Errorf("failed to verify ballot signature")
 	}
 
 	// set the proposal ID when received
+	// It mustn't contain the active set if layer >= AllowEmptyActiveSet
+	// (p.Initialize uses SignedBytes again).
 	if err := p.Initialize(); err != nil {
 		failedInit.Inc()
 		return errInitialize
 	}
-
 	if expHash != (types.Hash32{}) && p.ID().AsHash32() != expHash {
 		return fmt.Errorf("%w: proposal want %s, got %s", errWrongHash, expHash.ShortString(), p.ID().AsHash32().ShortString())
 	}
+	p.ActiveSet = set
 
 	if p.AtxID == types.EmptyATXID || p.AtxID == h.cfg.GoldenATXID {
 		badData.Inc()

diff --git a/proposals/handler_test.go b/proposals/handler_test.go
@@ -1398,3 +1398,104 @@ func TestHandleActiveSet(t *testing.T) {
 		})
 	}
 }
+
+func gproposal(signer *signing.EdSigner, atxid types.ATXID, activeset []types.ATXID,
+	layer types.LayerID, edata *types.EpochData,
+) types.Proposal {
+	p := types.Proposal{}
+	p.Layer = layer
+	p.AtxID = atxid
+	p.ActiveSet = activeset
+	p.EpochData = edata
+	p.Ballot.Signature = signer.Sign(signing.BALLOT, p.Ballot.SignedBytes())
+	p.Ballot.SmesherID = signer.NodeID()
+	p.Signature = signer.Sign(signing.PROPOSAL, p.SignedBytes())
+	p.SmesherID = signer.NodeID()
+	return p
+}
+
+func TestHandleSyncedProposalActiveSet(t *testing.T) {
+	signer, err := signing.NewEdSigner()
+	require.NoError(t, err)
+
+	set := []types.ATXID{{1}, {2}}
+	const acceptEmpty = 20
+	good := gproposal(signer, types.ATXID{1}, set, acceptEmpty-1, &types.EpochData{
+		ActiveSetHash: types.ATXIDList(set).Hash(),
+		Beacon:        types.Beacon{1},
+	})
+	require.NoError(t, good.Initialize())
+
+	woActive := good
+	woActive.ActiveSet = nil
+
+	notSigned := gproposal(signer, types.ATXID{1}, nil, acceptEmpty, &types.EpochData{
+		ActiveSetHash: types.ATXIDList(set).Hash(),
+		Beacon:        types.Beacon{1},
+	})
+	require.NoError(t, notSigned.Initialize())
+	notSigned.ActiveSet = set
+
+	notSignedEmpty := notSigned
+	notSignedEmpty.ActiveSet = nil
+	for _, tc := range []struct {
+		desc       string
+		data       []byte
+		id         types.Hash32
+		requestSet []types.ATXID
+		err        string
+	}{
+		{
+			desc: "before empty signed active set",
+			data: codec.MustEncode(&good),
+			id:   good.ID().AsHash32(),
+		},
+		{
+			desc: "before empty without active set",
+			data: codec.MustEncode(&woActive),
+			id:   woActive.ID().AsHash32(),
+			err:  "failed to verify proposal signature",
+		},
+		{
+			desc: "after empty not signed active set",
+			data: codec.MustEncode(&notSigned),
+			id:   notSigned.ID().AsHash32(),
+		},
+		{
+			desc:       "after empty not signed active set empty",
+			data:       codec.MustEncode(&notSignedEmpty),
+			id:         notSigned.ID().AsHash32(),
+			requestSet: set,
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			th := createTestHandler(t)
+			th.cfg.AllowEmptyActiveSet = acceptEmpty
+			pid := p2p.Peer("any")
+
+			th.mclock.EXPECT().LayerToTime(gomock.Any())
+			th.mf.EXPECT().RegisterPeerHashes(pid, gomock.Any()).AnyTimes()
+			th.md.EXPECT().GetMissingActiveSet(gomock.Any(), gomock.Any()).AnyTimes()
+			if tc.requestSet != nil {
+				id := types.ATXIDList(tc.requestSet).Hash()
+				th.mf.EXPECT().GetActiveSet(gomock.Any(), id)
+				require.NoError(t, activesets.Add(th.cdb, id, &types.EpochActiveSet{
+					Set: tc.requestSet,
+				}))
+			}
+			th.mf.EXPECT().GetAtxs(gomock.Any(), gomock.Any()).AnyTimes()
+			th.mf.EXPECT().GetBallots(gomock.Any(), gomock.Any()).AnyTimes()
+			th.mockSet.decodeAnyBallots()
+			th.mv.EXPECT().CheckEligibility(gomock.Any(), gomock.Any()).AnyTimes().Return(true, nil)
+			th.mm.EXPECT().AddBallot(gomock.Any(), gomock.Any()).AnyTimes()
+			th.mm.EXPECT().AddTXsFromProposal(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
+
+			err := th.HandleSyncedProposal(context.Background(), tc.id, pid, tc.data)
+			if len(tc.err) > 0 {
+				require.ErrorContains(t, err, tc.err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}