From 2b18825bf3cefad9227622e950b99bd39b354f56 Mon Sep 17 00:00:00 2001 From: "iguerNL@Functori" Date: Tue, 25 Oct 2022 15:37:46 +0200 Subject: [PATCH 1/3] Proto/Dal: check the page content's size in check_page_proof --- src/proto_alpha/lib_protocol/dal_slot_repr.ml | 38 ++++++++++++++++--- .../lib_protocol/dal_slot_repr.mli | 4 +- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/proto_alpha/lib_protocol/dal_slot_repr.ml b/src/proto_alpha/lib_protocol/dal_slot_repr.ml index 449e5570a782..237d0a306183 100644 --- a/src/proto_alpha/lib_protocol/dal_slot_repr.ml +++ b/src/proto_alpha/lib_protocol/dal_slot_repr.ml @@ -650,7 +650,9 @@ module History = struct pp_inclusion_proof next_inc_proof) - type error += Dal_proof_error of string + type error += + | Dal_proof_error of string + | Unexpected_page_size of {expected_size : int; page_size : int} let () = let open Data_encoding in @@ -664,6 +666,29 @@ module History = struct (function Dal_proof_error e -> Some e | _ -> None) (fun e -> Dal_proof_error e) + let () = + let open Data_encoding in + register_error_kind + `Permanent + ~id:"dal_slot_repr.slots_history.unexpected_page_size" + ~title:"Unexpected page size" + ~description: + "The size of the given page content doesn't match the expected one." + ~pp:(fun ppf (expected, size) -> + Format.fprintf + ppf + "The size of a Dal page is expected to be %d bytes. The given one \ + has %d" + expected + size) + (obj2 (req "expected_size" int16) (req "page_size" int16)) + (function + | Unexpected_page_size {expected_size; page_size} -> + Some (expected_size, page_size) + | _ -> None) + (fun (expected_size, page_size) -> + Unexpected_page_size {expected_size; page_size}) + let dal_proof_error reason = Dal_proof_error reason let proof_error reason = error @@ dal_proof_error reason @@ -687,11 +712,12 @@ module History = struct | Error `Segment_index_out_of_range -> fail_with_error_msg "Segment_index_out_of_range" | Error `Page_length_mismatch -> - fail_with_error_msg - @@ Format.sprintf - "Page_length_mismatch: Expected:%d. Got: %d" - dal_params.page_size - (Bytes.length page.content) + fail + @@ Unexpected_page_size + { + expected_size = dal_params.page_size; + page_size = Bytes.length data; + } let produce_proof_repr dal_params page_id ~page_info slots_hist hist_cache = let open Tzresult_syntax in diff --git a/src/proto_alpha/lib_protocol/dal_slot_repr.mli b/src/proto_alpha/lib_protocol/dal_slot_repr.mli index dd0faf469b74..4827e39a7fd4 100644 --- a/src/proto_alpha/lib_protocol/dal_slot_repr.mli +++ b/src/proto_alpha/lib_protocol/dal_slot_repr.mli @@ -312,7 +312,9 @@ module History : sig type error += Add_element_in_slots_skip_list_violates_ordering - type error += Dal_proof_error of string + type error += + | Dal_proof_error of string + | Unexpected_page_size of {expected_size : int; page_size : int} module Internal_for_tests : sig val content : t -> Header.t -- GitLab From e4abd9850e55b46ff406b18e9a43cda915f23fe6 Mon Sep 17 00:00:00 2001 From: "iguerNL@Functori" Date: Tue, 25 Oct 2022 09:09:11 +0200 Subject: [PATCH 2/3] Proto/Dal: generalize Dal_helpers.failing_check_produce_result's error --- .../lib_protocol/test/helpers/dal_helpers.ml | 22 +++++++++++-------- .../lib_protocol/test/helpers/dal_helpers.mli | 2 +- .../test/unit/test_dal_slot_proof.ml | 16 +++++++++----- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.ml b/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.ml index 84388e158a6e..2280d9f3c83d 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.ml @@ -210,9 +210,11 @@ struct input_opt ~expected:(expected_data page_info proof_status) - let failing_check_produce_result ~__LOC__ ~dal_proof_error res _page_info = - Assert.proto_error ~loc:__LOC__ res (function - | Hist.Dal_proof_error s -> String.equal s dal_proof_error + let failing_check_produce_result ~__LOC__ ~expected_error res _page_info = + Assert.proto_error ~loc:__LOC__ res (fun e -> + match (e, expected_error) with + | Hist.Dal_proof_error s, Hist.Dal_proof_error expected -> + String.equal s expected | _ -> false) let successful_check_verify_result ~__LOC__ proof_status res page_info = @@ -232,14 +234,16 @@ struct let slot_confirmed_but_page_data_not_provided ~__LOC__ = failing_check_produce_result ~__LOC__ - ~dal_proof_error: - "The page ID's slot is confirmed, but no page content and proof are \ - provided." + ~expected_error: + (Hist.Dal_proof_error + "The page ID's slot is confirmed, but no page content and proof are \ + provided.") let slot_not_confirmed_but_page_data_provided ~__LOC__ = failing_check_produce_result ~__LOC__ - ~dal_proof_error: - "The page ID's slot is not confirmed, but page content and proof are \ - provided." + ~expected_error: + (Hist.Dal_proof_error + "The page ID's slot is not confirmed, but page content and proof \ + are provided.") end diff --git a/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.mli b/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.mli index 20c6454aebc5..c4413f377f4a 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.mli @@ -157,7 +157,7 @@ end) : sig (** Helper for the case where produce_proof is expected to fail. *) val failing_check_produce_result : __LOC__:string -> - dal_proof_error:string -> + expected_error:Environment.Error_monad.error -> 'a tzresult -> 'b -> unit tzresult Lwt.t diff --git a/src/proto_alpha/lib_protocol/test/unit/test_dal_slot_proof.ml b/src/proto_alpha/lib_protocol/test/unit/test_dal_slot_proof.ml index e3ca62d4634a..b2b703475fdc 100644 --- a/src/proto_alpha/lib_protocol/test/unit/test_dal_slot_proof.ml +++ b/src/proto_alpha/lib_protocol/test/unit/test_dal_slot_proof.ml @@ -219,9 +219,11 @@ struct ~check_produce: (failing_check_produce_result ~__LOC__ - ~dal_proof_error: - "Wrong page content for the given page index and slot commitment \ - (page id=(published_level: 11, slot_index: 0, page_index: 2)).") + ~expected_error: + (Hist.Dal_proof_error + "Wrong page content for the given page index and slot \ + commitment (page id=(published_level: 11, slot_index: 0, \ + page_index: 2)).")) (** Test where a slot is confirmed, requesting a proof for a confirmed page, where correct page proof is provided, but given page data is altered. *) @@ -239,9 +241,11 @@ struct ~check_produce: (failing_check_produce_result ~__LOC__ - ~dal_proof_error: - "Wrong page content for the given page index and slot commitment \ - (page id=(published_level: 11, slot_index: 0, page_index: 0)).") + ~expected_error: + (Hist.Dal_proof_error + "Wrong page content for the given page index and slot \ + commitment (page id=(published_level: 11, slot_index: 0, \ + page_index: 0)).")) (* Variants of the tests above: Construct/verify proofs that attempt to unconfirm pages on top of a (confirmed) slot added in genesis_history skip -- GitLab From 1eb87e4b62a86ff1283b5174c8a86c53971fc721 Mon Sep 17 00:00:00 2001 From: "iguerNL@Functori" Date: Tue, 25 Oct 2022 09:17:31 +0200 Subject: [PATCH 3/3] Tests: add two DAL proofs tests with too short/long page data --- .../lib_protocol/test/helpers/dal_helpers.ml | 3 ++ .../test/unit/test_dal_slot_proof.ml | 46 ++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.ml b/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.ml index 2280d9f3c83d..90acd505d742 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/dal_helpers.ml @@ -215,6 +215,9 @@ struct match (e, expected_error) with | Hist.Dal_proof_error s, Hist.Dal_proof_error expected -> String.equal s expected + | ( Hist.Unexpected_page_size {expected_size = e1; page_size = p1}, + Hist.Unexpected_page_size {expected_size = e2; page_size = p2} ) -> + e1 = e2 && p1 = p2 | _ -> false) let successful_check_verify_result ~__LOC__ proof_status res page_info = diff --git a/src/proto_alpha/lib_protocol/test/unit/test_dal_slot_proof.ml b/src/proto_alpha/lib_protocol/test/unit/test_dal_slot_proof.ml index b2b703475fdc..c1e83567e500 100644 --- a/src/proto_alpha/lib_protocol/test/unit/test_dal_slot_proof.ml +++ b/src/proto_alpha/lib_protocol/test/unit/test_dal_slot_proof.ml @@ -247,6 +247,44 @@ struct commitment (page id=(published_level: 11, slot_index: 0, \ page_index: 0)).")) + (** Same as {!confirmed_slot_on_genesis_confirmed_page_bad_data_right_length} + but the data is too short. *) + let confirmed_slot_on_genesis_confirmed_page_bad_data_short = + let page_size = Parameters.dal_parameters.cryptobox_parameters.page_size in + helper_confirmed_slot_on_genesis + ~level:(Raw_level_repr.succ level_ten) + ~mk_page_info: + (mk_page_info + ~custom_data: + (Some + (fun ~default_char page_size -> + Some (Bytes.make (page_size - 1) default_char)))) + ~check_produce: + (failing_check_produce_result + ~__LOC__ + ~expected_error: + (Hist.Unexpected_page_size + {expected_size = page_size; page_size = page_size - 1})) + + (** Same as {!confirmed_slot_on_genesis_confirmed_page_bad_data_right_length} + but the data is too long. *) + let confirmed_slot_on_genesis_confirmed_page_bad_data_long = + let page_size = Parameters.dal_parameters.cryptobox_parameters.page_size in + helper_confirmed_slot_on_genesis + ~level:(Raw_level_repr.succ level_ten) + ~mk_page_info: + (mk_page_info + ~custom_data: + (Some + (fun ~default_char page_size -> + Some (Bytes.make (page_size + 1) default_char)))) + ~check_produce: + (failing_check_produce_result + ~__LOC__ + ~expected_error: + (Hist.Unexpected_page_size + {expected_size = page_size; page_size = page_size + 1})) + (* Variants of the tests above: Construct/verify proofs that attempt to unconfirm pages on top of a (confirmed) slot added in genesis_history skip list. @@ -360,8 +398,14 @@ struct "Confirmed slot on top of genesis: confirmed page with bad proof" confirmed_slot_on_genesis_confirmed_page_bad_page_proof; tztest - "Confirmed slot on top of genesis: confirmed page with bad data " + "Confirmed slot on top of genesis: confirmed page with bad data" confirmed_slot_on_genesis_confirmed_page_bad_data_right_length; + tztest + "Confirmed slot on top of genesis: confirmed page with too short data" + confirmed_slot_on_genesis_confirmed_page_bad_data_short; + tztest + "Confirmed slot on top of genesis: confirmed page with too long data" + confirmed_slot_on_genesis_confirmed_page_bad_data_long; ] in let confirmed_slot_on_genesis_unconfirmed_page_tests = -- GitLab