search

poanetwork / hbbft

An implementation of the paper "Honey Badger of BFT Protocols" in Rust. This is a modular library of consensus.
Other
357 stars 96 forks source link

Optimization: output early from Subset? #170

Closed afck closed 6 years ago

afck commented 6 years ago

Subset could output multiple times: Once for each accepted contribution, and then once more when it terminates, to say that the remaining contributions have been rejected. That way, Honey Badger can start exchanging decryption shares sooner, and by the time the agreement instances for lagging nodes' contributions finally output false, the accepted ones will already be decrypted and ready to be output.

(We should measure, of course, whether it actually improves performance and whether it's worth it.)

What needs to be changed

The Subset output needs to be an enum with two variants, e.g. Contribution(Vec<u8>) and Done.

Instead of outputting all accepted contributions when all have been decided, we should output a single Contribution(_) as soon as it gets accepted, and then only output an empty Done once all have been decided.

In Honey Badger, we wouldn't set the SubsetState to Complete on a Contribution(_) output, but only on Done.

The simulation example should be run a few times with and without the change, and maybe with different parameters, to test whether it improves performance.

DemiMarie commented 6 years ago

I will take this one.

DemiMarie commented 6 years ago

I am slightly confused: if I return from https://github.com/poanetwork/hbbft/blob/422d8ef55bbc658a6346eefa828e784963959426/src/subset.rs#L255-L257 the compiler (rightly) warns about unreachable code. Where should the code that comes afterward be placed?

(Sorry, I know very little about the codebase)

c0gent commented 6 years ago

Could you clarify your question please and give an example of exactly what you're trying to do?

vkomenda commented 6 years ago

@DemiMarie, if you return from the if let Some(output) clause then value needs to become a function, not a let-binding, because the else clause also returns. Hence the code after value is unreachable. Please correct if I understood something wrong.

afck commented 6 years ago

How confident is everyone that this doesn't hurt censorship resistance? I imagine it shouldn't make a difference, because even though the decrypted content of one accepted contribution can now influence your vote on the others, you never know the contents of any contribution while you can still influence its outcome.

But the fact that I can't imagine a problem doesn't mean there is none. (No need to hold off with merging this, fo course: If we have doubts, we could always make this a configuration option later.)

c0gent commented 6 years ago

I can't think of a problem either but since this change may not be that much of an optimization anyway I think keeping it an option or build feature is best.

afck commented 6 years ago

As discussed let's make it a runtime option. This will need to be passed through from the HoneyBadger builder down to Subset.