-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
threshold checking for hotshot state prover #1901
Conversation
…o state-prover-startup
…into state-prover-startup
Circuit logic is done. We may still need to alter the stake table design. As every user now has 2 keys. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be great if we can add unit tests incrementally as we build along.
We may still need to alter the stake table design. As every user now has 2 keys.
our current stake table impl is generic over K: Key
type, which could be just a tuple of (BLSVerKey, SchnorrVerKey)
.
also when we are computing the stake table commitment, we can commit over the compressed BLS points concatenated with the uncompressed Schnorr (since we need the full for verification), this will slightly improve the circuit size as we need to hash fewer elements.
(ofc, need consistency inside and outside circuit, luckily, we can hide this optimization under a uniformed abstracted StakeTable::commitment()
API)
/// - the signer's accumulated weight exceeds the quorum threshold | ||
/// - the commitment of the stake table | ||
/// - all schnorr signatures are valid | ||
pub fn build<ST, P>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is fn build_for_preprocessing
instead? (alternatively, maybe via an enum input argument to indicate preprocessing mode or proving mode)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking whether we should only export a single API for proof generation only, w/o exposing these details about circuit.
These interface issues could be finalized when doing proof generation: #1985
|
||
/// HotShot state Variable | ||
#[derive(Clone, Debug)] | ||
pub struct HotShotStateVar { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a From() method converting between HotShotStateVar
and HotshotState
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such From()
also needs to take a circuit as input. Doesn't seems possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…into state-prover-startup
* threshold checking circuit * change some variables to public * comments & tests(TBD) * doing test * test * Adding back the bls_comm * small refactor for test * use `STAKE_TABLE_CAPACITY` * Move state definition to another crate * Formatting * merge imports * augment test & comments * Addressing comments * duplicated items
closes: #1900
Should be simple enough