Improve fork config generation for develop. #14285

Sventimir · 2023-10-06T06:51:34Z

Explain your changes:

Make it possible to merge Improve fork config generation for berkeley #14270 into berkeley.

Explain how you tested your changes:

CI.

Checklist:

Dependency versions are unchanged
- Notify Velocity team if dependencies must change in CI
Modified the current draft of release notes with details on what is completed or incomplete within this project
Document code purpose, how to use it
- Mention expected invariants, implicit constraints
Tests were added for the new behavior
- Document test purpose, significance of failures
- Test names should reflect their purpose
All tests pass (CI will check this if you didn't)
Serialized types are in stable-versioned modules
Does this close issues? List them

Closes [RFC] Export genesis ledger for hard fork. MinaFoundation/mina#95

TEMP

when generating a fork config.

Sventimir · 2023-10-09T07:39:39Z

!ci-build-me

Sventimir · 2023-10-09T07:56:11Z

!ci-build-me

Sventimir · 2023-10-11T06:40:36Z

!ci-build-me

…x-fork-config-generation-berkeley

Sventimir · 2023-10-12T14:50:42Z

!ci-build-me

…ntimir/fix-fork-generation-develop

Sventimir · 2023-10-13T08:26:19Z

!ci-build-me

Sventimir · 2023-10-18T12:53:06Z

!ci-build-me

Sventimir · 2023-10-31T13:34:00Z

!ci-build-me

Sventimir · 2023-11-06T08:29:42Z

!ci-build-me

snaitmouloud-mf · 2023-11-13T10:04:29Z

@bkase any chance to review this PR ?

bkase

What happens when you hit this graphql endpoint for a node synced to mainnet? Does it hang the node for X seconds? What is X? Something we should keep in mind operationally, though I think we can survive even if it's really slow since only one (or a very small number of) nodes needs to run this

bkase · 2023-11-27T16:43:11Z

src/lib/runtime_config/runtime_config.ml

@@ -1,9 +1,21 @@
 open Core_kernel

 module Fork_config = struct
+  (* previous_global_slot here is a global slot since genesis. previous


comment bitrot a bit (previous_global_slot renamed)

can you inline these comments for each field, that will make it easier to notice when they change?

Done in a681c0f.

Sventimir · 2023-11-28T09:31:28Z

!ci-build-me

Sventimir · 2023-11-28T17:54:42Z

!ci-build-me

ghost-not-in-the-shell · 2023-11-28T18:09:48Z

src/lib/mina_graphql/mina_graphql.ml

@@ -2293,15 +2293,6 @@ module Queries = struct
        |> Runtime_config.to_yojson |> Yojson.Safe.to_basic )

  let fork_config =
-    let rec map_results ~f = function


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

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

ghost-not-in-the-shell · 2023-11-28T18:20:21Z

src/lib/mina_graphql/mina_graphql.ml

+            let runtime_config = Mina_lib.runtime_config mina in
+            match
+              let open Result.Let_syntax in
+              let%bind staking_ledger =


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.

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

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

Done in 27157c9.

ghost-not-in-the-shell · 2023-11-28T18:29:59Z

src/lib/mina_graphql/mina_graphql.ml

+              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


Why use the previous_state_hash here?

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?

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.

Done in 27157c9.

ghost-not-in-the-shell · 2023-11-28T18:32:05Z

Can you expanded a little more on how you tested this PR?

Sventimir · 2023-11-29T09:28:30Z

@ghost-not-in-the-shell , there's an integration test for this in review: #14342. Once it's merged, we can add more checks to it to test this change.

For the time being I tested manually by extracting the runtime config from a running node and started a new network from it. In particular I did synchronise with mainnet, extracted the config and started a berkeley' sandbox network with the state, I had to add an account that I control to the ledger and delegate from all accounts to that one so that I could produce blocks.

ghost-not-in-the-shell · 2023-12-01T22:26:09Z

src/lib/runtime_config/runtime_config.ml

+  }
+
+let ledger_accounts (ledger : Mina_ledger.Ledger.Any_ledger.witness) =
+  Mina_ledger.Ledger.Any_ledger.M.foldi ~init:[]


You are just using the foldi here to return a list of accounts. Then why not use the to_list_sequential function?

Indeed, see 27157c9.

stevenplatt · 2023-12-05T21:42:31Z

!ci-build-me

stevenplatt · 2023-12-06T18:00:21Z

!ci-build-me

nholland94 · 2024-01-02T18:57:14Z

I'm going to revert this PR as the compatible version was not reviewed yet. The correct order is to have the original PR reviewed first so that changes can be addressed there, before we land any conflict resolving PRs into other branches. The review comments made on this PR should be addressed on the original PR instead, and the review should be finalized there first. Typically, I don't recommend opening merge conflict branches until the first PR is approved in order to avoid this confusion (you will just end up redoing the merge PRs if you have to make changes to the originating PR during review anyway).

…k-generation-develop" This reverts commit ab24cde, reversing changes made to 79c6253.

Sventimir added 4 commits October 4, 2023 15:24

[mina_graphql] Extract fork config generation to runtime_config lib.

3e71a63

TEMP

[runtime_config] Add a Quickcheck generator for the config.

c425c05

TEMP

[runtime_config] Distinguish global slot and blockchain length.

05946c1

when generating a fork config.

[runtime_config] Include epoch data in the generated fork config.

a8c6f2b

Sventimir requested review from a team as code owners October 6, 2023 06:51

Sventimir self-assigned this Oct 6, 2023

Sventimir added hard fork MinaFoundation labels Oct 6, 2023

Merge branch 'sventimir/fix-fork-config-generation' into sventimir/fi…

eff2f2a

…x-fork-config-generation-berkeley

Sventimir force-pushed the sventimir/fix-fork-generation-develop branch from c932a4b to 2f3f5ab Compare October 12, 2023 14:50

Merge branch 'sventimir/fix-fork-config-generation-berkeley' into sve…

984d5fa

…ntimir/fix-fork-generation-develop

Sventimir force-pushed the sventimir/fix-fork-generation-develop branch from 2f3f5ab to 984d5fa Compare October 13, 2023 08:17

Sventimir added 2 commits October 13, 2023 10:54

Merge branch 'develop' into sventimir/fix-fork-generation-develop

9a874bc

Merge branch 'develop' into sventimir/fix-fork-generation-develop

a75ed96

Merge branch 'develop' into sventimir/fix-fork-generation-develop

4c2f269

Merge branch 'develop' into sventimir/fix-fork-generation-develop

6aa7a7c

bkase reviewed Nov 27, 2023

View reviewed changes

[runtime_config] Inline explanations on Fork_config.t fields.

c83dc68

Sventimir force-pushed the sventimir/fix-fork-generation-develop branch from a681c0f to c83dc68 Compare November 28, 2023 17:54

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

View reviewed changes

ghost-not-in-the-shell reviewed Dec 1, 2023

View reviewed changes

stevenplatt requested review from bkase and removed request for bkase December 4, 2023 15:28

Sventimir and others added 3 commits December 5, 2023 11:13

Merge branch 'develop' into sventimir/fix-fork-generation-develop

ef58153

Apply reviewer's suggestions.

27157c9

Merge branch 'develop' into sventimir/fix-fork-generation-develop

2f94025

ghost-not-in-the-shell approved these changes Dec 6, 2023

View reviewed changes

Merge branch 'develop' into sventimir/fix-fork-generation-develop

bc26d6d

bkase approved these changes Dec 7, 2023

View reviewed changes

Sventimir merged commit ab24cde into develop Dec 8, 2023
1 check passed

Sventimir deleted the sventimir/fix-fork-generation-develop branch December 8, 2023 07:55

nholland94 added a commit that referenced this pull request Jan 2, 2024

Revert "Merge pull request #14285 from MinaProtocol/sventimir/fix-for…

afae4c9

…k-generation-develop" This reverts commit ab24cde, reversing changes made to 79c6253.

This was referenced Jan 2, 2024

Improve fork config generation for berkeley #14270

Closed

Improve fork config generation #14266

Merged

mitschabaude mentioned this pull request Apr 10, 2024

Merge develop into o1js-main #15500

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve fork config generation for develop. #14285

Improve fork config generation for develop. #14285

Sventimir commented Oct 6, 2023

Sventimir commented Oct 9, 2023

Sventimir commented Oct 9, 2023

Sventimir commented Oct 11, 2023

Sventimir commented Oct 12, 2023

Sventimir commented Oct 13, 2023

Sventimir commented Oct 18, 2023

Sventimir commented Oct 31, 2023

Sventimir commented Nov 6, 2023

snaitmouloud-mf commented Nov 13, 2023

bkase left a comment

bkase Nov 27, 2023

bkase Nov 27, 2023

Sventimir Nov 28, 2023

Sventimir commented Nov 28, 2023

Sventimir commented Nov 28, 2023

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

edited

Loading

Sventimir Nov 29, 2023

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

Sventimir Nov 29, 2023

ghost-not-in-the-shell Dec 1, 2023

Sventimir Dec 5, 2023

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

Sventimir Nov 29, 2023

ghost-not-in-the-shell Dec 1, 2023

Sventimir Dec 5, 2023

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

Sventimir commented Nov 29, 2023

ghost-not-in-the-shell Dec 1, 2023

Sventimir Dec 5, 2023

stevenplatt commented Dec 5, 2023

stevenplatt commented Dec 6, 2023

nholland94 commented Jan 2, 2024

Improve fork config generation for develop. #14285

Improve fork config generation for develop. #14285

Conversation

Sventimir commented Oct 6, 2023

Sventimir commented Oct 9, 2023

Sventimir commented Oct 9, 2023

Sventimir commented Oct 11, 2023

Sventimir commented Oct 12, 2023

Sventimir commented Oct 13, 2023

Sventimir commented Oct 18, 2023

Sventimir commented Oct 31, 2023

Sventimir commented Nov 6, 2023

snaitmouloud-mf commented Nov 13, 2023

bkase left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sventimir commented Nov 28, 2023

Sventimir commented Nov 28, 2023

ghost-not-in-the-shell Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Sventimir commented Nov 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenplatt commented Dec 5, 2023

stevenplatt commented Dec 6, 2023

nholland94 commented Jan 2, 2024

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

edited

Loading