From 3abbabe36996ff662954c91ad554ce5d199b9f56 Mon Sep 17 00:00:00 2001 From: Valentin Chaboche Date: Thu, 9 Jun 2022 14:48:15 +0200 Subject: [PATCH 1/4] Scoru,Proto: avoid [consume_n_messages] unnecessary transfo to int for [n] --- src/proto_alpha/lib_protocol/alpha_context.mli | 2 +- src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml | 8 ++++---- src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli | 2 +- .../lib_protocol/sc_rollup_stake_storage.ml | 5 ++--- .../lib_protocol/test/unit/test_sc_rollup_inbox.ml | 10 ++++++---- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/proto_alpha/lib_protocol/alpha_context.mli b/src/proto_alpha/lib_protocol/alpha_context.mli index 7416846996dc..c470867e2dd7 100644 --- a/src/proto_alpha/lib_protocol/alpha_context.mli +++ b/src/proto_alpha/lib_protocol/alpha_context.mli @@ -2776,7 +2776,7 @@ module Sc_rollup : sig val number_of_available_messages : t -> Z.t - val consume_n_messages : int -> t -> t option tzresult + val consume_n_messages : int32 -> t -> t option tzresult module Hash : S.HASH diff --git a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml index 733fafcc6c01..f33d698beb47 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml @@ -353,11 +353,11 @@ let empty rollup level = let consume_n_messages n ({nb_available_messages; _} as inbox) : t option tzresult = - if Compare.Int.(n < 0) then - error (Invalid_number_of_messages_to_consume (Int64.of_int n)) - else if Compare.Int64.(Int64.of_int n > nb_available_messages) then ok None + let n = Int64.of_int32 n in + if Compare.Int64.(n < 0L) then error (Invalid_number_of_messages_to_consume n) + else if Compare.Int64.(n > nb_available_messages) then ok None else - let nb_available_messages = Int64.(sub nb_available_messages (of_int n)) in + let nb_available_messages = Int64.(sub nb_available_messages n) in ok (Some {inbox with nb_available_messages}) let key_of_message = Data_encoding.Binary.to_string_exn Data_encoding.z diff --git a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli index 6aba77b97fdf..6d944879bfca 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli @@ -159,7 +159,7 @@ val starting_level_of_current_commitment_period : t -> Raw_level_repr.t (** [consume_n_messages n inbox] returns an inbox where [n] messages have been consumed, or [None] if there are strictly less than [n] messages available in [inbox]. *) -val consume_n_messages : int -> t -> t option tzresult +val consume_n_messages : int32 -> t -> t option tzresult module Hash : S.HASH diff --git a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml index 28064f93948a..46ef8abf8d45 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml @@ -372,9 +372,8 @@ let cement_commitment ctxt rollup new_lcc = consume_n_messages ctxt rollup - (Int32.to_int - @@ Sc_rollup_repr.Number_of_messages.to_int32 - new_lcc_commitment.number_of_messages) + (Sc_rollup_repr.Number_of_messages.to_int32 + new_lcc_commitment.number_of_messages) let remove_staker ctxt rollup staker = let open Lwt_tzresult_syntax in diff --git a/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_inbox.ml b/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_inbox.ml index f00a9b1a1856..1483dd89fc07 100644 --- a/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_inbox.ml +++ b/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_inbox.ml @@ -80,16 +80,18 @@ let test_add_messages payloads = (err "Invalid number of available messages.") let test_consume_messages (payloads, nb_consumed_messages) = - let nb_payloads = List.length payloads in + let nb_payloads = List.length payloads |> Int32.of_int in setup_inbox_with_messages [payloads] @@ fun _messages _history inbox _inboxes -> consume_n_messages nb_consumed_messages inbox |> Environment.wrap_tzresult >>?= function | Some inbox -> - let available_messages = nb_payloads - nb_consumed_messages in + let available_messages = Int32.sub nb_payloads nb_consumed_messages in fail_unless Z.( - equal (number_of_available_messages inbox) (of_int available_messages)) + equal + (number_of_available_messages inbox) + (of_int32 available_messages)) (err "Invalid number of available messages.") | None -> fail_unless @@ -215,7 +217,7 @@ let tests = QCheck2.Gen.( let* l = list_size small_int string in let* n = 0 -- ((List.length l * 2) + 1) in - return (l, n)) + return (l, Int32.of_int n)) test_consume_messages; ] @ -- GitLab From b60e7c3611f80210a076a7a0f0a4d6d59e57c482 Mon Sep 17 00:00:00 2001 From: Valentin Chaboche Date: Thu, 9 Jun 2022 14:52:10 +0200 Subject: [PATCH 2/4] Scoru,Proto: use [fail_when] when possible --- .../lib_protocol/sc_rollup_inbox_repr.ml | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml index f33d698beb47..ea0698c8931e 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml @@ -617,18 +617,20 @@ module MakeHashingScheme (Tree : TREE) : let add_messages_aux history inbox level payloads messages = let open Lwt_tzresult_syntax in - if Raw_level_repr.(level < inbox.level) then - fail (Invalid_level_add_messages level) - else - let history, inbox = archive_if_needed history inbox level in - let* messages, inbox = - List.fold_left_es - (fun (messages, inbox) payload -> add_message inbox payload messages) - (messages, inbox) - payloads - in - let current_messages_hash () = hash_messages messages in - return (messages, history, {inbox with current_messages_hash}) + let* () = + fail_when + Raw_level_repr.(level < inbox.level) + (Invalid_level_add_messages level) + in + let history, inbox = archive_if_needed history inbox level in + let* messages, inbox = + List.fold_left_es + (fun (messages, inbox) payload -> add_message inbox payload messages) + (messages, inbox) + payloads + in + let current_messages_hash () = hash_messages messages in + return (messages, history, {inbox with current_messages_hash}) let add_external_messages history inbox level payloads messages = let open Lwt_tzresult_syntax in -- GitLab From 94097b1a7d924121cfc79f9eb948967cc65ed470 Mon Sep 17 00:00:00 2001 From: Valentin Chaboche Date: Thu, 9 Jun 2022 14:53:00 +0200 Subject: [PATCH 3/4] Scoru,Proto: clean up a bit the comments --- .../lib_protocol/sc_rollup_inbox_repr.ml | 3 +- .../lib_protocol/sc_rollup_inbox_repr.mli | 83 +++++++++---------- 2 files changed, 41 insertions(+), 45 deletions(-) diff --git a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml index ea0698c8931e..3e1c0d3a7b59 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml @@ -32,8 +32,7 @@ This module is designed to: - 1. give a constant-time access to the number of available messages - ; + 1. give a constant-time access to the number of available messages ; 2. provide a space-efficient representation for proofs of inbox inclusions (only for inboxes obtained at the end of block diff --git a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli index 6d944879bfca..a2d95c1ef414 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli @@ -136,7 +136,7 @@ val encoding : t Data_encoding.t val empty : Sc_rollup_repr.t -> Raw_level_repr.t -> t (** [inbox_level inbox] returns the maximum level of message insertion in - [inbox] or its initial level. *) + [inbox] or its initial level. *) val inbox_level : t -> Raw_level_repr.t (** [number_of_available_messages inbox] returns the number of @@ -176,10 +176,10 @@ module type MerkelizedOperations = sig type messages = tree (** The history is a merkelized sequence of [messages], one per - level. The history is typically used by the rollup node to - produce inclusion proofs. The protocol only manipulates an empty - history as it does not remember previous messages and only keeps - a witness of the latest state of the history. *) + level. The history is typically used by the rollup node to + produce inclusion proofs. The protocol only manipulates an empty + history as it does not remember previous messages and only keeps + a witness of the latest state of the history. *) type history val history_encoding : history Data_encoding.t @@ -192,23 +192,19 @@ module type MerkelizedOperations = sig val history_at_genesis : bound:int64 -> history (** [add_external_messages history inbox level payloads messages] inserts a - list of [payloads] as new messages in the [messages] of the current - [level] of the [inbox]. This function returns the new sequence - of messages as well as updated [inbox] and [history]. - - If the [inbox]'s level is older than [level], the [inbox] is updated - so that the messages of the levels older than [level] are archived. - To archive a sequence of [messages] for a given [level], we push - it at the end of the [history] and update the witness of this - history in the [inbox]. The [inbox]'s messages for the current - level are also emptied to insert the [payloads] in a fresh sequence - of [messages] for [level]. - - This function fails if [level] is older than [inbox]'s [level]. - - This function fails with {!Max_number_of_available_messages_reached} - if the inbox is full. - + list of [payloads] as new messages in the [messages] of the current + [level] of the [inbox]. This function returns the new sequence + of messages as well as updated [inbox] and [history]. + + If the [inbox]'s level is older than [level], the [inbox] is updated + so that the messages of the levels older than [level] are archived. + To archive a sequence of [messages] for a given [level], we push + it at the end of the [history] and update the witness of this + history in the [inbox]. The [inbox]'s messages for the current + level are also emptied to insert the [payloads] in a fresh sequence + of [messages] for [level]. + + This function fails if [level] is older than [inbox]'s [level]. *) val add_external_messages : history -> @@ -219,7 +215,7 @@ module type MerkelizedOperations = sig (messages * history * t) tzresult Lwt.t (** [add_messages_no_history inbox level payloads messages] behaves - a [add_external_messages] except that it does not remember the inbox + as {!add_external_messages} except that it does not remember the inbox history. *) val add_messages_no_history : t -> @@ -229,9 +225,9 @@ module type MerkelizedOperations = sig (messages * t, error trace) result Lwt.t (** [get_message messages idx] returns [Some message] if the - sequence of [messages] has a more than [idx] messages and - [message] is at position [idx] in this sequence. - Returns [None] otherwise. *) + sequence of [messages] has a more than [idx] messages and + [message] is at position [idx] in this sequence. + Returns [None] otherwise. *) val get_message : messages -> Z.t -> message option Lwt.t (** [get_message_payload messages idx] returns [Some payload] if the @@ -241,15 +237,15 @@ module type MerkelizedOperations = sig val get_message_payload : messages -> Z.t -> string option Lwt.t (** Given a inbox [A] at some level [L] and another inbox [B] at - some level [L' >= L], an [inclusion_proof] guarantees that [A] is - an older version of [B]. + some level [L' >= L], an [inclusion_proof] guarantees that [A] is + an older version of [B]. - To be more precise, an [inclusion_proof] guarantees that the - previous levels messages of [A] are included in the previous - levels messages of [B]. The current messages of [A] and [B] - are not considered. + To be more precise, an [inclusion_proof] guarantees that the + previous levels messages of [A] are included in the previous + levels messages of [B]. The current messages of [A] and [B] + are not considered. - The size of this proof is O(log_basis (L' - L)). *) + The size of this proof is O(log_basis (L' - L)). *) type inclusion_proof val inclusion_proof_encoding : inclusion_proof Data_encoding.t @@ -259,14 +255,14 @@ module type MerkelizedOperations = sig (** [number_of_proof_steps proof] returns the length of [proof]. *) val number_of_proof_steps : inclusion_proof -> int - (** [produce_inclusion_proof history inbox1 inbox2] exploits - [history] to produce a self-contained proof that [inbox1] is an - older version of [inbox2]. *) + (** [produce_inclusion_proof history inboxA inboxB] exploits + [history] to produce a self-contained proof that [inboxA] is an + older version of [inboxB]. *) val produce_inclusion_proof : history -> t -> t -> inclusion_proof option - (** [verify_inclusion_proof proof inbox1 inbox2] returns [true] iff - [proof] is a minimal and valid proof that [inbox1] is included in - [inbox2]. *) + (** [verify_inclusion_proof proof inboxA inboxA] returns [true] iff + [proof] is a minimal and valid proof that [inboxA] is included in + [inboxB]. *) val verify_inclusion_proof : inclusion_proof -> t -> t -> bool end @@ -314,10 +310,11 @@ type inbox = t earlier in this file into the inbox proof as it is required by a refutation. *) module Proof : sig + type starting_point := Raw_level_repr.t * Z.t + (** An inbox proof has three parameters: - - the [starting_point], of type [Raw_level_repr.t * Z.t], specifying - a location in the inbox ; + - the {!starting_point} specifying a location in the inbox ; - the [message], of type [Sc_rollup_PVM_sem.input option] ; @@ -349,12 +346,12 @@ module Proof : sig val encoding : t Data_encoding.t - (** See the docstring for [Proof.t] for details of proof semantics. + (** See the docstring for {t} for details of proof semantics. [valid starting_point inbox proof] will return the third parameter of the proof, [message], iff the proof is valid. *) val valid : - Raw_level_repr.t * Z.t -> + starting_point -> inbox -> t -> (Sc_rollup_PVM_sem.input option, error) result Lwt.t -- GitLab From 1c37bf85258f7de96eeece33e40f6d0a622ef192 Mon Sep 17 00:00:00 2001 From: Valentin Chaboche Date: Mon, 13 Jun 2022 09:09:42 +0200 Subject: [PATCH 4/4] Proto,Scoru: use a record for [starting_point] in inbox_repr --- src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml | 10 ++++++++-- src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli | 2 +- src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml index 3e1c0d3a7b59..6ac78a7131ae 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.ml @@ -723,6 +723,8 @@ include ( type inbox = t module Proof = struct + type starting_point = {inbox_level : Raw_level_repr.t; message_counter : Z.t} + type t = { skips : (inbox * inclusion_proof) list; (* The [skips] value in this record makes it potentially unbounded @@ -812,7 +814,7 @@ module Proof = struct let*! result = promise in match result with Ok r -> return r | Error _ -> proof_error reason - let rec valid (l, n) inbox proof = + let rec valid {inbox_level = l; message_counter = n} inbox proof = assert (Z.(geq n zero)) ; let open Lwt_result_syntax in match split_proof proof with @@ -846,7 +848,11 @@ module Proof = struct verify_inclusion_proof inc level (bottom_level remaining_proof) && Raw_level_repr.equal (inbox_level level) l && Z.equal level.message_counter n - then valid (Raw_level_repr.succ l, Z.zero) inbox remaining_proof + then + valid + {inbox_level = Raw_level_repr.succ l; message_counter = Z.zero} + inbox + remaining_proof else proof_error "Inbox proof parameters don't match (lower level)" (* TODO #2997 This needs to be implemented when the inbox structure is diff --git a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli index a2d95c1ef414..9b322fbdf989 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_inbox_repr.mli @@ -310,7 +310,7 @@ type inbox = t earlier in this file into the inbox proof as it is required by a refutation. *) module Proof : sig - type starting_point := Raw_level_repr.t * Z.t + type starting_point = {inbox_level : Raw_level_repr.t; message_counter : Z.t} (** An inbox proof has three parameters: diff --git a/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml b/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml index 074de5883e56..84333262f7ae 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml @@ -80,12 +80,12 @@ let valid snapshot commit_level ~pvm_name proof = | Sc_rollup_PVM_sem.No_input_required, None -> return None | Sc_rollup_PVM_sem.Initial, Some inbox_proof -> Sc_rollup_inbox_repr.Proof.valid - (Raw_level_repr.root, Z.zero) + {inbox_level = Raw_level_repr.root; message_counter = Z.zero} snapshot inbox_proof | Sc_rollup_PVM_sem.First_after (level, counter), Some inbox_proof -> Sc_rollup_inbox_repr.Proof.valid - (level, Z.succ counter) + {inbox_level = level; message_counter = Z.succ counter} snapshot inbox_proof | _ -> -- GitLab