From 4e737efb3ce31c8d3c53e1941b40b79313e5688a Mon Sep 17 00:00:00 2001 From: Valentin Chaboche Date: Mon, 20 Jun 2022 17:45:51 +0200 Subject: [PATCH] Scoru,Proto: avoid redundant storage's lookups for [publish_commitment] --- .../lib_protocol/sc_rollup_stake_storage.ml | 26 +++++++++++++------ .../lib_protocol/sc_rollup_stake_storage.mli | 25 +++++++++++++----- .../test/unit/test_sc_rollup_storage.ml | 3 ++- .../Alpha- consecutive commitments.out | 24 ++++++++--------- 4 files changed, 50 insertions(+), 28 deletions(-) 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 b7a25fcae81d..bb090522fe5f 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml @@ -86,7 +86,7 @@ let deposit_stake ctxt rollup staker = in let* ctxt, _size = Store.Stakers.init (ctxt, rollup) staker lcc in let* ctxt = modify_staker_count ctxt rollup Int32.succ in - return (ctxt, balance_updates) + return (ctxt, balance_updates, lcc) let withdraw_stake ctxt rollup staker = let open Lwt_tzresult_syntax in @@ -251,10 +251,9 @@ let increase_commitment_stake_count ctxt rollup node = + 0 for Staker_count_update entry *) let commitment_storage_size_in_bytes = 85 -let refine_stake ctxt rollup staker commitment = +let refine_stake ctxt rollup staker staked_on commitment = let open Lwt_tzresult_syntax in let* lcc, ctxt = Commitment_storage.last_cemented_commitment ctxt rollup in - let* staked_on, ctxt = find_staker_unsafe ctxt rollup staker in let* ctxt = assert_refine_conditions_met ctxt rollup lcc commitment in let new_hash = Commitment.hash commitment in (* TODO: https://gitlab.com/tezos/tezos/-/issues/2559 @@ -310,12 +309,14 @@ 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.mem (ctxt, rollup) staker in - let* ctxt, balance_updates = - if res then return (ctxt, []) else deposit_stake ctxt rollup staker + let* ctxt, staked_on_opt = Store.Stakers.find (ctxt, rollup) staker in + let* ctxt, balance_updates, staked_on = + match staked_on_opt with + | Some staked_on -> return (ctxt, [], staked_on) + | None -> deposit_stake ctxt rollup staker in let+ commitment_hash, ctxt, level = - refine_stake ctxt rollup staker commitment + refine_stake ctxt rollup staker staked_on commitment in (commitment_hash, ctxt, level, balance_updates) @@ -424,5 +425,14 @@ let remove_staker ctxt rollup staker = module Internal_for_tests = struct let deposit_stake = deposit_stake - let refine_stake = refine_stake + let refine_stake ctxt rollup staker ?staked_on commitment = + let open Lwt_tzresult_syntax in + match staked_on with + | Some staked_on -> refine_stake ctxt rollup staker staked_on commitment + | None -> + (* This allows to call {!refine_stake} without explicitely passing the + staked_on parameter, it's more convenient for tests. However, + it still enforce that {!deposit_stake} was called before. *) + let* _ctxt, staked_on = Store.Stakers.get (ctxt, rollup) staker in + refine_stake ctxt rollup staker staked_on commitment end 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 11a3f756fa3d..b5481e05849a 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.mli @@ -167,7 +167,8 @@ val withdraw_stake : module Internal_for_tests : sig (** [deposit_stake context rollup staker] stakes [staker] at the last cemented commitment, freezing [sc_rollup_stake_amount] from [staker]'s - account balance. + account balance. It also returns the last cemented commitment of the + [rollup] on which the staker just deposited. Warning: must be called only if [rollup] exists and [staker] is not to be found in {!Store.Stakers.} @@ -186,13 +187,22 @@ module Internal_for_tests : sig Raw_context.t -> Sc_rollup_repr.t -> Sc_rollup_repr.Staker.t -> - (Raw_context.t * Receipt_repr.balance_updates) tzresult Lwt.t + (Raw_context.t + * Receipt_repr.balance_updates + * Sc_rollup_commitment_repr.Hash.t) + tzresult + Lwt.t + + (** [refine_stake context rollup staker ?staked_on commitment] moves the stake + of [staker] on [?staked_on] to [commitment]. The function exposed + in [Internal_for_tests] allows [staked_on] to be [None] and fetches + the real value from the storage, but, the production code uses the + already existing commitment on which the staker is staked. - (** [refine_stake context rollup staker commitment] moves the stake of - [staker] to [commitment]. Because we do not assume any form of coordination - between validators, we do not distinguish between {i adding new} - commitments and {i staking on existing commitments}. The storage of - commitments is content-addressable to minimize storage duplication. + Because we do not assume any form of coordination between validators, we + do not distinguish between {i adding new} commitments and {i staking on + existing commitments}. The storage of commitments is content-addressable + to minimize storage duplication. Subsequent calls to [refine_stake] and [cement_commitment] must use a [context] with greater level, or this function call will fail. @@ -223,6 +233,7 @@ module Internal_for_tests : sig Raw_context.t -> Sc_rollup_repr.t -> Sc_rollup_repr.Staker.t -> + ?staked_on:Sc_rollup_commitment_repr.Hash.t -> Sc_rollup_commitment_repr.t -> (Sc_rollup_commitment_repr.Hash.t * Raw_level_repr.t * Raw_context.t) tzresult 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 165e49f55f23..41ccd2886dc3 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 @@ -140,7 +140,7 @@ let deposit_stake_and_check_balances ctxt rollup staker = rollup staker (fun ctxt rollup staker_contract stake -> - let* ctxt', _ = + let* ctxt', _, _ = lift @@ Sc_rollup_stake_storage.Internal_for_tests.deposit_stake ctxt @@ -2142,6 +2142,7 @@ let test_refine_stake_of_missing_rollup () = ctxt rollup Sc_rollup_repr.Staker.zero + ~staked_on:Sc_rollup_commitment_repr.Hash.zero Commitment_repr. { predecessor = Commitment_repr.Hash.zero; diff --git a/tezt/tests/expected/sc_rollup.ml/Alpha- consecutive commitments.out b/tezt/tests/expected/sc_rollup.ml/Alpha- consecutive commitments.out index 1d5e61040091..a3e393cae8cb 100644 --- a/tezt/tests/expected/sc_rollup.ml/Alpha- consecutive commitments.out +++ b/tezt/tests/expected/sc_rollup.ml/Alpha- consecutive commitments.out @@ -32,7 +32,7 @@ This sequence of operations was run: ./tezos-client --wait none publish commitment from '[PUBLIC_KEY_HASH]' for sc rollup '[SC_ROLLUP_HASH]' with compressed state scs11VNjWyZw4Tgbvsom8epQbox86S2CKkE1UAZkXMM7Pj8MQMLzMf at inbox level 32 and predecessor '[SC_ROLLUP_COMMITMENT_HASH]' and number of messages 0 and number of ticks 1 Node is bootstrapped. -Estimated gas: 6251.140 units (will add 100 for safety) +Estimated gas: 5781.076 units (will add 100 for safety) Estimated storage: no bytes added Operation successfully injected in the node. Operation hash is '[OPERATION_HASH]' @@ -43,13 +43,13 @@ and/or an external block explorer to make sure that it has been included. This sequence of operations was run: Manager signed operations: From: [PUBLIC_KEY_HASH] - Fee to the baker: ꜩ0.000977 + Fee to the baker: ꜩ0.00093 Expected counter: 2 - Gas limit: 6352 + Gas limit: 5882 Storage limit: 0 bytes Balance updates: - [PUBLIC_KEY_HASH] ... -ꜩ0.000977 - payload fees(the block proposer) ....... +ꜩ0.000977 + [PUBLIC_KEY_HASH] ... -ꜩ0.00093 + payload fees(the block proposer) ....... +ꜩ0.00093 Publish commitment SCORU Commitment: compressed_state: scs11VNjWyZw4Tgbvsom8epQbox86S2CKkE1UAZkXMM7Pj8MQMLzMf inbox_level: 32 @@ -57,7 +57,7 @@ This sequence of operations was run: number_of_messages: 0 number_of_ticks: 1 in the smart contract rollup at address [SC_ROLLUP_HASH] This smart contract rollup commitment publishing was successfully applied - Consumed gas: 6251.140 + Consumed gas: 5781.076 Hash of commit: [SC_ROLLUP_COMMITMENT_HASH] Commitment published at level: 3 Balance updates: @@ -67,7 +67,7 @@ This sequence of operations was run: ./tezos-client --wait none publish commitment from '[PUBLIC_KEY_HASH]' for sc rollup '[SC_ROLLUP_HASH]' with compressed state scs11VNjWyZw4Tgbvsom8epQbox86S2CKkE1UAZkXMM7Pj8MQMLzMf at inbox level 62 and predecessor '[SC_ROLLUP_COMMITMENT_HASH]' and number of messages 0 and number of ticks 1 Node is bootstrapped. -Estimated gas: 4479.904 units (will add 100 for safety) +Estimated gas: 4244.904 units (will add 100 for safety) Estimated storage: no bytes added Operation successfully injected in the node. Operation hash is '[OPERATION_HASH]' @@ -78,13 +78,13 @@ and/or an external block explorer to make sure that it has been included. This sequence of operations was run: Manager signed operations: From: [PUBLIC_KEY_HASH] - Fee to the baker: ꜩ0.000799 + Fee to the baker: ꜩ0.000776 Expected counter: 3 - Gas limit: 4580 + Gas limit: 4345 Storage limit: 0 bytes Balance updates: - [PUBLIC_KEY_HASH] ... -ꜩ0.000799 - payload fees(the block proposer) ....... +ꜩ0.000799 + [PUBLIC_KEY_HASH] ... -ꜩ0.000776 + payload fees(the block proposer) ....... +ꜩ0.000776 Publish commitment SCORU Commitment: compressed_state: scs11VNjWyZw4Tgbvsom8epQbox86S2CKkE1UAZkXMM7Pj8MQMLzMf inbox_level: 62 @@ -92,7 +92,7 @@ This sequence of operations was run: number_of_messages: 0 number_of_ticks: 1 in the smart contract rollup at address [SC_ROLLUP_HASH] This smart contract rollup commitment publishing was successfully applied - Consumed gas: 4479.904 + Consumed gas: 4244.904 Hash of commit: [SC_ROLLUP_COMMITMENT_HASH] Commitment published at level: 4 -- GitLab