[NC-1909] IBFT message gossiping

[Errorific]

PR description

Need to retransmit IBFT messages that we receive. Need to not send them back to who we received them from and not to who originally signed them

[rain-on]

The 'msg specific' functions could be removed, and the IbftController invoke gossipMessage directly. The "excludeAddress" would be the signer in that instance, rather than the sender.
I don't THINK there's a need to re-serialise the SignedData<?> into a messageData.

[Errorific]

The sender is available from the message object, the signer is available from the SignedData object, the signature I need to cache is in the SignedData object, dispatching a message requires it to be a MessageData. So at the end of everything all 3 (Message, MessageData, SignedData) need to be utilised at one level or another.

I've tried writing it a few ways and I've gotten to this version because it means the IbftController only needs to worry about the transport layer addresses when excluding things and the gossiper worries about the message contents. The other versions I've written didn't end up with a clean division of responsibilities.

[rain-on]

I know its not ideal - but in IbftController, can we not write:
gossiper.gossipMessage(payload.getSignature(),
messageData,
payload.getSender(),
payload.getSender(), message.getConnection().getPeer().getAddress());

Inside the handleMessage()? Or something similar in the new processMessage function?

[Errorific]

I don't think that's an improvement. This moves the concern for identifying a message from the gossiper to the controller. Ideally I'd like to pass a Message into the gossiper but with the type information that's lost about it's contents and the requirements to re decode the message I compromised and picked the most descriptive object that contained as much of the information I required as it could.

[rain-on]

Even better:

  public boolean gossipMessage(
      final SignedData<?> signedMessage,
      final Message message)  {
    final Signature msgUniqueId = signedMessage.getSignature();
    if (seenMessages.contains(msgUniqueId)) {
      return false;
    } else {
      final Address receivedFrom = message.getConnection().getPeer().getAddress();
      final List<Address> excludeAddressesList =
          Lists.newArrayList(receivedFrom, signedMessage.getSender());
      peers.multicastToValidatorsExcept(message.getData(), excludeAddressesList);
      seenMessages.add(msgUniqueId);
      return true;
    }
  }

[Errorific]

This doesn't make encapsulation any better, the invoker needs to co-ordinate that the message contains that SignedData.

[rain-on]

If this is reworked as per:
private

void processMessage(
final Message message,
final SignedData

signedPayload,
final Consumer<SignedData

> handleMessage) {
if (processMessage(signedPayload, message)) {
gossiper.gossipMessage(signedPayload.getSignature(),
message.getData(),
signedPayload.getSender(),
signedPayload.getSender(), message.getConnection().getPeer().getAddress());
handleMessage.accept(signedPayload);
}
}

The gossip Biconsumer can be removed, and the msg specific handlers in Gossiper can be removed.

[Errorific]

I don't think this is an improvement. There's nothing wrong with using a Biconsumer. The suggested version makes using the Gossiper require additional co-ordination and moves the responsibility for identifying the message and it's signer to the invoker which is worse encapsulation.

[CjHare]

LGTM