BlockSyncService#Start Overly Complex

Open oxnr opened this issue 2 years ago • 0 comments

Finding 004 - BlockSyncService#Start Overly Complex

ID 004

Finding BlockSyncService#Start Overly Complex

Severity 2 - Informational

Description The BlockSyncService#Start method is rather complex and large. It is responsible for many tasks such as starting and subscribing to a subscriber, starting the block store, P2P logic, creating a syncer and evaluating P2P responses. In addition, the method is not very well documented.In the case this method is bug-prone or problematic or even needs to be updated in the future, it might be difficult for developers to truly understand and debug root causes.

Recommendation We recommend splitting up the method into simpler, more testable methods, each that are clearly documented and well tested.In addition, Start should take a Context in order for the caller to manage the lifecycle of the call in a predictable manner.

Code References https://github.com/rollkit/rollkit/blob/eccdd0f1793a5ac532011ef4d896de9e0d8bcb9d/block/block_sync.go#L122

ID	004
Finding	BlockSyncService#Start Overly Complex
Severity	2 - Informational
Description	The BlockSyncService#Start method is rather complex and large. It is responsible for many tasks such as starting and subscribing to a subscriber, starting the block store, P2P logic, creating a syncer and evaluating P2P responses. In addition, the method is not very well documented.In the case this method is bug-prone or problematic or even needs to be updated in the future, it might be difficult for developers to truly understand and debug root causes.
Recommendation	We recommend splitting up the method into simpler, more testable methods, each that are clearly documented and well tested.In addition, Start should take a Context in order for the caller to manage the lifecycle of the call in a predictable manner.
Code References	https://github.com/rollkit/rollkit/blob/eccdd0f1793a5ac532011ef4d896de9e0d8bcb9d/block/block_sync.go#L122

Jan 17 '24 22:01 oxnr