From 888090d406fe82b8334fe7269eafa014b39a65f3 Mon Sep 17 00:00:00 2001 From: Lucas Randazzo Date: Wed, 21 Feb 2024 11:44:36 +0100 Subject: [PATCH 1/3] Proto/tests: change "raise Not_found" to be more informative --- .../lib_protocol/test/helpers/state.ml | 11 ++++-- .../test/helpers/state_account.ml | 36 ++++++++++++------- .../integration/test_scenario_autostaking.ml | 6 +++- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/state.ml b/src/proto_alpha/lib_protocol/test/helpers/state.ml index 911d3b8249b2..75671b168efa 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/state.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/state.ml @@ -46,7 +46,9 @@ let unstake_wait state = (** From a name, returns the corresponding account *) let find_account (account_name : string) (state : t) : account_state = match String.Map.find account_name state.account_map with - | None -> raise Not_found + | None -> + Log.error "State.find_account: account %s not found" account_name ; + assert false | Some r -> r let find_account_from_pkh (pkh : Signature.public_key_hash) (state : t) : @@ -56,7 +58,12 @@ let find_account_from_pkh (pkh : Signature.public_key_hash) (state : t) : state.account_map |> String.Map.choose |> function - | None -> raise Not_found + | None -> + Log.error + "State.find_account_from_pkh: account %a not found" + Signature.Public_key_hash.pp + pkh ; + assert false | Some (name, acc) -> (name, acc) let liquid_delegated ~name state = diff --git a/src/proto_alpha/lib_protocol/test/helpers/state_account.ml b/src/proto_alpha/lib_protocol/test/helpers/state_account.ml index 543add894537..b2c8fbfe0197 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/state_account.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/state_account.ml @@ -13,6 +13,10 @@ module Tez = struct include Tez_helpers.Compare end +let fail_account_not_found func_name account_name = + Log.error "State_account.%s: account %s not found" func_name account_name ; + assert false + let join_errors e1 e2 = let open Lwt_result_syntax in match (e1, e2) with @@ -498,7 +502,7 @@ let balance_zero = let balance_of_account account_name (account_map : account_map) = match String.Map.find account_name account_map with - | None -> raise Not_found + | None -> fail_account_not_found "balance_of_account.src" account_name | Some { pkh = _; @@ -529,7 +533,7 @@ let balance_of_account account_name (account_map : account_map) = | None -> balance | Some d -> ( match String.Map.find d account_map with - | None -> raise Not_found + | None -> fail_account_not_found "balance_of_account.delegate" d | Some delegate_account -> { balance with @@ -742,7 +746,9 @@ let assert_balance_equal ~loc account_name let update_account ~f account_name account_map = String.Map.update account_name - (function None -> raise Not_found | Some x -> Some (f x)) + (function + | None -> fail_account_not_found "update_account" account_name + | Some x -> Some (f x)) account_map let add_liquid_rewards amount account_name account_map = @@ -793,12 +799,13 @@ let apply_transfer amount src_name dst_name account_map = in let account_map = update_account ~f:f_src src_name account_map in update_account ~f:f_dst dst_name account_map - | _ -> raise Not_found + | None, _ -> fail_account_not_found "apply_transfer.src" src_name + | _, None -> fail_account_not_found "apply_transfer.dst" dst_name let stake_from_unstake amount current_cycle consensus_rights_delay delegate_name account_map = match String.Map.find delegate_name account_map with - | None -> raise Not_found + | None -> fail_account_not_found "stake_from_unstake" delegate_name | Some ({unstaked_frozen; frozen_deposits; slashed_cycles; _} as account) -> let oldest_slashable_cycle = Cycle.(sub current_cycle (consensus_rights_delay + 1)) @@ -884,7 +891,7 @@ let unstake_values_real amount delegate_account = let apply_stake amount current_cycle consensus_rights_delay staker_name account_map = match String.Map.find staker_name account_map with - | None -> raise Not_found + | None -> fail_account_not_found "apply_stake" staker_name | Some staker -> ( match staker.delegate with | None -> @@ -971,13 +978,14 @@ let apply_stake amount current_cycle consensus_rights_delay staker_name let apply_unstake cycle amount staker_name account_map = match String.Map.find staker_name account_map with - | None -> raise Not_found + | None -> fail_account_not_found "apply_unstake.staker" staker_name | Some staker -> ( match staker.delegate with | None -> (* Invalid operation: no delegate *) account_map | Some delegate_name -> ( match String.Map.find delegate_name account_map with - | None -> raise Not_found + | None -> + fail_account_not_found "apply_unstake.delegate" delegate_name | Some delegate -> if delegate_name = staker_name then (* Case self stake *) @@ -1090,7 +1098,7 @@ let apply_unslashable_for_all cycle account_map = let apply_finalize staker_name account_map = match String.Map.find staker_name account_map with - | None -> raise Not_found + | None -> fail_account_not_found "apply_finalize" staker_name | Some _staker -> (* Because an account can still have finalizable funds from a delegate that is not its own, we iterate over all of them *) @@ -1229,7 +1237,8 @@ let apply_slashing in let culprit_account = String.Map.find culprit_name account_map - |> Option.value_f ~default:(fun () -> raise Not_found) + |> Option.value_f ~default:(fun () -> + fail_account_not_found "apply_slashing" culprit_name) in let slashed_culprit_account, total_slashed = slash_culprit culprit_account in let account_map = @@ -1324,7 +1333,10 @@ let assert_pseudotokens_consistency ~loc balance account account_name if account_name = delegate_name then return_unit else match String.Map.find delegate_name account_map with - | None -> raise Not_found + | None -> + fail_account_not_found + "assert_pseudotokens_consistency" + delegate_name | Some delegate_account -> let total_co = Frozen_tez.total_co_current_q @@ -1357,7 +1369,7 @@ let assert_pseudotokens_consistency ~loc balance account account_name let assert_balance_check ~loc ctxt account_name account_map = let open Lwt_result_syntax in match String.Map.find account_name account_map with - | None -> raise Not_found + | None -> fail_account_not_found "assert_balance_check" account_name | Some account -> let* balance_ctxt, total_balance_ctxt = get_balance_from_context ctxt account.contract diff --git a/src/proto_alpha/lib_protocol/test/integration/test_scenario_autostaking.ml b/src/proto_alpha/lib_protocol/test/integration/test_scenario_autostaking.ml index d539e73d8207..9636285d50cd 100644 --- a/src/proto_alpha/lib_protocol/test/integration/test_scenario_autostaking.ml +++ b/src/proto_alpha/lib_protocol/test/integration/test_scenario_autostaking.ml @@ -41,7 +41,11 @@ let assert_balance_evolution ~loc ~for_accounts ~part ~name ~old_balance Log.debug "@[Old Balance@ %a@]@." balance_pp old_balance ; Log.debug "@[New Balance@ %a@]@." balance_pp new_balance ; failwith "%s Unexpected stake evolution for %s" loc name) - else raise Not_found + else ( + Log.error + "Test_scenario_autostaking.assert_balance_evolution: account %s not found" + name ; + assert false) let delegate = "delegate" -- GitLab From 51796201c08a34a2588ea2184d097bd6436fb009 Mon Sep 17 00:00:00 2001 From: Lucas Randazzo Date: Wed, 21 Feb 2024 11:46:00 +0100 Subject: [PATCH 2/3] Proto/tests: better comments --- src/proto_alpha/lib_protocol/test/helpers/state_account.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/state_account.ml b/src/proto_alpha/lib_protocol/test/helpers/state_account.ml index b2c8fbfe0197..11f0df35b108 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/state_account.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/state_account.ml @@ -120,7 +120,7 @@ module Frozen_tez = struct assert (Q.(equal rem zero)) ; Tez.(tez +! a.self_current) - (* 0 <= quantity < 1 && co_current + quantity is int *) + (* Precondition: 0 <= quantity < 1 && co_current + quantity is int *) let add_q_to_all_co_current quantity co_current = let s = total_co_current_q co_current in if Q.(equal quantity zero) then co_current @@ -214,7 +214,7 @@ module Frozen_tez = struct in ({a with co_current}, amount) - (* Remove a partial amount to the co frozen tez table. *) + (* Remove a partial amount from the co frozen tez table. *) let sub_current_q amount_q account a = if account = a.delegate then assert false else -- GitLab From 3a5e82a5cd5687dea2fbb49be58ab17eb1c7e117 Mon Sep 17 00:00:00 2001 From: Lucas Randazzo Date: Wed, 21 Feb 2024 11:49:18 +0100 Subject: [PATCH 3/3] Proto/tests: assert false in impossible branch --- src/proto_alpha/lib_protocol/test/helpers/state_account.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/state_account.ml b/src/proto_alpha/lib_protocol/test/helpers/state_account.ml index 11f0df35b108..66109684335c 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/state_account.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/state_account.ml @@ -219,7 +219,7 @@ module Frozen_tez = struct if account = a.delegate then assert false else match String.Map.find account a.co_current with - | None -> a + | None -> assert false | Some frozen -> if Q.(geq amount_q frozen) then let co_current = String.Map.remove account a.co_current in -- GitLab