From f75c44c02a98c8d785ce97364207088e85c963c3 Mon Sep 17 00:00:00 2001 From: acud <12988138+acud@users.noreply.github.com> Date: Wed, 31 Jul 2024 17:57:20 -0600 Subject: [PATCH] test: remove compact function injection --- hare4/hare.go | 35 +++++++++++------------------- hare4/hare_test.go | 54 ++++++++++++++++++++++++++++------------------ 2 files changed, 46 insertions(+), 43 deletions(-) diff --git a/hare4/hare.go b/hare4/hare.go index cffa2ec366..96803f05a5 100644 --- a/hare4/hare.go +++ b/hare4/hare.go @@ -46,6 +46,7 @@ var ( errResponseTooBig = errors.New("response too big") errCannotFindProposal = errors.New("cannot find proposal") errNoEligibilityProofs = errors.New("no eligibility proofs") + errSigTooShort = errors.New("signature too short") fetchFullTimeout = 5 * time.Second ) @@ -224,7 +225,6 @@ func New( atxsdata: atxsdata, proposals: proposals, verifier: verif, - compactFn: compactTruncate, oracle: &legacyOracle{ log: zap.NewNop(), oracle: oracle, @@ -268,7 +268,6 @@ type Hare struct { atxsdata *atxsdata.Data proposals *store.Store verifier verifier - compactFn func([]byte) []byte oracle *legacyOracle sync system.SyncStateProvider patrol *layerpatrol.LayerPatrol @@ -397,7 +396,7 @@ func (h *Hare) reconstructProposals(ctx context.Context, peer p2p.Peer, msgId ty if len(proposals) == 0 { return errNoLayerProposals } - compacted := h.compactProposals(h.compactFn, msg.Layer, proposals) + compacted := h.compactProposals(msg.Layer, proposals) proposalIds := make([]proposalTuple, len(proposals)) for i := range proposals { proposalIds[i] = proposalTuple{id: proposals[i].ID(), compact: compacted[i]} @@ -718,7 +717,7 @@ func (h *Hare) onOutput(session *session, ir IterRound, out output) error { msg.Signature = session.signers[i].Sign(signing.HARE, msg.ToMetadata().ToBytes()) if ir.Round == preround { var err error - msg.Body.Value.CompactProposals, err = h.compactProposalIds(h.compactFn, msg.Layer, + msg.Body.Value.CompactProposals, err = h.compactProposalIds(msg.Layer, out.message.Body.Value.Proposals) if err != nil { h.log.Debug("failed to compact proposals", zap.Error(err)) @@ -887,32 +886,20 @@ type session struct { vrfs []*types.HareEligibility } -type compactFunc func([]byte) []byte - -// compactFunc will truncate a given byte slice to a shorter -// byte slice by reslicing. -func compactTruncate(b []byte) []byte { - return b[:4] -} - -func compactVrf(compacter compactFunc, v types.VrfSignature) (c types.CompactProposalID) { - b := compacter(v[:]) - copy(c[:], b) - return c -} - -func (h *Hare) compactProposals(compacter compactFunc, layer types.LayerID, +func (h *Hare) compactProposals(layer types.LayerID, proposals []*types.Proposal, ) []types.CompactProposalID { compactProposals := make([]types.CompactProposalID, len(proposals)) for i, prop := range proposals { vrf := prop.EligibilityProofs[0].Sig - compactProposals[i] = compactVrf(compacter, vrf) + var c types.CompactProposalID + copy(c[:], vrf[:4]) + compactProposals[i] = c } return compactProposals } -func (h *Hare) compactProposalIds(compacter compactFunc, layer types.LayerID, +func (h *Hare) compactProposalIds(layer types.LayerID, proposals []types.ProposalID, ) ([]types.CompactProposalID, error) { compactProposals := make([]types.CompactProposalID, len(proposals)) @@ -927,7 +914,11 @@ func (h *Hare) compactProposalIds(compacter compactFunc, layer types.LayerID, if len(fp.EligibilityProofs) == 0 { return nil, errNoEligibilityProofs } - compactProposals[i] = compactVrf(compacter, fp.EligibilityProofs[0].Sig) + + var c types.CompactProposalID + copy(c[:], fp.EligibilityProofs[0].Sig[:4]) + + compactProposals[i] = c } return compactProposals, nil } diff --git a/hare4/hare_test.go b/hare4/hare_test.go index 6f5d00147a..8b4a87887d 100644 --- a/hare4/hare_test.go +++ b/hare4/hare_test.go @@ -308,9 +308,9 @@ func withMockVerifier() clusterOpt { } } -func withMockCompactFn(f func([]byte) []byte) clusterOpt { +func withCollidingProposals() clusterOpt { return func(cluster *lockstepCluster) { - cluster.mockCompactFn = f + cluster.collidingProposals = true } } @@ -349,9 +349,9 @@ type lockstepCluster struct { nodes []*node signers []*node // nodes that active on consensus but don't run hare instance - mockVerify bool - mockCompactFn func([]byte) []byte - units struct { + mockVerify bool + collidingProposals bool + units struct { min, max int } proposals struct { @@ -396,9 +396,6 @@ func (cl *lockstepCluster) addActive(n int) *lockstepCluster { if cl.mockVerify { nn = nn.withVerifier() } - if cl.mockCompactFn != nil { - nn.hare.compactFn = cl.mockCompactFn - } cl.addNode(nn) } return cl @@ -502,7 +499,11 @@ func (cl *lockstepCluster) genProposals(lid types.LayerID, skipNodes ...int) { proposal.SetID(id) proposal.Ballot.SetID(bid) var vrf types.VrfSignature - cl.t.rng.Read(vrf[:]) + if !cl.collidingProposals { + // if we want non-colliding proposals we copy from the rng + // otherwise it is kept as an array of zeroes + cl.t.rng.Read(vrf[:]) + } proposal.Ballot.EligibilityProofs = append(proposal.Ballot.EligibilityProofs, types.VotingEligibility{Sig: vrf}) proposal.SetBeacon(proposal.EpochData.Beacon) @@ -897,7 +898,7 @@ func TestHandler(t *testing.T) { msg1.Signature = n.signer.Sign(signing.HARE, msg1.ToMetadata().ToBytes()) msg1.Value.Proposals = nil msg1.Value.CompactProposals = []types.CompactProposalID{ - compactVrf(compactTruncate, p1.Ballot.EligibilityProofs[0].Sig), + compactVrf(p1.Ballot.EligibilityProofs[0].Sig), } msg2 := &Message{} @@ -908,7 +909,7 @@ func TestHandler(t *testing.T) { msg2.Signature = n.signer.Sign(signing.HARE, msg2.ToMetadata().ToBytes()) msg2.Value.Proposals = nil msg2.Value.CompactProposals = []types.CompactProposalID{ - compactVrf(compactTruncate, p2.Ballot.EligibilityProofs[0].Sig), + compactVrf(p2.Ballot.EligibilityProofs[0].Sig), } require.NoError(t, n.hare.Handler(context.Background(), "", codec.MustEncode(msg1))) @@ -1184,7 +1185,7 @@ func TestHare_ReconstructForward(t *testing.T) { n.mpublisher.EXPECT(). Publish(gomock.Any(), gomock.Any(), gomock.Any()). Do(func(ctx context.Context, proto string, msg []byte) error { - // here we wanna call the handler on the second node + // here we want to call the handler on the second node // but then call the handler on the third with the incoming peer id // of the second node, this way we know the peers could resolve between // themselves without having the connection to the original sender @@ -1345,7 +1346,7 @@ func TestHare_ReconstructAll(t *testing.T) { } // TestHare_ReconstructCollision tests that the nodes go into a -// full message exchange in the case that there's a siphash collision. +// full message exchange in the case that there's a compacted id collision. func TestHare_ReconstructCollision(t *testing.T) { cfg := DefaultConfig() cfg.LogStats = true @@ -1359,10 +1360,7 @@ func TestHare_ReconstructCollision(t *testing.T) { genesis: types.GetEffectiveGenesis(), } - fn := func(_ []byte) []byte { - return []byte{0xab, 0xab, 0xab, 0xab} - } - cluster := newLockstepCluster(tst, withMockCompactFn(fn), withProposals(1)). + cluster := newLockstepCluster(tst, withProposals(1), withCollidingProposals()). addActive(2) if cluster.signersCount > 0 { cluster = cluster.addSigner(cluster.signersCount) @@ -1371,16 +1369,24 @@ func TestHare_ReconstructCollision(t *testing.T) { layer := tst.genesis + 1 // scenario: - // node 1 has generated 1 proposal that (mocked) hash into 0xab as prefix - both nodes know the proposal - // node 2 has generated 1 proposal that hash into 0xab (but node 1 doesn't know about it) + // node 1 has generated 1 proposal that hash into 0x00 as prefix - both nodes know the proposal + // node 2 has generated 1 proposal that hash into 0x00 (but node 1 doesn't know about it) // so the two proposals collide and then we check that the nodes actually go into a round of // exchanging the missing/colliding hashes and then the signature verification (not mocked) // should pass and that a full exchange of all hashes is not triggered (disambiguates case of // failed signature vs. hashes colliding - there's a difference in number of single prefixes // that are sent, but the response should be the same) - go func() { <-cluster.nodes[1].tracer.compactReq }() // node 2 gets the request - go func() { <-cluster.nodes[0].tracer.compactResp }() // node 1 gets the response + var wg sync.WaitGroup + wg.Add(2) + go func() { + <-cluster.nodes[1].tracer.compactReq + wg.Done() + }() // node 2 gets the request + go func() { + <-cluster.nodes[0].tracer.compactResp + wg.Done() + }() // node 1 gets the response cluster.setup() @@ -1411,6 +1417,12 @@ func TestHare_ReconstructCollision(t *testing.T) { default: t.Fatal("no result") } + wg.Wait() require.Empty(t, n.hare.Running()) } } + +func compactVrf(v types.VrfSignature) (c types.CompactProposalID) { + copy(c[:], v[:4]) + return c +}