Fix Layered World State issue #5076

ahamlat · 2023-02-09T21:45:03Z

Signed-off-by: Ameziane H [email protected]
Signed-off-by: Karim TAAM [email protected]

PR description

We found the problem :

During a remember block we added a copy of the worldstate during the saveTrieLog in order to fix the get balance == 0. By making a copy we have a BonsaiInMemoryWorldstate which does not persist the trielog in rocksdb. We saw the log that tells us that we can’t commit.
The problem is that during a FCU when we persist a block if we have a reorg we finally save the trielog after the different Rollback / Forward. Because of that we do not save a valid log. Instead of saving a Trielog for 1 to 2 bis we have a Trielog which is a mix of 2-1 + 1-2bis . This happens because it is not normal to not already have the trielog in base during a reorg.

Because of that we can't rollback/rollfoward anymore with this invalid trielog so we have the layered copy worldstate error

The worldstate copy is not really needed and we decided to not create a copy and to subscribe to the worldstateStorage directly in the BonsaiLayeredWorldstate.

This fixed was not fixing yet the problem because we had another problem. During a getAccount we are getting the BonsaiLayeredWorldstate and we are closing this one after the getAccount. Because of that we are unsubscribing the worldstateStorage and losing the instance. to fix that we decided to use the same code we re using for the validate/transaction pool etc and to create a copy of the layeredWorldstate instead

Fixed Issue(s)

Documentation

I thought about documentation and added the doc-change-required label to this PR if
updates are required.

Acceptance Tests (Non Mainnet)

I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.

Changelog

I thought about the changelog and included a changelog update if required.

…ater during BonsaiInMemoryWorldState copy Signed-off-by: Ameziane H <[email protected]>

Signed-off-by: Ameziane H <[email protected]>

Signed-off-by: Karim TAAM <[email protected]>

Signed-off-by: Ameziane H <[email protected]>

Signed-off-by: Karim TAAM <[email protected]>

ahamlat · 2023-02-13T16:09:22Z

I can confirm that it is working, I tested getBalance on latest several times and with a specific block. We also managed to run two nodes, one with the fix and other one with commit e715010. After several hours, the reorg happened, the node with the fix (this PR) was able to handle it, while the other node crashed " Unable to copy Layered Worldstate for 0x58c372dc871df277ad86bdfdc3aa623b75d12c6d04718e6835b7c0e4fd4854cc" error.

garyschulte · 2023-02-13T16:58:32Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiLayeredWorldState.java

    this.nextWorldView = nextWorldView;
+    maybeSubscribe(); // subscribe the next view


garyschulte

🙏

Signed-off-by: Ameziane H <[email protected]>

Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]>

Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Signed-off-by: garyschulte <[email protected]>

* Use the copy during prepareTrieLog instead of saveTrieLog * add final flag on BonsaiWorldStateUpdater * Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog * add link to persisted worldstate storage * fix tests * Make a copy of the worldstate after committing changes to the trielog * spotless + remove maybeUnSubscribe in setNextWorldView * subscribe storage on layered worldstate * fix null issue * not close layered worldstate during getAccount * clean code * Add changelog entry --------- Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Karim TAAM <[email protected]>

* Use the copy during prepareTrieLog instead of saveTrieLog * add final flag on BonsaiWorldStateUpdater * Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog * add link to persisted worldstate storage * fix tests * Make a copy of the worldstate after committing changes to the trielog * spotless + remove maybeUnSubscribe in setNextWorldView * subscribe storage on layered worldstate * fix null issue * not close layered worldstate during getAccount * clean code * Add changelog entry --------- Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Karim TAAM <[email protected]> Signed-off-by: garyschulte <[email protected]>

* Use the copy during prepareTrieLog instead of saveTrieLog * add final flag on BonsaiWorldStateUpdater * Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog * add link to persisted worldstate storage * fix tests * Make a copy of the worldstate after committing changes to the trielog * spotless + remove maybeUnSubscribe in setNextWorldView * subscribe storage on layered worldstate * fix null issue * not close layered worldstate during getAccount * clean code * Add changelog entry --------- Signed-off-by: ahamlat <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Karim TAAM <[email protected]>

* Use the copy during prepareTrieLog instead of saveTrieLog * add final flag on BonsaiWorldStateUpdater * Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog * add link to persisted worldstate storage * fix tests * Make a copy of the worldstate after committing changes to the trielog * spotless + remove maybeUnSubscribe in setNextWorldView * subscribe storage on layered worldstate * fix null issue * not close layered worldstate during getAccount * clean code * Add changelog entry --------- Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Karim TAAM <[email protected]>

* Use the copy during prepareTrieLog instead of saveTrieLog * add final flag on BonsaiWorldStateUpdater * Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog * add link to persisted worldstate storage * fix tests * Make a copy of the worldstate after committing changes to the trielog * spotless + remove maybeUnSubscribe in setNextWorldView * subscribe storage on layered worldstate * fix null issue * not close layered worldstate during getAccount * clean code * Add changelog entry --------- Signed-off-by: ahamlat <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Karim TAAM <[email protected]>

* Prepare for version 23.1.1-SNAPSHOT (#5067) Signed-off-by: Simon Dudley <[email protected]> * Add getPayloadBodiesByRangeV1 and getPayloadBodiesByHash engine methods (#4980) * Add engine get payload body methods and test Signed-off-by: Zhenyang Shi <[email protected]> Signed-off-by: Gabriel Fukushima <[email protected]> * Add header Signed-off-by: Zhenyang Shi <[email protected]> * Update result struct & add test Signed-off-by: Zhenyang Shi <[email protected]> * Change constant to use upper case Signed-off-by: Gabriel Fukushima <[email protected]> * Add PayloadBody class and withdrawals to response of methods Signed-off-by: Gabriel Fukushima <[email protected]> * Add unit tests Signed-off-by: Gabriel Fukushima <[email protected]> * Add changelog Signed-off-by: Gabriel Fukushima <[email protected]> * spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Add check to prevent returning trailing null results past the head Signed-off-by: Gabriel Fukushima <[email protected]> * Add test to check trailing null post head scenario Signed-off-by: Gabriel Fukushima <[email protected]> * Split tests into pre and post shanghai Signed-off-by: Gabriel Fukushima <[email protected]> * spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Rename methods Signed-off-by: Gabriel Fukushima <[email protected]> * Use getName() to log method name Signed-off-by: Gabriel Fukushima <[email protected]> * spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Rename variable Signed-off-by: Gabriel Fukushima <[email protected]> * Call constructor directly Signed-off-by: Gabriel Fukushima <[email protected]> * Fix ByHash json parsing Signed-off-by: Simon Dudley <[email protected]> * Fix json parsing again Signed-off-by: Simon Dudley <[email protected]> * Add check to prevent unnecessary queries Signed-off-by: Gabriel Fukushima <[email protected]> * Refactor method Signed-off-by: Gabriel Fukushima <[email protected]> * Add new error code for EngineGetPayloadBodies methods Signed-off-by: Gabriel Fukushima <[email protected]> * Add return error for request above the API limit Signed-off-by: Gabriel Fukushima <[email protected]> * Add constructor for empty response Signed-off-by: Gabriel Fukushima <[email protected]> * add check for number of blocks requested and for requests of post head Signed-off-by: Gabriel Fukushima <[email protected]> * Add test to check error code when request exceeds API limits Signed-off-by: Gabriel Fukushima <[email protected]> * add constant for max blocks allowed per request Signed-off-by: Gabriel Fukushima <[email protected]> * spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Fix some nits Signed-off-by: Gabriel Fukushima <[email protected]> * Add invalid params check Signed-off-by: Gabriel Fukushima <[email protected]> * Add tests for invalid params check Signed-off-by: Gabriel Fukushima <[email protected]> * Fix test and spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Revert "Fix json parsing again" This reverts commit 558d325. Signed-off-by: Gabriel Fukushima <[email protected]> * Revert "Fix ByHash json parsing Signed-off-by: Simon Dudley <[email protected]>" This reverts commit 663e11e Signed-off-by: Gabriel Fukushima <[email protected]> * Use UnsignedLongParameter to cast params of the request Signed-off-by: Gabriel Fukushima <[email protected]> * Add optional withdrawals to the NewPayload log (#5021) Signed-off-by: Simon Dudley <[email protected]> * kubernetes and errorprone - update versions (#5013) * update errorprone and kubernetes versions * fixed errorprone issues in prod cod * fixed errorprone issues in test code --------- Signed-off-by: Sally MacFarlane <[email protected]> * Rename JsonRpcService to EngineJsonRpcService (#5036) * rename JsonRpcService to EngineJsonRpcService Signed-off-by: Daniel Lehrner <[email protected]> * Params should be single item of array type, not outer array of strings (#5037) Signed-off-by: Simon Dudley <[email protected]> * Add EIP-2537 (BLS12-381 curve precompiles) to Cancun (#5017) Add the BLS curve precompiles into the registry for cancun. All of the curve precompiles have been here since berlin, so this is just wiring them in. Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> * Use UnsignedLongParameter to cast params of the request Signed-off-by: Gabriel Fukushima <[email protected]> * Add optional withdrawals to the NewPayload log (#5021) Signed-off-by: Simon Dudley <[email protected]> * kubernetes and errorprone - update versions (#5013) * update errorprone and kubernetes versions * fixed errorprone issues in prod cod * fixed errorprone issues in test code --------- Signed-off-by: Sally MacFarlane <[email protected]> * Rename JsonRpcService to EngineJsonRpcService (#5036) * rename JsonRpcService to EngineJsonRpcService Signed-off-by: Daniel Lehrner <[email protected]> * Params should be single item of array type, not outer array of strings (#5037) Signed-off-by: Simon Dudley <[email protected]> * Add EIP-2537 (BLS12-381 curve precompiles) to Cancun (#5017) Add the BLS curve precompiles into the registry for cancun. All of the curve precompiles have been here since berlin, so this is just wiring them in. Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> * Convert start and count from hex to match JSON-RPC Spec standard Signed-off-by: Gabriel Fukushima <[email protected]> --------- Signed-off-by: Zhenyang Shi <[email protected]> Signed-off-by: Gabriel Fukushima <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Zhenyang Shi <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Danno Ferrin <[email protected]> * Revert "Keep Worldstate Storage open for Bonsai archive latest layer (#5039)" (#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]> * Support post merge forks at genesis for hive tests (#5019) Signed-off-by: Jason Frame <[email protected]> * Fix manifest docker not skipping interim builds for RCs (#5068) Signed-off-by: Jason Frame <[email protected]> * Fix PoS checkpoint validation (#5081) * change validation to lessThan Signed-off-by: Gabriel Fukushima <[email protected]> * Change exception message to PoS instead of Near head Signed-off-by: Gabriel Fukushima <[email protected]> * Add unit test and fix message of existing unit test Signed-off-by: Gabriel Fukushima <[email protected]> --------- Signed-off-by: Gabriel Fukushima <[email protected]> * moves check for init code length before balance check (#5077) Signed-off-by: Justin Florentine <[email protected]> * bump revision for 23.1.x release branch Signed-off-by: garyschulte <[email protected]> * Burn in build of 23.1.0 (#5093) * rebase off of burn-in release, remove unreleased 23.1.0-RC2 from CHANGELOG Signed-off-by: garyschulte <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> * re-default global max rpc batch size to 1k (#5104) (#5105) * default global max rpc batch size to 1000 for now Signed-off-by: garyschulte <[email protected]> * Fix Layered World State issue (#5076) * Use the copy during prepareTrieLog instead of saveTrieLog * add final flag on BonsaiWorldStateUpdater * Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog * add link to persisted worldstate storage * fix tests * Make a copy of the worldstate after committing changes to the trielog * spotless + remove maybeUnSubscribe in setNextWorldView * subscribe storage on layered worldstate * fix null issue * not close layered worldstate during getAccount * clean code * Add changelog entry --------- Signed-off-by: ahamlat <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Karim TAAM <[email protected]> * Add shanghaiTime to sepolia (#5088) Update and fix forkId tests Move timestamp forks from getForkBlockNumbers to getForkBlockTimestamps in JsonGenesisConfigOptions - this ultimately gets used to popoulate the ForkIdManager which handles lists of blocks and timestamps the same way so this hasn't changed any actual behaviour, but rather supports the test fixes. Implement TransitionProtocolSchedule.streamMilestoneBlocks as a concatenation of blockNumbers++blockTimestamps. This may have been a latent bug since it's used to update the node record when a fork transition occurs. Signed-off-by: Simon Dudley <[email protected]> * If a PoS block creation repetition takes less than a configurable dur… (#5048) * If a PoS block creation repetition takes less than a configurable duration, then waits before next repetition Signed-off-by: Fabio Di Fabio <[email protected]> * Update CHANGELOG Signed-off-by: Fabio Di Fabio <[email protected]> * Add unit test Signed-off-by: Fabio Di Fabio <[email protected]> * Update besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> * Update besu/src/main/java/org/hyperledger/besu/cli/options/unstable/MiningOptions.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> --------- Signed-off-by: Fabio Di Fabio <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> * Allow dashes in ethstats password (#5090) Signed-off-by: Simon Dudley <[email protected]> * reintroduce checking of block height for certain tasks when we are not PoS (Revert PR#3911) (#5083) * reintroduce checking of block height for certain tasks when we are not PoS (Revert PR#3911) Signed-off-by: Stefan Pingel <[email protected]> * Allow other users to read the /opt/besu dir when using docker (#5092) Signed-off-by: Rafael Matias <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> * Only use MAINNET version of KZG (#5095) Signed-off-by: Fabio Di Fabio <[email protected]> * add more context to exception messages and debug logging (#5066) Signed-off-by: Sally MacFarlane <[email protected]> * Fix block value calculation (#5100) * Add gasUsed calculation Signed-off-by: Gabriel Fukushima <[email protected]> * Fix unit test Signed-off-by: Gabriel Fukushima <[email protected]> * Fix typo Signed-off-by: Gabriel Fukushima <[email protected]> --------- Signed-off-by: Gabriel Fukushima <[email protected]> * Add 23.1.1-RC1 changelog Signed-off-by: Jason Frame <[email protected]> * Change version to 23.1.1-RC1 Signed-off-by: Jason Frame <[email protected]> --------- Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Zhenyang Shi <[email protected]> Signed-off-by: Gabriel Fukushima <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Signed-off-by: Jason Frame <[email protected]> Signed-off-by: Justin Florentine <[email protected]> Signed-off-by: garyschulte <[email protected]> Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Signed-off-by: ahamlat <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Stefan Pingel <[email protected]> Signed-off-by: Rafael Matias <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Gabriel Fukushima <[email protected]> Co-authored-by: Zhenyang Shi <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Danno Ferrin <[email protected]> Co-authored-by: Justin Florentine <[email protected]> Co-authored-by: garyschulte <[email protected]> Co-authored-by: ahamlat <[email protected]> Co-authored-by: Karim TAAM <[email protected]> Co-authored-by: Fabio Di Fabio <[email protected]> Co-authored-by: Stefan Pingel <[email protected]> Co-authored-by: Rafael Matias <[email protected]>

* Use the copy during prepareTrieLog instead of saveTrieLog * add final flag on BonsaiWorldStateUpdater * Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog * add link to persisted worldstate storage * fix tests * Make a copy of the worldstate after committing changes to the trielog * spotless + remove maybeUnSubscribe in setNextWorldView * subscribe storage on layered worldstate * fix null issue * not close layered worldstate during getAccount * clean code * Add changelog entry --------- Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Karim TAAM <[email protected]>

ahamlat and others added 5 commits February 9, 2023 22:34

commit e715010 + Fix Layered World State issue, add a copy of the upd…

5948edb

…ater during BonsaiInMemoryWorldState copy Signed-off-by: Ameziane H <[email protected]>

Use the copy during prepareTrieLog instead of saveTrieLog

0a449aa

Signed-off-by: Ameziane H <[email protected]>

add final flag on BonsaiWorldStateUpdater

7bd84fb

Signed-off-by: Ameziane H <[email protected]>

Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog

924a496

Signed-off-by: Ameziane H <[email protected]>

add link to persisted worldstate storage

d862160

Signed-off-by: Karim TAAM <[email protected]>

matkt force-pushed the fix-layered-worldstate-issue branch from fd9d2c4 to d862160 Compare February 10, 2023 12:38

matkt and others added 7 commits February 10, 2023 13:53

fix tests

e3ba704

Signed-off-by: Karim TAAM <[email protected]>

Make a copy of the worldstate after committing changes to the trielog

e8ce58d

Signed-off-by: Ameziane H <[email protected]>

spotless + remove maybeUnSubscribe in setNextWorldView

2dcb8c7

Signed-off-by: Ameziane H <[email protected]>

subscribe storage on layered worldstate

220d706

Signed-off-by: Karim TAAM <[email protected]>

fix null issue

4e788e0

Signed-off-by: Karim TAAM <[email protected]>

not close layered worldstate during getAccount

ac4f6f7

Signed-off-by: Karim TAAM <[email protected]>

clean code

a8852dd

Signed-off-by: Karim TAAM <[email protected]>

matkt marked this pull request as ready for review February 13, 2023 15:47

ahamlat changed the title ~~Attempt to fix Layered World State issue~~ Fix Layered World State issue Feb 13, 2023

Merge branch 'main' into fix-layered-worldstate-issue

fe93100

matkt requested a review from garyschulte February 13, 2023 16:30

garyschulte reviewed Feb 13, 2023

View reviewed changes

garyschulte approved these changes Feb 13, 2023

View reviewed changes

Add changelog entry

4308890

Signed-off-by: Ameziane H <[email protected]>

ahamlat merged commit 3151c60 into hyperledger:main Feb 13, 2023

garyschulte pushed a commit that referenced this pull request Feb 14, 2023

cherry-pick of Fix Layered World State issue (#5076)

feb9453

Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]>

garyschulte pushed a commit to garyschulte/besu that referenced this pull request Feb 14, 2023

Cherry pick of Fix Layered World State issue (hyperledger#5076)

2f0c2f6

Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]>

garyschulte mentioned this pull request Feb 14, 2023

Burn in build of 23.1.0 #5093

Merged

3 tasks

siladu mentioned this pull request Mar 6, 2023

eth_getBalance returning 0x0 for latest block #5038

Closed

garyschulte mentioned this pull request Mar 7, 2023

copy layered worldstate fix for eth_call #5179

Merged

3 tasks

non-fungible-nelson mentioned this pull request May 15, 2023

eth_getCode unpredictable behaviour #5456

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Layered World State issue #5076

Fix Layered World State issue #5076

ahamlat commented Feb 9, 2023 •

edited by matkt

Loading

ahamlat commented Feb 13, 2023

garyschulte Feb 13, 2023

garyschulte left a comment

		this.nextWorldView = nextWorldView;
		maybeSubscribe(); // subscribe the next view

Fix Layered World State issue #5076

Fix Layered World State issue #5076

Conversation

ahamlat commented Feb 9, 2023 • edited by matkt Loading

PR description

Fixed Issue(s)

Documentation

Acceptance Tests (Non Mainnet)

Changelog

ahamlat commented Feb 13, 2023

garyschulte Feb 13, 2023

Choose a reason for hiding this comment

garyschulte left a comment

Choose a reason for hiding this comment

ahamlat commented Feb 9, 2023 •

edited by matkt

Loading