Skip to content
This repository was archived by the owner on Aug 22, 2023. It is now read-only.

Refactor QC traits #64

Merged
alxiong merged 5 commits into from
Jul 3, 2023
Merged

Refactor QC traits #64

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bfa75ed
reorg qc trait and impl
alxiong Jun 21, 2023
8085561
rename: QCValidation->QC, BitvectorQuorumCertificate->BitVectorQC, ::…
alxiong Jun 21, 2023
a419267
add QC::trace() API
alxiong Jun 21, 2023
1cc2965
remove CheckedType since it's unused (and unclear why we need it)
alxiong Jun 21, 2023
c9429f0
add back check()->Result<QuorumSize>
alxiong Jun 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/qc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ pub trait QuorumCertificate<A: AggregateableSignatureSchemes + Serialize + for<'
/// Type of the actual quorum certificate object
type QC;

/// Type of some auxiliary information returned by the check function in order to feed oth
type CheckedType;

/// Produces a partial signature on a message with a single user signing key
/// NOTE: the original message (vote) should be prefixed with the hash of the stake table.
/// * `agg_sig_pp` - public parameters for aggregate signature
Expand Down Expand Up @@ -67,7 +64,7 @@ pub trait QuorumCertificate<A: AggregateableSignatureSchemes + Serialize + for<'
qc_vp: &Self::QCVerifierParams,
message: &GenericArray<A::MessageUnit, Self::MessageLength>,
qc: &Self::QC,
) -> Result<Self::CheckedType, PrimitivesError>;
) -> Result<(), PrimitivesError>;
Copy link

@nyospe nyospe Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I remember why this is here now. It's not the Checked<T>. And it is needed.

I'm even the person who requested it, I just forgot.

And I think the entire discussion motivating this was on github.

... 90 minutes of searching later...

https://github.com/EspressoSystems/hotshot-primitives/pull/27/files/907f07d85af77fee0ba1b328a77944cdce4e64d5#r1195601782

Short version: we absolutely, non-negotiably, need the CheckedType if we want to be able to use this from HotShot.

Copy link
Contributor Author

@alxiong alxiong Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for digging this comment for us! could you clarify what would the number CheckedType = U256 signify?
the number of votes inside the qc?

Copy link

@nyospe nyospe Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of HotShot, the total number of voted shares, I think...

Copy link
Contributor Author

@alxiong alxiong Jun 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added it back, see if you like c9429f0 @nyospe


/// Trace the list of signers given a qc.
fn trace(
Expand Down
3 changes: 1 addition & 2 deletions src/qc/bit_vector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ where

type QC = (A::Signature, BitVec);
type MessageLength = U32;
type CheckedType = ();

fn sign<R: CryptoRng + RngCore>(
agg_sig_pp: &A::PublicParameter,
Expand Down Expand Up @@ -112,7 +111,7 @@ where
qc_vp: &Self::QCVerifierParams,
message: &GenericArray<A::MessageUnit, Self::MessageLength>,
qc: &Self::QC,
) -> Result<Self::CheckedType, PrimitivesError> {
) -> Result<(), PrimitivesError> {
let (sig, signers) = qc;
if signers.len() != qc_vp.stake_entries.len() {
return Err(ParameterError(format!(
Expand Down