Skip to content

Update BonsaiLayeredWorldState implementation#1999

Merged
matkt merged 11 commits into
besu-eth:masterfrom
matkt:feature/tracing-fix
Mar 16, 2021
Merged

Update BonsaiLayeredWorldState implementation#1999
matkt merged 11 commits into
besu-eth:masterfrom
matkt:feature/tracing-fix

Conversation

@matkt
Copy link
Copy Markdown
Contributor

@matkt matkt commented Mar 10, 2021

Signed-off-by: Karim TAAM t2am.karim@gmail.com

PR description

Update BonsaiLayeredWorldState implementation to find valid code, storage and account corresponding to a block

Test performed

  • Fast sync bonsai trie goerli

Fixed Issue(s)

Changelog

matkt and others added 5 commits March 10, 2021 15:02
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>
public Optional<MutableWorldState> getWorldState(final Hash rootHash, final Hash blockHash) {
if (layeredWorldStates.containsKey(blockHash)) {
return Optional.of(layeredWorldStates.get(blockHash));
} else {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we don't load TrieLog layered world states from disk how do we then do traces beyond near chain head? We won't be able to do eth_call 1MM blocks back but we should be able to do trace_* from 1MM blocks back if we have the TrieLogs.

Copy link
Copy Markdown
Contributor Author

@matkt matkt Mar 11, 2021

Choose a reason for hiding this comment

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

Indeed I just put back the loading of the layer from the disk but in this case I put the nextLayer (=parent) to empty and I load it on demand (call getNextWorldView : https://github.com/hyperledger/besu/pull/1999/files#diff-b24e30196f7e0d254b3a27e89926831bb450c356a094f435e6a6daa80b10b09dR64)

so as not to load all layers for maybe nothing when we call getMutable

I tried it's work

}
final MutableWorldState mutableWorldState =
worldStateArchive.getMutable(previous.getStateRoot(), previous.getHash()).orElse(null);
worldStateArchive.getWorldState(previous.getStateRoot(), previous.getHash()).orElse(null);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like there is a lingering type/name issue. If the return type is a MutableWorldState then the method name should be getMutable or getMutableWorldState. This should either go to just a WorldState or the method name should be getMutable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done renamed to getMutable

currentLayer.trieLog.getStorageBySlotHash(address, slotHash);
if (maybeValue.isPresent()) {
final Optional<UInt256> maybeOriginalValue =
currentLayer.trieLog.getOriginalStorageBySlotHash(address, slotHash);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is a good place for getOriginalStorageBySlotHash. Original in the sense of EIP-2200 gas costing. I think we need to go to renaming BonsaiValue getOriginal as getPrior to remove that confusion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

indeed I wanted to rename it in another PR but why not do it now


private final BonsaiWorldStateArchive worldStateArchive;
private final BonsaiWorldView parent;
private Optional<BonsaiWorldView> parent;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this being re-worked so as only used for blocks that were before the persisted state? (eg persisted #100, this is a trie log for #99) In that case parent may not be the best name. perhaps nextTrieLog

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So we can have a layer or a persistate Worldstate so I renamed it to nextWorldView because those are both BonsaiWorldView.

Layer9 (parent = = Layer10), Layer10 (parent= = Layer11), Layer11 (parent==PersistedWorldState)

matkt added 5 commits March 11, 2021 15:21
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>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@matkt matkt marked this pull request as ready for review March 13, 2021 19:55
@matkt
Copy link
Copy Markdown
Contributor Author

matkt commented Mar 15, 2021

@shemnon Do you have any other remarks before its merge into master ?

@matkt matkt merged commit 8a0d002 into besu-eth:master Mar 16, 2021
RichardH92 pushed a commit to RichardH92/besu that referenced this pull request Mar 29, 2021
Update BonsaiLayeredWorldState implementation to find valid code, storage and account corresponding to a block with Bonsai trie storage mode

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Richard Hart <richardhart92@gmail.com>
@matkt matkt deleted the feature/tracing-fix branch April 13, 2021 11:21
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Update BonsaiLayeredWorldState implementation to find valid code, storage and account corresponding to a block with Bonsai trie storage mode

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.

2 participants