Update holesky with fixed extraData, genesis time, shanghaiTime #5890

siladu · 2023-09-18T21:44:43Z

Incorporates eth-clients/holesky#73
and also @fab-10's changes from eth-clients/holesky#72

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

github-actions · 2023-09-18T21:44:57Z

I thought about documentation and added the doc-change-required label to this PR if updates are required.
I thought about the changelog and included a changelog update if required.
If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

gfukushima

LGTM, double-checked against the expected block hash and it matches:
0xb5f7f912443c940f21fd611f12828d75b534364ed9e95ca4e307729a4661bde4

I'm going to wait until eth-clients/holesky#73 gets merged to approve this.

garyschulte

It looks like the missing brace would have parsed the remaining accounts as storage fields? I presume this was possible because of a json parsing policy that ignores missing fields, otherwise we should have thrown when "balance":xx wasn't part of the json schema for storage fields.

can we add stricter json parsing for storage fields and/or test coverage?

siladu · 2023-09-19T00:58:48Z

@fab-10 I have some doubt about the removal of cancunTime (which is currently breaking the tests).

Eth-clients genesis didn't have it but geth did: https://github.com/ethereum/go-ethereum/blob/90d5bd85bcf2919ac2735a47fde675213348a0a6/params/config.go#L84

Nethermind didn't: https://github.com/NethermindEth/nethermind/blob/8ce10d324c05a44dc1fb9e07e6b8618350addac9/src/Nethermind/Nethermind.Specs/HoleskySpecProvider.cs#L15

I think it's probably fine either way...just not 100% sure if "fork next" having a different value affects peering

After changing shanghaiTime and removing cancunTime Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu · 2023-09-19T01:27:00Z

besu/src/test/java/org/hyperledger/besu/ForkIdsNetworkConfigTest.java

-                new ForkId(Bytes.ofUnsignedInt(0x840a3b53L), 2000000000L),
-                new ForkId(Bytes.ofUnsignedInt(0x30771f90), 0L),
-                new ForkId(Bytes.ofUnsignedInt(0x30771f90), 0L))
+                new ForkId(Bytes.ofUnsignedInt(0xc61a6098L), 1696000704L),


Couldn't find a reference for the ForkId hash, would be good to cross reference with other clients once they've made the changes

fab-10 · 2023-09-19T05:04:29Z

@fab-10 I have some doubt about the removal of cancunTime (which is currently breaking the tests).

Eth-clients genesis didn't have it but geth did: https://github.com/ethereum/go-ethereum/blob/90d5bd85bcf2919ac2735a47fde675213348a0a6/params/config.go#L84

Nethermind didn't: https://github.com/NethermindEth/nethermind/blob/8ce10d324c05a44dc1fb9e07e6b8618350addac9/src/Nethermind/Nethermind.Specs/HoleskySpecProvider.cs#L15

I think it's probably fine either way...just not 100% sure if "fork next" having a different value affects peering

Ok, now I remember why I added it in the first place, so should I readd it or have you found a way to fix the tests?

siladu · 2023-09-19T06:01:00Z

Ok, now I remember why I added it in the first place, so should I readd it or have you found a way to fix the tests?

@fab-10 I fixed the tests and confirmed with Stefan that a future cancunTime shouldn't affect peering so think we're good.
Would still like to confirm with other EL clients if the forkId is the expected value before we re-launch

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

jframe · 2023-09-19T21:53:56Z

It looks like the missing brace would have parsed the remaining accounts as storage fields? I presume this was possible because of a json parsing policy that ignores missing fields, otherwise we should have thrown when "balance":xx wasn't part of the json schema for storage fields.

can we add stricter json parsing for storage fields and/or test coverage?

Definitely a good idea to do that. Happy for that to be in another PR to not hold up the release for the Holesky relaunch. Even better would be to be compatible with the same genesis config that other clients use.

macfarla · 2023-09-19T22:13:13Z

Created this issue for tightening up the parsing #5900

jframe · 2023-09-19T22:18:02Z

config/src/main/resources/holesky.json

+      "balance": "0xd3c21bcecceda1000000"
+    },
+    "0x4BC656B34De23896fa6069C9862F355b740401aF": {
+      "balance": "0x084595161401484a000000"
    }
  },
  "coinbase": "0x0000000000000000000000000000000000000000",
  "baseFeePerGas": "0x3B9ACA00",


Why does the Besu config need baseFeePerGas but the default genesis.json doesn't?

Just double checked and we technically don't because it is the default initial value: https://github.com/hyperledger/besu/blob/main/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java#L43 so we still get same genesis block when it loads.

However, if we change it in this PR then we will no longer exactly match https://github.com/eth-clients/holesky/blob/main/custom_config_data/besu.json if anyone cares about that.

I actually prefer it being in, more explicit, since it is actually part of the genesis block.

garyschulte

🚢 LGTM. AFAIK, next fork shouldn't present a peering problem until we 'get there'

--- Drop Kotti Network support (ETC) (hyperledger#5816) Signed-off-by: Diego López León <dieguitoll@gmail.com> fix ForkId if there are no Forks and the starting timestamp is not 0 (hyperledger#5819) Signed-off-by: Stefan <stefan.pingel@consensys.net> enforce that BlobTransactions have at least one blob (hyperledger#5826) * enforce that BlobTransactions have at least one blob Signed-off-by: Stefan <stefan.pingel@consensys.net> Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Do not create ignorable segments on `revert storage-variables` (hyperledger#5830) * fix the bug that creates the ignorable chain pruner segment, add rocks exception parsing to RocksDBColumnarKeyValueStorage subclasses * parse rocksdb error for unprintable column family id's Signed-off-by: garyschulte <garyschulte@gmail.com> add versioned hashes and number of blobs to toString() (hyperledger#5831) Signed-off-by: Stefan <stefan.pingel@consensys.net> add parent beacon block root to payload id calculation (hyperledger#5843) Signed-off-by: Stefan <stefan.pingel@consensys.net> bump version to 23.7.3-SNAPSHOT (hyperledger#5854) Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> set the beacon root address to the correct value (hyperledger#5853) Signed-off-by: Stefan <stefan.pingel@consensys.net> docs(readme): fix broken link to installation of binaries page (hyperledger#5859) Fixes hyperledger#5858 Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com> Update RocksDB version from 8.0.0 to 8.3.2 (hyperledger#5832) Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> use non-deprecated authenticate methods (hyperledger#5852) Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> move to Hyperledger shared runners for current github actions (hyperledger#5860) Signed-off-by: garyschulte <garyschulte@gmail.com> Add range tracing with worldstate (hyperledger#5844) Implement a method to trace a range of blocks and have access to the worldstate before and after the tracing Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Layered txpool by default and txpool options hoverhaul (hyperledger#5772) Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Fix issue 5824 - Duplicate key errors in EthScheduler-Transactions (hyperledger#5857) Fix issue 5824 - Duplicate key errors in EthScheduler-Transactions Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net> updated gradle verification metadata (hyperledger#5870) * removed old artefacts [skip ci] * works with compileTestJava * restored metadata needed for codeQL and trusted-artifacts block for javadoc/sources Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> --------- Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> [4844] Add encodingContext to TransactionEncoder and TransactionDecoder (hyperledger#5820) * Add decode type to TransactionDecoder * Refactoring TransactionDecoder * Invert methods order * Use Transaction encoder instead of writeTo * Move enter and leave list to inner method as pr suggestion * Size calculation should use opaque bytes instead of rlp --------- Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Stefan <stefan.pingel@consensys.net> payload attributes: fix wrong warning and fail if beacon root is available before cancun (hyperledger#5872) Signed-off-by: Stefan <stefan.pingel@consensys.net> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Merge MutableAccount and EVMAccount (hyperledger#5863) Merge MutableAccount and EVMAccount functionalities by removing EVMAccount, all calls to getMutable, and the WrappedEVMAccount that was wrapping non-EVMAccounts in a mutable fashion. Instead, use a MutableAccount in all cases an EVMAccount would have been used. This also tends to reduce a level of layering in many places. Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com> Add world context to transaction tracing API (hyperledger#5836) * Add world context to transaction tracing API Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update changelog with PR ID Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Add the Transaction to traceEndTransaction Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Rebase on main Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Add receipt-linked information to the transaction tracer Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> --------- Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Bonsai based reference test worldstate (hyperledger#5686) * create a bonsai based reference test worldstate -> getOrCreate in BonsaiWorldStateUpdateAccumulator - do not throw if we discover an empty account in a non-null BonsaiValue<Account> -> add curentStateRoot to t8n -> storageEntriesFrom and streamAccounts implemented in BonsaiWorldStateKeyValueStorage -> add endKey version of streamFromKey * bonsai fix for self-destruct and create2 at the same address and same block Signed-off-by: garyschulte <garyschulte@gmail.com> Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Co-authored-by: Karim TAAM <karim.t2am@gmail.com> Don't start BFT mining coordinators until initial sync has completed (hyperledger#5861) * Don't start BFT mining coordinators until initial sync has completed Signed-off-by: Matthew Whitehead <matthew1001@gmail.com> * Fix unit tests Signed-off-by: Matthew Whitehead <matthew1001@gmail.com> * Fix 'enable' logic Signed-off-by: Matthew Whitehead <matthew1001@gmail.com> --------- Signed-off-by: Matthew Whitehead <matthew1001@gmail.com> display only peers ready for requets on ethstats (hyperledger#5880) * display only ready for requets peers in ethstats Signed-off-by: Karim TAAM <karim.t2am@gmail.com> * cast to int Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> --------- Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> [MINOR] test RLP used for encode/decode blob tx should contain to field (hyperledger#5883) * validate to field on encode/decode for blob tx Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> * revert decode/encode checks - tis done later in tx validation Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> --------- Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> Fix: correctly convert percentage options in TOML configuration file (hyperledger#5886) Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> EIP7516 - Add BlobBaseFee opcode to Cancun EVM (hyperledger#5884) Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net> Fix snapsync heal (hyperledger#5838) Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Upgrade besu-native (hyperledger#5893) Upgrade besu-native to 0.8.2 Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com> Tune G1GC to reduce Besu memory footprint (hyperledger#5879) Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Add updated storage to evmtool json trace (hyperledger#5892) Add the EIP-3155 "storage" option to the standard tracer, with the caveat only updated storage is logged. Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com> Update holesky with fixed extraData, genesis time, shanghaiTime (hyperledger#5890) Signed-off-by: Simon Dudley <simon.dudley@consensys.net> [CHANGELOG] removed duplicated line (hyperledger#5904) * removed duplicated line [skip ci] Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> * fixed spelling on Holesky Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> --------- Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>