Add DynamicHoneyBadger (part 1).

[afck]

DynamicHoneyBadger is supposed to allow adding and removing nodes. (See #47 (comment)) Internally, it performs Distributed Key Generation and restarts Honey Badger, but all that should be hidden away from the user.

This is still incomplete in several ways, but removing a node works.

[vkomenda]

Can you update the batch size here given that the number of nodes has increased?

[vkomenda]

...or decreased.

[afck]

It depends on the exact requirements; there is still a TODO in the code for that. I created issue #88 for it.

For now, the code does the right thing in the situation of the issue's first bullet point, i.e. where we want a constant block size.

[DrPeterVanNostrand]

I think that a better abstraction here would be to implement a conversion trait instead of using a method (because &self isn't being used):

impl<Tx, NodeUid> From<Input<Tx, NodeUid>> for Transaction<Tx, NodeUid> {
    fn from(input: Input<Tx, NodeUid>) -> Self {
        match input {
            Input::User(tx) => Transaction::User(tx),
            Input::Change(change) => Transaction::Change(change),
        }
    }
}

Then converting from Input -> Transaction could be written:

let tx = input.into();

or, if the type parameters are necessary:

let tx: Transaction<Self::Input, Self::NodeUid> = input.into();

[afck]

You're right, I'll change it. (When I originally wrote the method, I still needed self.)

[DrPeterVanNostrand]

Hmm... this allocates a secret key on stack. I'm not sure what can be done about it though.

I guess we could change self.netinfo.secret_key() to return a pointer to the heap allocated secret-key, and then change SyncKeyGen.sec_key to be of type ClearOnDrop<Box<SecretKey>>.

I'm not sure, but that may be a change that would touch a lot of code, and may be outside the scope of the issue of adding DynamicHoneyBadger.

[afck]

That's true. I wonder whether the right thing to do would be to turn what's currently SecretKey into a private SecretKeyInner, and put a ClearOnDrop<Box<SecretKeyInner>> inside the new SecretKey, so that outside of the crypto module there's no way to circumvent it anymore?

[afck]

I created #89 for it.

[vkomenda]

Would the following code be equivalent to insert_vote and yet shorter?

if self.votes.insert(id, change.clone()).is_none() {
    *self.vote_counts.entry(change).or_insert(0) += 1;
}

[afck]

Not quite. Sorry for my lousy code comments: The idea is that every node can only have one active vote at a time; voting again revokes the previous vote.
So when we insert a vote, we need to increment the count for that vote, and also decrement the count for the previous vote of that node. Not sure how that can be expressed more concisely.

[vkomenda]

One way would be to avoid duplication by removing vote_counts but it does look like a handy optimization. Maybe you could mention this semantics of revoking votes in a comment to Change so that it is visible?

[afck]

No, you're right, removing it is better: the optimization is pointless since it's only computed twice per epoch. Also it required more cloning, and if there are a lot of votes it might even be slower, so I removed it.

[vkomenda]

I suppose there are going to be other messages in this enum. Otherwise it would have been a newtype.

[afck]

Yes, we will need additional messages so that the joining node can take part in the key generation (DKG): It will have to send its Propose and Accepts as messages to the other nodes.

In fact, I think I'll insert this messaging step for all nodes, to prevent censorship. So every node will send its votes and DKG messages first as messages, and everyone who receives a signed message from a peer will propose it in Honey Badger. (Instead of only proposing their own messages, like they do now.)

[afck]

I think something is still wrong with this (or with the test): Shouldn't the to-be-removed node panic here?.
Ah, I found it! I'll push a fix soon…