From 4cf20fff71863807b193134746998be0b1a1df01 Mon Sep 17 00:00:00 2001 From: Eugen Zalinescu Date: Tue, 20 May 2025 15:57:13 +0200 Subject: [PATCH 1/4] DAL: remove references to closed issues --- src/lib_dal_node/constants.ml | 4 ---- src/lib_dal_node/daemon.ml | 5 ----- src/lib_dal_node/dal_metrics.ml | 6 ------ src/lib_dal_node/errors.ml | 5 ----- src/lib_dal_node/gossipsub/gs_interface.ml | 4 +--- .../gossipsub/gs_transport_connection.ml | 14 ++------------ .../transport_layer_default_parameters.ml | 5 +---- .../gossipsub/transport_layer_interface.ml | 6 ------ src/lib_dal_node/kvs_skip_list_cells_store.mli | 11 ----------- src/lib_dal_node/store.ml | 4 ---- src/lib_gossipsub/gossipsub_automaton.ml | 3 +-- src/proto_alpha/lib_protocol/alpha_context.mli | 5 ++--- .../lib_protocol/dal_attestation_repr.ml | 17 ----------------- src/proto_alpha/lib_protocol/dal_slot_repr.ml | 2 -- 14 files changed, 7 insertions(+), 84 deletions(-) diff --git a/src/lib_dal_node/constants.ml b/src/lib_dal_node/constants.ml index 835333368876..6a293185b95b 100644 --- a/src/lib_dal_node/constants.ml +++ b/src/lib_dal_node/constants.ml @@ -23,10 +23,6 @@ (* *) (*****************************************************************************) -(* FIXME: https://gitlab.com/tezos/tezos/-/issues/4458 - - Better handling of this limitation. *) - (* Each entry in the cache maintains two open file descriptors (one via regular file opening and one via mmap on the bitset region). So the selected value should be bigger than twice the number of slots per level, diff --git a/src/lib_dal_node/daemon.ml b/src/lib_dal_node/daemon.ml index 5d23431aac31..1e714753c8bd 100644 --- a/src/lib_dal_node/daemon.ml +++ b/src/lib_dal_node/daemon.ml @@ -81,8 +81,6 @@ let on_new_finalized_head ctxt cctxt crawler = loop () let daemonize handlers = - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/3605 - Improve concurrent tasks by using workers *) let open Lwt_result_syntax in let* handlers = List.map_es (fun x -> x) handlers in let (_ : Lwt_exit.clean_up_callback_id) = @@ -224,9 +222,6 @@ let update_and_register_profiles ctxt = let*! () = Node_context.set_profile_ctxt ctxt profile_ctxt in return_unit -(* FIXME: https://gitlab.com/tezos/tezos/-/issues/3605 - Improve general architecture, handle L1 disconnection etc -*) let run ~data_dir ~configuration_override = let open Lwt_result_syntax in let log_cfg = Tezos_base_unix.Logs_simple_config.default_cfg in diff --git a/src/lib_dal_node/dal_metrics.ml b/src/lib_dal_node/dal_metrics.ml index df2f7c04cd27..502c250e6763 100644 --- a/src/lib_dal_node/dal_metrics.ml +++ b/src/lib_dal_node/dal_metrics.ml @@ -265,9 +265,6 @@ module GS = struct Format.asprintf "%a" Signature.Public_key_hash.pp pkh; ] - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/6593 - - Be able to clean peers from metrics once they are disconnected. *) let collect_peers_per_topic_metrics gs_state = let module W = Gossipsub.Worker in W.GS.Topic.Map.fold @@ -282,9 +279,6 @@ module GS = struct gs_state.W.GS.Introspection.mesh Prometheus.LabelSetMap.empty - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/6593 - - Be able to clean peers from metrics onces they are disconnected. *) let collect_scores_of_peers_metrics gs_state = let module W = Gossipsub.Worker in W.GS.Peer.Map.fold diff --git a/src/lib_dal_node/errors.ml b/src/lib_dal_node/errors.ml index 7827e8dbe62c..d42d6ec462f1 100644 --- a/src/lib_dal_node/errors.ml +++ b/src/lib_dal_node/errors.ml @@ -34,11 +34,6 @@ type error += | Not_enough_l1_history of {stored_cycles : int; minimal_cycles : int} | Amplificator_initialization_failed -(* TODO: https://gitlab.com/tezos/tezos/-/issues/4622 - - Move errors from different DAL modules to this file. -*) - let () = register_error_kind `Permanent diff --git a/src/lib_dal_node/gossipsub/gs_interface.ml b/src/lib_dal_node/gossipsub/gs_interface.ml index 6990c98154ff..c6ce6d3a18ee 100644 --- a/src/lib_dal_node/gossipsub/gs_interface.ml +++ b/src/lib_dal_node/gossipsub/gs_interface.ml @@ -108,11 +108,9 @@ module Worker_config : let maybe_reachable_point Types.Peer.{maybe_reachable_point; _} = maybe_reachable_point - (* TODO: https://gitlab.com/tezos/tezos/-/issues/5596 - - Use Seq_s instead of Lwt_stream to implement module Stream. *) module Stream = struct type 'a t = { + (* In principle [Seq_s] could also be used instead. *) stream : 'a Lwt_stream.t; pusher : 'a option -> unit; mutable length : int; diff --git a/src/lib_dal_node/gossipsub/gs_transport_connection.ml b/src/lib_dal_node/gossipsub/gs_transport_connection.ml index 2e0b20767676..feac0889b9f3 100644 --- a/src/lib_dal_node/gossipsub/gs_transport_connection.ml +++ b/src/lib_dal_node/gossipsub/gs_transport_connection.ml @@ -139,9 +139,8 @@ let new_connections_handler gs_worker p2p_layer peer_id conn = fold_pool_opt P2p_pool.Points.get_trusted (addr, port)) in let trusted = trusted_peer || trusted_point in - (* TODO: https://gitlab.com/tezos/tezos/-/issues/5584 - - Add the ability to have direct peers. *) + (* Currently direct peers are not supported. The "private mode" mechanism + present in octez-p2p could maybe be used for that. *) let direct = false in let peer = peer_of_connection p2p_layer conn in Worker.( @@ -190,10 +189,6 @@ let try_connect ?expected_peer_id gs_worker p2p_layer ~trusted point = wipe / regenerate their peer identities while keeping the same IP addresses. The [expected_peer_id] check, if enabled, will make Octez-p2p reject any other connection with a different identity. *) - (* DAL/TODO: https://gitlab.com/tezos/tezos/-/issues/7646 - - Implement a better PX exchange. - *) ignore expected_peer_id ; let* (result : _ P2p.connection tzresult) = P2p.connect ~trusted p2p_layer point @@ -230,11 +225,6 @@ let gs_worker_p2p_output_handler gs_worker p2p_layer = | None -> (* This could happen when the peer is disconnected or the connection is accepted but not running (authenticated) yet. *) - (* TODO: https://gitlab.com/tezos/tezos/-/issues/5649 - - Are there weird cases in which there is no connection - associated to the peer, but the peer is still registered as - connected on the GS side? *) Events.(emit no_connection_for_peer to_peer.peer_id) | Some conn -> ( let* (res : unit tzresult) = diff --git a/src/lib_dal_node/gossipsub/transport_layer_default_parameters.ml b/src/lib_dal_node/gossipsub/transport_layer_default_parameters.ml index b0b8c0ae837f..8bf17451a8d3 100644 --- a/src/lib_dal_node/gossipsub/transport_layer_default_parameters.ml +++ b/src/lib_dal_node/gossipsub/transport_layer_default_parameters.ml @@ -24,10 +24,7 @@ (* *) (*****************************************************************************) -(* TODO: https://gitlab.com/tezos/tezos/-/issues/5565 - - Document these values (taken from octez-node), and chage them if needed. -*) +(* Most of the values below are taken from the octez-node. *) module P2p_limits = struct let connection_timeout = Time.System.Span.of_seconds_exn 10. diff --git a/src/lib_dal_node/gossipsub/transport_layer_interface.ml b/src/lib_dal_node/gossipsub/transport_layer_interface.ml index a59fd98a6dc3..b250118b81e9 100644 --- a/src/lib_dal_node/gossipsub/transport_layer_interface.ml +++ b/src/lib_dal_node/gossipsub/transport_layer_interface.ml @@ -26,9 +26,6 @@ module Types = Tezos_dal_node_services.Types -(* FIXME: https://gitlab.com/tezos/tezos/-/issues/5583 - - Version this type to ease future migrations. *) module P2p_message_V1 = struct type px_peer = Types.Peer.t @@ -204,9 +201,6 @@ module P2p_message_V1 = struct let distributed_db_version = Distributed_db_version.zero - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/5638 - - Decide how to safely choose the node db version. *) let distributed_db_versions = [distributed_db_version] let message_config ~network_name : p2p_message P2p_params.message_config = diff --git a/src/lib_dal_node/kvs_skip_list_cells_store.mli b/src/lib_dal_node/kvs_skip_list_cells_store.mli index 729889f11097..f2fa16da0af9 100644 --- a/src/lib_dal_node/kvs_skip_list_cells_store.mli +++ b/src/lib_dal_node/kvs_skip_list_cells_store.mli @@ -24,17 +24,6 @@ (* *) (*****************************************************************************) -(* TODO: https://gitlab.com/tezos/tezos/-/issues/7068 - - Think about a better implementation of the skip list cells store that: - - - Doesn't add padding (KVS with values of variable size) - - - Doesn't encode to intermediate structures (like - [Dal_proto_types.Skip_list_cell]). Maybe have a per-protocol store in this - case, and move most of the store creating and updating to the proto plugin - to avoid existential type variables issues. -*) (** This module instantiates the key value store to provide facilities for storing and retrieving the cells of the DAL skip list. The store maintains: diff --git a/src/lib_dal_node/store.ml b/src/lib_dal_node/store.ml index 4854f35f7ba5..7b8663fee649 100644 --- a/src/lib_dal_node/store.ml +++ b/src/lib_dal_node/store.ml @@ -204,10 +204,6 @@ module Shards = struct shards |> Errors.other_lwt_result in - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/4974 - - DAL/Node: rehaul the store abstraction & notification system. - *) return_unit let read_all shards_store slot_id ~number_of_shards = diff --git a/src/lib_gossipsub/gossipsub_automaton.ml b/src/lib_gossipsub/gossipsub_automaton.ml index 32ef277c37f3..a0cb0d5a6b55 100644 --- a/src/lib_gossipsub/gossipsub_automaton.ml +++ b/src/lib_gossipsub/gossipsub_automaton.ml @@ -750,8 +750,7 @@ module Make (C : AUTOMATON_CONFIG) : | `unknown_peer -> return Subscribe_to_unknown_peer | `subscribed connections -> let* () = set_connections connections in - (* TODO: https://gitlab.com/tezos/tezos/-/issues/5143 - rust-libp2p adds the peer to the mesh if needed here. *) + (* Note: rust-libp2p adds the peer to the mesh if needed here. *) return Subscribed end diff --git a/src/proto_alpha/lib_protocol/alpha_context.mli b/src/proto_alpha/lib_protocol/alpha_context.mli index 230b0fc3c056..f4c3fb0fea45 100644 --- a/src/proto_alpha/lib_protocol/alpha_context.mli +++ b/src/proto_alpha/lib_protocol/alpha_context.mli @@ -3126,9 +3126,8 @@ end (** This module re-exports definitions from {!Dal_errors_repr}. *) module Dal_errors : sig - (* DAL/FIXME: https://gitlab.com/tezos/tezos/-/issues/3168 - do not expose these errors and return them in functions - from Dal_slot_repr or Dal_attestation_repr. *) + (* We could not expose these errors and instead return them in functions from + [Dal_slot_repr] or [Dal_attestation_repr]. *) type error += | Dal_feature_disabled | Dal_incentives_disabled diff --git a/src/proto_alpha/lib_protocol/dal_attestation_repr.ml b/src/proto_alpha/lib_protocol/dal_attestation_repr.ml index 1e151b0a47d9..f72917eeda47 100644 --- a/src/proto_alpha/lib_protocol/dal_attestation_repr.ml +++ b/src/proto_alpha/lib_protocol/dal_attestation_repr.ml @@ -23,23 +23,6 @@ (* *) (*****************************************************************************) -(* DAL/FIXME https://gitlab.com/tezos/tezos/-/issues/3103 - - This may be a bit heavy in practice. We could also assume that in - practice, many bits in this bitfield will be set to one. Hence, we - could consider a better encoding which is smaller in the optimistic - case. For example: - - 1. When all the slots are attested, the encoding can be represented - in one bit. - - 2. Otherwise, we can pack slots by [8]. Have a header of [slots/8] - which is [1] if all the slots in this set are [1], [0] - otherwise. For all pack with a bit set to [0], we give the explicit - representation. Hence, if there are [256] slots, and [2] are not - attested, this representation will be of size [32] bits + [16] bits - = [48] bits which is better than [256] bits. *) - (* A set of (attested) slot indexes. *) type t = Bitset.t diff --git a/src/proto_alpha/lib_protocol/dal_slot_repr.ml b/src/proto_alpha/lib_protocol/dal_slot_repr.ml index febb1cd8bc15..1085f5e8ff5a 100644 --- a/src/proto_alpha/lib_protocol/dal_slot_repr.ml +++ b/src/proto_alpha/lib_protocol/dal_slot_repr.ml @@ -987,8 +987,6 @@ module History = struct type proof = bytes - (** DAL/FIXME: https://gitlab.com/tezos/tezos/-/issues/4084 - DAL proof's encoding should be bounded *) let proof_encoding = Data_encoding.(bytes Hex) type error += Dal_invalid_proof_serialization -- GitLab From 829ef021b0fc737378adc25324927d65dfd4a3af Mon Sep 17 00:00:00 2001 From: Eugen Zalinescu Date: Tue, 20 May 2025 15:58:09 +0200 Subject: [PATCH 2/4] DAL/Node: remove reference to issue #6391 --- .../gossipsub/transport_layer_default_parameters.ml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/lib_dal_node/gossipsub/transport_layer_default_parameters.ml b/src/lib_dal_node/gossipsub/transport_layer_default_parameters.ml index 8bf17451a8d3..6653c3ee12d0 100644 --- a/src/lib_dal_node/gossipsub/transport_layer_default_parameters.ml +++ b/src/lib_dal_node/gossipsub/transport_layer_default_parameters.ml @@ -32,12 +32,8 @@ module P2p_limits = struct let greylist_timeout = Time.System.Span.of_seconds_exn 86400. (* one day *) - (* Some of the default values below taken from L1 node instantiation of - Octez-p2p library. We amplify the value by this constant. *) - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/6391 - - Decide which default value we should choose for P2P parameters and wheter we - specialize them per profile kind. *) + (* Some of the values taken from the L1 node's instantiation of the + octez-p2p library are amplified by the following constant. *) let dal_amplification_factor = 8 (* [maintenance_idle_time] is set to [None] to disable the internal maintenance -- GitLab From 8598e16025c551a774198d1913f9e401f18190f9 Mon Sep 17 00:00:00 2001 From: Eugen Zalinescu Date: Tue, 20 May 2025 16:14:02 +0200 Subject: [PATCH 3/4] DAL/GS: reference #5415 as a TODO not a FIXME --- src/lib_gossipsub/gossipsub_intf.ml | 6 +++++- src/lib_gossipsub/gossipsub_worker.ml | 4 ---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib_gossipsub/gossipsub_intf.ml b/src/lib_gossipsub/gossipsub_intf.ml index 3d6f20c3ac17..1c010c0ff7a2 100644 --- a/src/lib_gossipsub/gossipsub_intf.ml +++ b/src/lib_gossipsub/gossipsub_intf.ml @@ -1106,7 +1106,11 @@ module type WORKER = sig include WORKER_CONFIGURATION (** A message together with a header, that is, a topic and an id. - This corresponds to what the spec calls a "full message". *) + This corresponds to what the spec calls a "full message". + + TODO: https://gitlab.com/tezos/tezos/-/issues/5415 + The topic can be inferred from the message_id and can thus be omitted; + this also applies for the IHave message. *) type message_with_header = { message : GS.Message.t; topic : GS.Topic.t; diff --git a/src/lib_gossipsub/gossipsub_worker.ml b/src/lib_gossipsub/gossipsub_worker.ml index ba4dc25fba98..1389e3520bc1 100644 --- a/src/lib_gossipsub/gossipsub_worker.ml +++ b/src/lib_gossipsub/gossipsub_worker.ml @@ -560,10 +560,6 @@ module Make (C : Gossipsub_intf.WORKER_CONFIGURATION) : | `Message message -> Introspection.update_count_recv_iwants state.stats `Incr ; let topic = Message_id.get_topic message_id in - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/5415 - - Don't provide a topic when it can be inferred (e.g. from - Message_id.t). This also applies for Message_with_header and IHave. *) let message_with_header = {message; topic; message_id} in let p2p_message = Message_with_header message_with_header in emit_p2p_message state p2p_message (Seq.return peer) -- GitLab From 2dac8a8848c00249b99c41c216fc8196fb5761c4 Mon Sep 17 00:00:00 2001 From: Eugen Zalinescu Date: Tue, 20 May 2025 16:01:23 +0200 Subject: [PATCH 4/4] DAL/Node: add event when commitment not found in cache --- src/lib_dal_node/event.ml | 12 ++++++++++++ src/lib_dal_node/slot_manager.ml | 17 ++++++----------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/lib_dal_node/event.ml b/src/lib_dal_node/event.ml index d2885b13c986..ed2b6fb5ee40 100644 --- a/src/lib_dal_node/event.ml +++ b/src/lib_dal_node/event.ml @@ -1007,6 +1007,15 @@ open struct ~msg:"catching up done" ~level:Notice () + + let commitment_not_found_in_cache = + declare_1 + ~section + ~prefix_name_with_section:true + ~name:"commitment_not_found_in_cache" + ~msg:"commitment {commitment} was not found in the cache" + ~level:Warning + ("commitment", Cryptobox.Commitment.encoding) end (* DAL node event emission functions *) @@ -1267,3 +1276,6 @@ let emit_start_catchup ~start_level ~end_level ~levels_to_clean_up = let emit_catching_up ~current_level = emit catching_up current_level let emit_end_catchup () = emit end_catchup () + +let emit_commitment_not_found_in_cache ~commitment = + emit commitment_not_found_in_cache commitment diff --git a/src/lib_dal_node/slot_manager.ml b/src/lib_dal_node/slot_manager.ml index ef15a02fc82a..25a2f075388a 100644 --- a/src/lib_dal_node/slot_manager.ml +++ b/src/lib_dal_node/slot_manager.ml @@ -347,17 +347,12 @@ let publish_slot_data ctxt ~level_committee ~slot_size gs_worker let cache = Store.cache node_store in match Store.Commitment_indexed_cache.find_opt cache commitment with | None -> - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/5676 - - If this happens, we should probably tell the user that - something bad happened. Either: - - 1. The proofs where not stored properly (an invariant is broken) - - 2. The node was restarted (unlikely to happen given the time frame) - - 3. The cache was full (unlikely to happen if - [shards_proofs_cache_size] is set properly. *) + (* This is unexpected. Either: + 1. The proofs where not stored properly (an invariant is broken) + 2. The node was restarted (unlikely to happen given the time frame) + 3. The cache was full (unlikely to happen if + [shards_proofs_cache_size] is set properly. *) + let*! () = Event.emit_commitment_not_found_in_cache ~commitment in return_unit | Some (slot, shares, shard_proofs) -> let shards = -- GitLab