From af6964dc694c756f181195d2b756b85a8c52c773 Mon Sep 17 00:00:00 2001 From: lin Date: Thu, 4 May 2023 22:28:18 +0900 Subject: [PATCH 1/5] Gossipsub: Expose get_topic_params for testing purpose --- src/lib_gossipsub/gossipsub_intf.ml | 7 +++++++ src/lib_gossipsub/peers_score.ml | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/src/lib_gossipsub/gossipsub_intf.ml b/src/lib_gossipsub/gossipsub_intf.ml index 350af6ec385f..3612c9b97a4e 100644 --- a/src/lib_gossipsub/gossipsub_intf.ml +++ b/src/lib_gossipsub/gossipsub_intf.ml @@ -409,6 +409,13 @@ module type SCORE = sig include PRINTABLE with type t := t val pp_value : Format.formatter -> value -> unit + + module Internal_for_tests : sig + val get_topic_params : + ('topic, 'span) score_parameters -> + 'topic -> + 'span per_topic_score_parameters + end end module type AUTOMATON = sig diff --git a/src/lib_gossipsub/peers_score.ml b/src/lib_gossipsub/peers_score.ml index 6ad3e338a803..3f4906d0875a 100644 --- a/src/lib_gossipsub/peers_score.ml +++ b/src/lib_gossipsub/peers_score.ml @@ -522,4 +522,8 @@ struct ] let pp_value = Fmt.float + + module Internal_for_tests = struct + let get_topic_params = get_topic_params + end end -- GitLab From 8ac3a20565b63c12a58f93a1546c646ca7d2ed81 Mon Sep 17 00:00:00 2001 From: lin Date: Thu, 4 May 2023 02:34:27 +0900 Subject: [PATCH 2/5] Gossipsub/Test: Change Time.elapse to take Milliseconds --- src/lib_gossipsub/test/test_gossipsub_shared.ml | 6 +++--- src/lib_gossipsub/test/test_unit.ml | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib_gossipsub/test/test_gossipsub_shared.ml b/src/lib_gossipsub/test/test_gossipsub_shared.ml index 4a75519721f0..b74d8a843477 100644 --- a/src/lib_gossipsub/test/test_gossipsub_shared.ml +++ b/src/lib_gossipsub/test/test_gossipsub_shared.ml @@ -94,9 +94,9 @@ module Time = struct let now () = !t - let elapse s = - assert (s >= 0) ; - t := Milliseconds.add !t (Milliseconds.of_int_s s) + let elapse ms = + assert (Milliseconds.(ms >= zero)) ; + t := Milliseconds.add !t ms let set now = t := now diff --git a/src/lib_gossipsub/test/test_unit.ml b/src/lib_gossipsub/test/test_unit.ml index 47ca42fcb4d0..3c62d8fd0e17 100644 --- a/src/lib_gossipsub/test/test_unit.ml +++ b/src/lib_gossipsub/test/test_unit.ml @@ -946,7 +946,7 @@ let test_do_not_graft_within_backoff_period rng limits parameters = |> WithExceptions.Result.get_ok ~loc:__LOC__ |> List.fold_left (fun state i -> - Time.elapse 1 ; + Time.elapse @@ Milliseconds.of_int_s 1 ; Log.info "%d time tick(s) elapsed..." i ; let state, Heartbeat {to_graft; _} = GS.heartbeat state in let grafts = Peer.Map.bindings to_graft in @@ -960,7 +960,7 @@ let test_do_not_graft_within_backoff_period rng limits parameters = in (* After elapsing one more second, the backoff should be cleared and the graft should be emitted. *) - Time.elapse 1 ; + Time.elapse @@ Milliseconds.of_int_s 1 ; let _state, Heartbeat {to_graft; _} = GS.heartbeat state in let grafts = Peer.Map.bindings to_graft in Check.((List.length grafts = 1) int ~error_msg:"Expected %R, got %L" ~__LOC__) ; @@ -1009,7 +1009,7 @@ let test_unsubscribe_backoff rng limits parameters = |> WithExceptions.Result.get_ok ~loc:__LOC__ |> List.fold_left (fun state i -> - Time.elapse 1 ; + Time.elapse @@ Milliseconds.of_int_s 1 ; Log.info "%d time tick(s) elapsed..." i ; let state, Heartbeat {to_graft; _} = GS.heartbeat state in let grafts = Peer.Map.bindings to_graft in @@ -1023,7 +1023,7 @@ let test_unsubscribe_backoff rng limits parameters = in (* After elapsing one more second, the backoff should be cleared and the graft should be emitted. *) - Time.elapse 1 ; + Time.elapse @@ Milliseconds.of_int_s 1 ; let _state, Heartbeat {to_graft; _} = GS.heartbeat state in let grafts = Peer.Map.bindings to_graft in Check.((List.length grafts = 1) int ~error_msg:"Expected %R, got %L" ~__LOC__) ; -- GitLab From 0bd770d2c5bc7b834548eef9d3250f8083071af2 Mon Sep 17 00:00:00 2001 From: lin Date: Thu, 4 May 2023 22:27:42 +0900 Subject: [PATCH 3/5] Gossipsub/Test: Add Milliseconds.to_int_ms --- src/lib_gossipsub/test/test_gossipsub_shared.ml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib_gossipsub/test/test_gossipsub_shared.ml b/src/lib_gossipsub/test/test_gossipsub_shared.ml index b74d8a843477..0c7f21a35446 100644 --- a/src/lib_gossipsub/test/test_gossipsub_shared.ml +++ b/src/lib_gossipsub/test/test_gossipsub_shared.ml @@ -36,6 +36,8 @@ module Milliseconds = struct let of_int_s n = {ms = n * 1000} + let to_int_ms {ms} = ms + let zero = {ms = 0} let add m1 m2 = of_int_ms (m1.ms + m2.ms) @@ -63,6 +65,8 @@ module Milliseconds = struct let to_int_s = to_int_s + let to_int_ms = to_int_ms + let to_float_s = to_float_s let of_int_s = of_int_s -- GitLab From 43275669dde4fbb5f26ba8ef60711fbed1f1027c Mon Sep 17 00:00:00 2001 From: lin Date: Thu, 4 May 2023 02:37:35 +0900 Subject: [PATCH 4/5] Gossipsub/Test: Lower heartbeat interval in test_unsubscribe_backoff This is to prevent exceeding [mesh_message_deliveries_activation], which defaults to 5 seconds, within this test --- src/lib_gossipsub/test/test_unit.ml | 40 +++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/src/lib_gossipsub/test/test_unit.ml b/src/lib_gossipsub/test/test_unit.ml index 3c62d8fd0e17..7fd637b1d0c8 100644 --- a/src/lib_gossipsub/test/test_unit.ml +++ b/src/lib_gossipsub/test/test_unit.ml @@ -978,20 +978,40 @@ let test_unsubscribe_backoff rng limits parameters = ~tags:["gossipsub"; "heartbeat"; "join"; "leave"] @@ fun () -> let topic = "topic" in + let per_topic_score_parameters = + GS.Score.Internal_for_tests.get_topic_params limits.score_parameters topic + in + let mesh_message_deliveries_activation = + per_topic_score_parameters.mesh_message_deliveries_activation + in (* Only one peer => mesh too small and will try to regraft as early as possible *) let peers = make_peers ~number:1 in + (* Number of ticks until the unsubscribe backoff expires. *) + let backoff_ticks = 5 in + (* We must set [heartbeat_interval] so that + number_of_heartbeats_in_the_test * [heartbeat_interval] < [mesh_message_deliveries_activation] + holds. This prevents surpassing [mesh_message_deliveries_activation] within the test, + thus avoiding the activation of p3 penalty. Since number_of_heartbeats_in_the_test + is [backoff_ticks + 2], we set [heartbeat_interval] as the following.*) + let heartbeat_interval = + (Milliseconds.to_int_ms mesh_message_deliveries_activation - 1) + / (backoff_ticks + 2) + |> Milliseconds.of_int_ms + in + (* Time required until unsubscribe backoff expires. *) + let unsubscribe_backoff = + Milliseconds.(of_int_ms @@ (backoff_ticks * to_int_ms heartbeat_interval)) + in let state = init_state ~rng ~limits: { limits with + heartbeat_interval; (* Run backoff clearing on every heartbeat tick. *) backoff_cleanup_ticks = 1; - (* We will run the heartbeat tick on each time tick to simplify the test. *) - heartbeat_interval = Milliseconds.of_int_s 1; - (* Set unsubscribe backoff to 5. *) - unsubscribe_backoff = Milliseconds.of_int_s 5; + unsubscribe_backoff; } ~parameters ~peers @@ -1002,14 +1022,14 @@ let test_unsubscribe_backoff rng limits parameters = (* Peer unsubscribes then subscribes from topic. *) let state, _ = GS.leave {topic} state in let state, _ = GS.join {topic} state in - (* No graft should be emitted until 7 time ticks pass. - The additional 2 time ticks from the backoff is due to the "backoff slack". *) + (* No graft should be emitted until [(backoff_ticks + 2) * heartbeat_interval] elapse. + The additional 2 [heartbeat_interval] is due to the "backoff slack". *) let state = - List.init ~when_negative_length:() 6 (fun i -> i + 1) + List.init ~when_negative_length:() (backoff_ticks + 1) (fun i -> i + 1) |> WithExceptions.Result.get_ok ~loc:__LOC__ |> List.fold_left (fun state i -> - Time.elapse @@ Milliseconds.of_int_s 1 ; + Time.elapse @@ heartbeat_interval ; Log.info "%d time tick(s) elapsed..." i ; let state, Heartbeat {to_graft; _} = GS.heartbeat state in let grafts = Peer.Map.bindings to_graft in @@ -1021,9 +1041,9 @@ let test_unsubscribe_backoff rng limits parameters = state) state in - (* After elapsing one more second, + (* After elapsing one more [heartbeat_interval], the backoff should be cleared and the graft should be emitted. *) - Time.elapse @@ Milliseconds.of_int_s 1 ; + Time.elapse @@ heartbeat_interval ; let _state, Heartbeat {to_graft; _} = GS.heartbeat state in let grafts = Peer.Map.bindings to_graft in Check.((List.length grafts = 1) int ~error_msg:"Expected %R, got %L" ~__LOC__) ; -- GitLab From 4476f14d92ff1b484647c1af1260b4a2ec919dc8 Mon Sep 17 00:00:00 2001 From: lin Date: Wed, 3 May 2023 19:59:24 +0900 Subject: [PATCH 5/5] Gossipsub: Properly notify scoring about graft We are missing some Score.graft calls --- src/lib_gossipsub/tezos_gossipsub.ml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/lib_gossipsub/tezos_gossipsub.ml b/src/lib_gossipsub/tezos_gossipsub.ml index 2ece1d7a3780..34b4f907a5f5 100644 --- a/src/lib_gossipsub/tezos_gossipsub.ml +++ b/src/lib_gossipsub/tezos_gossipsub.ml @@ -1110,6 +1110,15 @@ module Make (C : AUTOMATON_CONFIG) : in return (Peer.Set.union more_peers valid_fanout_peers) in + (* Notify scoring about the graft. *) + let scores = + Peer.Set.fold + (fun peer scores -> + update_scores_score peer (fun s -> Score.graft s topic) scores) + peers + scores + in + let* () = set_scores scores in let* () = set_mesh_topic topic peers in let* () = delete_fanout topic in Joining_topic {to_graft = peers} |> return @@ -1608,6 +1617,16 @@ module Make (C : AUTOMATON_CONFIG) : backoff |> set_backoff in + (* Notify scoring about the grafts. *) + let scores = + Peer.Map.fold + (fun peer -> + Topic.Set.fold (fun topic -> + update_scores_score peer (fun s -> Score.graft s topic))) + to_graft + scores + in + let* () = set_scores scores in (* Update mesh for grafted and pruned peers *) let* () = update_mesh mesh ~to_graft ~to_prune in return (to_graft, to_prune, noPX_peers) -- GitLab