diff --git a/snow/engine/snowman/transitive_test.go b/snow/engine/snowman/transitive_test.go index 30a23baf0c04..6657f8ae83b6 100644 --- a/snow/engine/snowman/transitive_test.go +++ b/snow/engine/snowman/transitive_test.go @@ -2710,6 +2710,10 @@ func TestEngineNonPreferredAmplification(t *testing.T) { // B func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { vdr, _, sender, vm, te, gBlk := setupDefaultConfig(t) + expectedVdrSet := set.Set[ids.NodeID]{} + expectedVdrSet.Add(vdr) + + require := require.New(t) // [blk1] is a child of [gBlk] and currently passes verification blk1 := &snowman.TestBlock{ @@ -2742,13 +2746,14 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { case bytes.Equal(b, blk2.Bytes()): return blk2, nil default: - t.Fatalf("Unknown block bytes") + require.FailNow("Unknown block bytes") return nil, nil } } - // The VM should only be able to retrieve [gBlk] from storage - // TODO GetBlockF should be updated after blocks are verified/accepted + // for now, this VM should only be able to retrieve [gBlk] from storage + // this "GetBlockF" will be updated after blocks are verified/accepted + // in the following tests vm.GetBlockF = func(_ context.Context, blkID ids.ID) (snowman.Block, error) { switch blkID { case gBlk.ID(): @@ -2762,54 +2767,40 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { reqID := new(uint32) sender.SendGetF = func(_ context.Context, inVdr ids.NodeID, requestID uint32, blkID ids.ID) { *reqID = requestID - if *asked { - t.Fatalf("Asked multiple times") - } - if blkID != blk1.ID() { - t.Fatalf("Expected engine to request blk1") - } - if inVdr != vdr { - t.Fatalf("Expected engine to request blk2 from vdr") - } + require.False(*asked) + require.Equal(blk1.ID(), blkID) + require.Equal(vdr, inVdr) *asked = true } - // Receive Gossip message for [blk2] first and expect the sender to issue a Get request for - // its ancestor: [blk1]. - if err := te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk2.Bytes()); err != nil { - t.Fatal(err) - } - - if !*asked { - t.Fatalf("Didn't ask for missing blk1") - } + // This engine receives a Gossip message for [blk2] which was "unknown" in this engine. + // The engine thus learns about its ancestor [blk1] and should send a Get request for it. + // (see above for expected "Get" request) + require.NoError(te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk2.Bytes())) + require.True(*asked) // Prepare to PushQuery [blk1] after our Get request is fulfilled. We should not PushQuery // [blk2] since it currently fails verification. queried := new(bool) queryRequestID := new(uint32) sender.SendPushQueryF = func(_ context.Context, inVdrs set.Set[ids.NodeID], requestID uint32, blkBytes []byte) { - if *queried { - t.Fatalf("Asked multiple times") - } + require.False(*queried) *queried = true + *queryRequestID = requestID vdrSet := set.Set[ids.NodeID]{} vdrSet.Add(vdr) - if !inVdrs.Equals(vdrSet) { - t.Fatalf("Asking wrong validator for preference") - } - if !bytes.Equal(blk1.Bytes(), blkBytes) { - t.Fatalf("Asking for wrong block") - } - } - - // Answer the request, this should allow [blk1] to be issued and cause [blk2] to - // fail verification. - if err := te.Put(context.Background(), vdr, *reqID, blk1.Bytes()); err != nil { - t.Fatal(err) - } - - // now blk1 is verified, vm can return it + require.Equal(vdrSet, inVdrs) + require.Equal(blk1.Bytes(), blkBytes) + } + // This engine now handles the response to the "Get" request. This should cause [blk1] to be issued + // which will result in attempting to issue [blk2]. However, [blk2] should fail verification and be dropped. + // By issuing [blk1], this node should fire a "PushQuery" request for [blk1]. + // (see above for expected "PushQuery" request) + require.NoError(te.Put(context.Background(), vdr, *reqID, blk1.Bytes())) + require.True(*asked) + require.True(*queried, "Didn't query the newly issued blk1") + + // now [blk1] is verified, vm can return it vm.GetBlockF = func(_ context.Context, blkID ids.ID) (snowman.Block, error) { switch blkID { case gBlk.ID(): @@ -2821,52 +2812,27 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { } } - if !*queried { - t.Fatalf("Didn't ask for preferences regarding blk1") - } - sendReqID := new(uint32) reqVdr := new(ids.NodeID) // Update GetF to produce a more detailed error message in the case that receiving a Chits // message causes us to send another Get request. sender.SendGetF = func(_ context.Context, inVdr ids.NodeID, requestID uint32, blkID ids.ID) { - switch blkID { - case blk1.ID(): - t.Fatal("Unexpectedly sent a Get request for blk1") - case blk2.ID(): - *sendReqID = requestID - *reqVdr = inVdr - return - default: - t.Fatal("Unexpectedly sent a Get request for unknown block") - } - } + require.Equal(blk2.ID(), blkID) - sender.SendPullQueryF = func(_ context.Context, _ set.Set[ids.NodeID], _ uint32, blkID ids.ID) { - switch blkID { - case blk1.ID(): - t.Fatal("Unexpectedly sent a PullQuery request for blk1") - case blk2.ID(): - t.Fatal("Unexpectedly sent a PullQuery request for blk2") - default: - t.Fatal("Unexpectedly sent a PullQuery request for unknown block") - } + *sendReqID = requestID + *reqVdr = inVdr } - // Now we are expecting a Chits message, and we receive it for blk2 instead of blk1 - // The votes should be bubbled through blk2 despite the fact that it is failing verification. - if err := te.Chits(context.Background(), vdr, *queryRequestID, []ids.ID{blk2.ID()}, nil); err != nil { - t.Fatal(err) - } + // Now we are expecting a Chits message, and we receive it for [blk2] instead of [blk1]. + // This will cause the node to again request [blk2]. + require.NoError(te.Chits(context.Background(), vdr, *queryRequestID, []ids.ID{blk2.ID()}, nil)) - if err := te.Put(context.Background(), *reqVdr, *sendReqID, blk2.Bytes()); err != nil { - t.Fatal(err) - } + // The votes should be bubbled through [blk2] despite the fact that it is failing verification. + require.NoError(te.Put(context.Background(), *reqVdr, *sendReqID, blk2.Bytes())) // The vote should be bubbled through [blk2], such that [blk1] gets marked as Accepted. - if blk1.Status() != choices.Accepted { - t.Fatalf("Expected blk1 to be Accepted, but found status: %s", blk1.Status()) - } + require.Equal(choices.Accepted, blk1.Status()) + require.Equal(choices.Processing, blk2.Status()) // Now that [blk1] has been marked as Accepted, [blk2] can pass verification. blk2.VerifyV = nil @@ -2885,37 +2851,19 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { *queried = false // Prepare to PushQuery [blk2] after receiving a Gossip message with [blk2]. sender.SendPushQueryF = func(_ context.Context, inVdrs set.Set[ids.NodeID], requestID uint32, blkBytes []byte) { - if *queried { - t.Fatalf("Asked multiple times") - } + require.False(*queried) *queried = true *queryRequestID = requestID - vdrSet := set.Set[ids.NodeID]{} - vdrSet.Add(vdr) - if !inVdrs.Equals(vdrSet) { - t.Fatalf("Asking wrong validator for preference") - } - if !bytes.Equal(blk2.Bytes(), blkBytes) { - t.Fatalf("Asking for wrong block") - } + require.Equal(expectedVdrSet, inVdrs) + require.Equal(blk2.Bytes(), blkBytes) } // Expect that the Engine will send a PushQuery after receiving this Gossip message for [blk2]. - if err := te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk2.Bytes()); err != nil { - t.Fatal(err) - } - - if !*queried { - t.Fatalf("Didn't ask for preferences regarding blk2") - } + require.NoError(te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk2.Bytes())) + require.True(*queried) // After a single vote for [blk2], it should be marked as accepted. - if err := te.Chits(context.Background(), vdr, *queryRequestID, []ids.ID{blk2.ID()}, nil); err != nil { - t.Fatal(err) - } - - if blk2.Status() != choices.Accepted { - t.Fatalf("Expected blk2 to be Accepted, but found status: %s", blk2.Status()) - } + require.NoError(te.Chits(context.Background(), vdr, *queryRequestID, []ids.ID{blk2.ID()}, nil)) + require.Equal(choices.Accepted, blk2.Status()) } // Test that in the following scenario, if block B fails verification, votes @@ -2933,6 +2881,10 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { // C func TestEngineBubbleVotesThroughInvalidChain(t *testing.T) { vdr, _, sender, vm, te, gBlk := setupDefaultConfig(t) + expectedVdrSet := set.Set[ids.NodeID]{} + expectedVdrSet.Add(vdr) + + require := require.New(t) // [blk1] is a child of [gBlk] and currently passes verification blk1 := &snowman.TestBlock{ @@ -2978,7 +2930,7 @@ func TestEngineBubbleVotesThroughInvalidChain(t *testing.T) { case bytes.Equal(b, blk3.Bytes()): return blk3, nil default: - t.Fatalf("Unknown block bytes") + require.FailNow("Unknown block bytes") return nil, nil } } @@ -2999,26 +2951,15 @@ func TestEngineBubbleVotesThroughInvalidChain(t *testing.T) { reqID := new(uint32) sender.SendGetF = func(_ context.Context, inVdr ids.NodeID, requestID uint32, blkID ids.ID) { *reqID = requestID - if *asked { - t.Fatalf("Asked multiple times") - } - if blkID != blk2.ID() { - t.Fatalf("Expected engine to request blk2") - } - if inVdr != vdr { - t.Fatalf("Expected engine to request blk2 from vdr") - } + require.False(*asked) + require.Equal(blk2.ID(), blkID) + require.Equal(vdr, inVdr) *asked = true } // Receive Gossip message for [blk3] first and expect the sender to issue a // Get request for its ancestor: [blk2]. - if err := te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk3.Bytes()); err != nil { - t.Fatal(err) - } - - if !*asked { - t.Fatalf("Didn't ask for missing blk2") - } + require.NoError(te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk3.Bytes())) + require.True(*asked) // Prepare to PushQuery [blk1] after our request for [blk2] is fulfilled. // We should not PushQuery [blk2] since it currently fails verification. @@ -3026,29 +2967,16 @@ func TestEngineBubbleVotesThroughInvalidChain(t *testing.T) { queried := new(bool) queryRequestID := new(uint32) sender.SendPushQueryF = func(_ context.Context, inVdrs set.Set[ids.NodeID], requestID uint32, blkBytes []byte) { - if *queried { - t.Fatalf("Asked multiple times") - } + require.False(*queried) *queried = true *queryRequestID = requestID - vdrSet := set.Set[ids.NodeID]{} - vdrSet.Add(vdr) - if !inVdrs.Equals(vdrSet) { - t.Fatalf("Asking wrong validator for preference") - } - if !bytes.Equal(blk1.Bytes(), blkBytes) { - t.Fatalf("Asking for wrong block") - } + require.Equal(expectedVdrSet, inVdrs) + require.Equal(blk1.Bytes(), blkBytes) } // Answer the request, this should result in [blk1] being issued as well. - if err := te.Put(context.Background(), vdr, *reqID, blk2.Bytes()); err != nil { - t.Fatal(err) - } - - if !*queried { - t.Fatalf("Didn't ask for preferences regarding blk1") - } + require.NoError(te.Put(context.Background(), vdr, *reqID, blk2.Bytes())) + require.True(*queried) sendReqID := new(uint32) reqVdr := new(ids.NodeID) @@ -3057,7 +2985,7 @@ func TestEngineBubbleVotesThroughInvalidChain(t *testing.T) { sender.SendGetF = func(_ context.Context, inVdr ids.NodeID, requestID uint32, blkID ids.ID) { switch blkID { case blk1.ID(): - t.Fatal("Unexpectedly sent a Get request for blk1") + require.FailNow("Unexpectedly sent a Get request for blk1") case blk2.ID(): t.Logf("sending get for blk2 with %d", requestID) *sendReqID = requestID @@ -3069,41 +2997,22 @@ func TestEngineBubbleVotesThroughInvalidChain(t *testing.T) { *reqVdr = inVdr return default: - t.Fatal("Unexpectedly sent a Get request for unknown block") + require.FailNow("Unexpectedly sent a Get request for unknown block") } } - sender.SendPullQueryF = func(_ context.Context, _ set.Set[ids.NodeID], _ uint32, blkID ids.ID) { - switch blkID { - case blk1.ID(): - t.Fatal("Unexpectedly sent a PullQuery request for blk1") - case blk2.ID(): - t.Fatal("Unexpectedly sent a PullQuery request for blk2") - case blk3.ID(): - t.Fatal("Unexpectedly sent a PullQuery request for blk3") - default: - t.Fatal("Unexpectedly sent a PullQuery request for unknown block") - } - } + // Now we are expecting a Chits message and we receive it for [blk3]. + // This will cause the node to again request [blk3]. + require.NoError(te.Chits(context.Background(), vdr, *queryRequestID, []ids.ID{blk3.ID()}, nil)) - // Now we are expecting a Chits message, and we receive it for [blk3] - // instead of blk1. This will cause the node to again request [blk3]. - if err := te.Chits(context.Background(), vdr, *queryRequestID, []ids.ID{blk3.ID()}, nil); err != nil { - t.Fatal(err) - } - - // Drop the re-request for blk3 to cause the poll to termindate. The votes - // should be bubbled through blk3 despite the fact that it hasn't been + // Drop the re-request for [blk3] to cause the poll to terminate. The votes + // should be bubbled through [blk3] despite the fact that it hasn't been // issued. - if err := te.GetFailed(context.Background(), *reqVdr, *sendReqID); err != nil { - t.Fatal(err) - } + require.NoError(te.GetFailed(context.Background(), *reqVdr, *sendReqID)) // The vote should be bubbled through [blk3] and [blk2] such that [blk1] // gets marked as Accepted. - if blk1.Status() != choices.Accepted { - t.Fatalf("Expected blk1 to be Accepted, but found status: %s", blk1.Status()) - } + require.Equal(choices.Accepted, blk1.Status()) } func TestMixedQueryNumPushSet(t *testing.T) {