fix: missing reorged blocks #250
Conversation
DNK90
force-pushed
the
fix/missing_new_chain_blocks_in_reorg
branch
from
41abbca
to
216e024
Compare
| @@ -161,6 +162,9 @@ func makeFullNode(ctx *cli.Context) (*node.Node, ethapi.Backend) { | |||
| } | |||
| backend, eth := utils.RegisterEthService(stack, &cfg.Eth) | |||
|
|
|||
| // store internal transaction is enabled flag | |||
| rawdb.WriteStoreInternalTransactionsEnabled(backend.ChainDb(), ctx.GlobalBool(utils.StoreInternalTransactions.Name)) | |||
There was a problem hiding this comment.
In my opinion, it's better to add a field to blockchain object and read from it to check whether this feature is enabled instead of reading from DB
There was a problem hiding this comment.
it would be complicated to pass the config to blockchain object from cli and a lot of code changes. Therefore I think, the most simple wait is add it to db
There was a problem hiding this comment.
I've added a field shouldStoreInternalTxs in blockchain object, and get it from db to reduce validation cost everytine WriteInternalTransactions is triggered
core/blockchain_reader.go
Outdated
| bc.internalTransactionsCache.Add(hash, internalTxs) | ||
|
|
||
| // check if store internal transactions is enabled or not | ||
| if !rawdb.ReadStoreInternalTransactionsEnabled(bc.db) { |
There was a problem hiding this comment.
I think we should check this before caching the internalTxs
| if !rawdb.ReadStoreInternalTransactionsEnabled(bc.db) { | ||
| return | ||
| } | ||
| rawdb.WriteInternalTransactions(bc.db, hash, internalTxs) |
There was a problem hiding this comment.
The internal transactions are written to db and never deleted. I think we should cache them to LRU cache and store them to DB only on blockchain.Stop, load them from DB when block.NewBlockChain. The same logic can apply to dirty accounts.
There was a problem hiding this comment.
I think if we have the options of storing internal transactions within db. We should accept the increment of the storage. I agree with the logic of storing dirtyAccounts to db when blockchain is stop
| @@ -1712,6 +1721,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er | |||
|
|
|||
| if dirtyAccounts != nil { | |||
| bc.dirtyAccountFeed.Send(dirtyAccounts) | |||
| bc.dirtyAccountsCache.Add(block.Hash(), dirtyAccounts) | |||
There was a problem hiding this comment.
I think it's better to cache dirty accounts when a configuration is on only, not to always do this.
There was a problem hiding this comment.
I think dirty accounts cache is not big compare to other caches such as block. Therefore, we don't need to add more configuration for it
…ck if internal txs should be stored into db or not
core/blockchain.go
Outdated
| return bc, nil | ||
| } | ||
|
|
||
| func (bc *BlockChain) loadLatestDirtyAccounts() { | ||
| for i := uint64(dirtyAccountsCacheLimit); i > 0; i-- { | ||
| hash := rawdb.ReadCanonicalHash(bc.db, bc.CurrentBlock().NumberU64()-i) |
There was a problem hiding this comment.
We want to store the dirty accounts of side chain, with this logic, dirty accounts of side chain are not loaded. I think we store them as an array of struct { blockHash, dirtyAccounts }
There was a problem hiding this comment.
Please check it again
…nonical or sidechain blockhash can be found
* fix: add `added blocks` to ChainEvent if reorg happened * chore: fix unit test failed * chore: move read/write store internal transaction flag to `accessors_chain` * chore: add `shouldStoreInternalTxs` into blockchain and use it to check if internal txs should be stored into db or not * chore: backup dirty accounts to db when blockchain is stop * chore: change struct of storing latest dirty accounts to make sure canonical or sidechain blockhash can be found
Purpose:
After reorg happened, only canonical block is published. Added blocks during this process will not be published.
How to
chainFeed