Skip to content

Commit 72f8ec3

Browse files
poszuposzu
committed
Review feedback
1 parent 7d0e2c6 commit 72f8ec3

File tree

5 files changed

+98
-39
lines changed

5 files changed

+98
-39
lines changed

‎activation/e2e/atx_merge_test.go‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -66,17 +66,18 @@ type nipostData struct {
6666

6767
func buildNipost(
6868
nb *activation.NIPostBuilder,
69-
sig *signing.EdSigner,
69+
signer *signing.EdSigner,
7070
publish types.EpochID,
7171
previous, positioning types.ATXID,
7272
) (nipostData, error) {
73-
challenge := wire.NIPostChallengeV2{
74-
PublishEpoch: publish,
75-
PrevATXID: previous,
76-
PositioningATXID: positioning,
73+
postChallenge := &types.NIPostChallenge{
74+
PublishEpoch: publish,
75+
PrevATXID: previous,
76+
PositioningATX: positioning,
7777
}
78-
nipost, err := nb.BuildNIPost(context.Background(), sig, challenge.PublishEpoch, challenge.Hash())
79-
nb.ResetState(sig.NodeID())
78+
challenge := wire.NIPostChallengeToWireV2(postChallenge).Hash()
79+
nipost, err := nb.BuildNIPost(context.Background(), signer, challenge, postChallenge)
80+
nb.ResetState(signer.NodeID())
8081
return nipostData{previous, nipost}, err
8182
}
8283

@@ -304,6 +305,7 @@ func Test_MarryAndMerge(t *testing.T) {
304305
logger.Named("nipostBuilder"),
305306
poetCfg,
306307
clock,
308+
validator,
307309
activation.WithPoetClients(poetClient),
308310
)
309311
require.NoError(t, err)
@@ -338,17 +340,18 @@ func Test_MarryAndMerge(t *testing.T) {
338340
eg = errgroup.Group{}
339341
for i, signer := range signers {
340342
eg.Go(func() error {
341-
post, postInfo, err := nb.Proof(context.Background(), signer.NodeID(), types.EmptyHash32[:])
343+
post, postInfo, err := nb.Proof(context.Background(), signer.NodeID(), types.EmptyHash32[:], nil)
342344
if err != nil {
343345
return err
344346
}
345347

346-
challenge := wire.NIPostChallengeV2{
347-
PublishEpoch: publish,
348-
PositioningATXID: goldenATX,
349-
InitialPost: wire.PostToWireV1(post),
348+
postChallenge := &types.NIPostChallenge{
349+
PublishEpoch: publish,
350+
PositioningATX: goldenATX,
351+
InitialPost: post,
350352
}
351-
nipost, err := nb.BuildNIPost(context.Background(), signer, challenge.PublishEpoch, challenge.Hash())
353+
challenge := wire.NIPostChallengeToWireV2(postChallenge).Hash()
354+
nipost, err := nb.BuildNIPost(context.Background(), signer, challenge, postChallenge)
352355
if err != nil {
353356
return err
354357
}

‎activation/handler_v2.go‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"errors"
77
"fmt"
8+
"math"
89
"math/bits"
910
"slices"
1011
"time"
@@ -435,7 +436,8 @@ func (h *HandlerV2) validateMarriages(atx *wire.ActivationTxV2) ([]types.NodeID,
435436
if len(atx.Marriages) == 0 {
436437
return nil, nil
437438
}
438-
var marryingIDs []types.NodeID
439+
marryingIDsSet := make(map[types.NodeID]struct{}, len(atx.Marriages))
440+
var marryingIDs []types.NodeID // for deterministic order
439441
for i, m := range atx.Marriages {
440442
var id types.NodeID
441443
if m.ReferenceAtx == types.EmptyATXID {
@@ -451,6 +453,10 @@ func (h *HandlerV2) validateMarriages(atx *wire.ActivationTxV2) ([]types.NodeID,
451453
if !h.edVerifier.Verify(signing.MARRIAGE, id, atx.SmesherID.Bytes(), m.Signature) {
452454
return nil, fmt.Errorf("invalid marriage[%d] signature", i)
453455
}
456+
if _, ok := marryingIDsSet[id]; ok {
457+
return nil, fmt.Errorf("more than 1 marriage certificate for ID %s", id)
458+
}
459+
marryingIDsSet[id] = struct{}{}
454460
marryingIDs = append(marryingIDs, id)
455461
}
456462
return marryingIDs, nil
@@ -515,22 +521,21 @@ func (n nipostSizes) minTicks() uint64 {
515521
}
516522

517523
func (n nipostSizes) sumUp() (units uint32, weight uint64, err error) {
518-
var totalEffectiveNumUnits uint32
524+
var totalUnits uint64
519525
var totalWeight uint64
520526
for _, ns := range n {
521-
sum, carry := bits.Add32(totalEffectiveNumUnits, ns.units, 0)
522-
if carry != 0 {
523-
return 0, 0, fmt.Errorf("total units overflow (%d + %d)", totalEffectiveNumUnits, ns.units)
524-
}
525-
totalEffectiveNumUnits = sum
527+
totalUnits += uint64(ns.units)
526528

527529
hi, weight := bits.Mul64(uint64(ns.units), ns.ticks)
528530
if hi != 0 {
529531
return 0, 0, fmt.Errorf("weight overflow (%d * %d)", ns.units, ns.ticks)
530532
}
531533
totalWeight += weight
532534
}
533-
return totalEffectiveNumUnits, totalWeight, nil
535+
if totalUnits > math.MaxUint32 {
536+
return 0, 0, fmt.Errorf("total units overflow: %d", totalUnits)
537+
}
538+
return uint32(totalUnits), totalWeight, nil
534539
}
535540

536541
func (h *HandlerV2) verifyIncludedIDsUniqueness(atx *wire.ActivationTxV2) error {
@@ -595,7 +600,6 @@ func (h *HandlerV2) syntacticallyValidateDeps(
595600
}
596601
}
597602
nipostSizes[i].addUnits(effectiveNumUnits)
598-
599603
}
600604
}
601605

@@ -708,7 +712,6 @@ func (h *HandlerV2) syntacticallyValidateDeps(
708712
}
709713

710714
func (h *HandlerV2) checkMalicious(
711-
ctx context.Context,
712715
tx *sql.Tx,
713716
watx *wire.ActivationTxV2,
714717
marrying []types.NodeID,
@@ -721,7 +724,7 @@ func (h *HandlerV2) checkMalicious(
721724
return true, nil, nil
722725
}
723726

724-
proof, err := h.checkDoubleMarry(tx, watx, marrying)
727+
proof, err := h.checkDoubleMarry(tx, marrying)
725728
if err != nil {
726729
return false, nil, fmt.Errorf("checking double marry: %w", err)
727730
}
@@ -739,11 +742,7 @@ func (h *HandlerV2) checkMalicious(
739742
return false, nil, nil
740743
}
741744

742-
func (h *HandlerV2) checkDoubleMarry(
743-
tx *sql.Tx,
744-
watx *wire.ActivationTxV2,
745-
marrying []types.NodeID,
746-
) (*mwire.MalfeasanceProof, error) {
745+
func (h *HandlerV2) checkDoubleMarry(tx *sql.Tx, marrying []types.NodeID) (*mwire.MalfeasanceProof, error) {
747746
for _, id := range marrying {
748747
married, err := identities.Married(tx, id)
749748
if err != nil {
@@ -776,7 +775,7 @@ func (h *HandlerV2) storeAtx(
776775
)
777776
if err := h.cdb.WithTx(ctx, func(tx *sql.Tx) error {
778777
var err error
779-
malicious, proof, err = h.checkMalicious(ctx, tx, watx, marrying)
778+
malicious, proof, err = h.checkMalicious(tx, watx, marrying)
780779
if err != nil {
781780
return fmt.Errorf("check malicious: %w", err)
782781
}

‎activation/handler_v2_test.go‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/spacemeshos/go-spacemesh/activation/wire"
2020
"github.com/spacemeshos/go-spacemesh/atxsdata"
2121
"github.com/spacemeshos/go-spacemesh/codec"
22+
"github.com/spacemeshos/go-spacemesh/common/fixture"
2223
"github.com/spacemeshos/go-spacemesh/common/types"
2324
"github.com/spacemeshos/go-spacemesh/datastore"
2425
mwire "github.com/spacemeshos/go-spacemesh/malfeasance/wire"
@@ -121,8 +122,8 @@ func (h *handlerMocks) expectVerifyNIPoSTs(
121122
}
122123

123124
func (h *handlerMocks) expectStoreAtxV2(atx *wire.ActivationTxV2) {
124-
h.mbeacon.EXPECT().OnAtx(gomock.Any())
125-
h.mtortoise.EXPECT().OnAtx(gomock.Any(), gomock.Any(), gomock.Any())
125+
h.mbeacon.EXPECT().OnAtx(fixture.MatchId(atx.ID()))
126+
h.mtortoise.EXPECT().OnAtx(atx.PublishEpoch+1, atx.ID(), gomock.Any())
126127
h.mValidator.EXPECT().IsVerifyingFullPost().Return(false)
127128
}
128129

@@ -183,17 +184,24 @@ func (h *v2TestHandler) createAndProcessInitial(t *testing.T, sig *signing.EdSig
183184
t.Helper()
184185
atx := newInitialATXv2(t, h.handlerMocks.goldenATXID)
185186
atx.Sign(sig)
186-
p, err := h.processInitial(atx)
187+
p, err := h.processInitial(t, atx)
187188
require.NoError(t, err)
188189
require.Nil(t, p)
189190
return atx
190191
}
191192

192-
func (h *v2TestHandler) processInitial(atx *wire.ActivationTxV2) (*mwire.MalfeasanceProof, error) {
193+
func (h *v2TestHandler) processInitial(t *testing.T, atx *wire.ActivationTxV2) (*mwire.MalfeasanceProof, error) {
194+
t.Helper()
193195
h.expectInitialAtxV2(atx)
194196
return h.processATX(context.Background(), peer.ID("peer"), atx, codec.MustEncode(atx), time.Now())
195197
}
196198

199+
func (h *v2TestHandler) processSoloAtx(t *testing.T, atx *wire.ActivationTxV2) (*mwire.MalfeasanceProof, error) {
200+
t.Helper()
201+
h.expectAtxV2(atx)
202+
return h.processATX(context.Background(), peer.ID("peer"), atx, codec.MustEncode(atx), time.Now())
203+
}
204+
197205
func TestHandlerV2_SyntacticallyValidate(t *testing.T) {
198206
t.Parallel()
199207
golden := types.RandomATXID()
@@ -1234,7 +1242,7 @@ func Test_ValidateMarriages(t *testing.T) {
12341242
}
12351243
marriage.Sign(sig)
12361244

1237-
p, err := atxHandler.processInitial(marriage)
1245+
p, err := atxHandler.processInitial(t, marriage)
12381246
require.NoError(t, err)
12391247
require.Nil(t, p)
12401248

@@ -1569,10 +1577,9 @@ func Test_Marriages(t *testing.T) {
15691577
Signature: otherSig.Sign(signing.MARRIAGE, sig.NodeID().Bytes()),
15701578
},
15711579
}
1572-
15731580
atx.Sign(sig)
15741581

1575-
p, err := atxHandler.processInitial(atx)
1582+
p, err := atxHandler.processInitial(t, atx)
15761583
require.NoError(t, err)
15771584
require.Nil(t, p)
15781585

@@ -1588,7 +1595,39 @@ func Test_Marriages(t *testing.T) {
15881595
require.NoError(t, err)
15891596
require.ElementsMatch(t, []types.NodeID{sig.NodeID(), otherSig.NodeID()}, set)
15901597
})
1591-
t.Run("can't marry twice", func(t *testing.T) {
1598+
t.Run("can't marry twice in the same marriage ATX", func(t *testing.T) {
1599+
t.Parallel()
1600+
atxHandler := newV2TestHandler(t, golden)
1601+
1602+
otherSig, err := signing.NewEdSigner()
1603+
require.NoError(t, err)
1604+
othersAtx := atxHandler.createAndProcessInitial(t, otherSig)
1605+
1606+
othersSecondAtx := newSoloATXv2(t, othersAtx.PublishEpoch+1, othersAtx.ID(), othersAtx.ID())
1607+
othersSecondAtx.Sign(otherSig)
1608+
_, err = atxHandler.processSoloAtx(t, othersSecondAtx)
1609+
require.NoError(t, err)
1610+
1611+
atx := newInitialATXv2(t, golden)
1612+
atx.Marriages = []wire.MarriageCertificate{
1613+
{
1614+
Signature: sig.Sign(signing.MARRIAGE, sig.NodeID().Bytes()),
1615+
},
1616+
{
1617+
ReferenceAtx: othersAtx.ID(),
1618+
Signature: otherSig.Sign(signing.MARRIAGE, sig.NodeID().Bytes()),
1619+
},
1620+
{
1621+
ReferenceAtx: othersSecondAtx.ID(),
1622+
Signature: otherSig.Sign(signing.MARRIAGE, sig.NodeID().Bytes()),
1623+
},
1624+
}
1625+
atx.Sign(sig)
1626+
1627+
_, err = atxHandler.validateMarriages(atx)
1628+
require.ErrorContains(t, err, "more than 1 marriage certificate for ID")
1629+
})
1630+
t.Run("can't marry twice (separate marriages)", func(t *testing.T) {
15921631
t.Parallel()
15931632
atxHandler := newV2TestHandler(t, golden)
15941633

‎activation/wire/wire_v2.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ func (sp *SubPostV2) Root(prevATXs []types.ATXID) []byte {
229229
}
230230
tree.AddLeaf(prevATXs[sp.PrevATXIndex].Bytes())
231231

232-
var leafIndex [8]byte
232+
var leafIndex types.Hash32
233233
binary.LittleEndian.PutUint64(leafIndex[:], sp.MembershipLeafIndex)
234234
tree.AddLeaf(leafIndex[:])
235235

‎common/fixture/atxs.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,21 @@ func ToAtx(t testing.TB, watx *wire.ActivationTxV1) *types.ActivationTx {
7777
atx.TickCount = 1
7878
return atx
7979
}
80+
81+
type idMatcher types.ATXID
82+
83+
func MatchId(id types.ATXID) idMatcher {
84+
return idMatcher(id)
85+
}
86+
87+
func (m idMatcher) Matches(x any) bool {
88+
type hasID interface {
89+
ID() types.ATXID
90+
}
91+
v, ok := x.(hasID)
92+
return ok && v.ID() == types.ATXID(m)
93+
}
94+
95+
func (m idMatcher) String() string {
96+
return "is ATX ID " + types.ATXID(m).String()
97+
}

0 commit comments

Comments
 (0)