search

koinos / koinos-p2p

The p2p microservice orchestrates the distribution of blocks and transactions between peers.
MIT License
6 stars 4 forks source link

Fork Broadcast #99

Closed mvandeberg closed 3 years ago

mvandeberg commented 3 years ago

Closes #93

theoreticalbts commented 3 years ago

Shared MyTopologyCache resolution

Suggested long-term fix is to move MyTopologyCache to top-level (i.e. Node or SyncManager) and make it threadsafe. However it seems like a bit of work, so I suggest that for this ticket, we just acknowledge that it's currently racy, and put writing the fix into another ticket.

Suggested fixes

Reinstate the comment and create mutex on TestRPC methods. p.heightRange = heightRange issue will be fixed by #100. Another ticket should be filed for implementing shared, thread-safe MyTopologyCache (and possibly mentioned as related to #98).

mvandeberg commented 3 years ago

The deleted TODO comment suggesting to re-implement GetAncestorTopologyAtHeights() in the block store should be reinstated (implementing in the block store this will still give great performance benefits due to fewer round trips and low-level seek optimizations)

The comment was deleted because it suggested that p2p should unbox the block to access previous in the block header. When we redesigned the block structure, I thought it was understood by all that we were doing this in part so that the header could be freely access via microservices instead of slapping an additional topology on every struct that contains a block. (See koinos/koinos-block-store#57)

And upon further review, it appears that this function is not called anywhere. So instead of restoring a comment I do not agree with, I think we should remove the method entirely.

theoreticalbts commented 3 years ago

In the interests of getting this merged, we added the thread-safe MyTopologyCache as #101 and thread-safe TestRPC as #102.