Skip to content

Improve fork config generation for develop. #14285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/lib/mina_graphql/dune
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
graphql_lib
block_time
currency
merkle_ledger
mina_lib
mina_commands
mina_state
Expand Down
105 changes: 53 additions & 52 deletions src/lib/mina_graphql/mina_graphql.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2293,15 +2293,6 @@ module Queries = struct
|> Runtime_config.to_yojson |> Yojson.Safe.to_basic )

let fork_config =
let rec map_results ~f = function
Copy link
Contributor

@ghost-not-in-the-shell ghost-not-in-the-shell Nov 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map_results ~f xs should be equivalent to List.map ~f xs |> Result.all. In case you find it tedius to define it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it's less efficient, as it iterates over the list twice, doesn't it?

| [] ->
Result.return []
| r :: rs ->
let open Result.Let_syntax in
let%bind r' = f r in
let%map rs = map_results ~f rs in
r' :: rs
in
field "fork_config"
~doc:
"The runtime configuration for a blockchain fork intended to be a \
Expand All @@ -2314,59 +2305,69 @@ module Queries = struct
`Assoc [ ("error", `String "Daemon is bootstrapping") ]
| `Active best_tip -> (
let block = Transition_frontier.Breadcrumb.(block best_tip) in
let blockchain_length = Mina_block.blockchain_length block in
let global_slot =
Mina_block.blockchain_length block |> Unsigned.UInt32.to_int
Mina_block.consensus_state block
|> Consensus.Data.Consensus_state.curr_global_slot
in
let accounts_or_error =
let staged_ledger =
Transition_frontier.Breadcrumb.staged_ledger best_tip
|> Staged_ledger.ledger
|> Ledger.foldi ~init:[] ~f:(fun _ accum act -> act :: accum)
|> map_results
~f:Runtime_config.Json_layout.Accounts.Single.of_account
in
let protocol_state =
Transition_frontier.Breadcrumb.protocol_state best_tip
in
match accounts_or_error with
let consensus =
Mina_state.Protocol_state.consensus_state protocol_state
in
let staking_epoch =
Consensus.Proof_of_stake.Data.Consensus_state.staking_epoch_data
consensus
in
let next_epoch =
Consensus.Proof_of_stake.Data.Consensus_state.next_epoch_data
consensus
in
let staking_epoch_seed =
Mina_base.Epoch_seed.to_base58_check
staking_epoch.Mina_base.Epoch_data.Poly.seed
in
let next_epoch_seed =
Mina_base.Epoch_seed.to_base58_check
next_epoch.Mina_base.Epoch_data.Poly.seed
in
let runtime_config = Mina_lib.runtime_config mina in
match
let open Result.Let_syntax in
let%bind staking_ledger =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a check that the ledger hash of the staking_ledger equal to the staking_epoch.ledger and also the ledger hash of the next_epoch_ledger equal to the next_epoch.ledger.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I obtain a hash from the ledger in order to compare it? I cannot find a function for this in the interface.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get that by calling Ledger.Any_ledger.M.merkle_root.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 27157c9.

match Mina_lib.staking_ledger mina with
| None ->
Error "Staking ledger is not initialized."
| Some (Genesis_epoch_ledger l) ->
Ok (Ledger.Any_ledger.cast (module Ledger) l)
| Some (Ledger_db l) ->
Ok (Ledger.Any_ledger.cast (module Ledger.Db) l)
in
let%bind next_epoch_ledger =
match Mina_lib.next_epoch_ledger mina with
| None ->
Error "Next epoch ledger is not initialized."
| Some `Notfinalized ->
Ok None
| Some (`Finalized (Genesis_epoch_ledger l)) ->
Ok (Some (Ledger.Any_ledger.cast (module Ledger) l))
| Some (`Finalized (Ledger_db l)) ->
Ok (Some (Ledger.Any_ledger.cast (module Ledger.Db) l))
in
Runtime_config.make_fork_config ~staged_ledger ~global_slot
~staking_ledger ~staking_epoch_seed ~next_epoch_ledger
~next_epoch_seed ~blockchain_length
~protocol_state_hash:protocol_state.previous_state_hash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use the previous_state_hash here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What else should I use? As far as I can see, the protocol_state only contains that and the genesis_state_hash within body, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be the current protocol_state_hash instead of the previous one. You can get the current state_hash by Transition_frontier.Breadcrumb.state_hash best tip.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 27157c9.

runtime_config
with
| Error e ->
`Assoc [ ("error", `String e) ]
| Ok accounts ->
let runtime_config = Mina_lib.runtime_config mina in
let ledger = Option.value_exn runtime_config.ledger in
let previous_length =
let open Option.Let_syntax in
let%bind proof = runtime_config.proof in
let%map fork = proof.fork in
fork.previous_length + global_slot
in
let fork =
Runtime_config.Fork_config.
{ previous_state_hash =
State_hash.to_base58_check
protocol_state.previous_state_hash
; previous_length =
Option.value ~default:global_slot previous_length
; genesis_slot = global_slot
}
in
let update =
Runtime_config.make
(* add_genesis_winner must be set to false, because this
config effectively creates a continuation of the current
blockchain state and therefore the genesis ledger already
contains the winner of the previous block. No need to
artificially add it. In fact, it wouldn't work at all,
because the new node would try to create this account at
startup, even though it already exists, leading to an error.*)
~ledger:
{ ledger with
base = Accounts accounts
; add_genesis_winner = Some false
}
~proof:(Runtime_config.Proof_keys.make ~fork ())
()
in
let new_config = Runtime_config.combine runtime_config update in
| Ok new_config ->
Runtime_config.to_yojson new_config |> Yojson.Safe.to_basic ) )

let thread_graph =
Expand Down
5 changes: 4 additions & 1 deletion src/lib/runtime_config/dune
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,19 @@
data_hash_lib
kimchi_backend.pasta
kimchi_backend.pasta.basic
merkle_ledger
mina_base
mina_base.import
mina_ledger
mina_numbers
signature_lib
snark_params
unsigned_extended
pickles
pickles.backend
pickles_types
with_hash
signature_lib
staged_ledger
)
(instrumentation (backend bisect_ppx))
(preprocess (pps ppx_custom_printf ppx_sexp_conv ppx_let ppx_deriving_yojson
Expand Down
Loading