hyperledger-labs / Scorex

Scorex 2.0 Core
Apache License 2.0
543 stars 115 forks source link

moved peerSynchronizer creation to app #364

Closed scasplte2 closed 4 years ago

scasplte2 commented 4 years ago

Addressed the following todo in Network controller

//todo: make usage more clear, now we're relying on preStart logic in a actor which is described by a never used val

This note is regarding the registration process in PeerSynchronizer that happens on creation. There seems to be no reason this actor cannot be instantiated within the primary application which will also avoid the redundant computation of the the PeerFeature serializers.

Additionally, while not implemented in the Scorex framework, the shutdown hook has been changed in Ergo App to include a Seq of actors that should be sent a poison pill on application shutdown. By including the PeerSynchronizer in this sequence, a grayed out value would be avoided which may help dissuade an overzealous maintainer from removing the needed instantiation in the future.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 46.984% when pulling 29429ce791345781db456a142b49f309fd442c15 on scasplte2:move_peerSync_creation into 1cfffb3dd84308202ec3a28725d64b9566a92921 on ScorexFoundation:master.