-
Notifications
You must be signed in to change notification settings - Fork 41
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
fnv/core verifier #939
fnv/core verifier #939
Conversation
Test Results 3 files ±0 17 suites +1 7m 5s ⏱️ +19s Results for commit 22972f7. ± Comparison against base commit 09dc5e6. This pull request removes 2 and adds 4 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
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.
Good overall. Left some comments. However, I'm not convinced in the naming. verify_avk
could be interpreted as verifying the aggregate key. Maybe instead of verify
and verify_avk
, we could call core_verify
and verify
. And both functions should have good documentation explaining their differences.
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.
Made another pass. Still haven't looked at the tests. I would also make the suggestion of not using SignerAvk, as it does not seem clear. I would probably distinguish the two signers (and any other two structures) with the core
keyword. SignerCore for the one that does not need the merkle tree, and Signer for the one that needs it.
Will go over the tests once the comments are addressed. Good work!
Co-authored-by: Iñigo Querejeta Azurmendi <31273774+iquerejeta@users.noreply.github.com>
538688f
to
6824cbe
Compare
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.
With the tests passing, this implies that the serialisation is backwards compatible. It looks good to me 👍
Content
A full node verifier feature that does not require checking signers' identities externally.
Pre-submit checklist
Comments
The idea is to provide a core verifier (previously full-node verifier) that has
and be able to verify the signatures that are generated without the registration information, i.e.,
avk
.In this case, the signer who generates this signature has all the fields of
StmSigner
except fromclosed_reg
.So,
closed_reg
field is updated asOption
. If the signer wants to participate in Mithril certificate generation, then it must have theclosed_reg
and this signer is generated with the functionStmInitializer::new_signer()
. Otherwise, the signer can only generate signatures to be validated by a core verifier. We call this signer core_signer, and it is created with the functionStmInitializer::new_core_signer()
. Note that in this version the core signer is built upon theStmInitializer,
but it does not need to be built upon theStmInitializer
depending on the functionality it provides.StmSigner
now provides two signing functions, one for a core signature and one for a signature to participate in certificate generation.A core signature is generated by
StmSigner::core_sign()
function without closed registration and can be verified by the core verifier. This function simply creates a signature for a given message.StmSigner::sign()
function callscore_sign()
withmsgp = (Commitment || msg)
, and it produces a signature for the Mithril certificate.Similarly,
StmSig
is updated to providecore_verify
: Verifiessigma
and checks the indices. This function verifies theStmSig
s to beverified by full node verifier.
verify
: Runscore_verify
withmsgp
. This function is responsible for verifyingStmSig
s for mithril functionality.A new struct
StmSigRegParty
includingStmSig
andRegParty
is created. Consequently, the fieldsignatures
ofStmAggrSig
is now of the formVec<StmSigRegParty>
instead ofVec<(StmSig, RegParty)>
. This change is done to avoid duplicate codes since the core verifier andStmAggrSig
have many common operations.A struct
CoreVerifier
containingeligible_parties
andtotal_stake
is created. Core verifier implements the following:setup
: Collects unique signers ineligible_parties
, calculates thetotal_stake
, and returns theCoreVerifier
.dedup_sigs_for_indices
: This function is adopted fromStmClerk::dedup_sigs_for_indices
, and the new function can be used by bothStmClerk
andCoreVerifier
.preliminary_verify
: Verify all signatures whether they all won the lottery, if the indices are unique, and if the quorum is achieved.collect_sigs_vks
: Collect and return the signatures and verification keys for given lists ofStmSigRegParty
.verify
: Maps the signatures with the eligible parties bymap_sig_party
. Runsdedup_sigs_for_indices
to get unique signatures. After runningpreliminary_verify
, it collects sigs and vks withcollect_sigs_vks
and verifies the aggregate.The functions of
CoreVerifier
is designed to be used byStmAggrSig
since they have similar operations.StmAggrSig::preliminary_verify
verifies all checks from signatures except for the signature verification itself.Indices and quorum are checked by
CoreVerifier::preliminary_verify
withmsgp
.It collects leaves from signatures and checks the batch proof. After the batch proof is checked, it collects and returns the signatures and verification keys to be used by aggregate verification.
Minor changes:
Error enums are updated for new functionality.
to_bytes
andfrom_bytes
functions are implemented forStmSigRegParty
, and they are used inStmAggrSig
. Unused functionsStmSigner::get_closed_reg()
andStmSigner::compute_avk()
are removed.ToDo
CoreVerifier
.StmClerk::dedup_sigs_for_indices()
and its related tests.CoreVerifier::dedup_sigs_for_indices()
.CoreVerifier
.CoreVerifier
.Issue(s)
Closes #839