poanetwork / hbbft

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

Make non-threshold crypto in SyncKeyGen pluggable. #424

Closed afck closed 5 years ago

afck commented 5 years ago

It uses default arguments so that existing code does not need to be changed.

afck commented 5 years ago

I'm unsure; please let me know your opinion:

vkomenda commented 5 years ago
  • Is it overengineering to make Part<C> and Ack<C> generic in the ciphertext type C? Almost all crypto implementations will probably just use C = Vec<u8> there, so we might as well hard-code it, and include serialization in the PublicKey/SecretKey implementations?

I think C can be hardcoded as Vec<u8> the same way as ProposedValue in subset. We have other examples of hardcoded Vec<u8>. That might rule out some character encodings though but there is an argument that in fact sounds more in favour of using Vec<u8> in cryptography than text strings.

  • I'll probably update DynamicHoneyBadger accordingly, too. Or is it better to keep it simple, because it's going to be used as a reference implementation rather than directly?

Maybe for the reference sake you could show how DHB would use pluggable SyncKeyGen. But that could be a PR of its own.

afck commented 5 years ago

Done. The disadvantage with this approach is that serialization and deserialization of the BLS ciphertext are now part of the PublicKey/SecretKey trait implementation. That means those traits have an associated Error type, and with BLS we do some redundant serialization, i.e. serialize Ciphertext into a Vec<u8> that is part of a message that gets serialized later. But I guess it's still simpler this way.

dforsten commented 5 years ago

The EngineSigner passed to Parity Consensus Engines only exposes "sign" and "address" functions, which will not be sufficient for general-purpose encryption/decryption. We will have to acquire the signer's keypair through other means, e.g. from an account provider, which allows access to the secret.

As soon as we have access to the secret in Parity I see no problem implementing SyncKeyGen's SecretKey and PublicKey traits.