Skip to content
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

PR #16186, split for re-review #16502

Open
wants to merge 25 commits into
base: georgeee/redo-16186-for-better-review-base
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
37fde44
Move transaction_capacity_log_2 to Runtime_config
georgeee Jan 22, 2025
33ce3c3
Introduce Runtime_config.Constants
georgeee Jan 22, 2025
71bd217
Rename Genesis_ledger_helper.init_from_config_file
georgeee Jan 22, 2025
e053563
Split out legacy part from inputs_from_config_file
georgeee Jan 22, 2025
788ab19
Add compile_config to Genesis_proof.Inputs
georgeee Jan 22, 2025
37e3584
Add config_files flag to Cli_lib
georgeee Jan 22, 2025
f070475
Remove default_transaction_fee from node_config
georgeee Jan 22, 2025
ffbfe45
Implement init_from_config_file (non-legacy)
georgeee Jan 22, 2025
b2eb3df
Use init_from_config_file in mina_lib tests
georgeee Jan 22, 2025
34d7a98
Use init_from_config_file in archive
georgeee Jan 22, 2025
d5a56e3
Accept optional --config-file in standalone SW
georgeee Jan 22, 2025
7860cdf
Accept optional --config-file in 'internal snark-worker'
georgeee Jan 22, 2025
3062393
Use load_constants in snarky_tests
georgeee Jan 22, 2025
eaba812
Accept optional --config-file in vrf cli commands
georgeee Jan 22, 2025
09bd9bd
Use load_constants in delegation_verify app
georgeee Jan 22, 2025
2249d75
Accept --config-file in hash ledger and two other
georgeee Jan 22, 2025
10df606
Accept optional --config-file for compile-time-constants
georgeee Jan 22, 2025
39b7c74
Accept optional --config-file in constraint_system_digests
georgeee Jan 22, 2025
eefcc50
Accept optional --config-file in 'internal run-prover'
georgeee Jan 22, 2025
bb8ebe1
Accept optional --config-file in 'internal run-verifier', 'internal r…
georgeee Jan 22, 2025
c1d7fbd
Move load_config_files to Genesis_ledger_helpers
georgeee Jan 22, 2025
b47cf86
Remove default_snark_worker_fee usage
georgeee Jan 22, 2025
cb395b2
Use non-legacy inputs_from_config_file in load_config_files
georgeee Jan 22, 2025
b98f42f
Remove legacy functions from Config_loader
georgeee Jan 22, 2025
60b5ab3
Remainder of changes from PR #16186
georgeee Jan 22, 2025
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
65 changes: 21 additions & 44 deletions src/app/cli/src/cli_entrypoint/mina_cli_entrypoint.ml
Original file line number Diff line number Diff line change
Expand Up @@ -640,23 +640,15 @@ let setup_daemon logger ~itn_features =
in
let pids = Child_processes.Termination.create_pid_table () in
let mina_initialization_deferred () =
let genesis_constants =
Genesis_constants.Compiled.genesis_constants
in
let constraint_constants =
Genesis_constants.Compiled.constraint_constants
in
let compile_config = Mina_compile_config.Compiled.t in
let%bind precomputed_values, config =
Genesis_ledger_helper.Config_loader.load_config_files ~logger
~conf_dir ~genesis_dir
~proof_level:Genesis_constants.Compiled.proof_level config_files
~genesis_constants ~constraint_constants ~cli_proof_level
~conf_dir ?genesis_dir ?cli_proof_level ~itn_features config_files
|> Deferred.Or_error.ok_exn
in

constraint_constants.block_window_duration_ms |> Float.of_int
|> Time.Span.of_ms |> Mina_metrics.initialize_all ;
let constraint_constants = precomputed_values.consensus_constants in
let compile_config = precomputed_values.compile_config in
constraint_constants.block_window_duration_ms |> Block_time.Span.to_ms
|> Float.of_int64 |> Time.Span.of_ms |> Mina_metrics.initialize_all ;

let module DC = Runtime_config.Daemon in
(* The explicit typing here is necessary to prevent type inference from specializing according
Expand Down Expand Up @@ -1332,11 +1324,9 @@ Pass one of -peer, -peer-list-file, -seed, -peer-list-url.|} ;
let () = Mina_plugins.init_plugins ~logger mina plugins in
return mina )

let daemon logger =
let compile_config = Mina_compile_config.Compiled.t in
let daemon logger ~itn_features =
Command.async ~summary:"Mina daemon"
(Command.Param.map
(setup_daemon logger ~itn_features:compile_config.itn_features)
(Command.Param.map (setup_daemon logger ~itn_features)
~f:(fun setup_daemon () ->
(* Immediately disable updating the time offset. *)
Block_time.Controller.disable_setting_offset () ;
Expand All @@ -1345,7 +1335,7 @@ let daemon logger =
[%log info] "Daemon ready. Clients can now connect" ;
Async.never () ) )

let replay_blocks logger =
let replay_blocks ~itn_features logger =
let replay_flag =
let open Command.Param in
flag "--blocks-filename" ~aliases:[ "-blocks-filename" ] (required string)
Expand All @@ -1356,10 +1346,9 @@ let replay_blocks logger =
flag "--format" ~aliases:[ "-format" ] (optional string)
~doc:"json|sexp The format to read lines of the file in (default: json)"
in
let compile_config = Mina_compile_config.Compiled.t in
Command.async ~summary:"Start mina daemon with blocks replayed from a file"
(Command.Param.map3 replay_flag read_kind
(setup_daemon logger ~itn_features:compile_config.itn_features)
(setup_daemon logger ~itn_features)
~f:(fun blocks_filename read_kind setup_daemon () ->
(* Enable updating the time offset. *)
Block_time.Controller.enable_setting_offset () ;
Expand Down Expand Up @@ -1551,7 +1540,7 @@ let snark_hashes =
let json = Cli_lib.Flag.json in
fun () -> if json then Core.printf "[]\n%!"]

let internal_commands logger =
let internal_commands ~itn_features logger =
[ ( Snark_worker.Intf.command_name
, Snark_worker.command ~commit_id:Mina_version.commit_id )
; ("snark-hashes", snark_hashes)
Expand Down Expand Up @@ -1591,6 +1580,7 @@ let internal_commands logger =
flag "--file" (required string)
~doc:"File containing the s-expression of the snark work to execute"
and config_file = Cli_lib.Flag.config_files in

fun () ->
let open Deferred.Let_syntax in
let logger = Logger.create () in
Expand Down Expand Up @@ -1793,18 +1783,12 @@ let internal_commands logger =
() ) ;
Deferred.return ()) )
; ("dump-type-shapes", dump_type_shapes)
; ("replay-blocks", replay_blocks logger)
; ("replay-blocks", replay_blocks ~itn_features logger)
; ("audit-type-shapes", audit_type_shapes)
; ( "test-genesis-block-generation"
, Command.async ~summary:"Generate a genesis proof"
(let open Command.Let_syntax in
let%map_open config_files =
flag "--config-file" ~aliases:[ "config-file" ]
~doc:
"PATH path to a configuration file (overrides MINA_CONFIG_FILE, \
default: <config_dir>/daemon.json). Pass multiple times to \
override fields from earlier config files"
(listed string)
let%map_open config_file = Cli_lib.Flag.config_files
and conf_dir = Cli_lib.Flag.conf_dir
and genesis_dir =
flag "--genesis-ledger-dir" ~aliases:[ "genesis-ledger-dir" ]
Expand All @@ -1818,17 +1802,10 @@ let internal_commands logger =
Parallel.init_master () ;
let logger = Logger.create () in
let conf_dir = Mina_lib.Conf_dir.compute_conf_dir conf_dir in
let genesis_constants =
Genesis_constants.Compiled.genesis_constants
in
let constraint_constants =
Genesis_constants.Compiled.constraint_constants
in
let proof_level = Genesis_constants.Proof_level.Full in
let%bind precomputed_values, _ =
Genesis_ledger_helper.Config_loader.load_config_files ~logger
~conf_dir ~genesis_dir ~genesis_constants ~constraint_constants
~proof_level config_files ~cli_proof_level:None
~conf_dir ?genesis_dir ~cli_proof_level:Full ~itn_features
Copy link
Member Author

Choose a reason for hiding this comment

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

cli_proof_level is used here so that whatever is provided in compile-time config and initialized via load_config_files is overridden with Full.

config_file
|> Deferred.Or_error.ok_exn
in
let pids = Child_processes.Termination.create_pid_table () in
Expand All @@ -1837,7 +1814,7 @@ let internal_commands logger =
realistic test.
*)
Prover.create ~commit_id:Mina_version.commit_id ~logger ~pids
~conf_dir ~proof_level
~conf_dir ~proof_level:precomputed_values.proof_level
~constraint_constants:precomputed_values.constraint_constants ()
in
match%bind
Expand All @@ -1857,13 +1834,14 @@ let internal_commands logger =

let mina_commands logger ~itn_features =
[ ("accounts", Client.accounts)
; ("daemon", daemon logger)
; ("daemon", daemon ~itn_features logger)
; ("client", Client.client)
; ("advanced", Client.advanced ~itn_features)
; ("ledger", Client.ledger)
; ("libp2p", Client.libp2p)
; ( "internal"
, Command.group ~summary:"Internal commands" (internal_commands logger) )
, Command.group ~summary:"Internal commands"
Copy link
Member Author

Choose a reason for hiding this comment

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

Commit cb395b2 also propagates ~itn_features down daemon and internal_commands -> replay_blocks so that it's read from compile time config in the only place in mina_cli_entrypoint

(internal_commands ~itn_features logger) )
; (Parallel.worker_command_name, Parallel.worker_command)
; ("transaction-snark-profiler", Transaction_snark_profiler.command)
]
Expand Down Expand Up @@ -1901,11 +1879,10 @@ let () =
| [| _mina_exe; version |] when is_version_cmd version ->
Mina_version.print_version ()
| _ ->
let compile_config = Mina_compile_config.Compiled.t in
let itn_features = Mina_compile_config.Compiled.t.itn_features in
Command.run
(Command.group ~summary:"Mina" ~preserve_subcommand_order:()
(mina_commands logger ~itn_features:compile_config.itn_features) )
) ;
(mina_commands logger ~itn_features) ) ) ;
Core.exit 0

let linkme = ()
32 changes: 17 additions & 15 deletions src/lib/genesis_ledger_helper/genesis_ledger_helper.ml
Original file line number Diff line number Diff line change
Expand Up @@ -783,16 +783,14 @@ module type Config_loader_intf = sig
-> (Precomputed_values.t * Runtime_config.t) Deferred.Or_error.t

val load_config_files :
logger:Logger.t
-> genesis_constants:Genesis_constants.t
-> constraint_constants:Genesis_constants.Constraint_constants.t
-> conf_dir:string
-> genesis_dir:string option
-> cli_proof_level:Genesis_constants.Proof_level.t option
-> proof_level:Genesis_constants.Proof_level.t
?overwrite_version:Unsigned.uint32
-> ?genesis_dir:string
-> ?itn_features:bool
-> ?cli_proof_level:Genesis_constants.Proof_level.t
-> ?conf_dir:string
-> logger:Logger.t
-> string list
-> (Precomputed_values.t * Runtime_config.t)
Async_kernel.Deferred.Or_error.t
-> (Precomputed_values.t * Runtime_config.t) Or_error.t Deferred.t
end

module Config_loader : Config_loader_intf = struct
Expand Down Expand Up @@ -933,20 +931,24 @@ module Config_loader : Config_loader_intf = struct
let values = Genesis_proof.create_values_no_proof inputs in
(values, config)

let load_config_files ~logger ~genesis_constants ~constraint_constants
~conf_dir ~genesis_dir ~cli_proof_level ~proof_level
(config_files : string list) =
let load_config_files ?overwrite_version ?genesis_dir ?(itn_features = false)
?cli_proof_level ?conf_dir ~logger (config_files : string list) =
let open Deferred.Or_error.Let_syntax in
let genesis_dir =
let%map.Option conf_dir = conf_dir in
Option.value ~default:(conf_dir ^/ "genesis") genesis_dir
in
let%bind.Deferred constants =
Runtime_config.Constants.load_constants ?conf_dir ?cli_proof_level
Copy link
Member Author

Choose a reason for hiding this comment

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

Note Json_loader.load_config_files is called twice: within load_constants and via a direct call below.

This results in Reading configuration files being printed twice when the node starts. Worth fixing in compatible.

Copy link
Member

Choose a reason for hiding this comment

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

Not only did this changeset break commands, but it also added output at the start of the program that has embarrassing duplication? Fantastic.

Copy link
Member

Choose a reason for hiding this comment

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

I would like this to be fixed before the release.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is already fixed in compatible

~itn_features ~logger config_files
in
let%bind config =
Runtime_config.Json_loader.load_config_files ~conf_dir ~logger
Runtime_config.Json_loader.load_config_files ?conf_dir ~logger
Copy link
Member Author

Choose a reason for hiding this comment

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

Logging behavior doesn't change comparing to previous version, which was present in mina_cli_entrypoint

config_files
in
match%bind.Deferred
init_from_config_file_legacy ~cli_proof_level ~genesis_dir ~logger
~genesis_constants ~constraint_constants ~proof_level config
init_from_config_file ?overwrite_version ?genesis_dir ~logger ~constants
config
with
| Ok a ->
return a
Expand Down