Make Subset optimization optional.

[afck]

Let's make the optimization introduced in #170 an option, configurable per Honey Badger instance. We should allow disabling it to

• allow behavior exactly as specified in the paper,
• make it easy to compare performance with and without it,
• enable other features that might conflict with it, like Remove the randomness feature from Honey Badger. #183.

[ErichDonGubler]

I'd be more than happy to tackle this! Just wanting to confirm: is the scope of this issue ONLY to make this optimization a knob to turn?

[afck]

Thank you! Yes, it should be an option in the DynamicHoneyBadgerBuilder.

Since it's basically a per-epoch setting, it could also be made dynamic itself later, and reconfigurable at runtime, as part of #103.

[ErichDonGubler]

Okay! This should be fun -- I haven't dealt with blockchain tech at all before, so I'll just disclaim now that I'm unfamiliar with the domain. :) Would it be better to implement the optimization behavior inside of EpochState or the Subset algorithm? I can't really tell immediately if Subset actually does anything other than handle incoming messages (i.e., sends messages across the network), and if EpochState is what actually sends messages out then it might be cleaner to implement the behavior switching there instead of in Subset. I'm experimenting with how to make it work at the Subset level right now given my uncertainty above, but am more than happy to change my approach.

[vkomenda]

@ErichDonGubler, can I suggest you trying to start from HoneyBadger? Then trace how the new config option takes effect on Subset. Ideally the option should be configurable in HoneyBadger, DynamicHoneyBadger and QueueingHoneyBadger which are the user-facing consensus instance types. Also, should the user wish to instantiate Subset directly, they should be able to configure that option too.

[afck]

Note that the optimization itself is already implemented since #233.
This is just about making it optional (and then, possibly, configurable at runtime).

None of the algorithms do any networking: They just have methods to handle messages, which return any messages that need to be sent to other instances. Networking is handled by the user. Since HoneyBadger uses Subset, the HoneyBadger messages wrap the Subset messages, and add information like which epoch's Subset instance the message belongs to.

I agree with @vkomenda: the change will probably mostly affect the honey_badger module.

[ErichDonGubler]

@afck:

Note that the optimization itself is already implemented....

Sorry, yes, I understood that the optimization had already been implemented; s/implement the optimization behavior/implement the optimization behavior **toggle**/. :)

@vkomenda: I actually have a mostly-coded implementation going down from the HoneyBadger and its builder to Subset, I just need to tweak the DynamicHoneyBadgerBuilder, fix tests, and I think I'll be ready for at least a WIP review -- it doesn't seem that QueueingHoneyBadger needs any changes beyond recompilation once DynamicHoneyBadgerBuilder is changed, since its new method signature is pub fn new(dyn_hb: DynamicHoneyBadger<Vec<T>, N>) -> Self.

I'm sure I'll be missing things implementing this, and really do appreciate the fast response here! :)

[afck]

going down from the HoneyBadger and its builder to Subset

I'm not sure whether Subset needs to change at all: It might be simpler to just collect Subset's outputs in HoneyBadger until we receive Done, and only then send the decryption shares.

I'm sorry I didn't think of this earlier and gave the wrong impression in the description: I don't think it's desirable to make Subset itself configurable, since whether it returns the outputs as soon as it knows them or not is only a local difference and doesn't change the message flow at all. The important part here is that HoneyBadger doesn't send the decryption shares before the Done if the optimization is turned off.

(But if the code is simpler with the change in Subset, that's a possibility, too, of course.)

[ErichDonGubler]

@afck: Got it. That's why I asked about the outgoing message flow -- it SEEMED like the only time EpochState and Subset together were sending messages was with the decryption share sending. I think it'd be much cleaner to allow whatever consumes Subset to determine if it will delay or immediately send out a message about a Subset contribution by itself -- the incremental optimization allows client code to do whatever it wants, so I think that Subset itself shouldn't actually change.

I changed Subset to be conservative with its API, but I'll move those changes to EpochState.

[ErichDonGubler]

As an addendum: I'd be more than happy to make a [WIP] PR to show what I'm doing -- if anybody is interested, just let me know and I'll get one going.

[afck]

Sure, I'm happy to take a look.