ETCM-938 Refactor Blockchain and extract methods getEvmCodeByHash and mptStateSavedKeys #1017
Conversation
| */ | ||
| class BlockchainHostActor( | ||
| blockchain: Blockchain, | ||
| evmCodeStorage: EvmCodeStorage, |
There was a problem hiding this comment.
Do you expect it will happen more during the refactoring that actors will have access to the storages directly? (To clarify: I don't really see a problem with it.)
There was a problem hiding this comment.
I think might happen for state storage also as the Blockchain class does not really bring anything in term of read access (I think it's ok for read access).
We will probably want write access to be through a single service though, or redefined what we call a storage to be a class that manage several rocksdb namespace internally, because we have some writes that span several storages (for instance when adding a block we want to store the block itself but also update the transaction mapping and potentially some other metadata).
| appStateStorage: AppStateStorage, | ||
| blockchain: Blockchain, | ||
| evmCodeStorage: EvmCodeStorage, | ||
| nodeStorage: NodeStorage, |
There was a problem hiding this comment.
I wonder at what point we'll create a case class MantisEnv or something that has all these instances in it.
There was a problem hiding this comment.
We already have something similar BlockchainStorages which is extended by Storages in the trait StoragesComponent, and with a concrete implementation in DefaultStorages.
It is a big bag with every storage in it so we will probably want to separate them to have storages that work together in the same container, and identify storages that are a "view" of other storages.
| ) | ||
| assert( | ||
| peer1.bl | ||
| .getBestBlockNumber() == peer3.bl.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset |
There was a problem hiding this comment.
nitpick: the line break in peer1.bl.getBestBlockNumber() makes it less readable imho
There was a problem hiding this comment.
I've put it back!
| */ | ||
| class BlockchainHostActor( | ||
| blockchain: Blockchain, | ||
| evmCodeStorage: EvmCodeStorage, |
There was a problem hiding this comment.
I think might happen for state storage also as the Blockchain class does not really bring anything in term of read access (I think it's ok for read access).
We will probably want write access to be through a single service though, or redefined what we call a storage to be a class that manage several rocksdb namespace internally, because we have some writes that span several storages (for instance when adding a block we want to store the block itself but also update the transaction mapping and potentially some other metadata).
| appStateStorage: AppStateStorage, | ||
| blockchain: Blockchain, | ||
| evmCodeStorage: EvmCodeStorage, | ||
| nodeStorage: NodeStorage, |
There was a problem hiding this comment.
We already have something similar BlockchainStorages which is extended by Storages in the trait StoragesComponent, and with a concrete implementation in DefaultStorages.
It is a big bag with every storage in it so we will probably want to separate them to have storages that work together in the same container, and identify storages that are a "view" of other storages.
leo-bogastry
force-pushed
the
refactor/ETCM-938
branch
from
3c076f6 to
589135c
Compare
… mptStateSavedKeys
leo-bogastry
force-pushed
the
refactor/ETCM-938
branch
from
589135c to
c001c80
Compare
Description
In order to simplify the Blockchain complexity, methods
getEvmCodeByHashandmptStateSavedKeyswhere removed, since they are used in just a few locationsgetEvmCodeByHashis just agetdone directly to the EvmCodeStoragemptStateSavedKeysis used only inSyncStateSchedulerso it was moved thereTesting
All should work as before