-
Notifications
You must be signed in to change notification settings - Fork 0
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
Spark modified frost implemenation #1
Open
zhenlu
wants to merge
6
commits into
secp256k1-tr-base
Choose a base branch
from
spark-frost
base: secp256k1-tr-base
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
52d022f
Add sign_spark function for round 2
zhenlu 6845a63
Aggregation and api functions
zhenlu cbc140e
Fix challenge
zhenlu cb3dc00
Add the functions to frost-secp256k1
zhenlu 50f2051
Make it work with secp256k1-tr
zhenlu 115983a
Fix aggregate_spark
zhenlu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Ah I see your approach here is to assume we have two distinct sets of signer IDs, the inner and the outer sets. This should work, but presently the code assumes the two sets don't intersect.
Let's say we set
outer_coef_set_x = {1, 2}
andinner_coef_set_x = {1, 2, 3, 4, 5, ...}
. Signer1
is the SE and signer2
is the user. Thesigning_package
stores all commitments, inner and outer, in aBTreeMap<Identifier<C>, round1::SigningCommitments<C>>
.If the inner signer (SO) with identifier
2
participates in the signing session, their commitment will overwrite the user's commitment, or vise-versa. Both commitments can't exist in the BTree at once.My first thought is to adjust the structure of the
SigningPackage
orSigningParameters
to include the outer signers' commitments as an extra field, rather than lumping them in with the inner signers' commitments.How do you think that we should handle this?
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.
Actually I should include this script on how to use this to make this more clear
https://gist.github.com/zhenlu/6183156fd6151bc82dedca1f6888c24e
So my approach is that each signer have 2 identifiers here,
inner_id
andouter_id
. TheSigningCommitments
here is only using theinner_id
, so theinner_id
should be different for each signers. However, theouter_id
is only used for calculate the Lagrange coefficient for the second round, so it's ok to intersect with inner.In the above script, I have the inner_ids for the group as
1, 2, ..., n
, and assume user's inner_id as100
(Note this is only a hack for now, assuming we have less than 100 participants in the group.) And always set group's outer_id to 1, user's outer_id to 2 (I hardcoded this since our use case only has two participants in this round).I think your approach might work better since we don't need these extra parameters if we include the outer signers' commitments as an extra field, (probably still need the outer signer's identifier to be able to calculate Lagrange coefficient for outer signers). My approach here is just to make the minimum change needed to test this works.
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.
Makes sense, as long as we're aware of that limitation and can guard against it 👍