From 9d416f3d24184c59b64e6230b9557dd17c605e27 Mon Sep 17 00:00:00 2001 From: Valentin Chaboche Date: Wed, 25 May 2022 10:23:41 +0200 Subject: [PATCH] Proto,Scoru: avoid double look-up for deposit stake Put the responsability on the caller to check if the staker has already deposited. --- .../lib_protocol/sc_rollup_errors.ml | 11 -------- .../lib_protocol/sc_rollup_stake_storage.ml | 27 ++++++++----------- .../lib_protocol/sc_rollup_stake_storage.mli | 5 ++-- .../test/unit/test_sc_rollup_storage.ml | 23 ---------------- 4 files changed, 14 insertions(+), 52 deletions(-) diff --git a/src/proto_alpha/lib_protocol/sc_rollup_errors.ml b/src/proto_alpha/lib_protocol/sc_rollup_errors.ml index 16e43e641ae0..e07643d89171 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_errors.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_errors.ml @@ -25,7 +25,6 @@ (*****************************************************************************) type error += - | (* `Temporary *) Sc_rollup_already_staked | (* `Temporary *) Sc_rollup_disputed | (* `Temporary *) Sc_rollup_does_not_exist of Sc_rollup_repr.t | (* `Temporary *) Sc_rollup_no_conflict @@ -106,16 +105,6 @@ let () = Some () | _ -> None) (fun () -> Sc_rollup_max_number_of_messages_reached_for_commitment_period) ; - let description = "Already staked." in - register_error_kind - `Temporary - ~id:"Sc_rollup_already_staked" - ~title:"Already staked" - ~description - ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) - Data_encoding.empty - (function Sc_rollup_already_staked -> Some () | _ -> None) - (fun () -> Sc_rollup_already_staked) ; let description = "Attempted to cement a disputed commitment." in register_error_kind `Temporary 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 45f8054adae1..03baddb4e56c 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml @@ -47,19 +47,17 @@ let modify_staker_count ctxt rollup f = assert (Compare.Int.(size_diff = 0)) ; return ctxt +(** Warning: must be called only if [rollup] exists and [staker] is not to be + found in {!Store.Stakers.} *) let deposit_stake ctxt rollup staker = let open Lwt_tzresult_syntax in let* lcc, ctxt = Commitment_storage.last_cemented_commitment ctxt rollup in - let* ctxt, res = Store.Stakers.find (ctxt, rollup) staker in - match res with - | None -> - (* TODO: https://gitlab.com/tezos/tezos/-/issues/2449 - We should lock stake here, and fail if there aren't enough funds. - *) - let* ctxt, _size = Store.Stakers.init (ctxt, rollup) staker lcc in - let* ctxt = modify_staker_count ctxt rollup Int32.succ in - return ctxt - | Some _ -> fail Sc_rollup_already_staked + (* TODO: https://gitlab.com/tezos/tezos/-/issues/2449 + We should lock stake here, and fail if there aren't enough funds. + *) + let* ctxt, _size = Store.Stakers.init (ctxt, rollup) staker lcc in + let* ctxt = modify_staker_count ctxt rollup Int32.succ in + return ctxt let withdraw_stake ctxt rollup staker = let open Lwt_tzresult_syntax in @@ -268,12 +266,9 @@ let refine_stake ctxt rollup staker commitment = let publish_commitment ctxt rollup staker commitment = let open Lwt_tzresult_syntax in - let* ctxt, res = Store.Stakers.find (ctxt, rollup) staker in - match res with - | None -> - let* ctxt = deposit_stake ctxt rollup staker in - refine_stake ctxt rollup staker commitment - | Some _ -> refine_stake ctxt rollup staker commitment + let* ctxt, res = Store.Stakers.mem (ctxt, rollup) staker in + let* ctxt = if res then return ctxt else deposit_stake ctxt rollup staker in + refine_stake ctxt rollup staker commitment (** Try to consume n messages. *) let consume_n_messages ctxt rollup n = diff --git a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.mli b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.mli index 3ce6e4c7055e..19979776411b 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.mli @@ -126,10 +126,11 @@ module Internal_for_tests : sig cemented commitment, freezing [sc_rollup_deposit] from [staker]'s account balance. + Warning: must be called only if [rollup] exists and [staker] is not to be + found in {!Store.Stakers.} + May fail with: {ul - {li [Sc_rollup_does_not_exist] if [rollup] does not exist} - {li [Sc_rollup_already_staked] if [staker] is already staked} {li [Sc_rollup_staker_funds_too_low] if [staker] does not have enough funds to cover the deposit} } diff --git a/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_storage.ml b/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_storage.ml index 52b4e5734e7a..33e6a1e129cc 100644 --- a/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_storage.ml +++ b/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_storage.ml @@ -227,28 +227,6 @@ let test_deposit_then_withdraw () = in assert_true ctxt -let test_can_not_stake_twice () = - let* ctxt = new_context () in - let* rollup, ctxt = lift @@ new_sc_rollup ctxt in - let staker = - Signature.Public_key_hash.of_b58check_exn - "tz1SdKt9kjPp1HRQFkBmXtBhgMfvdgFhSjmG" - in - let* ctxt = - lift - @@ Sc_rollup_stake_storage.Internal_for_tests.deposit_stake - ctxt - rollup - staker - in - assert_fails_with - ~loc:__LOC__ - (Sc_rollup_stake_storage.Internal_for_tests.deposit_stake - ctxt - rollup - staker) - "Already staked." - let test_withdrawal_from_missing_rollup () = assert_fails_with_missing_rollup ~loc:__LOC__ (fun ctxt rollup -> Sc_rollup_stake_storage.Internal_for_tests.withdraw_stake @@ -2456,7 +2434,6 @@ let tests = "deposit to existing rollup" `Quick test_deposit_to_existing_rollup; - Tztest.tztest "can not deposit twice" `Quick test_can_not_stake_twice; Tztest.tztest "deposit, then withdraw" `Quick test_deposit_then_withdraw; Tztest.tztest "cement with zero stakers fails" -- GitLab