Open lyulka opened 3 months ago
Most of the things in this issue has been implemented up to the last commit, the last things have to do with the networking
module and the Certificate::is_correct
method (may do the latter in the future instead of now).
Affected version
v0.4 (branch:
dev/v0.4
)Related issues
Definitely related: #49. May be related: #46.
Problem
Cryptography, signed messages, and certificates-related definitions are currently split up into seven modules:
crate::messages
crate::types::keypair
crate::types::validators
crate::types::collectors
crate::state::safety
crate::hotstuff::types
crate::pacemaker::types
.I feel like the way these types are split up leaves a lot to be desired, for several reasons:
QuorumCertificate::is_correct
andBlock::is_correct
on the one hand, andsafe_qc
andsafe_block
on the other:is_correct
pair to only check the cryptographic well-formedness of their respective types, and so their signatures should only take in verifying key(s), just likeSignedMessage::is_correct
. However, as it turns out, both theis_correct
andsafe_
pairs currently take inBlockTree
. The similarity in the signatures of both pairs of functions make the distinction between them unclear.QuorumCertificate::is_correct
are always followed by a call tosafe_qc
, and all calls toBlock::is_correct
are always followed by a call tosafe_block
. The opposite is also true, all calls tosafe_qc
are always preceded by a call toQuorumCertificate::is_correct
, and all calls tosafe_block
are always preceded by a call toBlock::is_correct
. This suggests thatQuorumCertificate::is_correct
should just be called insafe_qc
, andBlock::is_correct
should just be called insafe_block
.ed25519_dalek
are re-exported from two separate modules:crate::types::keypair
,crate::types::validators
. This does not feel right, because:validators
define complex types, not primitives like the three Dalek types it currently re-exports (i.e.,Signature
,SigningKey
, andVerifyingKey
).crate::types::keypair
defines only one type, that isKeypair
. This is out of place when compared to its sibling modules, which defines many types.SignedMessage
,Certificate
, andCollectors
, are all intimately related, yetSignedMessages
is defined at the top-levelcrate::messages
module, whileCertificate
andCollectors
are defined incrate::types
.Proposed solution
Organization
I propose that we reorganize the top-level cryptography, signed messages, and certificates-related definitions into the following six modules:
mod networking
enum Message
enum ProgressMessage
trait Cacheable
mod state::safety
fn safe_block
(callssafe_qc
).fn safe_nudge
(callssafe_qc
).fn safe_qc
(callsQuorumCertificate::is_correct
).mod types::crypto_primitives
struct Keypair
ed25519::Signature
ed25519::SigningKey
ed25519::VerifyingKey
mod types::signed_messages
trait SignedMessage
fn message_bytes
fn signature_bytes
fn is_correct
.trait Certificate
fn message_bytes(&self) -> &[u8]
fn signature_set(&self) -> &SignatureSet
fn is_correct(&self, vk: VerifyingKey) -> bool
(provided)fn quorum(total_power: TotalPower) -> TotalPower
(provided)trait Collector<SignedMessage: SignedMessage, Certificate: Certificate>
fn collect(signer: VerifyingKey, message: Self::SignedMessage) -> Self::Certificate
mod hotstuff::types
struct QuorumCertificate
impl Certificate for QuorumCertificate
struct VoteCollector
impl Collector for VoteCollector
mod pacemaker::types
struct TimeoutCertificate
impl Certificate for TimeoutCertificate
struct TimeoutVoteCollector
impl Collector for TimeoutVoteCollector
Changes
The changes that the above organization makes on the current organization are:
ed25519_dalek
re-exports into a single module:types::crypto_primitives
, enhancing coherence.SignedMessage
,Certificate
, andCollector
in a single module:types::signed_messages
, enhancing coherence.crate::messages
that are related to networking tocrate::networking
, enhancing coherence.Certificate::is_correct
to take in only aValidatorSet
, clarifying the difference in the responsibilities ofis_correct
andsafe_
:is_correct
checks the cryptographic well-formedness of the type, andsafe_
checks everything else, most significantly the properties that can only be checked by reading the block tree.is_correct
fromsafe_
methods, simplifying calling code and allowing us to remove the preconditions of thesafe_
methods.