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 bonsai storage inconsistency issue #2492

Merged
merged 7 commits into from
Jul 6, 2021

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Jul 1, 2021

PR description

This ticket fixes a consistency issue between the blockchain storage part and the worldstate storage .

Fixed Issue(s)

Changelog

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

LGTM - non-blocking feedback about the Optional.get()'s

if (persistedHeader.isEmpty()) {
getTrieLogLayer(persistedState.blockHash()).ifPresent(rollBacks::add);
} else {
BlockHeader targetHeader = blockchain.getBlockHeader(blockHash).get();
Copy link
Contributor

Choose a reason for hiding this comment

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

non-blocking: it would help with readability to resolve the Optional persistedHeader once here and use it in the rest of the block. all of the naked Optional.get() make me nervous (despite being safe here). If we had a small refactor of the flow, those get's could be landmines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok I will do that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines +156 to +158
assertThat(bonsaiWorldStateArchive.getMutable(null, blockHeader.getHash()))
.containsInstanceOf(BonsaiPersistedWorldState.class);

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

garyschulte and others added 2 commits July 5, 2021 08:46
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@matkt matkt enabled auto-merge (squash) July 6, 2021 14:15
@matkt matkt merged commit 3072bc1 into hyperledger:master Jul 6, 2021
jflo pushed a commit to jflo/besu that referenced this pull request Jul 6, 2021
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
RatanRSur pushed a commit to RatanRSur/besu that referenced this pull request Jul 8, 2021
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
RatanRSur pushed a commit to RatanRSur/besu that referenced this pull request Jul 8, 2021
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
RatanRSur pushed a commit to RatanRSur/besu that referenced this pull request Jul 8, 2021
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
RatanRSur pushed a commit to RatanRSur/besu that referenced this pull request Jul 8, 2021
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
@matkt matkt deleted the fix-bonsai-storage-issue branch March 11, 2022 08:06
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
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.

None yet

2 participants