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 and update Besu genesis config #72

Closed
wants to merge 5 commits into from
Closed

Conversation

fab-10
Copy link
Contributor

@fab-10 fab-10 commented Sep 15, 2023

No description provided.

custom_config_data/besu.json Outdated Show resolved Hide resolved
@@ -14,7 +14,14 @@
"preMergeForkBlock": 0,
"terminalTotalDifficulty": 0,
"shanghaiTime": 1694884704,
"ethash": {}
"ethash": {},
"cancunTime": 2000000000,

Choose a reason for hiding this comment

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

The repo says Cancun TBD. Was this meant to be 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.

right not required to be here

@gfukushima
Copy link

It would also be good to update the timestamp field at the end of our genesis file to match what @barnabasbusa is proposing in his PR just to make sure we don't revert his changes in case his PR gets merged before this one.

fab-10 and others added 2 commits September 18, 2023 15:13
Co-authored-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
@fab-10
Copy link
Contributor Author

fab-10 commented Sep 18, 2023

It would also be good to update the timestamp field at the end of our genesis file to match what @barnabasbusa is proposing in his PR just to make sure we don't revert his changes in case his PR gets merged before this one.

There should no risk of overriding, git should sort out it.

@barnabasbusa could you review this? the main fix is that there was a misplaced } that excluded some prefunded addresses to be included in the genesis.

@barnabasbusa
Copy link
Collaborator

yes plan to adjust my PR to match this syntax too. Apologies for the delay, will fix it tomorrow. cc: @parithosh

@@ -13,8 +13,14 @@
"londonBlock": 0,
"preMergeForkBlock": 0,
Copy link

@siladu siladu Sep 18, 2023

Choose a reason for hiding this comment

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

Think this could be changed to match the geth genesis.json (although neither are used AFAIK)

Suggested change
"preMergeForkBlock": 0,
"mergeNetsplitBlock": 0,

@parithosh
Copy link
Member

@fab-10 @siladu Could you have a look at the PR here: #73
I merged in the changes from this PR to have everything in a single PR to avoid rebasing issues. If its fine now, please close this PR and we can merge the other one.

@siladu
Copy link

siladu commented Sep 19, 2023

LGTM thanks @parithosh
I think @fab-10 might have to close this issue

@fab-10
Copy link
Contributor Author

fab-10 commented Sep 20, 2023

closing since these changes are integrated in #73

@parithosh parithosh closed this Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants