dot/network: refactor `createNotificationsMessageHandle` into smaller functions

[kishansagathiya]

Issue summary

• It would be nice to break up createNotificationsMessageHandler into two smaller functions for handshake and notification message
• right now, it is easier to confuse between notification message and handshake. They both use the same interface. It might be worth it to separate them, that could potentially make it easier for errors to get spot. Hash() method is never implemented in case of Handshake. https://github.com/ChainSafe/gossamer/blob/7ab31f0c84b16df6565f98553dd9fc6f9ae39e88/dot/network/block_announce.go#L148-L150

And then we have isHandshake() method differentiate between them. Same interface looks forced than natural here. We should split it into two different interfaces.

• https://github.com/ChainSafe/gossamer/blob/7ab31f0c84b16df6565f98553dd9fc6f9ae39e88/dot/network/notifications.go#L258 Don't think we should be allowing for message cache to be nil. We should throw and error if it is nil.
• Should change this ... comments to something more meaningful. They are at a bunch of places. https://github.com/ChainSafe/gossamer/blob/7ab31f0c84b16df6565f98553dd9fc6f9ae39e88/dot/network/block_announce.go#L147
• I think more ideas will start coming on the task
• https://github.com/ChainSafe/gossamer/pull/2435#discussion_r835161686

Other information and links

[timwu20]

It would be nice to break up createNotificationsMessageHandler into two smaller functions for handshake and notification message

After looking at the codeI agree I think we should break up createNotificationsMessageHandler. There is a good amount of code wrapped in a if msg.IsHandshake()condition, which returns at the end. It makes sense to move both cases to other functions to handle each case of handshake vs message.

right now, it is easier to confuse between notification message and handshake. They both use the same interface. It might be worth it to separate them, that could potentially make it easier for errors to get spot. Hash() method is never implemented in case of Handshake.

And then we have isHandshake() method differentiate between them. Same interface looks forced than natural here. We should split it into two different interfaces.

I'm not sure that will be worthwhile at this point. The cast to NotificationMessage is pretty clear at this point, and the IsHandshake() is kinda ugly. I think there's a better way maybe via generics to expose an interface that constrains only on handshake types in the network package.

Don't think we should be allowing for message cache to be nil. We should throw and error if it is nil.

Should change this ... comments to something more meaningful. They are at a bunch of places.

I agree with this.

[kishansagathiya]

@timwu20 It looks like I will have to take up this in order to fix https://github.com/ChainSafe/gossamer/issues/2636