From 0e9febddf080c258c9480989e8c1434870f4fda3 Mon Sep 17 00:00:00 2001 From: Guillaume Genestier Date: Thu, 9 Oct 2025 11:31:51 +0200 Subject: [PATCH 1/3] DAL/Tests: Fix the init_logger function to get a polymorphic log_step --- tezt/tests/dal.ml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index 2bffb1db24dc..7f3358da5430 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -58,13 +58,17 @@ module Dal_RPC = struct include Dal.RPC.Local end -let init_logger () = +type logger = {log_step : 'a. ('a, Format.formatter, unit, unit) format4 -> 'a} + +let init_logger () : logger = let counter = ref 1 in - fun fmt -> + let log_step fmt = let color = Log.Color.(bold ++ FG.blue) in let prefix = "step-" ^ string_of_int !counter in incr counter ; Log.info ~color ~prefix fmt + in + {log_step} let read_dir dir = let* dir = Lwt_unix.opendir dir in @@ -9579,7 +9583,7 @@ let test_inject_accusation_aggregated_attestation nb_attesting_tz4 protocol let test_producer_attester (protocol : Protocol.t) (dal_params : Dal_common.Parameters.t) (_cryptobox : Cryptobox.t) (node : Node.t) (client : Client.t) (_dal_node : Dal_node.t) : unit Lwt.t = - let log_step = init_logger () in + let {log_step} = init_logger () in let index = 3 in let slot_size = dal_params.cryptobox.slot_size in log_step "Declaration of a producer" ; @@ -9662,7 +9666,7 @@ let test_producer_attester (protocol : Protocol.t) let test_attester_did_not_attest (protocol : Protocol.t) (dal_params : Dal_common.Parameters.t) (_cryptobox : Cryptobox.t) (node : Node.t) (client : Client.t) (_dal_node : Dal_node.t) : unit Lwt.t = - let log_step = init_logger () in + let {log_step} = init_logger () in let index = 3 in let slot_size = dal_params.cryptobox.slot_size in log_step "Declaration of a producer" ; @@ -11184,7 +11188,7 @@ let baker_at_round_n ?level round client : string Lwt.t = - [node2]: B (on top of A2) After reconnecting the nodes, node1 switches to the A2 -> B branch (due to higher fitness). - Then, test that shards are propagated after one level is baked on top of + Then, test that shards are propagated after one level is baked on top of the block which included the commitment publication, at (n+1). *) let test_dal_one_level_reorg protocol dal_parameters _cryptobox node1 client1 dal_bootstrap = -- GitLab From 870c1656a07782719b3ef3d7f41b4245dfb5a2fb Mon Sep 17 00:00:00 2001 From: Guillaume Genestier Date: Thu, 9 Oct 2025 11:33:26 +0200 Subject: [PATCH 2/3] DAL/Tests: Fix the simple_producer which used too low fees hence got publications rejected by the baker --- tezt/tests/dal.ml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index 7f3358da5430..83d1e27a702e 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -1219,6 +1219,13 @@ let simple_slot_producer ~slot_index ~slot_size ~from ~into dal_node l1_node let* _op_hash = publish_commitment ~force:true + (* This value is quite high (way higher than required). But the + default value of 1200µtez was not sufficient for a publication to + be validated by the mempool in the test + [test_migration_accuser_issue]. I do not understand why, since + publication fees on real networks are less than 1000µtez. + *) + ~fee:5000 ~source ~index:slot_index ~commitment -- GitLab From 9a0c79c735f1799ac8bdc1b2a3b12af642b8a56b Mon Sep 17 00:00:00 2001 From: Guillaume Genestier Date: Thu, 9 Oct 2025 11:34:23 +0200 Subject: [PATCH 3/3] DAL/Tests: Check that when attestation_lag changes, publication cannot be attested in the protocol next to the one of their publication --- tezt/tests/dal.ml | 196 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 194 insertions(+), 2 deletions(-) diff --git a/tezt/tests/dal.ml b/tezt/tests/dal.ml index 83d1e27a702e..5127c599b274 100644 --- a/tezt/tests/dal.ml +++ b/tezt/tests/dal.ml @@ -4649,7 +4649,7 @@ let test_migration_accuser_issue ~migrate_from ~migrate_to = ~activation_timestamp:Now ~operator_profiles:[slot_index] ~minimal_block_delay: - (* to be sure there is enough time to published slots *) + (* to be sure there is enough time to publish slots *) "2" ~blocks_per_cycle:32 ~consensus_committee_size:512 @@ -4664,6 +4664,197 @@ let test_migration_accuser_issue ~migrate_from ~migrate_to = ~migrate_to () +(* This test checks that the migration goes well when the attestation lag is + reduced. It uses the baker and features publication at the last level of + "previous" protocol. It verifies that this publication is not attested in + the "new" protocol. + + Here is an explanation of the potential issue: + We denote by M the migration level. We assume a migration from attestation + lag from 8 to 5 in this explanation. + + If we use publication time protocol, we expect + + M - 9 -> attested at M - 1 + M - 8 -> attested at M + M - 7 -> attested at M + 1 + M - 6 -> attested at M + 2 + M - 5 -> attested at M + 3 + M - 4 -> attested at M + 4 + M - 3 -> attested at M + 5 <<< Conflict + M - 2 -> attested at M + 6 <<< + M - 1 -> attested at M + 7 <<< + M -> attested at M + 5 <<< Conflict + M + 1 -> attested at M + 6 <<< + M + 2 -> attested at M + 7 <<< + M + 3 -> attested at M + 8 + + We see that attestations at levels M + 5, M + 6 and M + 7 are ambiguous + since there are 2 publication levels which are expected to be attested at + the same level. + The publication levels targetting attestation at an ambiguous level are + marked with <<<. + + If we use attestation time protocol, we expect + + M - 10 -> attested at M - 2 + M - 9 -> attested at M - 1 + M - 8 -> attested at M + M - 7 -> undefined + M - 6 -> undefined + M - 5 -> undefined + M - 4 -> attested at M + 1 + M - 3 -> attested at M + 2 + + There is an issue in both cases, so one has to make a choice. + We assume in this test that the publications between + M - previous_attestations_lag + 1 and M are not attested. +*) +let test_migration_with_attestation_lag_change ~migrate_from ~migrate_to = + let tags = ["migration"; "dal"; "attestation_lag"] in + let description = "test migration with reduction of the attestation_lag" in + let {log_step} = init_logger () in + (* We select a slot index to publish on. *) + let slot_index = 0 in + let scenario ~migration_level (dal_parameters : Dal.Parameters.t) client node + dal_node = + let* level = Client.level client in + (* We will publish random data for a whole cycle, which includes a migration + in the middle. *) + let first_level = level + 1 in + let last_publication_level = 22 in + (* [dal_parameters] should contain the parameters of the "previous" + protocol. *) + log_step "Getting the parameters of the next protocol" ; + let base = Either.right (migrate_to, None) in + let* proto_parameters = generate_protocol_parameters base migrate_to [] in + let new_dal_parameters = + Dal.Parameters.from_protocol_parameters proto_parameters + in + let old_lag = dal_parameters.attestation_lag in + let new_lag = new_dal_parameters.attestation_lag in + (* We want to have enough levels to attest in "previous" and to + attest in "next" commitments published in "next". + *) + log_step + "Checking that the migration level is at good distance of both ends" ; + assert (first_level + old_lag < migration_level) ; + assert (migration_level + new_lag < last_publication_level) ; + log_step + "Launching a producer publishing from first_level = %d to last_level = %d" + first_level + last_publication_level ; + let _promise = + simple_slot_producer + ~slot_size:dal_parameters.cryptobox.slot_size + ~slot_index + ~from:first_level + ~into:last_publication_level + dal_node + node + client + in + log_step "Start baker" ; + let baker = + let dal_node_rpc_endpoint = Dal_node.as_rpc_endpoint dal_node in + Agnostic_baker.create + ~dal_node_rpc_endpoint + ~delegates: + (List.map + (fun x -> x.Account.public_key_hash) + Constant.all_secret_keys) + node + client + in + let* () = Agnostic_baker.run baker in + let* _level = Node.wait_for_level node migration_level in + log_step "Migrated to next protocol" ; + let* _level = Node.wait_for_level node last_publication_level in + log_step "Turning the baker off" ; + let* () = Agnostic_baker.terminate baker in + let check_if_metadata_contain_expected_dal lvl metadata = + if lvl <= migration_level then ( + log_step + "Checking that level %d which is before migration is considered as \ + attested" + lvl ; + Check.is_true + ((function + | None -> false + | Some vec -> Array.length vec > slot_index && vec.(slot_index)) + metadata.RPC.dal_attestation) + ~__LOC__ + ~error_msg:"Slot before migration is expected to be attested") + else if lvl = migration_level + 1 then + Check.is_false + ((function + | None -> false | Some vec -> Array.fold_left ( || ) false vec) + metadata.dal_attestation) + ~__LOC__ + ~error_msg:"The migration level block is not supposed to be attested" + else if lvl <= migration_level + new_lag then ( + log_step + "Checking that level %d which is after migration but refers to a \ + publication before is attested only if the attestation_level did \ + not change between the 2 protocols." + lvl ; + if new_lag = old_lag then + Check.is_true + ((function + | None -> false + | Some vec -> Array.length vec > slot_index && vec.(slot_index)) + metadata.dal_attestation) + ~__LOC__ + ~error_msg: + "Slot published before migration is expected to be attested \ + after migration" + else + Check.is_false + ((function + | None -> true | Some vec -> Array.fold_left ( || ) false vec) + metadata.dal_attestation) + ~__LOC__ + ~error_msg: + "Slot published before migration is expected to not be attested \ + after migration") + else ( + log_step + "Checking that level %d which is after migration and refers to a \ + publication after migration is considered as attested" + lvl ; + Check.is_true + ((function + | None -> false + | Some vec -> Array.length vec > slot_index && vec.(slot_index)) + metadata.dal_attestation) + ~__LOC__ + ~error_msg:"Slot published after migration is expected to be attested") + in + let rec loop level = + if level <= last_publication_level then + let* metadata = + Node.RPC.call node + @@ RPC.get_chain_block_metadata ~block:(string_of_int level) () + in + let () = check_if_metadata_contain_expected_dal level metadata in + loop (level + 1) + else unit + in + let* () = loop (first_level + old_lag + 1) in + unit + in + test_l1_migration_scenario + ~scenario + ~tags + ~description + ~uses:[Constant.octez_agnostic_baker] + ~activation_timestamp:Now + ~operator_profiles:[slot_index] + ~migration_level:12 + ~migrate_from + ~migrate_to + () + let test_operator_profile _protocol _dal_parameters _cryptobox _node _client dal_node = let index = 0 in @@ -11915,7 +12106,8 @@ let register_migration ~migrate_from ~migrate_to = ~migrate_from ~migrate_to ; tests_start_dal_node_around_migration ~migrate_from ~migrate_to ; - test_migration_accuser_issue ~migration_level:4 ~migrate_from ~migrate_to + test_migration_accuser_issue ~migration_level:4 ~migrate_from ~migrate_to ; + test_migration_with_attestation_lag_change ~migrate_from ~migrate_to let () = Regression.register -- GitLab