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

fix: replace mainnet TTD so geth mainnet genesis parsing works #5290

Merged
merged 3 commits into from
Nov 4, 2023

Conversation

Rjected
Copy link
Member

@Rjected Rjected commented Nov 4, 2023

This is a hack to make parsing default geth mainnet genesis.json files work. This all stems from fundamental limitations of serde_json, which does not support numbers larger than u64 by default, meaning the TTD cannot be deserialized. This limitation also makes visit_u128 implementations useless. Theoretically this would be solved by using the arbitrary_precision feature in serde_json, but that has its own problems, which propagates to users:
gakonst/ethers-rs#2206

This replaces the mainnet TTD with the TTD surrounded by quotes, because JsonU256 is able to deserialize strings. This solution does not work for other networks. AFAIK all test networks do not have TTD values larger than u64, so they do not need this hack in the first place.

The hack:

if raw.contains("58750000000000000000000") && !raw.contains("\"58750000000000000000000\"") {
    raw = raw.replace("58750000000000000000000", "\"58750000000000000000000\"");
}

@Rjected Rjected added the C-bug An unexpected or incorrect behavior label Nov 4, 2023
@Rjected Rjected requested a review from mattsse November 4, 2023 02:22
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm,

unfortunately, this is very bad, but it is the best solution here -.-

Comment on lines +82 to +83
if raw.contains("58750000000000000000000") &&
!raw.contains("\"58750000000000000000000\"")
Copy link
Collaborator

Choose a reason for hiding this comment

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

since we only call this once during launch, this should be fine and this only affects the mainnet ttd

bin/reth/src/args/utils.rs Outdated Show resolved Hide resolved
@mattsse
Copy link
Collaborator

mattsse commented Nov 4, 2023

ref gakonst/ethers-rs#2617

@mattsse mattsse added this pull request to the merge queue Nov 4, 2023
Merged via the queue into main with commit cf18249 Nov 4, 2023
22 checks passed
@mattsse mattsse deleted the dan/replace-mainnet-ttd-geth-genesis branch November 4, 2023 06:17
mattsse added a commit that referenced this pull request Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants