From 2e153996c286687a79ac4edd71760299b16c3e7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Thu, 25 Apr 2024 10:11:53 +0200 Subject: [PATCH 01/11] DAL: don't wrap status encoding in an obj1 --- src/lib_dal_node_services/types.ml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib_dal_node_services/types.ml b/src/lib_dal_node_services/types.ml index c3a60adb90b6..81419d7aa130 100644 --- a/src/lib_dal_node_services/types.ml +++ b/src/lib_dal_node_services/types.ml @@ -361,19 +361,19 @@ let header_status_encoding : header_status Data_encoding.t = case ~title:"waiting_attestation" (Tag 0) - (obj1 (req "status" (constant "waiting_attestation"))) + (constant "waiting_attestation") (function `Waiting_attestation -> Some () | _ -> None) (function () -> `Waiting_attestation); case ~title:"attested" (Tag 1) - (obj1 (req "status" (constant "attested"))) + (constant "attested") (function `Attested -> Some () | _ -> None) (function () -> `Attested); case ~title:"unattested" (Tag 2) - (obj1 (req "status" (constant "unattested"))) + (constant "unattested") (function `Unattested -> Some () | _ -> None) (function () -> `Unattested); ] @@ -385,9 +385,9 @@ let slot_header_encoding = (fun (slot_id, (commitment, status)) -> {slot_id; commitment; status}) (merge_objs slot_id_encoding - (merge_objs - (obj1 (req "commitment" Cryptobox.Commitment.encoding)) - header_status_encoding)) + (obj2 + (req "commitment" Cryptobox.Commitment.encoding) + (req "status" header_status_encoding))) let profile_encoding = let open Data_encoding in @@ -428,7 +428,7 @@ let with_proof_encoding = let header_status_arg = let destruct s = - let s = `O [("status", `String s)] in + let s = `String s in try Ok (Data_encoding.Json.destruct header_status_encoding s) with _ -> Error "Cannot parse header status value" in -- GitLab From d49e840b4c1283ef81ce9d39e0e87ac1f75974a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 10:58:20 +0200 Subject: [PATCH 02/11] DAL/Test: Remove unused check_headers function This is dead code. --- tezt/tests/dal.ml | 37 +------------------------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index d01a0f63e768..7cd1cd23fe36 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1384,8 +1384,7 @@ let check_published_level_headers ~__LOC__ dal_node ~pub_level (* Checks that [response] contains zero or one slot header. If [expected_status] is not given, the expected response is the empty list; if it is given, the - response should contain exactly one header, with the given status. The - function [check_headers] generalizes this check to more headers. *) + response should contain exactly one header, with the given status. *) let get_headers_succeeds ~__LOC__ ?expected_status response = let headers = JSON.( @@ -1414,40 +1413,6 @@ let get_headers_succeeds ~__LOC__ ?expected_status response = commitment, got %d." (List.length headers) -(* Similar to [get_headers_succeeds], except that it is more general, it does - not assume that there will be just 0 or 1 header. *) -let check_headers ~__LOC__ expected_headers response = - let headers = - JSON.( - parse ~origin:"check_headers" response.RPC_core.body - |> Dal_RPC.slot_headers_of_json) - in - Check.(List.length headers = List.length expected_headers) - ~__LOC__ - Check.int - ~error_msg:"check_headers: Expected %R headers, got %L." ; - let headers = - List.map (fun h -> (h.Dal_RPC.slot_level, h.slot_index, h.status)) headers - in - List.iter - (fun (level, index, expected_status) -> - let status_opt = - List.find_map - (fun (l, i, status) -> - if level = l && index = i then Some status else None) - headers - in - Check.(status_opt = Some expected_status) - ~__LOC__ - Check.(option string) - ~error_msg: - (Format.asprintf - "Expected to find for slot level %d and slot index %d the status \ - (%%R), found (%%L)." - level - index)) - expected_headers - let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node client dal_node = let check_published_level_headers = check_published_level_headers dal_node in -- GitLab From 0b0cf505384d0e7a81e46b9ea454b02940d5fadb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 10:36:14 +0200 Subject: [PATCH 03/11] DAL/Test: simplify check_get_commitment_headers This commit makes `check_get_commitment_headers` use `Dal_RPC.call` instead of `Dal_RPC.call_raw` + `JSON.parse`. This simplifies the code a bit and gives a static guarantee that we call get_headers_succeeds with the appropriate type. --- tezt/tests/dal.ml | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index 7cd1cd23fe36..ad1894e498c0 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1361,12 +1361,11 @@ let check_get_commitment_headers dal_node ~slot_level check_result slots_info = if not query_string then (None, None) else (Some slot_level, Some slot_index) in - let* response = + let* headers = Dal_RPC.( - call_raw dal_node - @@ get_commitment_headers ?slot_index ?slot_level commit) + call dal_node @@ get_commitment_headers ?slot_index ?slot_level commit) in - return @@ check_result response + return @@ check_result headers in let* () = Lwt_list.iter_s (test check_result ~query_string:true) slots_info in Lwt_list.iter_s (test check_result ~query_string:false) slots_info @@ -1382,15 +1381,10 @@ let check_published_level_headers ~__LOC__ dal_node ~pub_level ~error_msg:"Unexpected slot headers length (got = %L, expected = %R)" ; unit -(* Checks that [response] contains zero or one slot header. If [expected_status] +(* Checks that [headers] contains zero or one slot header. If [expected_status] is not given, the expected response is the empty list; if it is given, the response should contain exactly one header, with the given status. *) -let get_headers_succeeds ~__LOC__ ?expected_status response = - let headers = - JSON.( - parse ~origin:"get_headers_succeeds" response.RPC_core.body - |> Dal_RPC.slot_headers_of_json) - in +let get_headers_succeeds ~__LOC__ ?expected_status headers = match (headers, expected_status) with | [], None -> () | _ :: _, None -> -- GitLab From 5a7c2c012ba4efa43a445a8b1aaad31c2945e18c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 11:13:40 +0200 Subject: [PATCH 04/11] DAL/Test: check_get_commitment_headers after get_headers_succeeds In the next commit, we will make check_get_commitment_headers call get_headers_succeeds, this commit moves the former after the latter. --- tezt/tests/dal.ml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index ad1894e498c0..7fe84450d34e 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1355,21 +1355,6 @@ let get_commitment_succeeds expected_commitment response = let get_commitment_not_found _commit r = RPC_core.check_string_response ~code:404 r -let check_get_commitment_headers dal_node ~slot_level check_result slots_info = - let test check_result ~query_string (slot_index, commit) = - let slot_level, slot_index = - if not query_string then (None, None) - else (Some slot_level, Some slot_index) - in - let* headers = - Dal_RPC.( - call dal_node @@ get_commitment_headers ?slot_index ?slot_level commit) - in - return @@ check_result headers - in - let* () = Lwt_list.iter_s (test check_result ~query_string:true) slots_info in - Lwt_list.iter_s (test check_result ~query_string:false) slots_info - let check_published_level_headers ~__LOC__ dal_node ~pub_level ~number_of_headers = let* slot_headers = @@ -1407,6 +1392,21 @@ let get_headers_succeeds ~__LOC__ ?expected_status headers = commitment, got %d." (List.length headers) +let check_get_commitment_headers dal_node ~slot_level check_result slots_info = + let test check_result ~query_string (slot_index, commit) = + let slot_level, slot_index = + if not query_string then (None, None) + else (Some slot_level, Some slot_index) + in + let* headers = + Dal_RPC.( + call dal_node @@ get_commitment_headers ?slot_index ?slot_level commit) + in + return @@ check_result headers + in + let* () = Lwt_list.iter_s (test check_result ~query_string:true) slots_info in + Lwt_list.iter_s (test check_result ~query_string:false) slots_info + let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node client dal_node = let check_published_level_headers = check_published_level_headers dal_node in -- GitLab From 98e20a22c1e6fb8934aef177ec7d0275e357cb26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 11:15:52 +0200 Subject: [PATCH 05/11] DAL/Tests: check_get_commitment_headers without callback This commit makes check_get_commitment_headers call get_headers_succeeds directly rather than through the check_result callback. --- tezt/tests/dal.ml | 49 ++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index 7fe84450d34e..a1aac87cbd8d 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1392,8 +1392,9 @@ let get_headers_succeeds ~__LOC__ ?expected_status headers = commitment, got %d." (List.length headers) -let check_get_commitment_headers dal_node ~slot_level check_result slots_info = - let test check_result ~query_string (slot_index, commit) = +let check_get_commitment_headers ~__LOC__ ?expected_status dal_node ~slot_level + slots_info = + let test ~query_string (slot_index, commit) = let slot_level, slot_index = if not query_string then (None, None) else (Some slot_level, Some slot_index) @@ -1402,10 +1403,10 @@ let check_get_commitment_headers dal_node ~slot_level check_result slots_info = Dal_RPC.( call dal_node @@ get_commitment_headers ?slot_index ?slot_level commit) in - return @@ check_result headers + return @@ get_headers_succeeds ~__LOC__ ?expected_status headers in - let* () = Lwt_list.iter_s (test check_result ~query_string:true) slots_info in - Lwt_list.iter_s (test check_result ~query_string:false) slots_info + let* () = Lwt_list.iter_s (test ~query_string:true) slots_info in + Lwt_list.iter_s (test ~query_string:false) slots_info let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node client dal_node = @@ -1449,7 +1450,7 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node check_get_commitment_headers dal_node ~slot_level:level - (get_headers_succeeds ~__LOC__) + ~__LOC__ [slot0; slot1; slot2_a; slot2_b; slot3; slot4] in let* () = @@ -1468,7 +1469,7 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node check_get_commitment_headers dal_node ~slot_level:level - (get_headers_succeeds ~__LOC__) + ~__LOC__ [slot0; slot1; slot2_a; slot2_b; slot3; slot4] in @@ -1499,8 +1500,12 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node ~error_msg: "Published header is different from stored header (current = %L, \ expected = %R)" ; - let check_get_commitment_headers = - check_get_commitment_headers dal_node ~slot_level:pub_level + let check_get_commitment_headers ?expected_status l = + check_get_commitment_headers + ?expected_status + dal_node + ~slot_level:pub_level + l in let check_get_commitment = check_get_commitment dal_node ~slot_level:pub_level @@ -1510,13 +1515,12 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node let* () = check_get_commitment get_commitment_succeeds ok in let* () = check_get_commitment_headers - (get_headers_succeeds ~__LOC__ ~expected_status:"waiting_attestation") + ~__LOC__ + ~expected_status:"waiting_attestation" ok in (* slot_2_a is not selected. *) - let* () = - check_get_commitment_headers (get_headers_succeeds ~__LOC__) [slot2_a] - in + let* () = check_get_commitment_headers ~__LOC__ [slot2_a] in (* Slots not published or not included in blocks. *) let* () = check_get_commitment get_commitment_not_found ko in @@ -1550,31 +1554,24 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node let* () = check_get_commitment get_commitment_succeeds ok in (* Slot that were waiting for attestation and now attested. *) let* () = - check_get_commitment_headers - (get_headers_succeeds ~__LOC__ ~expected_status:"attested") - attested + check_get_commitment_headers ~__LOC__ ~expected_status:"attested" attested in (* Slots not published or not included in blocks. *) let* () = check_get_commitment get_commitment_not_found ko in (* Slot that were waiting for attestation and now unattested. *) let* () = check_get_commitment_headers - (get_headers_succeeds ~__LOC__ ~expected_status:"unattested") + ~__LOC__ + ~expected_status:"unattested" unattested in (* slot2_a is still not selected. *) - let* () = - check_get_commitment_headers (get_headers_succeeds ~__LOC__) [slot2_a] - in + let* () = check_get_commitment_headers ~__LOC__ [slot2_a] in (* slot3 never finished in an L1 block, so the DAL node did not store a status for it. *) - let* () = - check_get_commitment_headers (get_headers_succeeds ~__LOC__) [slot3] - in + let* () = check_get_commitment_headers ~__LOC__ [slot3] in (* slot4 is never injected in any of the nodes. So, it's not known by the Dal node. *) - let* () = - check_get_commitment_headers (get_headers_succeeds ~__LOC__) [slot4] - in + let* () = check_get_commitment_headers ~__LOC__ [slot4] in (* The number of published slots has not changed *) let* () = check_published_level_headers -- GitLab From 2e5459cb7b646d5d6504bf833fb248bce9495ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 09:14:35 +0200 Subject: [PATCH 06/11] DAL: update RPC GET /commitments//headers - Use slot_id instead of commitment, - Return a single value, - Only return the status. --- src/bin_dal_node/RPC_server.ml | 16 ++++----- src/bin_dal_node/slot_manager.ml | 8 ++--- src/bin_dal_node/slot_manager.mli | 16 ++++----- src/bin_dal_node/store.ml | 50 +++++--------------------- src/bin_dal_node/store.mli | 17 ++++----- src/lib_dal_node_services/services.ml | 18 +++++----- src/lib_dal_node_services/services.mli | 10 +++--- 7 files changed, 42 insertions(+), 93 deletions(-) diff --git a/src/bin_dal_node/RPC_server.ml b/src/bin_dal_node/RPC_server.ml index 48715aba94f1..b9d9d7bd17a2 100644 --- a/src/bin_dal_node/RPC_server.ml +++ b/src/bin_dal_node/RPC_server.ml @@ -186,14 +186,10 @@ module Slots_handlers = struct store |> to_option_tzresult) - let get_commitment_headers ctxt commitment (slot_level, slot_index) () = + let get_slot_status ctxt slot_level slot_index () () = call_handler1 ctxt (fun store -> - Slot_manager.get_commitment_headers - commitment - ?slot_level - ?slot_index - store - |> Errors.to_tzresult) + let slot_id : Types.slot_id = {slot_level; slot_index} in + Slot_manager.get_slot_status ~slot_id store |> Errors.to_tzresult) let get_published_level_headers ctxt published_level header_status () = call_handler1 ctxt (fun store -> @@ -416,9 +412,9 @@ let register : Services.get_profiles (Profile_handlers.get_profiles ctxt) |> add_service - Tezos_rpc.Directory.register1 - Services.get_commitment_headers - (Slots_handlers.get_commitment_headers ctxt) + Tezos_rpc.Directory.opt_register2 + Services.get_slot_status + (Slots_handlers.get_slot_status ctxt) |> add_service Tezos_rpc.Directory.register2 Services.get_assigned_shard_indices diff --git a/src/bin_dal_node/slot_manager.ml b/src/bin_dal_node/slot_manager.ml index 111a4ccaba2c..41f10edf35e6 100644 --- a/src/bin_dal_node/slot_manager.ml +++ b/src/bin_dal_node/slot_manager.ml @@ -427,12 +427,8 @@ let get_commitment_by_published_level_and_index ~level ~slot_index node_store = ~slot_index node_store -let get_commitment_headers commitment ?slot_level ?slot_index node_store = - Store.Legacy.get_commitment_headers - commitment - ?slot_level - ?slot_index - node_store +let get_slot_status ~slot_id node_store = + Store.Legacy.get_slot_status ~slot_id node_store let get_published_level_headers ~published_level ?header_status store = Store.Legacy.get_published_level_headers ~published_level ?header_status store diff --git a/src/bin_dal_node/slot_manager.mli b/src/bin_dal_node/slot_manager.mli index a071d7baae30..3dc14853b4ab 100644 --- a/src/bin_dal_node/slot_manager.mli +++ b/src/bin_dal_node/slot_manager.mli @@ -177,18 +177,14 @@ val get_commitment_by_published_level_and_index : Store.t -> (Cryptobox.commitment, [Errors.decoding | Errors.not_found]) result Lwt.t -(** [get_commitment_headers commitment ?slot_level ?slot_index store] returns - the list of slot headers {!Types.slot_header} known by the DAL. - The result is filtered by [slot_level] and [slot_index] if provided. - - The function may return an decoding error in case of failure. +(** [get_slot_status ~slot_id store] returns the status associated to the + accepted slot of id [slot_id] or [None] if no status is currently + stored for that slot id. *) -val get_commitment_headers : - Cryptobox.commitment -> - ?slot_level:Types.level -> - ?slot_index:Types.slot_index -> +val get_slot_status : + slot_id:Types.slot_id -> Store.t -> - (Types.slot_header list, Errors.decoding) result Lwt.t + (Types.header_status option, Errors.decoding) result Lwt.t (** [get_published_level_headers ~published_level ?header_status store] returns the list of slot headers {!Types.slot_header} that are published diff --git a/src/bin_dal_node/store.ml b/src/bin_dal_node/store.ml index 99cfbdf7f86d..a4eca12c633f 100644 --- a/src/bin_dal_node/store.ml +++ b/src/bin_dal_node/store.ml @@ -293,12 +293,6 @@ let decode_header_status v = ~tztrace_of_error:tztrace_of_read_error @@ Data_encoding.Binary.of_string Types.header_status_encoding v -let decode_slot_id v = - trace_decoding_error - ~data_kind:Types.Store.Slot_id - ~tztrace_of_error:tztrace_of_read_error - @@ Data_encoding.Binary.of_string Types.slot_id_encoding v - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/4975 DAL/Node: Replace Irmin storage for paths @@ -310,8 +304,6 @@ module Legacy = struct val to_string : ?prefix:string -> t -> string module Commitment : sig - val headers : Cryptobox.commitment -> Irmin.Path.t - val header : Cryptobox.commitment -> Types.slot_id -> Irmin.Path.t end @@ -482,21 +474,6 @@ module Legacy = struct ~some:(fun c_str -> Lwt.return @@ decode_commitment c_str) commitment_str_opt - (** Filter the given list of indices according to the values of the given slot - level and index. *) - let filter_indexes = - let keep_field v = function None -> true | Some f -> f = v in - fun ?slot_level ?slot_index indexes -> - let open Result_syntax in - let* indexes = - List.map_e (fun (slot_id, _) -> decode_slot_id slot_id) indexes - in - List.filter - (fun {Types.Slot_id.slot_level = l; slot_index = i} -> - keep_field l slot_level && keep_field i slot_index) - indexes - |> return - let get_headers ~skip_commitment slot_ids store accu = let open Lwt_result_syntax in List.fold_left_es @@ -526,27 +503,16 @@ module Legacy = struct accu slot_ids - let get_headers_of_commitment commitment slot_ids store accu = - let encoded_commitment = encode_commitment commitment in - let skip_commitment read_commitment = - Result_syntax.return - (if String.equal read_commitment encoded_commitment then - `Keep commitment - else `Skip) - in - get_headers ~skip_commitment slot_ids store accu - - let get_commitment_headers commitment ?slot_level ?slot_index node_store = - (* TODO: https://gitlab.com/tezos/tezos/-/issues/4528 - Improve the implementation of this handler. - *) + let get_slot_status ~slot_id node_store = let open Lwt_result_syntax in let store = node_store.store in - (* Get the list of known slot identifiers for [commitment]. *) - let*! indexes = Irmin.list store @@ Path.Commitment.headers commitment in - (* Filter the list of indices by the values of [slot_level] [slot_index]. *) - let*? slot_ids = filter_indexes ?slot_level ?slot_index indexes in - get_headers_of_commitment commitment slot_ids store [] + let status_path = Path.Level.status slot_id in + let*! status_opt = Irmin.find store status_path in + match status_opt with + | None -> return_none + | Some status_str -> + let*? status = decode_header_status status_str in + return_some status let get_published_level_headers ~published_level ?header_status node_store = let open Lwt_result_syntax in diff --git a/src/bin_dal_node/store.mli b/src/bin_dal_node/store.mli index 01e157845172..9b09ca8144b7 100644 --- a/src/bin_dal_node/store.mli +++ b/src/bin_dal_node/store.mli @@ -186,18 +186,13 @@ module Legacy : sig result Lwt.t - (** [get_commitment_headers commitment ?slot_level ?slot_index - store] returns all the slot headers (slot identifiers and - statuses) associated to the given commitment. The optional - arguments can be used for filtering; when set, only the - slot headers for the provided level or slot index are - considered. *) - val get_commitment_headers : - commitment -> - ?slot_level:int32 -> - ?slot_index:int -> + (** [get_slot_status ~slot_id store] returns the status associated + to the given accepted [slot_id], or [None] if no status is + associated to the [slot_id]. *) + val get_slot_status : + slot_id:Types.slot_id -> t -> - ( Types.slot_header list, + ( Types.header_status option, [> `Decoding_failed of Types.Store.kind * tztrace] ) result Lwt.t diff --git a/src/lib_dal_node_services/services.ml b/src/lib_dal_node_services/services.ml index e452b0d5cb50..e4f6df88fbb5 100644 --- a/src/lib_dal_node_services/services.ml +++ b/src/lib_dal_node_services/services.ml @@ -141,21 +141,21 @@ let get_commitment_by_published_level_and_index : open_root / "levels" /: Tezos_rpc.Arg.int32 / "slot_indices" /: Tezos_rpc.Arg.int / "commitment") -let get_commitment_headers : +let get_slot_status : < meth : [`GET] ; input : unit - ; output : slot_header list + ; output : header_status ; prefix : unit - ; params : unit * Cryptobox.commitment - ; query : level option * slot_index option > + ; params : (unit * level) * slot_index + ; query : unit > service = Tezos_rpc.Service.get_service - ~description: - "Return the known headers for the slot whose commitment is given." - ~query:slot_id_query - ~output:(Data_encoding.list slot_header_encoding) + ~description:"Return the status for the given slot." + ~query:Tezos_rpc.Query.empty + ~output:header_status_encoding Tezos_rpc.Path.( - open_root / "commitments" /: Cryptobox.Commitment.rpc_arg / "headers") + open_root / "levels" /: Tezos_rpc.Arg.int32 / "slots" /: Tezos_rpc.Arg.int + / "status") let get_published_level_headers : < meth : [`GET] diff --git a/src/lib_dal_node_services/services.mli b/src/lib_dal_node_services/services.mli index 8a3e7ef4fbcf..1b397179d4d4 100644 --- a/src/lib_dal_node_services/services.mli +++ b/src/lib_dal_node_services/services.mli @@ -108,14 +108,14 @@ val get_commitment_by_published_level_and_index : ; query : unit > service -(** Return the known headers for the slot whose commitment is given. *) -val get_commitment_headers : +(** Return the status for the given slot. *) +val get_slot_status : < meth : [`GET] ; input : unit - ; output : Types.slot_header list + ; output : Types.header_status ; prefix : unit - ; params : unit * Cryptobox.commitment - ; query : Types.level option * Types.slot_index option > + ; params : (unit * Types.level) * Types.slot_index + ; query : unit > service (** Return the known slot headers for the given published level. *) -- GitLab From 98b01b8d381cb8a52d05bd278868d4ee9c77ed81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Thu, 25 Apr 2024 09:54:28 +0200 Subject: [PATCH 07/11] DAL/Test: reset regression traces --- ...lpha- Testing DAL node (dal node list RPCs).out | 14 ++++++-------- ...aris- Testing DAL node (dal node list RPCs).out | 14 ++++++-------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/tezt/tests/expected/dal.ml/Alpha- Testing DAL node (dal node list RPCs).out b/tezt/tests/expected/dal.ml/Alpha- Testing DAL node (dal node list RPCs).out index 2d62f7b22a20..35536b1cf3b3 100644 --- a/tezt/tests/expected/dal.ml/Alpha- Testing DAL node (dal node list RPCs).out +++ b/tezt/tests/expected/dal.ml/Alpha- Testing DAL node (dal node list RPCs).out @@ -3,14 +3,10 @@ Available services: - + commitments// - - GET /commitments//headers - Return the known headers for the slot whose commitment is given. - - GET /commitments//proof - Compute the proof associated with a commitment. - - GET /commitments//slot - Retrieve the content of the slot associated with the given - commitment. + - GET /commitments//proof + Compute the proof associated with a commitment. + - GET /commitments//slot + Retrieve the content of the slot associated with the given commitment. + levels// - GET /levels//headers Return the known headers for the given published level. @@ -19,6 +15,8 @@ Available services: published at the given level. - GET /levels//slots//pages Fetch slot as list of pages + - GET /levels//slots//status + Return the status for the given slot. + p2p/ - POST /p2p/connect Connect to a new peer. diff --git a/tezt/tests/expected/dal.ml/Paris- Testing DAL node (dal node list RPCs).out b/tezt/tests/expected/dal.ml/Paris- Testing DAL node (dal node list RPCs).out index 2d62f7b22a20..35536b1cf3b3 100644 --- a/tezt/tests/expected/dal.ml/Paris- Testing DAL node (dal node list RPCs).out +++ b/tezt/tests/expected/dal.ml/Paris- Testing DAL node (dal node list RPCs).out @@ -3,14 +3,10 @@ Available services: - + commitments// - - GET /commitments//headers - Return the known headers for the slot whose commitment is given. - - GET /commitments//proof - Compute the proof associated with a commitment. - - GET /commitments//slot - Retrieve the content of the slot associated with the given - commitment. + - GET /commitments//proof + Compute the proof associated with a commitment. + - GET /commitments//slot + Retrieve the content of the slot associated with the given commitment. + levels// - GET /levels//headers Return the known headers for the given published level. @@ -19,6 +15,8 @@ Available services: published at the given level. - GET /levels//slots//pages Fetch slot as list of pages + - GET /levels//slots//status + Return the status for the given slot. + p2p/ - POST /p2p/connect Connect to a new peer. -- GitLab From ccca22d1e455b3ac21d1e9b65eb9002d552427a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 09:18:28 +0200 Subject: [PATCH 08/11] DAL/Tezt: Adapt the RPC wrapper to the update of the previous commit --- tezt/lib_tezos/dal_common.ml | 17 +++++++++-------- tezt/lib_tezos/dal_common.mli | 12 ++++-------- tezt/tests/dal.ml | 27 +++++++++++++++++++-------- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/tezt/lib_tezos/dal_common.ml b/tezt/lib_tezos/dal_common.ml index 8204b07af549..931f71214f03 100644 --- a/tezt/lib_tezos/dal_common.ml +++ b/tezt/lib_tezos/dal_common.ml @@ -274,16 +274,17 @@ module Dal_RPC = struct let mk_query_arg ~to_string field v_opt = Option.fold ~none:[] ~some:(fun v -> [(field, to_string v)]) v_opt - let get_commitment_headers ?slot_level ?slot_index commitment = - let query_string = - mk_query_arg ~to_string:string_of_int "slot_level" slot_level - @ mk_query_arg ~to_string:string_of_int "slot_index" slot_index - in + let get_level_slot_status ~slot_level ~slot_index = make - ~query_string GET - ["commitments"; commitment; "headers"] - slot_headers_of_json + [ + "levels"; + string_of_int slot_level; + "slots"; + string_of_int slot_index; + "status"; + ] + JSON.as_string let get_assigned_shard_indices ~level ~pkh = make diff --git a/tezt/lib_tezos/dal_common.mli b/tezt/lib_tezos/dal_common.mli index 47fc2a366485..9fb4dde2962c 100644 --- a/tezt/lib_tezos/dal_common.mli +++ b/tezt/lib_tezos/dal_common.mli @@ -191,14 +191,10 @@ module RPC : sig the DAL node. *) val get_profiles : unit -> profile RPC_core.t - (** Call RPC "GET /commitments//headers" to get the headers and - statuses known about the given commitment. The resulting list can be filtered by a - given header publication level and slot index. *) - val get_commitment_headers : - ?slot_level:int -> - ?slot_index:int -> - commitment -> - slot_header list RPC_core.t + (** Call RPC "GET /levels//slots//status" to + get the status known about the given slot. *) + val get_level_slot_status : + slot_level:int -> slot_index:int -> string RPC_core.t (** Call RPC "GET /profiles//attested_levels//assigned_shard_indices" diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index a1aac87cbd8d..68359be9774d 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1394,19 +1394,30 @@ let get_headers_succeeds ~__LOC__ ?expected_status headers = let check_get_commitment_headers ~__LOC__ ?expected_status dal_node ~slot_level slots_info = - let test ~query_string (slot_index, commit) = - let slot_level, slot_index = - if not query_string then (None, None) - else (Some slot_level, Some slot_index) + let test (slot_index, commitment) = + let* commitment_is_published = + let rpc = Dal_RPC.get_level_index_commitment ~slot_level ~slot_index in + let* response = Dal_RPC.call_raw dal_node rpc in + match response.code with + | 200 -> + let published_commitment = + rpc.decode @@ JSON.parse ~origin:"RPC response" response.body + in + return (commitment = published_commitment) + | _ -> return false in let* headers = - Dal_RPC.( - call dal_node @@ get_commitment_headers ?slot_index ?slot_level commit) + if commitment_is_published then + let* status = + Dal_RPC.( + call dal_node @@ get_level_slot_status ~slot_level ~slot_index) + in + return [Dal_RPC.{status; slot_level; slot_index; commitment}] + else return [] in return @@ get_headers_succeeds ~__LOC__ ?expected_status headers in - let* () = Lwt_list.iter_s (test ~query_string:true) slots_info in - Lwt_list.iter_s (test ~query_string:false) slots_info + Lwt_list.iter_s test slots_info let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node client dal_node = -- GitLab From f06730e725c610b95263c37476d3a603bcd9a2c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 11:42:02 +0200 Subject: [PATCH 09/11] DAL/Test: inline get_headers_succeeds --- tezt/tests/dal.ml | 51 ++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index 68359be9774d..003235fec5d2 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1366,32 +1366,6 @@ let check_published_level_headers ~__LOC__ dal_node ~pub_level ~error_msg:"Unexpected slot headers length (got = %L, expected = %R)" ; unit -(* Checks that [headers] contains zero or one slot header. If [expected_status] - is not given, the expected response is the empty list; if it is given, the - response should contain exactly one header, with the given status. *) -let get_headers_succeeds ~__LOC__ ?expected_status headers = - match (headers, expected_status) with - | [], None -> () - | _ :: _, None -> - Test.fail - ~__LOC__ - "It was expected that there is no slot id for the given commitment, \ - got %d." - (List.length headers) - | [header], Some expected_status -> - Check.(header.Dal_RPC.status = expected_status) - ~__LOC__ - Check.string - ~error_msg: - "The value of the fetched status should match the expected one \ - (current = %L, expected = %R)" - | _, Some _ -> - Test.fail - ~__LOC__ - "It was expected that there is exactly one slot id for the given \ - commitment, got %d." - (List.length headers) - let check_get_commitment_headers ~__LOC__ ?expected_status dal_node ~slot_level slots_info = let test (slot_index, commitment) = @@ -1415,7 +1389,30 @@ let check_get_commitment_headers ~__LOC__ ?expected_status dal_node ~slot_level return [Dal_RPC.{status; slot_level; slot_index; commitment}] else return [] in - return @@ get_headers_succeeds ~__LOC__ ?expected_status headers + let () = + match (headers, expected_status) with + | [], None -> () + | _ :: _, None -> + Test.fail + ~__LOC__ + "It was expected that there is no slot id for the given \ + commitment, got %d." + (List.length headers) + | [header], Some expected_status -> + Check.(header.Dal_RPC.status = expected_status) + ~__LOC__ + Check.string + ~error_msg: + "The value of the fetched status should match the expected one \ + (current = %L, expected = %R)" + | _, Some _ -> + Test.fail + ~__LOC__ + "It was expected that there is exactly one slot id for the given \ + commitment, got %d." + (List.length headers) + in + unit in Lwt_list.iter_s test slots_info -- GitLab From 3583a4254090643ae2535702c5d340f063403fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 11:43:41 +0200 Subject: [PATCH 10/11] DAL/Test: simplify check_get_commitment_headers --- tezt/tests/dal.ml | 49 +++++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index 003235fec5d2..d85ac16f56ff 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1380,39 +1380,30 @@ let check_get_commitment_headers ~__LOC__ ?expected_status dal_node ~slot_level return (commitment = published_commitment) | _ -> return false in - let* headers = - if commitment_is_published then + match (commitment_is_published, expected_status) with + | false, None -> unit + | true, None -> + Test.fail + ~__LOC__ + "It was expected that the given commitment was not published but it \ + was." + | true, Some expected_status -> let* status = Dal_RPC.( call dal_node @@ get_level_slot_status ~slot_level ~slot_index) in - return [Dal_RPC.{status; slot_level; slot_index; commitment}] - else return [] - in - let () = - match (headers, expected_status) with - | [], None -> () - | _ :: _, None -> - Test.fail - ~__LOC__ - "It was expected that there is no slot id for the given \ - commitment, got %d." - (List.length headers) - | [header], Some expected_status -> - Check.(header.Dal_RPC.status = expected_status) - ~__LOC__ - Check.string - ~error_msg: - "The value of the fetched status should match the expected one \ - (current = %L, expected = %R)" - | _, Some _ -> - Test.fail - ~__LOC__ - "It was expected that there is exactly one slot id for the given \ - commitment, got %d." - (List.length headers) - in - unit + Check.(status = expected_status) + ~__LOC__ + Check.string + ~error_msg: + "The value of the fetched status should match the expected one \ + (current = %L, expected = %R)" ; + unit + | false, Some _ -> + Test.fail + ~__LOC__ + "The commitment published for the given slot id is not the expected \ + one." in Lwt_list.iter_s test slots_info -- GitLab From 489fd58edd2e2771e0e7fa376e8fb9d953cc8fd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Wed, 24 Apr 2024 11:44:58 +0200 Subject: [PATCH 11/11] DAL/Test: rename check_get_commitment_headers into check_slot_status --- tezt/tests/dal.ml | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index d85ac16f56ff..3dfc90d850df 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1366,8 +1366,8 @@ let check_published_level_headers ~__LOC__ dal_node ~pub_level ~error_msg:"Unexpected slot headers length (got = %L, expected = %R)" ; unit -let check_get_commitment_headers ~__LOC__ ?expected_status dal_node ~slot_level - slots_info = +let check_slot_status ~__LOC__ ?expected_status dal_node ~slot_level slots_info + = let test (slot_index, commitment) = let* commitment_is_published = let rpc = Dal_RPC.get_level_index_commitment ~slot_level ~slot_index in @@ -1446,7 +1446,7 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node Log.info "Just after injecting slots and before baking, there are no headers" ; (* because headers are stored based on information from finalized blocks *) let* () = - check_get_commitment_headers + check_slot_status dal_node ~slot_level:level ~__LOC__ @@ -1465,7 +1465,7 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node "After baking one block, there is still no header, because the block is \ not final" ; let* () = - check_get_commitment_headers + check_slot_status dal_node ~slot_level:level ~__LOC__ @@ -1499,12 +1499,8 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node ~error_msg: "Published header is different from stored header (current = %L, \ expected = %R)" ; - let check_get_commitment_headers ?expected_status l = - check_get_commitment_headers - ?expected_status - dal_node - ~slot_level:pub_level - l + let check_slot_status ?expected_status l = + check_slot_status ?expected_status dal_node ~slot_level:pub_level l in let check_get_commitment = check_get_commitment dal_node ~slot_level:pub_level @@ -1513,13 +1509,10 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node (* Slots waiting for attestation. *) let* () = check_get_commitment get_commitment_succeeds ok in let* () = - check_get_commitment_headers - ~__LOC__ - ~expected_status:"waiting_attestation" - ok + check_slot_status ~__LOC__ ~expected_status:"waiting_attestation" ok in (* slot_2_a is not selected. *) - let* () = check_get_commitment_headers ~__LOC__ [slot2_a] in + let* () = check_slot_status ~__LOC__ [slot2_a] in (* Slots not published or not included in blocks. *) let* () = check_get_commitment get_commitment_not_found ko in @@ -1552,25 +1545,20 @@ let test_dal_node_slots_headers_tracking _protocol parameters _cryptobox node (* Slots confirmed. *) let* () = check_get_commitment get_commitment_succeeds ok in (* Slot that were waiting for attestation and now attested. *) - let* () = - check_get_commitment_headers ~__LOC__ ~expected_status:"attested" attested - in + let* () = check_slot_status ~__LOC__ ~expected_status:"attested" attested in (* Slots not published or not included in blocks. *) let* () = check_get_commitment get_commitment_not_found ko in (* Slot that were waiting for attestation and now unattested. *) let* () = - check_get_commitment_headers - ~__LOC__ - ~expected_status:"unattested" - unattested + check_slot_status ~__LOC__ ~expected_status:"unattested" unattested in (* slot2_a is still not selected. *) - let* () = check_get_commitment_headers ~__LOC__ [slot2_a] in + let* () = check_slot_status ~__LOC__ [slot2_a] in (* slot3 never finished in an L1 block, so the DAL node did not store a status for it. *) - let* () = check_get_commitment_headers ~__LOC__ [slot3] in + let* () = check_slot_status ~__LOC__ [slot3] in (* slot4 is never injected in any of the nodes. So, it's not known by the Dal node. *) - let* () = check_get_commitment_headers ~__LOC__ [slot4] in + let* () = check_slot_status ~__LOC__ [slot4] in (* The number of published slots has not changed *) let* () = check_published_level_headers -- GitLab