From 574f549f4435a3e7ce0775d1c5fead6af8c947ad Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Thu, 1 Sep 2022 20:39:07 +0200 Subject: [PATCH 1/7] Proto: expose Dal.Slot_index.to_int in Alpha_context --- src/proto_alpha/lib_protocol/alpha_context.mli | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/proto_alpha/lib_protocol/alpha_context.mli b/src/proto_alpha/lib_protocol/alpha_context.mli index 83d837adb7c7..0a14bfd1c481 100644 --- a/src/proto_alpha/lib_protocol/alpha_context.mli +++ b/src/proto_alpha/lib_protocol/alpha_context.mli @@ -2807,6 +2807,8 @@ module Dal : sig val of_int : int -> t option + val to_int : t -> int + val compare : t -> t -> int end -- GitLab From 872e18af953e46178b2d15481e87517c4f1887dc Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Thu, 1 Sep 2022 20:39:44 +0200 Subject: [PATCH 2/7] Dal/Rollup node: store slot headers --- src/proto_alpha/bin_sc_rollup_node/daemon.ml | 27 ++- src/proto_alpha/bin_sc_rollup_node/store.ml | 234 ++++++++++++++++++- 2 files changed, 248 insertions(+), 13 deletions(-) diff --git a/src/proto_alpha/bin_sc_rollup_node/daemon.ml b/src/proto_alpha/bin_sc_rollup_node/daemon.ml index ae1191cf1bf8..e01d160e312b 100644 --- a/src/proto_alpha/bin_sc_rollup_node/daemon.ml +++ b/src/proto_alpha/bin_sc_rollup_node/daemon.ml @@ -70,7 +70,7 @@ module Make (PVM : Pvm.S) = struct for the first time. {b Note}: this function does not process inboxes for the rollup, which is done instead by {!Inbox.process_head}. *) let process_included_l1_operation (type kind) (node_ctxt : Node_context.t) - ~source:_ (operation : kind manager_operation) + head ~source:_ (operation : kind manager_operation) (result : kind successful_manager_operation_result) = let open Lwt_syntax in match (operation, result) with @@ -91,18 +91,31 @@ module Make (PVM : Pvm.S) = struct Store.Last_cemented_commitment_level.set node_ctxt.store inbox_level in Store.Last_cemented_commitment_hash.set node_ctxt.store commitment + | Dal_publish_slot_header {slot}, Dal_publish_slot_header_result _ -> + let {Dal.Slot.index; _} = slot in + (* DAL/FIXME: https://gitlab.com/tezos/tezos/-/issues/3510 + We store slot headers for all slots. In practice, + it would be convenient to store only information about + headers of slots to which the rollup node is subscribed to. + We do not have the information about DAL slots subscribed to + at this time. *) + Store.Dal_slots.add + node_ctxt.store + ~primary_key:head + ~secondary_key:index + slot | _, _ -> (* Other manager operations *) return_unit (** Process an L1 SCORU operation (for the node's rollup) which is finalized for the first time. *) - let process_finalized_l1_operation (type kind) _node_ctxt ~source:_ + let process_finalized_l1_operation (type kind) _node_ctxt _head ~source:_ (_operation : kind manager_operation) (_result : kind successful_manager_operation_result) = Lwt.return_unit - let process_l1_operation (type kind) ~finalized node_ctxt ~source + let process_l1_operation (type kind) ~finalized node_ctxt head ~source (operation : kind manager_operation) (result : kind Apply_results.manager_operation_result) = let open Lwt_syntax in @@ -116,14 +129,14 @@ module Make (PVM : Pvm.S) = struct | Sc_rollup_recover_bond {sc_rollup = rollup} | Sc_rollup_dal_slot_subscribe {rollup; _} -> Sc_rollup.Address.(rollup = node_ctxt.Node_context.rollup_address) + | Dal_publish_slot_header _ -> true | Reveal _ | Transaction _ | Origination _ | Delegation _ | Register_global_constant _ | Set_deposits_limit _ | Increase_paid_storage _ | Tx_rollup_origination | Tx_rollup_submit_batch _ | Tx_rollup_commit _ | Tx_rollup_return_bond _ | Tx_rollup_finalize_commitment _ | Tx_rollup_remove_commitment _ | Tx_rollup_rejection _ | Tx_rollup_dispatch_tickets _ | Transfer_ticket _ - | Dal_publish_slot_header _ | Sc_rollup_originate _ - | Zk_rollup_origination _ -> + | Sc_rollup_originate _ | Zk_rollup_origination _ -> false in if not (is_for_my_rollup operation) then return_unit @@ -136,7 +149,7 @@ module Make (PVM : Pvm.S) = struct if finalized then process_finalized_l1_operation else process_included_l1_operation in - process node_ctxt ~source operation success_result + process node_ctxt head ~source operation success_result | _ -> (* No action for non successful operations *) return_unit @@ -148,7 +161,7 @@ module Make (PVM : Pvm.S) = struct result = let open Lwt_syntax in let* () = accu in - process_l1_operation ~finalized node_ctxt ~source operation result + process_l1_operation ~finalized node_ctxt hash ~source operation result in let apply_internal (type kind) accu ~source:_ (_operation : kind Apply_internal_results.internal_operation) diff --git a/src/proto_alpha/bin_sc_rollup_node/store.ml b/src/proto_alpha/bin_sc_rollup_node/store.ml index 3811ac199a33..4332452bf8c3 100644 --- a/src/proto_alpha/bin_sc_rollup_node/store.ml +++ b/src/proto_alpha/bin_sc_rollup_node/store.ml @@ -26,6 +26,10 @@ (* TODO: https://gitlab.com/tezos/tezos/-/issues/3471 Use indexed file for append-only instead of Irmin. *) +(* TODO: https://gitlab.com/tezos/tezos/-/issues/3739 + Refactor the store file to have functors in their own + separate module, and return errors within the Error monad. *) + open Protocol open Alpha_context module Maker = Irmin_pack_unix.Maker (Tezos_context_encoding.Context.Conf) @@ -86,6 +90,182 @@ module type KeyValue = sig val value_encoding : value Data_encoding.t end +(* An interface for constructing nested maps with a primary and secondary key. + This can be thought as a primary-key indexed maps whose values are + secondary-key indexed maps. *) +module type DoubleKeyValue = sig + val path : path + + val keep_last_n_entries_in_memory : int + + type primary_key + + type secondary_key + + (* `string_of_primary_key` should return a string that does not include + the character `/`, to avoid breaking the structure of Irmin paths used + by the underlying store. One possibility to guarantee that this + property is satisfied is to return strings in Base58 format, as + these only contain alphanumeric characters (and therefore not the '/' + character). + *) + val string_of_primary_key : primary_key -> string + + (* `string_of_secondary_key` should return a string that does not include + the character `/`, to avoid breaking the structure of Irmin paths used + by the underlying store. See + [Make_nested_map.list_secondary_keys_with_values] and + [Make_nested_map.add] for more details about how not enforcing this + property would break the store. One possibility to guarantee that this + property is satisfied is to return strings in Base58 format, as these only + contain alphanumeric characters (and therefore not the '/' character). + Furthermore, the function should satisfy the following equality for all + strings `s` for which `secondary_key_of_string_exn s` is defined: + `string_of_secondary_key @@ secondary_key_of_string_exn s = s`. + *) + val string_of_secondary_key : secondary_key -> string + + (* This function should satisfy the following equality for + all secondary_keys `k`: + `secondary_key_of_string_exn @@ string_of_secondary_key k = k`. + *) + val secondary_key_of_string_exn : string -> secondary_key + + type value + + val value_encoding : value Data_encoding.t + + val compare_secondary_keys : secondary_key -> secondary_key -> int +end + +(** A functor for building a nested storage map, with primary and secondary key, + from a module of type `DoubleKeyValue`. + This map provides the following functionalities: + {ul + {li inserting values in the map by specifying a primary and a secondary key} + {li fetching a value in the map associated with a given primary and + a given secondary key} + {li given a primary key, retrieving the list of secondary keys for which + there is a value associated with the primary and secondary keys in the map. + } + } + Similarly to storage maps with a single key, the contents of this map cannot be + overwritten. However, trying to overwrite the value for a key with the same + value will not result in an error. This is required to address the case + where the rollup node needs to rewrite some entries in the map as a + consequence of some work being redone after a violent interruption. +*) +module Make_nested_map (P : DoubleKeyValue) = struct + (* Ignored for now. *) + let _ = P.keep_last_n_entries_in_memory + + let path_key = P.path + + let make_primary_key primary_key = + path_key @ [P.string_of_primary_key primary_key] + + let make_composite_key primary_key secondary_key = + path_key + @ [ + P.string_of_primary_key primary_key; + P.string_of_secondary_key secondary_key; + ] + + let mem_primary store primary_key = + IStore.mem store (make_primary_key primary_key) + + let mem store ~primary_key ~secondary_key = + IStore.mem store (make_composite_key primary_key secondary_key) + + let decode_value encoded_value = + Data_encoding.Binary.of_bytes_exn P.value_encoding encoded_value + + let get store ~primary_key ~secondary_key = + let open Lwt_syntax in + let+ value = + IStore.get store (make_composite_key primary_key secondary_key) + in + decode_value value + + let list_secondary_keys_with_values store ~primary_key = + let open Lwt_syntax in + let primary_key_path = make_primary_key primary_key in + let* subtrees = IStore.list store primary_key_path in + let+ keys_with_values = + subtrees + |> List.map_s (fun (inner_key, tree) -> + let inner_key = P.secondary_key_of_string_exn inner_key in + let+ value = + match IStore.Tree.destruct tree with + | `Node _ -> + (* If `P.string_of_secondary_keys` never returns a + string containing the character `/`, then subtrees of a + primary key can only be of type `Contents. See + also the function `add`. *) + assert false + | `Contents (c, _metadata) -> + let+ raw_value = IStore.Tree.Contents.force_exn c in + decode_value raw_value + in + (inner_key, value)) + in + keys_with_values + |> List.sort (fun (key1, _value1) (key2, _value2) -> + P.compare_secondary_keys key1 key2) + + let list_secondary_keys store ~primary_key = + let open Lwt_syntax in + let+ keys_with_values = + list_secondary_keys_with_values store ~primary_key + in + keys_with_values |> List.map fst + + let list_values store ~primary_key = + let open Lwt_syntax in + let+ keys_with_values = + list_secondary_keys_with_values store ~primary_key + in + keys_with_values |> List.map snd + + let find store ~primary_key ~secondary_key = + let open Lwt_option_syntax in + let+ value = + IStore.find store (make_composite_key primary_key secondary_key) + in + decode_value value + + (* This function requires that `string_of_secondary_key secondary_key` + does not return a string containing the character `/`. If this + happens, then storing the value would create a child of `primary_key` + in the store tree with type `Node, which in turn breaks one of the + assumptions of `list_secondary_keys_with_values`. + If `string_of_secondary_key` never returns a string containing + the character `/`, then invoking `add` ensures that all the + children of `primary_key` in the store have type `Contents. *) + let add store ~primary_key ~secondary_key value = + let open Lwt_syntax in + let key_path = make_composite_key primary_key secondary_key in + let full_path = String.concat "/" key_path in + let* existing_value = find store ~primary_key ~secondary_key in + let encode v = Data_encoding.Binary.to_bytes_exn P.value_encoding v in + let encoded_value = encode value in + match existing_value with + | None -> + let info () = info full_path in + IStore.set_exn ~info store key_path encoded_value + | Some existing_value -> + (* To be robust to interruption in the middle of processes, + we accept to redo some work when we restart the node. + Hence, it is fine to insert twice the same value for a + given value. *) + if not (Bytes.equal (encode existing_value) encoded_value) then + Stdlib.failwith + (Printf.sprintf + "Key %s already exists with a different value" + full_path) + else return_unit +end + module Make_map (P : KeyValue) = struct (* Ignored for now. *) let _ = P.keep_last_n_entries_in_memory @@ -97,15 +277,17 @@ module Make_map (P : KeyValue) = struct let mem store key = IStore.mem store (make_key key) let decode_value encoded_value = - Lwt.return - @@ Data_encoding.Binary.of_bytes_exn P.value_encoding encoded_value + Data_encoding.Binary.of_bytes_exn P.value_encoding encoded_value - let get store key = IStore.get store (make_key key) >>= decode_value + let get store key = + let open Lwt_syntax in + let+ e = IStore.get store (make_key key) in + decode_value e let find store key = - let open Lwt_syntax in - let* value = IStore.find store (make_key key) in - Option.map_s decode_value value + let open Lwt_option_syntax in + let+ value = IStore.find store (make_key key) in + decode_value value let find_with_default store key ~on_default = let open Lwt_syntax in @@ -414,3 +596,43 @@ module Contexts = Make_append_only_map (struct let value_encoding = Context.hash_encoding end) + +module Dal_slots = Make_nested_map (struct + let path = ["dal"; "slot_headers"] + + (* FIXME: https://gitlab.com/tezos/tezos/-/issues/3527 + Check if we actually need `keep_last_n_entries_in_memory`. + *) + let keep_last_n_entries_in_memory = 10 + + type primary_key = Block_hash.t + + type secondary_key = Dal.Slot_index.t + + let string_of_primary_key block_hash = Block_hash.to_b58check block_hash + + let string_of_secondary_key slot_index = + string_of_int @@ Dal.Slot_index.to_int slot_index + + let secondary_key_of_string_exn slot_index = + match Dal.Slot_index.of_int @@ int_of_string slot_index with + | None -> + (* This value is never going to be `None. This is because the function + `secondary_key_of_string_exn` is only called by the function + `Dal_slots.list_secondary_keys_with_values`. The parameter of the + function is guaranteed to be a string `slot_index` from a path of + the forms `/dal/slot_headers//`, which must + have been previously created via the `Dal_slots.add` function, which + converts a slot_index - a bounded integer in the range [0, 256) - + into a string. Therefore, `slot_index` must be the representation as + a string of a valid slot index. + *) + assert false + | Some slot_index -> slot_index + + type value = Dal.Slot.t + + let value_encoding = Dal.Slot.encoding + + let compare_secondary_keys = Dal.Slot_index.compare +end) -- GitLab From 7b3013753d387c33d263bce3992cb0829078d169 Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Thu, 4 Aug 2022 13:46:35 +0100 Subject: [PATCH 3/7] Dal/Rollup node: endpoint for fetching last known slot headers --- src/proto_alpha/bin_sc_rollup_node/RPC_server.ml | 13 +++++++++++++ src/proto_alpha/lib_sc_rollup/sc_rollup_services.ml | 7 +++++++ tezt/lib_tezos/sc_rollup_client.ml | 4 +++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/proto_alpha/bin_sc_rollup_node/RPC_server.ml b/src/proto_alpha/bin_sc_rollup_node/RPC_server.ml index c88f41f70b72..86378943f2fc 100644 --- a/src/proto_alpha/bin_sc_rollup_node/RPC_server.ml +++ b/src/proto_alpha/bin_sc_rollup_node/RPC_server.ml @@ -46,6 +46,12 @@ let get_dal_slot_subscriptions_exn store = | None -> failwith "No slot subscriptions" | Some slot_subscriptions -> return slot_subscriptions +let get_dal_slots_exn store = + let open Lwt_result_syntax in + let* head = get_head_exn store in + let*! slot_headers = Store.Dal_slots.list_values store ~primary_key:head in + return slot_headers + let commitment_with_hash commitment = ( Protocol.Alpha_context.Sc_rollup.Commitment.hash_uncarbonated commitment, commitment ) @@ -230,6 +236,12 @@ module Make (PVM : Pvm.S) : S with module PVM = PVM = struct (Sc_rollup_services.Global.dal_slot_subscriptions ()) (fun () () -> get_dal_slot_subscriptions_exn store) + let register_dal_slots store dir = + RPC_directory.register0 + dir + (Sc_rollup_services.Global.dal_slots ()) + (fun () () -> get_dal_slots_exn store) + let register (node_ctxt : Node_context.t) configuration = RPC_directory.empty |> register_sc_rollup_address configuration @@ -244,6 +256,7 @@ module Make (PVM : Pvm.S) : S with module PVM = PVM = struct |> register_last_stored_commitment node_ctxt.store |> register_last_published_commitment node_ctxt.store |> register_dal_slot_subscriptions node_ctxt.store + |> register_dal_slots node_ctxt.store let start node_ctxt configuration = Common.start configuration (register node_ctxt configuration) diff --git a/src/proto_alpha/lib_sc_rollup/sc_rollup_services.ml b/src/proto_alpha/lib_sc_rollup/sc_rollup_services.ml index 0aaa6a88f540..8a2e64debb96 100644 --- a/src/proto_alpha/lib_sc_rollup/sc_rollup_services.ml +++ b/src/proto_alpha/lib_sc_rollup/sc_rollup_services.ml @@ -136,6 +136,13 @@ module Global = struct ~description:"Current data availability layer slot subscriptions" ~query:RPC_query.empty ~output:(Data_encoding.list Dal.Slot_index.encoding) + (prefix / "dal" / "slot_subscriptions") + + let dal_slots () = + RPC_service.get_service + ~description:"Data availability slots for a given block hash" + ~query:RPC_query.empty + ~output:(Data_encoding.list Dal.Slot.encoding) (prefix / "dal" / "slots") end diff --git a/tezt/lib_tezos/sc_rollup_client.ml b/tezt/lib_tezos/sc_rollup_client.ml index 15ef51cebefe..c0df79de1229 100644 --- a/tezt/lib_tezos/sc_rollup_client.ml +++ b/tezt/lib_tezos/sc_rollup_client.ml @@ -145,7 +145,9 @@ let last_published_commitment ?hooks sc_client = let dal_slot_subscriptions ?hooks sc_client = let open Lwt.Syntax in - let+ json = rpc_get ?hooks sc_client ["global"; "dal"; "slots"] in + let+ json = + rpc_get ?hooks sc_client ["global"; "dal"; "slot_subscriptions"] + in JSON.as_list json |> List.map JSON.as_int let spawn_generate_keys ?hooks ?(force = false) ~alias sc_client = -- GitLab From e27f479156851166cbe288ff6e9973d99c4aa156 Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Tue, 30 Aug 2022 17:11:38 +0200 Subject: [PATCH 4/7] Dal/Rollup node: reimplement nested maps as updatable map --- src/proto_alpha/bin_sc_rollup_node/store.ml | 303 ++++++-------------- 1 file changed, 86 insertions(+), 217 deletions(-) diff --git a/src/proto_alpha/bin_sc_rollup_node/store.ml b/src/proto_alpha/bin_sc_rollup_node/store.ml index 4332452bf8c3..634f5adc92fd 100644 --- a/src/proto_alpha/bin_sc_rollup_node/store.ml +++ b/src/proto_alpha/bin_sc_rollup_node/store.ml @@ -67,8 +67,6 @@ module type Mutable_value = sig val path_key : path - val decode_value : bytes -> value Lwt.t - val set : t -> value -> unit Lwt.t val get : t -> value Lwt.t @@ -90,182 +88,6 @@ module type KeyValue = sig val value_encoding : value Data_encoding.t end -(* An interface for constructing nested maps with a primary and secondary key. - This can be thought as a primary-key indexed maps whose values are - secondary-key indexed maps. *) -module type DoubleKeyValue = sig - val path : path - - val keep_last_n_entries_in_memory : int - - type primary_key - - type secondary_key - - (* `string_of_primary_key` should return a string that does not include - the character `/`, to avoid breaking the structure of Irmin paths used - by the underlying store. One possibility to guarantee that this - property is satisfied is to return strings in Base58 format, as - these only contain alphanumeric characters (and therefore not the '/' - character). - *) - val string_of_primary_key : primary_key -> string - - (* `string_of_secondary_key` should return a string that does not include - the character `/`, to avoid breaking the structure of Irmin paths used - by the underlying store. See - [Make_nested_map.list_secondary_keys_with_values] and - [Make_nested_map.add] for more details about how not enforcing this - property would break the store. One possibility to guarantee that this - property is satisfied is to return strings in Base58 format, as these only - contain alphanumeric characters (and therefore not the '/' character). - Furthermore, the function should satisfy the following equality for all - strings `s` for which `secondary_key_of_string_exn s` is defined: - `string_of_secondary_key @@ secondary_key_of_string_exn s = s`. - *) - val string_of_secondary_key : secondary_key -> string - - (* This function should satisfy the following equality for - all secondary_keys `k`: - `secondary_key_of_string_exn @@ string_of_secondary_key k = k`. - *) - val secondary_key_of_string_exn : string -> secondary_key - - type value - - val value_encoding : value Data_encoding.t - - val compare_secondary_keys : secondary_key -> secondary_key -> int -end - -(** A functor for building a nested storage map, with primary and secondary key, - from a module of type `DoubleKeyValue`. - This map provides the following functionalities: - {ul - {li inserting values in the map by specifying a primary and a secondary key} - {li fetching a value in the map associated with a given primary and - a given secondary key} - {li given a primary key, retrieving the list of secondary keys for which - there is a value associated with the primary and secondary keys in the map. - } - } - Similarly to storage maps with a single key, the contents of this map cannot be - overwritten. However, trying to overwrite the value for a key with the same - value will not result in an error. This is required to address the case - where the rollup node needs to rewrite some entries in the map as a - consequence of some work being redone after a violent interruption. -*) -module Make_nested_map (P : DoubleKeyValue) = struct - (* Ignored for now. *) - let _ = P.keep_last_n_entries_in_memory - - let path_key = P.path - - let make_primary_key primary_key = - path_key @ [P.string_of_primary_key primary_key] - - let make_composite_key primary_key secondary_key = - path_key - @ [ - P.string_of_primary_key primary_key; - P.string_of_secondary_key secondary_key; - ] - - let mem_primary store primary_key = - IStore.mem store (make_primary_key primary_key) - - let mem store ~primary_key ~secondary_key = - IStore.mem store (make_composite_key primary_key secondary_key) - - let decode_value encoded_value = - Data_encoding.Binary.of_bytes_exn P.value_encoding encoded_value - - let get store ~primary_key ~secondary_key = - let open Lwt_syntax in - let+ value = - IStore.get store (make_composite_key primary_key secondary_key) - in - decode_value value - - let list_secondary_keys_with_values store ~primary_key = - let open Lwt_syntax in - let primary_key_path = make_primary_key primary_key in - let* subtrees = IStore.list store primary_key_path in - let+ keys_with_values = - subtrees - |> List.map_s (fun (inner_key, tree) -> - let inner_key = P.secondary_key_of_string_exn inner_key in - let+ value = - match IStore.Tree.destruct tree with - | `Node _ -> - (* If `P.string_of_secondary_keys` never returns a - string containing the character `/`, then subtrees of a - primary key can only be of type `Contents. See - also the function `add`. *) - assert false - | `Contents (c, _metadata) -> - let+ raw_value = IStore.Tree.Contents.force_exn c in - decode_value raw_value - in - (inner_key, value)) - in - keys_with_values - |> List.sort (fun (key1, _value1) (key2, _value2) -> - P.compare_secondary_keys key1 key2) - - let list_secondary_keys store ~primary_key = - let open Lwt_syntax in - let+ keys_with_values = - list_secondary_keys_with_values store ~primary_key - in - keys_with_values |> List.map fst - - let list_values store ~primary_key = - let open Lwt_syntax in - let+ keys_with_values = - list_secondary_keys_with_values store ~primary_key - in - keys_with_values |> List.map snd - - let find store ~primary_key ~secondary_key = - let open Lwt_option_syntax in - let+ value = - IStore.find store (make_composite_key primary_key secondary_key) - in - decode_value value - - (* This function requires that `string_of_secondary_key secondary_key` - does not return a string containing the character `/`. If this - happens, then storing the value would create a child of `primary_key` - in the store tree with type `Node, which in turn breaks one of the - assumptions of `list_secondary_keys_with_values`. - If `string_of_secondary_key` never returns a string containing - the character `/`, then invoking `add` ensures that all the - children of `primary_key` in the store have type `Contents. *) - let add store ~primary_key ~secondary_key value = - let open Lwt_syntax in - let key_path = make_composite_key primary_key secondary_key in - let full_path = String.concat "/" key_path in - let* existing_value = find store ~primary_key ~secondary_key in - let encode v = Data_encoding.Binary.to_bytes_exn P.value_encoding v in - let encoded_value = encode value in - match existing_value with - | None -> - let info () = info full_path in - IStore.set_exn ~info store key_path encoded_value - | Some existing_value -> - (* To be robust to interruption in the middle of processes, - we accept to redo some work when we restart the node. - Hence, it is fine to insert twice the same value for a - given value. *) - if not (Bytes.equal (encode existing_value) encoded_value) then - Stdlib.failwith - (Printf.sprintf - "Key %s already exists with a different value" - full_path) - else return_unit -end - module Make_map (P : KeyValue) = struct (* Ignored for now. *) let _ = P.keep_last_n_entries_in_memory @@ -345,8 +167,7 @@ struct let path_key = P.path let decode_value encoded_value = - Lwt.return - @@ Data_encoding.Binary.of_bytes_exn P.value_encoding encoded_value + Data_encoding.Binary.of_bytes_exn P.value_encoding encoded_value let set store value = let encoded_value = @@ -355,12 +176,15 @@ struct let info () = info (String.concat "/" P.path) in IStore.set_exn ~info store path_key encoded_value - let get store = IStore.get store path_key >>= decode_value + let get store = + let open Lwt_syntax in + let+ value = IStore.get store path_key in + decode_value value let find store = - let open Lwt_syntax in - let* value = IStore.find store path_key in - Option.map_s decode_value value + let open Lwt_option_syntax in + let+ value = IStore.find store path_key in + decode_value value end (** Aggregated collection of messages from the L1 inbox *) @@ -569,6 +393,9 @@ module Commitments_published_at_level = Make_updatable_map (struct let value_encoding = Raw_level.encoding end) +(* Slot subscriptions per block hash, saved as a list of + `Dal.Slot_index.t`, which is a bounded integer between `0` and `255` + included. *) module Dal_slot_subscriptions = Make_append_only_map (struct let path = ["dal"; "slot_subscriptions"] @@ -597,42 +424,84 @@ module Contexts = Make_append_only_map (struct let value_encoding = Context.hash_encoding end) -module Dal_slots = Make_nested_map (struct - let path = ["dal"; "slot_headers"] +(* Published slot headers per block hash, + stored as a list of bindings from `Dal_slot_index.t` + to `Dal.Slot.t`. The encoding function converts this + list into a `Dal.Slot_index.t`-indexed map. *) +module Dal_slots = struct + module Dal_slots_map = Map.Make (struct + type t = Dal.Slot_index.t - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/3527 - Check if we actually need `keep_last_n_entries_in_memory`. - *) - let keep_last_n_entries_in_memory = 10 + let compare = Dal.Slot_index.compare + end) - type primary_key = Block_hash.t + include Make_updatable_map (struct + let keep_last_n_entries_in_memory = 10 - type secondary_key = Dal.Slot_index.t + let path = ["dal"; "slot_headers"] - let string_of_primary_key block_hash = Block_hash.to_b58check block_hash + type key = Block_hash.t - let string_of_secondary_key slot_index = - string_of_int @@ Dal.Slot_index.to_int slot_index + let string_of_key = Block_hash.to_b58check - let secondary_key_of_string_exn slot_index = - match Dal.Slot_index.of_int @@ int_of_string slot_index with - | None -> - (* This value is never going to be `None. This is because the function - `secondary_key_of_string_exn` is only called by the function - `Dal_slots.list_secondary_keys_with_values`. The parameter of the - function is guaranteed to be a string `slot_index` from a path of - the forms `/dal/slot_headers//`, which must - have been previously created via the `Dal_slots.add` function, which - converts a slot_index - a bounded integer in the range [0, 256) - - into a string. Therefore, `slot_index` must be the representation as - a string of a valid slot index. - *) - assert false - | Some slot_index -> slot_index - - type value = Dal.Slot.t - - let value_encoding = Dal.Slot.encoding - - let compare_secondary_keys = Dal.Slot_index.compare -end) + type value = Dal.Slot.t Dal_slots_map.t + + (* DAL/FIXME: https://gitlab.com/tezos/tezos/-/issues/3732 + Use data encodings for maps once they are available in the + `Data_encoding` library. + *) + let value_encoding = + Data_encoding.conv + (fun dal_slots_map -> Dal_slots_map.bindings dal_slots_map) + (fun dal_slots_bindings -> + Dal_slots_map.of_seq @@ List.to_seq dal_slots_bindings) + Data_encoding.( + list + @@ obj2 + (req "slot_index" Dal.Slot_index.encoding) + (req "slot_metadata" Dal.Slot.encoding)) + end) + + let list_secondary_keys_with_values store ~primary_key = + let open Lwt_syntax in + let+ slots_map = find store primary_key in + Option.fold ~none:[] ~some:Dal_slots_map.bindings slots_map + + let list_secondary_keys store ~primary_key = + let open Lwt_syntax in + let+ secondary_keys_with_values = + list_secondary_keys_with_values store ~primary_key + in + secondary_keys_with_values |> List.map fst + + let list_values store ~primary_key = + let open Lwt_syntax in + let+ secondary_keys_with_values = + list_secondary_keys_with_values store ~primary_key + in + secondary_keys_with_values |> List.map snd + + let add store ~primary_key ~secondary_key value = + let open Lwt_syntax in + let* slots_map = find store primary_key in + let slots_map = Option.value ~default:Dal_slots_map.empty slots_map in + let value_can_be_added = + match Dal_slots_map.find secondary_key slots_map with + | None -> true + | Some old_value -> + Data_encoding.Binary.( + Bytes.equal + (to_bytes_exn Dal.Slot.encoding old_value) + (to_bytes_exn Dal.Slot.encoding value)) + in + if value_can_be_added then + let new_slots_map = Dal_slots_map.add secondary_key value slots_map in + add store primary_key new_slots_map + else + Stdlib.failwith + (Printf.sprintf + "A binding for slot index %d under block_hash %s already exists \ + with a different value" + (Dal.Slot_index.to_int secondary_key) + (Block_hash.to_b58check primary_key)) +end -- GitLab From cd9f1a0ccee9be2f61d3e46eed8d8a15f0ad07a4 Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Thu, 4 Aug 2022 14:36:06 +0100 Subject: [PATCH 5/7] Dal/Tezt: check that rollup node stores dal slots with headers --- tezt/lib_tezos/sc_rollup_client.ml | 14 ++++ tezt/lib_tezos/sc_rollup_client.mli | 6 ++ tezt/tests/dal.ml | 72 ++++++++++++++++++- ...ctionality (rollup_node_dal_headers_s.out | 46 ++++++++++++ ...ctionality (rollup_node_dal_subscript.out | 6 +- 5 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_headers_s.out diff --git a/tezt/lib_tezos/sc_rollup_client.ml b/tezt/lib_tezos/sc_rollup_client.ml index c0df79de1229..9bbc867c7311 100644 --- a/tezt/lib_tezos/sc_rollup_client.ml +++ b/tezt/lib_tezos/sc_rollup_client.ml @@ -38,6 +38,8 @@ type commitment = { number_of_ticks : int; } +type slot_metadata = {level : int; header : string; index : int} + let commitment_from_json json = if JSON.is_null json then None else @@ -150,6 +152,18 @@ let dal_slot_subscriptions ?hooks sc_client = in JSON.as_list json |> List.map JSON.as_int +let dal_slots_metadata ?hooks sc_client = + let open Lwt.Syntax in + let+ json = rpc_get ?hooks sc_client ["global"; "dal"; "slots"] in + JSON.( + as_list json + |> List.map (fun obj -> + { + level = obj |> get "level" |> as_int; + header = obj |> get "header" |> as_string; + index = obj |> get "index" |> as_int; + })) + let spawn_generate_keys ?hooks ?(force = false) ~alias sc_client = spawn_command ?hooks diff --git a/tezt/lib_tezos/sc_rollup_client.mli b/tezt/lib_tezos/sc_rollup_client.mli index f9e7251db64d..e6476896eccb 100644 --- a/tezt/lib_tezos/sc_rollup_client.mli +++ b/tezt/lib_tezos/sc_rollup_client.mli @@ -33,6 +33,8 @@ type commitment = { number_of_ticks : int; } +type slot_metadata = {level : int; header : string; index : int} + (** [create ?name ?path ?base_dir ?path node] returns a fresh client identified by a specified [name], logging in [color], executing the program at [path], storing local information in [base_dir], and @@ -90,6 +92,10 @@ val last_published_commitment : (** [dal_slot_subscriptions client] gets the slots to which the rollup node is subscribed to *) val dal_slot_subscriptions : ?hooks:Process.hooks -> t -> int list Lwt.t +(** [dal_slots_metadata client] returns the dal slots metadata of the last tezos + head seen by the rollup node. *) +val dal_slots_metadata : ?hooks:Process.hooks -> t -> slot_metadata list Lwt.t + (** [generate_keys ~alias client] generates new unencrypted keys for [alias]. *) val generate_keys : ?hooks:Process.hooks -> ?force:bool -> alias:string -> t -> unit Lwt.t diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index 5794bfcfbcb1..1d6d97b74689 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -236,6 +236,8 @@ let slot_availability ~signer availability client = List.iter (fun i -> endorsement.(i) <- true) availability ; Operation.Consensus.(inject ~signer (slot_availability ~endorsement) client) +let header_of_slot_metadata {Sc_rollup_client.header; _} = header + type status = Applied | Failed of {error_id : string} let pp fmt = function @@ -509,6 +511,69 @@ let test_dal_node_startup = let* () = Dal_node.terminate dal_node in return () +let rollup_node_stores_dal_slots _protocol sc_rollup_node sc_rollup_address node + client = + (* Check that the rollup node stores the slots published in a block, along with slot headers: + + 1. Run rollup node for an originated rollup + 2. Execute a client command to publish a slot header, bake one level + 3. Repeat step two for a second slot header + 4. Get the list of slot headers for the rollup node + 5. Determine that the list contains the two slot headers published. + *) + let* parameters = Rollup.Dal.Parameters.from_client client in + let cryptobox = Rollup.Dal.make parameters in + let* genesis_info = + RPC.Client.call ~hooks client + @@ RPC.get_chain_block_context_sc_rollup_genesis_info sc_rollup_address + in + let init_level = JSON.(genesis_info |-> "level" |> as_int) in + let* () = Sc_rollup_node.run sc_rollup_node in + let sc_rollup_client = Sc_rollup_client.create sc_rollup_node in + let* level = Sc_rollup_node.wait_for_level sc_rollup_node init_level in + Check.(level = init_level) + Check.int + ~error_msg:"Current level has moved past origination level (%L = %R)" ; + let* _ = + publish_slot + ~source:Constant.bootstrap1 + ~fee:1_200 + ~index:0 + ~message:"CAFEBABE" + parameters + cryptobox + node + client + in + let* _ = + publish_slot + ~source:Constant.bootstrap2 + ~fee:1_200 + ~index:1 + ~message:"CAFEDEAD" + parameters + cryptobox + node + client + in + let* () = Client.bake_for_and_wait client in + let* _level = Sc_rollup_node.wait_for_level sc_rollup_node (init_level + 1) in + let* slots_metadata = + Sc_rollup_client.dal_slots_metadata ~hooks sc_rollup_client + in + let slot_headers = slots_metadata |> List.map header_of_slot_metadata in + let expected_slot_headers = + List.map + (fun msg -> + Tezos_crypto_dal.Cryptobox.Commitment.to_b58check + @@ Rollup.Dal.Commitment.dummy_commitment parameters cryptobox msg) + ["CAFEBABE"; "CAFEDEAD"] + in + Check.(slot_headers = expected_slot_headers) + (Check.list Check.string) + ~error_msg:"Unexpected list of slot headers (%L = %R)" ; + return () + let register ~protocols = test_dal_scenario "feature_flag_is_disabled" test_feature_flag protocols ; test_dal_scenario @@ -518,4 +583,9 @@ let register ~protocols = protocols ; test_slot_management_logic protocols ; test_dal_node_slot_management protocols ; - test_dal_node_startup protocols + test_dal_node_startup protocols ; + test_dal_scenario + ~dal_enable:true + "rollup_node_dal_headers_storage" + rollup_node_stores_dal_slots + protocols diff --git a/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_headers_s.out b/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_headers_s.out new file mode 100644 index 000000000000..3a227f037029 --- /dev/null +++ b/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_headers_s.out @@ -0,0 +1,46 @@ + +./tezos-client --wait none originate sc rollup from '[PUBLIC_KEY_HASH]' of kind arith of type unit booting with --burn-cap 9999999 +Node is bootstrapped. +Estimated gas: 3110.449 units (will add 100 for safety) +Estimated storage: 6655 bytes added (will add 20 for safety) +Operation successfully injected in the node. +Operation hash is '[OPERATION_HASH]' +NOT waiting for the operation to be included. +Use command + tezos-client wait for [OPERATION_HASH] to be included --confirmations 1 --branch [BLOCK_HASH] +and/or an external block explorer to make sure that it has been included. +This sequence of operations was run: + Manager signed operations: + From: [PUBLIC_KEY_HASH] + Fee to the baker: ꜩ0.000674 + Expected counter: 1 + Gas limit: 3211 + Storage limit: 6675 bytes + Balance updates: + [PUBLIC_KEY_HASH] ... -ꜩ0.000674 + payload fees(the block proposer) ....... +ꜩ0.000674 + Smart contract rollup origination: + Kind: arith + Parameter type: unit + Boot sector: '' + This smart contract rollup origination was successfully applied + Consumed gas: 3110.449 + Storage size: 6655 bytes + Address: [SC_ROLLUP_HASH] + Genesis commitment hash: [SC_ROLLUP_COMMITMENT_HASH] + Balance updates: + [PUBLIC_KEY_HASH] ... -ꜩ1.66375 + storage fees ........................... +ꜩ1.66375 + + +./tezos-client rpc get '/chains/main/blocks/head/context/sc_rollup/[SC_ROLLUP_HASH]/genesis_info' +{ "level": 2, + "commitment_hash": "[SC_ROLLUP_COMMITMENT_HASH]" } + +./tezos-sc-rollup-client-alpha rpc get /global/dal/slots +[ { "level": 2, "index": 0, + "header": + "sh1xjHVBc36jYtzCoktBUq1RCin8bFEwXTZxpFoS1RA5MqqBsX5Z5A2CqU7QZZQUuqdKfSsvab" }, + { "level": 2, "index": 1, + "header": + "sh27onybk3L4eVH8khVVk8oKZvMkjGi1YszeTLneRe5ps9LL4oqXxR12DKzTMS8tJa27w5imfP" } ] diff --git a/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_subscript.out b/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_subscript.out index eb286bde359c..d69846c4b197 100644 --- a/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_subscript.out +++ b/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_subscript.out @@ -37,11 +37,11 @@ This sequence of operations was run: { "level": 2, "commitment_hash": "[SC_ROLLUP_COMMITMENT_HASH]" } -./tezos-sc-rollup-client-alpha rpc get /global/dal/slots +./tezos-sc-rollup-client-alpha rpc get /global/dal/slot_subscriptions [] -./tezos-sc-rollup-client-alpha rpc get /global/dal/slots +./tezos-sc-rollup-client-alpha rpc get /global/dal/slot_subscriptions [ 0 ] -./tezos-sc-rollup-client-alpha rpc get /global/dal/slots +./tezos-sc-rollup-client-alpha rpc get /global/dal/slot_subscriptions [ 0, 1 ] -- GitLab From d1249d271f1e22866ab8858a6875993a5c58e904 Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Mon, 5 Sep 2022 11:12:38 +0100 Subject: [PATCH 6/7] Scoru/Rollup node rpc server: use _exn suffix consistently --- .../bin_sc_rollup_node/RPC_server.ml | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/proto_alpha/bin_sc_rollup_node/RPC_server.ml b/src/proto_alpha/bin_sc_rollup_node/RPC_server.ml index 86378943f2fc..b707417b2520 100644 --- a/src/proto_alpha/bin_sc_rollup_node/RPC_server.ml +++ b/src/proto_alpha/bin_sc_rollup_node/RPC_server.ml @@ -27,28 +27,28 @@ open Tezos_rpc open Tezos_rpc_http open Tezos_rpc_http_server -let get_head_exn store = +let get_head store = let open Lwt_result_syntax in let*! head = Layer1.current_head_hash store in match head with None -> failwith "No head" | Some head -> return head let get_state_info_exn store = let open Lwt_result_syntax in - let* head = get_head_exn store in + let* head = get_head store in let*! state = Store.StateInfo.get store head in return state let get_dal_slot_subscriptions_exn store = let open Lwt_result_syntax in - let* head = get_head_exn store in + let* head = get_head store in let*! slot_subscriptions = Store.Dal_slot_subscriptions.find store head in match slot_subscriptions with | None -> failwith "No slot subscriptions" | Some slot_subscriptions -> return slot_subscriptions -let get_dal_slots_exn store = +let get_dal_slots store = let open Lwt_result_syntax in - let* head = get_head_exn store in + let* head = get_head store in let*! slot_headers = Store.Dal_slots.list_values store ~primary_key:head in return slot_headers @@ -140,9 +140,9 @@ module Make (PVM : Pvm.S) : S with module PVM = PVM = struct include Common module PVM = PVM - let get_state_exn (node_ctxt : Node_context.t) = + let get_state (node_ctxt : Node_context.t) = let open Lwt_result_syntax in - let* head = get_head_exn node_ctxt.store in + let* head = get_head node_ctxt.store in let* ctxt = Node_context.checkout_context node_ctxt head in let*! state = PVM.State.find ctxt in match state with None -> failwith "No state" | Some state -> return state @@ -153,7 +153,7 @@ module Make (PVM : Pvm.S) : S with module PVM = PVM = struct (Sc_rollup_services.Global.current_total_ticks ()) (fun () () -> let open Lwt_result_syntax in - let* state = get_state_exn node_ctxt in + let* state = get_state node_ctxt in let*! tick = PVM.get_tick state in return tick) @@ -163,7 +163,7 @@ module Make (PVM : Pvm.S) : S with module PVM = PVM = struct (Sc_rollup_services.Global.current_state_hash ()) (fun () () -> let open Lwt_result_syntax in - let* state = get_state_exn node_ctxt in + let* state = get_state node_ctxt in let*! hash = PVM.state_hash state in return hash) @@ -173,7 +173,7 @@ module Make (PVM : Pvm.S) : S with module PVM = PVM = struct (Sc_rollup_services.Local.current_state_value ()) (fun {key} () -> let open Lwt_result_syntax in - let* state = get_state_exn node_ctxt in + let* state = get_state node_ctxt in let path = String.split_on_char '/' key in let*! value = PVM.State.lookup state path in match value with @@ -226,7 +226,7 @@ module Make (PVM : Pvm.S) : S with module PVM = PVM = struct (Sc_rollup_services.Global.current_status ()) (fun () () -> let open Lwt_result_syntax in - let* state = get_state_exn node_ctxt in + let* state = get_state node_ctxt in let*! status = PVM.get_status state in return (PVM.string_of_status status)) @@ -240,7 +240,7 @@ module Make (PVM : Pvm.S) : S with module PVM = PVM = struct RPC_directory.register0 dir (Sc_rollup_services.Global.dal_slots ()) - (fun () () -> get_dal_slots_exn store) + (fun () () -> get_dal_slots store) let register (node_ctxt : Node_context.t) configuration = RPC_directory.empty -- GitLab From b5bdb66ea09ae1c342c1318d90a11fc4c50ee129 Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Wed, 7 Sep 2022 11:03:29 +0100 Subject: [PATCH 7/7] Tezt: Reset regressions in dal.ml and sc_rollup.ml --- ...lity layer functionality (rollup_node_dal_headers_s.out | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_headers_s.out b/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_headers_s.out index 3a227f037029..03c8f113c10e 100644 --- a/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_headers_s.out +++ b/tezt/tests/expected/dal.ml/Alpha- Testing data availability layer functionality (rollup_node_dal_headers_s.out @@ -12,13 +12,13 @@ and/or an external block explorer to make sure that it has been included. This sequence of operations was run: Manager signed operations: From: [PUBLIC_KEY_HASH] - Fee to the baker: ꜩ0.000674 + Fee to the baker: ꜩ0.000672 Expected counter: 1 Gas limit: 3211 Storage limit: 6675 bytes Balance updates: - [PUBLIC_KEY_HASH] ... -ꜩ0.000674 - payload fees(the block proposer) ....... +ꜩ0.000674 + [PUBLIC_KEY_HASH] ... -ꜩ0.000672 + payload fees(the block proposer) ....... +ꜩ0.000672 Smart contract rollup origination: Kind: arith Parameter type: unit -- GitLab