Allowed adversary to control message delivery

[vkomenda]

Part of #45. Also required by #37.

The adversary fully controls message delivery.

[afck]

You should be able to avoid the cloning with:

if let TargetedMessage {
    target: Target::All,
    ref message,
} = msg.tm

[afck]

I think we should at least make sure that the original message, if it was from a correct to a correct node, is unchanged. E.g. we could just give a reference to push_message, and dispatch its outputs in addition to the original one.

Also, we should probably assert that all the adv_msgs have as sender one of the adversary's nodes: The adversary is not able to forge, modify or indefinitely block messages from a different sender.

[vkomenda]

For #37, push_message can withhold the message within the adversary and return an empty vector. In that case nothing should be dispatched. In the current tests this is still not implemented. That's why all push_messages return the same message. But it is not the case in general.

[vkomenda]

What you propose for adv_msgs would not allow implementing #37. adv_msgs is the result of the adversarial message transformation such as reordering of messages from correct nodes.

[afck]

But maybe reordering should be handled differently: by allowing adversaries to flip entries in the nodes' incoming queues? That, in combination with picking the next node to handle a message, should allow the adversary to do everything they are allowed to do.
(Alternatively, I think @mbr suggested to have just one queue for all the nodes, where entries have both sender and recipient IDs. Reordering that queue would replace both of the above: It allows the adversary to choose message order and the next node.)

At least I think that functionality should be separate from the method that informs the adversary about a message arriving in one of their own nodes. (Which is all that push_message currently does, I think. Not a great name, admittedly. 😬 )

[vkomenda]

I also think one queue for all nodes would be great. This PR is in that direction. The adversary owns that queue. Do we really need removing incoming queues of nodes? If so, can we remove the incoming queues in a separate PR? This would require modifying the current scheduling mechanism which is at the moment based on picking nodes and delivering all queued incoming messages to it. With the single queue, we could remove those messages from it which have the scheduled node as the recipient.

The alternative of the adversary directly modifying nodes (that is, their incoming queues) sounds more complicated. It would need to keep the order of all messages across all individual queues somehow.

[afck]

This would require modifying the current scheduling mechanism which is at the moment based on picking nodes and delivering all queued incoming messages to it.

Exactly; it would replace pick_node with something like shuffle_queue, and we could keep push_message in its current form. (Or later replace it with something else, too — I think one idea was to actually instantiate correct nodes for the adversary, and just allow the adversary to modify their incoming and outgoing messages?)

[vkomenda]

If we keep push_message in its current form, where do you think the messages should be pushed onto the single queue? shuffle_queue is a great idea. I don't quite see where the queue itself comes from in your proposal though.

[afck]

I was thinking that the TestNetwork itself would own the queue, and only allow the adversary to move messages inside it. (Not sure how the Adversary interface would have to look for that… maybe a method that gets a non-mutable reference to the queue and outputs a list of positions, i.e. a permutation?) That would make it impossible to write an adversary that cheats by removing or modifying entries.

But I guess it's also okay to do something simpler, and just make sure all our adversaries behave.

[vkomenda]

I'm maybe OK with the permutation interface. However, if we need another kind of attack in the future, we would probably rewrite the queue to have mutable access rather than add another interface to it.

@mbr, what do you think, does the single queue have to be owned by Adversary, in which case we have to ensure adversaries don't perform unexpected transformations on the queue, or should the queue be owned by TestNetwork with specialized interfaces for particular kinds of adversarial transformations?

[afck]

Or should that even be a different method? This one is for messages sent to the adversary, while the other invocation is kind of eavesdropping on any communication.

[vkomenda]

Or we could rename push_message. It is more of a transform_message now.

[afck]

I actually think it would be better to postpone the whole issue until we completed the API change, and the tests have been rewritten.

[vkomenda]

Closing in expectation of the ongoing work on #84.