aboimpinto
commented
6 years ago I think BlockPool is very important to serve as list of blocks that aren't yet persistent. Why we need that?
I prefer not to think that LevelDb is the only way to persist data. I prefer to think that persistence layer can be implement in any way and this way could be faster or slower to persist data. Don't think that all the process of sync block should slow down according with the selected persistence layer, therefore, I think we should dump the blocks in a place where doesn't interrupt the sync process and with time, the node can persistence that data in the selected persistence system.
Another reason is one that @shargon talked in the discord channel: we should not sync the block number 4 if we didn't receive the block number 3 this problem is described in this issue https://github.com/CityOfZion/neo-sharp/issues/286
removing the BlockPool will lead to this error over and over again as the current implementation, NEO2.0 where for reason that no one know some node stop sync in a certain node and cannot recover. They need a manual reboot.
I would strongly suggest to not remove the BlockPool. It's a important piece of code that will help us improve and scale the NEO3.0
I am not sure we need BlockPool
Justification: We sync block headers and after we sync blocks. Actually, we sync transactions because block is a header + transactions and the header was already synced. Instead of, saving them right away in a db we are, by some reason, trying to order blocks in memory (BlockPool). The question is why.
My theory is NEO LevelDB supports only sequential writing and blocks were aggregated in memory to be arranged before saving. In our case, both Redis and RocksDB do not suffer from such a design. They supports random write op.
Let's say we remove BlockPool and will save right to the db on sync. It means we will have transaction table with some gaps as long as we will write blocks in the order they come. Eventually, after full sync all gaps have to be filled and we will have full blockchain snapshot.
The only problem is moving CurrentBlock pointer in the db because it should move forward if there are no gaps on its way. Indeed, it is not hard to implement.