Skip to content

Stop using Genesis.Config#4313

Open
pmikolajczyk41 wants to merge 8 commits intomasterfrom
pmikolajczyk/nit-4412-migrate
Open

Stop using Genesis.Config#4313
pmikolajczyk41 wants to merge 8 commits intomasterfrom
pmikolajczyk/nit-4412-migrate

Conversation

@pmikolajczyk41
Copy link
Member

@pmikolajczyk41 pmikolajczyk41 commented Feb 3, 2026

The Genesis.Config field was used in nitro (excluding go-ethereum) in two places:

  1. genesis-generator tool - just checking that the field is empty and printing an error if not; this PR leaves this usage to ensure genesis.json doesn't use old format. However, this could be removed if we decide to remove the field instead of deprecating it.
  2. nitro init - this PR changes the logic to use the new getter that relies on the serialized format.

Reasoning for not migrating whole geth codebase and not removing the Config field

There are essentially two paths:

  1. mark Genesis.Config as deprecated - this should prevent us from using it again in nitro, but there's a follow up issue - what about geth code?
    a. leave Geth as it is - this leaves us with a small probability, that there is some utility in geth relying on the Config field, that we might some day use
    b. migrate Geth to use the SerializedChainConfig
  2. remove Genesis.Config - this will require doing 1b

Now, the problem with migrating Geth is that:

  • there are 330 usages of this field in total
    • there are 143 write usages - all these would require serializing the value being assigned
      • there are only 9 write usages outside tests
    • read usages would be easy to change, as we can add a getter Config() that would deserialized the string field, so the migration would be easy

This PR, after consultations, implements 1a variant.


closes NIT-4412
pulls in OffchainLabs/go-ethereum#618

@pmikolajczyk41 pmikolajczyk41 marked this pull request as ready for review February 3, 2026 13:34
@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.40%. Comparing base (4b0cc3a) to head (8bba6df).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4313      +/-   ##
==========================================
- Coverage   34.33%   32.40%   -1.94%     
==========================================
  Files         483      483              
  Lines       57343    57344       +1     
==========================================
- Hits        19690    18583    -1107     
- Misses      34149    35502    +1353     
+ Partials     3504     3259     -245     

@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2026

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@tsahee tsahee enabled auto-merge February 6, 2026 23:11
@tsahee tsahee added this pull request to the merge queue Feb 6, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Feb 7, 2026
@tsahee tsahee assigned pmikolajczyk41 and unassigned tsahee Feb 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants