diff --git a/src/proto_alpha/lib_protocol/sc_rollup_refutation_storage.ml b/src/proto_alpha/lib_protocol/sc_rollup_refutation_storage.ml index 59b9467c77acac929cdfd35a8183972d84709d82..ccf330c77a40ca2ccf3a2cc7531c7f327bd25e94 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_refutation_storage.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_refutation_storage.ml @@ -126,12 +126,15 @@ let get_conflict_point ctxt rollup staker1 staker2 = commit1_info.predecessor commit2_info.predecessor in - if Commitment_hash.(commit1 = commit2) then - (* This case will most dominantly happen when either commit is part of the other's history. - It occurs when the commit that is farther ahead gets dereferenced to its predecessor often - enough to land at the other commit. *) - fail Sc_rollup_no_conflict - else traverse_in_parallel ctxt commit1 commit2 + let* () = + fail_when + (* This case will most dominantly happen when either commit is part of the other's history. + It occurs when the commit that is farther ahead gets dereferenced to its predecessor often + enough to land at the other commit. *) + Commitment_hash.(commit1 = commit2) + Sc_rollup_no_conflict + in + traverse_in_parallel ctxt commit1 commit2 (** [get_or_init_game ctxt rollup refuter defender] returns the current game between the two stakers [refuter] and [defender] if it exists. @@ -205,10 +208,11 @@ let update_game ctxt rollup ~player ~opponent refutation = let* game, ctxt = get_or_init_game ctxt rollup ~refuter:player ~defender:opponent in - let* _ = - let turn = match game.turn with Alice -> alice | Bob -> bob in - if Sc_rollup_repr.Staker.equal turn player then return () - else fail Sc_rollup_wrong_turn + let* () = + fail_unless + (let turn = match game.turn with Alice -> alice | Bob -> bob in + Sc_rollup_repr.Staker.equal turn player) + Sc_rollup_wrong_turn in match Sc_rollup_game_repr.play game refutation with | Either.Left outcome -> return (Some outcome, ctxt) @@ -234,9 +238,12 @@ let timeout ctxt rollup stakers = let* ctxt, timeout_level = Store.Game_timeout.get (ctxt, rollup) (alice, bob) in - if Raw_level_repr.(level > timeout_level) then - return (Sc_rollup_game_repr.{loser = game.turn; reason = Timeout}, ctxt) - else fail Sc_rollup_timeout_level_not_reached + let* () = + fail_unless + Raw_level_repr.(level > timeout_level) + Sc_rollup_timeout_level_not_reached + in + return (Sc_rollup_game_repr.{loser = game.turn; reason = Timeout}, ctxt) (* TODO: #2926 this requires carbonation *) let apply_outcome ctxt rollup stakers (outcome : Sc_rollup_game_repr.outcome) = 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 8c4c19f27286e8d3816045f5d295b741eaccee56..45f8054adae1fb2b7cc92118de5e6e378276f0d3 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml @@ -68,15 +68,18 @@ let withdraw_stake ctxt rollup staker = match res with | None -> fail Sc_rollup_not_staked | Some staked_on_commitment -> - if Commitment_hash.(staked_on_commitment = lcc) then - (* TODO: https://gitlab.com/tezos/tezos/-/issues/2449 - We should refund stake here. - *) - let* ctxt, _size_freed = - Store.Stakers.remove_existing (ctxt, rollup) staker - in - modify_staker_count ctxt rollup Int32.pred - else fail Sc_rollup_not_staked_on_lcc + let* () = + fail_unless + Commitment_hash.(staked_on_commitment = lcc) + Sc_rollup_not_staked_on_lcc + in + (* TODO: https://gitlab.com/tezos/tezos/-/issues/2449 + We should refund stake here. + *) + let* ctxt, _size_freed = + Store.Stakers.remove_existing (ctxt, rollup) staker + in + modify_staker_count ctxt rollup Int32.pred let assert_commitment_not_too_far_ahead ctxt rollup lcc commitment = let open Lwt_tzresult_syntax in @@ -91,14 +94,16 @@ let assert_commitment_not_too_far_ahead ctxt rollup lcc commitment = return (ctxt, Commitment.(lcc.inbox_level)) in let max_level = Commitment.(commitment.inbox_level) in - if - let sc_rollup_max_lookahead = - Constants_storage.sc_rollup_max_lookahead_in_blocks ctxt - in - Compare.Int32.( - sc_rollup_max_lookahead < Raw_level_repr.diff max_level min_level) - then fail Sc_rollup_too_far_ahead - else return ctxt + let* () = + fail_when + (let sc_rollup_max_lookahead = + Constants_storage.sc_rollup_max_lookahead_in_blocks ctxt + in + Compare.Int32.( + sc_rollup_max_lookahead < Raw_level_repr.diff max_level min_level)) + Sc_rollup_too_far_ahead + in + return ctxt (** Enfore that a commitment's inbox level increases by an exact fixed amount over its predecessor. This property is used in several places - not obeying it causes severe breakage. @@ -136,11 +141,13 @@ let assert_commitment_period ctxt rollup commitment = let sc_rollup_commitment_period = Constants_storage.sc_rollup_commitment_period_in_blocks ctxt in - if - Raw_level_repr.( - commitment.inbox_level = add pred_level sc_rollup_commitment_period) - then return ctxt - else fail Sc_rollup_bad_inbox_level + let* () = + fail_unless + Raw_level_repr.( + commitment.inbox_level = add pred_level sc_rollup_commitment_period) + Sc_rollup_bad_inbox_level + in + return ctxt (** Check invariants on [inbox_level], enforcing overallocation of storage and regularity of block production. @@ -244,12 +251,13 @@ let refine_stake ctxt rollup staker commitment = assert (Compare.Int.(size_diff = 0 || size_diff = expected_size_diff)) ; return (new_hash, commitment_added_level, ctxt) (* See WARNING above. *)) - else if Commitment_hash.(node = lcc) then - (* We reached the LCC, but [staker] is not staked directly on it. - Thus, we backtracked. Note that everyone is staked indirectly on - the LCC. *) - fail Sc_rollup_staker_backtracked else + let* () = + (* We reached the LCC, but [staker] is not staked directly on it. + Thus, we backtracked. Note that everyone is staked indirectly on + the LCC. *) + fail_when Commitment_hash.(node = lcc) Sc_rollup_staker_backtracked + in let* pred, ctxt = Commitment_storage.get_predecessor_unsafe ctxt rollup node in @@ -290,43 +298,50 @@ let cement_commitment ctxt rollup new_lcc = in (* Get is safe, as [Stakers_size] is initialized on origination. *) let* ctxt, total_staker_count = Store.Staker_count.get ctxt rollup in - if Compare.Int32.(total_staker_count <= 0l) then fail Sc_rollup_no_stakers - else - let* new_lcc_commitment, ctxt = - Commitment_storage.get_commitment_unsafe ctxt rollup new_lcc - in - let* ctxt, new_lcc_added = - Store.Commitment_added.get (ctxt, rollup) new_lcc - in - if Commitment_hash.(new_lcc_commitment.predecessor <> old_lcc) then - fail Sc_rollup_parent_not_lcc - else - let* new_lcc_stake_count, ctxt = - get_commitment_stake_count ctxt rollup new_lcc - in - if Compare.Int32.(total_staker_count <> new_lcc_stake_count) then - fail Sc_rollup_disputed - else if - let level = (Raw_context.current_level ctxt).level in - Raw_level_repr.(level < add new_lcc_added refutation_deadline_blocks) - then fail Sc_rollup_too_recent - else - (* update LCC *) - let* ctxt, lcc_size_diff = - Store.Last_cemented_commitment.update ctxt rollup new_lcc - in - assert (Compare.Int.(lcc_size_diff = 0)) ; - (* At this point we know all stakers are implicitly staked - on the new LCC, and no one is directly staked on the old LCC. We - can safely deallocate the old LCC. - *) - let* ctxt = deallocate ctxt rollup old_lcc in - consume_n_messages - ctxt - rollup - (Int32.to_int - @@ Sc_rollup_repr.Number_of_messages.to_int32 - new_lcc_commitment.number_of_messages) + let* () = + fail_when Compare.Int32.(total_staker_count <= 0l) Sc_rollup_no_stakers + in + let* new_lcc_commitment, ctxt = + Commitment_storage.get_commitment_unsafe ctxt rollup new_lcc + in + let* ctxt, new_lcc_added = + Store.Commitment_added.get (ctxt, rollup) new_lcc + in + let* () = + fail_when + Commitment_hash.(new_lcc_commitment.predecessor <> old_lcc) + Sc_rollup_parent_not_lcc + in + let* new_lcc_stake_count, ctxt = + get_commitment_stake_count ctxt rollup new_lcc + in + let* () = + fail_when + Compare.Int32.(total_staker_count <> new_lcc_stake_count) + Sc_rollup_disputed + in + let* () = + fail_when + (let level = (Raw_context.current_level ctxt).level in + Raw_level_repr.(level < add new_lcc_added refutation_deadline_blocks)) + Sc_rollup_too_recent + in + (* update LCC *) + let* ctxt, lcc_size_diff = + Store.Last_cemented_commitment.update ctxt rollup new_lcc + in + assert (Compare.Int.(lcc_size_diff = 0)) ; + (* At this point we know all stakers are implicitly staked + on the new LCC, and no one is directly staked on the old LCC. We + can safely deallocate the old LCC. + *) + let* ctxt = deallocate ctxt rollup old_lcc in + consume_n_messages + ctxt + rollup + (Int32.to_int + @@ Sc_rollup_repr.Number_of_messages.to_int32 + new_lcc_commitment.number_of_messages) let remove_staker ctxt rollup staker = let open Lwt_tzresult_syntax in @@ -335,22 +350,23 @@ let remove_staker ctxt rollup staker = match res with | None -> fail Sc_rollup_not_staked | Some staked_on -> - if Commitment_hash.(staked_on = lcc) then fail Sc_rollup_remove_lcc - else - let* ctxt, _size_diff = - Store.Stakers.remove_existing (ctxt, rollup) staker - in - let* ctxt = modify_staker_count ctxt rollup Int32.pred in - let rec go node ctxt = - if Commitment_hash.(node = lcc) then return ctxt - else - let* pred, ctxt = - Commitment_storage.get_predecessor_unsafe ctxt rollup node - in - let* ctxt = decrease_commitment_stake_count ctxt rollup node in - (go [@ocaml.tailcall]) pred ctxt - in - go staked_on ctxt + let* () = + fail_when Commitment_hash.(staked_on = lcc) Sc_rollup_remove_lcc + in + let* ctxt, _size_diff = + Store.Stakers.remove_existing (ctxt, rollup) staker + in + let* ctxt = modify_staker_count ctxt rollup Int32.pred in + let rec go node ctxt = + if Commitment_hash.(node = lcc) then return ctxt + else + let* pred, ctxt = + Commitment_storage.get_predecessor_unsafe ctxt rollup node + in + let* ctxt = decrease_commitment_stake_count ctxt rollup node in + (go [@ocaml.tailcall]) pred ctxt + in + go staked_on ctxt module Internal_for_tests = struct let deposit_stake = deposit_stake diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_sc_rollup.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_sc_rollup.ml index 30f301f720a7f7d7e5fd9af6dc86f27d1efcd376..b04d3b36e5fb8df86d969387372274a7cc91f914 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_sc_rollup.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_sc_rollup.ml @@ -1,7 +1,7 @@ (*****************************************************************************) (* *) (* Open Source License *) -(* Copyright (c) 2021 Nomadic Labs *) +(* Copyright (c) 2022 Nomadic Labs *) (* *) (* Permission is hereby granted, free of charge, to any person obtaining a *) (* copy of this software and associated documentation files (the "Software"),*) @@ -43,14 +43,14 @@ let err x = Exn (Sc_rollup_test_error x) (** [context_init tup] initializes a context for testing in which the [sc_rollup_enable] constant is set to true. It returns the created context and contracts. *) -let context_init tup = +let context_init ?(sc_rollup_challenge_window_in_blocks = 10) tup = Context.init_with_constants_gen tup { Context.default_test_constants with sc_rollup_enable = true; consensus_threshold = 0; - sc_rollup_challenge_window_in_blocks = 10; + sc_rollup_challenge_window_in_blocks; } (** [test_disable_feature_flag ()] tries to originate a smart contract @@ -65,10 +65,7 @@ let test_disable_feature_flag () = | Environment.Ecoproto_error (Apply.Sc_rollup_feature_disabled as e) :: _ -> Assert.test_error_encodings e ; return_unit - | _ -> - failwith - "It should not be possible to send a smart contract rollup operation \ - when the feature flag is disabled." + | _ -> failwith "It should have failed with [Sc_rollup_feature_disabled]" in let*! _ = Incremental.add_operation ~expect_failure i op in return_unit @@ -105,8 +102,10 @@ let test_sc_rollups_all_well_defined () = all_names_are_valid () (** Initializes the context and originates a SCORU. *) -let init_and_originate tup = - let* ctxt, contracts = context_init tup in +let init_and_originate ?sc_rollup_challenge_window_in_blocks tup = + let* ctxt, contracts = + context_init ?sc_rollup_challenge_window_in_blocks tup + in let contract = Context.tup_hd tup contracts in let kind = Sc_rollup.Kind.Example_arith in let* operation, rollup = Op.sc_rollup_origination (B ctxt) contract kind "" in @@ -123,13 +122,6 @@ let number_of_ticks_exn n = | Some x -> x | None -> Stdlib.failwith "Bad Number_of_ticks" -let rec bake_until i top = - let level = Incremental.level i in - if level >= top then return i - else - Incremental.finalize_block i >>=? fun b -> - Incremental.begin_construction b >>=? fun i -> bake_until i top - let dummy_commitment ctxt rollup = let ctxt = Incremental.alpha_ctxt ctxt in let*! root_level = Sc_rollup.initial_level ctxt rollup in @@ -165,39 +157,16 @@ let test_publish_and_cement () = let* operation = Op.sc_rollup_publish (B ctxt) contract rollup c in let* i = Incremental.add_operation i operation in let* b = Incremental.finalize_block i in + let* constants = Context.get_constants (B b) in + let* b = + Block.bake_n constants.parametric.sc_rollup_challenge_window_in_blocks b + in let* i = Incremental.begin_construction b in - let* i = bake_until i 20l in let hash = Sc_rollup.Commitment.hash c in let* cement_op = Op.sc_rollup_cement (I i) contract rollup hash in let* _ = Incremental.add_operation i cement_op in return_unit -(** [test_cement_fails_if_premature] creates a rollup, publishes a - commitment and then tries to cement the commitment immediately - without waiting for the challenge period to elapse. We check that - this fails with the correct error. *) -let test_cement_fails_if_premature () = - let* ctxt, contracts, rollup = init_and_originate Context.T2 in - let _, contract = contracts in - let* i = Incremental.begin_construction ctxt in - let* c = dummy_commitment i rollup in - let* operation = Op.sc_rollup_publish (B ctxt) contract rollup c in - let* i = Incremental.add_operation i operation in - let* b = Incremental.finalize_block i in - let* i = Incremental.begin_construction b in - let hash = Sc_rollup.Commitment.hash c in - let* cement_op = Op.sc_rollup_cement (I i) contract rollup hash in - let expect_failure = function - | Environment.Ecoproto_error (Sc_rollup_errors.Sc_rollup_too_recent as e) - :: _ -> - Assert.test_error_encodings e ; - return_unit - | _ -> - failwith "It should not be possible to cement a commitment prematurely." - in - let* _ = Incremental.add_operation ~expect_failure i cement_op in - return_unit - (** [test_publish_fails_on_backtrack] creates a rollup and then publishes two different commitments with the same staker. We check that the second publish fails. *) @@ -220,7 +189,7 @@ let test_publish_fails_on_backtrack () = :: _ -> Assert.test_error_encodings e ; return_unit - | _ -> failwith "It should not be possible for a staker to backtrack." + | _ -> failwith "It should have failed with [Sc_rollup_staker_backtracked]" in let* _ = Incremental.add_operation ~expect_failure i operation2 in return_unit @@ -246,8 +215,11 @@ let test_cement_fails_on_conflict () = let* i = Incremental.begin_construction b in let* i = Incremental.add_operation i operation2 in let* b = Incremental.finalize_block i in + let* constants = Context.get_constants (B b) in + let* b = + Block.bake_n constants.parametric.sc_rollup_challenge_window_in_blocks b + in let* i = Incremental.begin_construction b in - let* i = bake_until i 20l in let hash = Sc_rollup.Commitment.hash commitment1 in let* cement_op = Op.sc_rollup_cement (I i) contract1 rollup hash in let expect_failure = function @@ -255,12 +227,61 @@ let test_cement_fails_on_conflict () = -> Assert.test_error_encodings e ; return_unit - | _ -> - failwith "It should not be possible to cement a contested commitment." + | _ -> failwith "It should have failed with [Sc_rollup_disputed]" in + let* _ = Incremental.add_operation ~expect_failure i cement_op in return_unit +let commit_and_cement_after_n_bloc ?expect_failure ctxt contract rollup n = + let* i = Incremental.begin_construction ctxt in + let* commitment = dummy_commitment i rollup in + let* operation = Op.sc_rollup_publish (B ctxt) contract rollup commitment in + let* i = Incremental.add_operation i operation in + let* b = Incremental.finalize_block i in + (* This pattern would add an additional block, so we decrement [n] by one. *) + let* b = Block.bake_n (n - 1) b in + let* i = Incremental.begin_construction b in + let hash = Sc_rollup.Commitment.hash commitment in + let* cement_op = Op.sc_rollup_cement (I i) contract rollup hash in + let* _ = Incremental.add_operation ?expect_failure i cement_op in + return_unit + +(** [test_challenge_window_period_boundaries] checks that cementing a commitment + without waiting for the whole challenge window period fails. Whereas, + succeeds when the period is over. *) +let test_challenge_window_period_boundaries () = + let sc_rollup_challenge_window_in_blocks = 10 in + let* ctxt, contract, rollup = + init_and_originate ~sc_rollup_challenge_window_in_blocks Context.T1 + in + (* Should fail because the waiting period is not strictly greater than the + challenge window period. *) + let* () = + let expect_failure = function + | Environment.Ecoproto_error (Sc_rollup_errors.Sc_rollup_too_recent as e) + :: _ -> + Assert.test_error_encodings e ; + return_unit + | _ -> failwith "It should have failed with [Sc_rollup_too_recent]" + in + commit_and_cement_after_n_bloc + ~expect_failure + ctxt + contract + rollup + (sc_rollup_challenge_window_in_blocks - 1) + in + (* Succeeds because the challenge period is over. *) + let* () = + commit_and_cement_after_n_bloc + ctxt + contract + rollup + sc_rollup_challenge_window_in_blocks + in + return_unit + let tests = [ Tztest.tztest @@ -275,10 +296,6 @@ let tests = "can publish a commit and then cement it" `Quick test_publish_and_cement; - Tztest.tztest - "cement will fail if it is too soon" - `Quick - test_cement_fails_if_premature; Tztest.tztest "publish will fail if staker is backtracking" `Quick @@ -287,4 +304,8 @@ let tests = "cement will fail if commitment is contested" `Quick test_cement_fails_on_conflict; + Tztest.tztest + "check the challenge window period boundaries" + `Quick + test_challenge_window_period_boundaries; ]