From 0d88aba7ced9398726bb566b01f2b47f5fcb88dc Mon Sep 17 00:00:00 2001 From: Adam Allombert-Goget Date: Thu, 15 May 2025 15:51:13 +0200 Subject: [PATCH 1/6] proto/main: update mempool initialization Minimal slots computation is no longer under [aggregate_attestation] feature flag. --- src/proto_alpha/lib_protocol/main.ml | 83 +++++++++------------------- 1 file changed, 25 insertions(+), 58 deletions(-) diff --git a/src/proto_alpha/lib_protocol/main.ml b/src/proto_alpha/lib_protocol/main.ml index 064aa4e28379..054a8ac67565 100644 --- a/src/proto_alpha/lib_protocol/main.ml +++ b/src/proto_alpha/lib_protocol/main.ml @@ -160,66 +160,33 @@ let init_consensus_rights_for_block ctxt mode ~predecessor_level = let init_consensus_rights_for_mempool ctxt ~predecessor_level = let open Lwt_result_syntax in let open Alpha_context in - if Constants.aggregate_attestation ctxt then - (* Under feature flag, we do compute minimal_slots for each level. *) - let allowed_levels = - let levels = [predecessor_level; Level.(succ ctxt predecessor_level)] in - match Level.pred ctxt predecessor_level with - | Some grandparent_level -> grandparent_level :: levels - | None -> levels - in - let* ctxt, minimal_slots = - List.fold_left_es - (fun (ctxt, minimal_slots) level -> - let* ctxt, level_minimal_slot = - Baking.attesting_rights_by_first_slot ctxt level - in - return (ctxt, Level.Map.add level level_minimal_slot minimal_slots)) - (ctxt, Level.Map.empty) - allowed_levels - in - let ctxt = - Consensus.initialize_consensus_operation - ctxt - ~allowed_attestations:None - ~allowed_preattestations:None - in - let ctxt = - Consensus.initialize_allowed_consensus ctxt (Some minimal_slots) - in - return ctxt - else - (* We don't want to compute the tables by first slot for all three - possible levels because it is time-consuming. So we don't compute - any [allowed_attestations] or [allowed_preattestations] tables. *) - let ctxt = - Consensus.initialize_consensus_operation - ctxt - ~allowed_attestations:None - ~allowed_preattestations:None - in - (* However, we want to ensure that the cycle rights are loaded in - the context, so that {!Stake_distribution.slot_owner} doesn't - have to initialize them each time it is called (we do this now - because the context is discarded at the end of the validation of - each operation, so we can't rely on the caching done by - [slot_owner] itself). *) - let cycle = (Level.current ctxt).cycle in - let* ctxt = Stake_distribution.load_sampler_for_cycle ctxt cycle in - let* ctxt = Stake_distribution.load_stake_info_for_cycle ctxt cycle in - (* If the cycle has changed between the grandparent level and the - current level, we also initialize the sampler for that - cycle. That way, all three allowed levels are covered. *) + (* For each allowed level, compute a map associating slots to their owners. + Only the lowest slot assigned to each delegate is included. *) + let allowed_levels = + let levels = [predecessor_level; Level.(succ ctxt predecessor_level)] in match Level.pred ctxt predecessor_level with - | Some gp_level when Cycle.(gp_level.cycle <> cycle) -> - let* ctxt = - Stake_distribution.load_sampler_for_cycle ctxt gp_level.cycle - in - let* ctxt = - Stake_distribution.load_stake_info_for_cycle ctxt gp_level.cycle + | Some grandparent_level -> grandparent_level :: levels + | None -> levels + in + let* ctxt, minimal_slots = + List.fold_left_es + (fun (ctxt, minimal_slots) level -> + let* ctxt, level_minimal_slot = + Baking.attesting_rights_by_first_slot ctxt level in - return ctxt - | Some _ | None -> return ctxt + return (ctxt, Level.Map.add level level_minimal_slot minimal_slots)) + (ctxt, Level.Map.empty) + allowed_levels + in + let ctxt = + Consensus.initialize_consensus_operation + ctxt + ~allowed_attestations:None + ~allowed_preattestations:None + in + (* Store the resulting map in the context as [allowed_consensus]. *) + let ctxt = Consensus.initialize_allowed_consensus ctxt (Some minimal_slots) in + return ctxt let prepare_ctxt ctxt mode ~(predecessor : Block_header.shell_header) = let open Lwt_result_syntax in -- GitLab From aa2447199df18038f39c6e48a556dfbe17e9f3f3 Mon Sep 17 00:00:00 2001 From: Adam Allombert-Goget Date: Thu, 15 May 2025 17:12:05 +0200 Subject: [PATCH 2/6] proto/validate: update mempool validation for consensus operations Minimal slot check is no longer under [aggregate_attestation] feature flag. --- src/proto_alpha/lib_protocol/validate.ml | 43 +++++++----------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/src/proto_alpha/lib_protocol/validate.ml b/src/proto_alpha/lib_protocol/validate.ml index 0c0ff0a33a5b..33b40d2def3b 100644 --- a/src/proto_alpha/lib_protocol/validate.ml +++ b/src/proto_alpha/lib_protocol/validate.ml @@ -599,17 +599,9 @@ module Consensus = struct [predecessor_level - 1 <= op_level <= predecessor_level + 1] (note that [predecessor_level + 1] is also known as [current_level]). - Note that we also don't check whether the slot is normalized - (that is, whether it is the delegate's smallest slot). Indeed, - we don't want to compute the right tables by first slot for all - three allowed levels. Checking the slot normalization is - therefore the responsability of the baker when it selects - the consensus operations to include in a new block. Moreover, - multiple attestations pointing to the same block with different - slots can be punished by a double-(pre)attestation operation. - - Return the slot owner's consensus key and a fake attesting power (the - latter won't be used anyway in Mempool mode). *) + The (pre)attested slot must be the lowest one assigned to its owner. + + Returns the corresponding delegate's consensus key and attesting power. *) let check_mempool_consensus vi consensus_info kind {level; slot; _} = let open Lwt_result_syntax in let*? () = @@ -623,26 +615,15 @@ module Consensus = struct (Consensus_operation_for_future_level {kind; expected; provided}) else ok_unit in - if Constants.aggregate_attestation vi.ctxt then - (* Under feature flag, minimal slots are pre-computed for all accepted - levels and stored in consensus_info.consensus_slot_map *) - let level = Level.from_raw vi.ctxt level in - match consensus_info.consensus_slot_map with - | None -> tzfail (Consensus.Slot_map_not_found {loc = __LOC__}) - | Some level_map -> - let slot_map = Level.Map.find level level_map in - let*? consensus_key, attesting_power, _dal_power = - get_delegate_details slot_map kind slot - in - return (consensus_key, attesting_power) - else - let* (_ctxt : t), consensus_key = - Stake_distribution.slot_owner - vi.ctxt - (Level.from_raw vi.ctxt level) - slot - in - return (consensus_key, 0 (* Fake attesting power *)) + let level = Level.from_raw vi.ctxt level in + match consensus_info.consensus_slot_map with + | None -> tzfail (Consensus.Slot_map_not_found {loc = __LOC__}) + | Some level_map -> + let slot_map = Level.Map.find level level_map in + let*? consensus_key, attesting_power, _dal_power = + get_delegate_details slot_map kind slot + in + return (consensus_key, attesting_power) (* We do not check that the frozen deposits are positive because this only needs to be true in the context of a block that actually contains the operation, which may not be the same as the current -- GitLab From 0a10049b61ad16dae7971d7a70d64d43342b854e Mon Sep 17 00:00:00 2001 From: Adam Allombert-Goget Date: Fri, 16 May 2025 10:52:36 +0200 Subject: [PATCH 3/6] proto/test_attestation: remove test for deprecated feature This feature has been removed: the mempool now systematically checks for slot minimality --- .../integration/consensus/test_attestation.ml | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/integration/consensus/test_attestation.ml b/src/proto_alpha/lib_protocol/test/integration/consensus/test_attestation.ml index a88624ee1bcb..879430d92631 100644 --- a/src/proto_alpha/lib_protocol/test/integration/consensus/test_attestation.ml +++ b/src/proto_alpha/lib_protocol/test/integration/consensus/test_attestation.ml @@ -132,39 +132,6 @@ let error_wrong_slot = function true | _ -> false -(** Test that the mempool accepts attestations with a non-normalized - slot (that is, a slot that belongs to the delegate but is not the - delegate's smallest slot) at all three allowed levels for - attestations (and various rounds). *) -let test_mempool_second_slot () = - let open Lwt_result_syntax in - let* genesis, grandparent = init_genesis () in - let* error = - (* Under [aggregate_attestation] feature flag, consensus operations with - non-minimal slots are no longer propagated by mempools anymore. *) - let* {parametric = {aggregate_attestation; _}; _} = - Context.get_constants (B genesis) - in - if aggregate_attestation then return_some error_wrong_slot else return_none - in - let* predecessor = Block.bake grandparent ~policy:(By_round 3) in - let* future_block = Block.bake predecessor ~policy:(By_round 5) in - let check_non_smallest_slot loc attested_block = - let* delegate, slot = delegate_and_second_slot attested_block in - Consensus_helpers.test_consensus_operation - ~loc - ~attested_block - ~predecessor - ~delegate - ~slot - ?error - Attestation - Mempool - in - let* () = check_non_smallest_slot __LOC__ grandparent in - let* () = check_non_smallest_slot __LOC__ predecessor in - check_non_smallest_slot __LOC__ future_block - (** {1 Negative tests} The following test scenarios are supposed to raise errors. *) @@ -988,7 +955,6 @@ let tests = Tztest.tztest "Arbitrary branch" `Quick test_arbitrary_branch; Tztest.tztest "Non-zero round" `Quick test_non_zero_round; Tztest.tztest "Fitness gap" `Quick test_fitness_gap; - Tztest.tztest "Mempool: non-smallest slot" `Quick test_mempool_second_slot; (* Negative tests *) (* Wrong slot *) Tztest.tztest "Attestation with slot -1" `Quick test_negative_slot; -- GitLab From 749b4be2c8ebd1799caa1c67ad1e4a4683059693 Mon Sep 17 00:00:00 2001 From: Adam Allombert-Goget Date: Fri, 16 May 2025 10:57:00 +0200 Subject: [PATCH 4/6] proto/test_attestation: update test for minimal-slot filtering Reflects the updated mempool behavior, which now filters out non-minimal slots regardless of the aggregate_attestation feature flag. --- .../integration/consensus/test_attestation.ml | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/integration/consensus/test_attestation.ml b/src/proto_alpha/lib_protocol/test/integration/consensus/test_attestation.ml index 879430d92631..11ddbe250aa8 100644 --- a/src/proto_alpha/lib_protocol/test/integration/consensus/test_attestation.ml +++ b/src/proto_alpha/lib_protocol/test/integration/consensus/test_attestation.ml @@ -158,20 +158,11 @@ let test_negative_slot () = (** Attestation with a non-normalized slot (that is, a slot that belongs to the delegate but is not the delegate's smallest slot). - It should fail in application and construction modes, but be - accepted in mempool mode. *) + This should fail in all validation modes. *) let test_not_smallest_slot () = let open Lwt_result_syntax in - let* genesis, b = init_genesis () in + let* _genesis, b = init_genesis () in let* delegate, slot = delegate_and_second_slot b in - let* mempool_error = - (* Under [aggregate_attestation] feature flag, consensus operations with - non-minimal slots are no longer propagated by mempools anymore. *) - let* {parametric = {aggregate_attestation; _}; _} = - Context.get_constants (B genesis) - in - if aggregate_attestation then return_some error_wrong_slot else return_none - in Consensus_helpers.test_consensus_operation_all_modes_different_outcomes ~loc:__LOC__ ~attested_block:b @@ -179,7 +170,7 @@ let test_not_smallest_slot () = ~slot ~application_error:error_wrong_slot ~construction_error:error_wrong_slot - ?mempool_error + ~mempool_error:error_wrong_slot Attestation let delegate_and_someone_elses_slot block = -- GitLab From be18d6413692e518485325dee58b609ddaeb8ccd Mon Sep 17 00:00:00 2001 From: Adam Allombert-Goget Date: Tue, 20 May 2025 11:15:45 +0200 Subject: [PATCH 5/6] proto/raw_context: update consensus setter functions - merging `initialize_with_attestations_and_preattestations` and `intialize_with_allowed_consensus` in a single `set_allowed_operations`. - merging `initialize_consensus_operation` and `initialize_allowed_consensus` in a single `initialize_consensus_operations`. --- src/proto_alpha/lib_protocol/main.ml | 5 ++-- src/proto_alpha/lib_protocol/raw_context.ml | 25 +++++++------------- src/proto_alpha/lib_protocol/raw_context.mli | 7 +----- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/proto_alpha/lib_protocol/main.ml b/src/proto_alpha/lib_protocol/main.ml index 054a8ac67565..4be507fadfec 100644 --- a/src/proto_alpha/lib_protocol/main.ml +++ b/src/proto_alpha/lib_protocol/main.ml @@ -146,6 +146,7 @@ let init_consensus_rights_for_block ctxt mode ~predecessor_level = ctxt ~allowed_attestations:(Some attestations_map) ~allowed_preattestations + ~allowed_consensus:None in return ctxt @@ -179,13 +180,13 @@ let init_consensus_rights_for_mempool ctxt ~predecessor_level = allowed_levels in let ctxt = + (* Store the resulting map in the context as [allowed_consensus]. *) Consensus.initialize_consensus_operation ctxt ~allowed_attestations:None ~allowed_preattestations:None + ~allowed_consensus:(Some minimal_slots) in - (* Store the resulting map in the context as [allowed_consensus]. *) - let ctxt = Consensus.initialize_allowed_consensus ctxt (Some minimal_slots) in return ctxt let prepare_ctxt ctxt mode ~(predecessor : Block_header.shell_header) = diff --git a/src/proto_alpha/lib_protocol/raw_context.ml b/src/proto_alpha/lib_protocol/raw_context.ml index ceb93b90420c..09c0a436add5 100644 --- a/src/proto_alpha/lib_protocol/raw_context.ml +++ b/src/proto_alpha/lib_protocol/raw_context.ml @@ -228,12 +228,9 @@ module Raw_consensus = struct t | None -> {t with preattestations_quorum_round = Some round} - let initialize_with_attestations_and_preattestations ~allowed_attestations - ~allowed_preattestations t = - {t with allowed_attestations; allowed_preattestations} - - let initialize_with_allowed_consensus ~allowed_consensus t = - {t with allowed_consensus} + let set_allowed_operations ~allowed_attestations ~allowed_preattestations + ~allowed_consensus t = + {t with allowed_attestations; allowed_preattestations; allowed_consensus} let locked_round_evidence t = t.locked_round_evidence @@ -2039,11 +2036,9 @@ module type CONSENSUS = sig t -> allowed_attestations:(consensus_pk * int * int) slot_map option -> allowed_preattestations:(consensus_pk * int * int) slot_map option -> + allowed_consensus:(consensus_pk * int * int) slot_map level_map option -> t - val initialize_allowed_consensus : - t -> (consensus_pk * int * int) slot_map level_map option -> t - val record_attestation : t -> initial_slot:slot -> power:int -> t tzresult val record_preattestation : @@ -2107,17 +2102,13 @@ module Consensus : Raw_consensus.locked_round_evidence ctxt.back.consensus let[@inline] initialize_consensus_operation ctxt ~allowed_attestations - ~allowed_preattestations = + ~allowed_preattestations ~allowed_consensus = update_consensus_with ctxt - (Raw_consensus.initialize_with_attestations_and_preattestations + (Raw_consensus.set_allowed_operations ~allowed_attestations - ~allowed_preattestations) - - let[@inline] initialize_allowed_consensus ctxt allowed_consensus = - update_consensus_with - ctxt - (Raw_consensus.initialize_with_allowed_consensus ~allowed_consensus) + ~allowed_preattestations + ~allowed_consensus) let[@inline] record_preattestation ctxt ~initial_slot ~power round = update_consensus_with_tzresult diff --git a/src/proto_alpha/lib_protocol/raw_context.mli b/src/proto_alpha/lib_protocol/raw_context.mli index 69c4986b2045..abba3935e5dd 100644 --- a/src/proto_alpha/lib_protocol/raw_context.mli +++ b/src/proto_alpha/lib_protocol/raw_context.mli @@ -403,14 +403,9 @@ module type CONSENSUS = sig t -> allowed_attestations:(consensus_pk * int * int) slot_map option -> allowed_preattestations:(consensus_pk * int * int) slot_map option -> + allowed_consensus:(consensus_pk * int * int) slot_map level_map option -> t - (** Initializes the map of allowed consensus operations, this - function must be called only once and before applying any consensus - operation. *) - val initialize_allowed_consensus : - t -> (consensus_pk * int * int) slot_map level_map option -> t - (** [record_attestation ctx ~initial_slot ~power] records an attestation for the current block. -- GitLab From 2867d21165903f7517a86e873d9368e1814b2304 Mon Sep 17 00:00:00 2001 From: Adam Allombert-Goget Date: Mon, 19 May 2025 14:03:36 +0200 Subject: [PATCH 6/6] proto/changelog: add entry for mempool filtering non-minimal slots --- docs/protocols/alpha.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/protocols/alpha.rst b/docs/protocols/alpha.rst index 636eb4866337..500cc08ef317 100644 --- a/docs/protocols/alpha.rst +++ b/docs/protocols/alpha.rst @@ -102,6 +102,9 @@ Minor Changes - Allow ``finalise_unstake`` to be performed by any account (:gl:`!17950`). This allows finalisation of unstake requests to be done automatically by a third party - for example a finalisation bot. +- Consensus operations with non-minimal slots are filtered by mempools + (MR :gl:`!18040`). + - Consensus operations with different slots are no longer considered a punishable misbehaviour (MR :gl:`!18043`) -- GitLab