Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Trusted Setup client binaries #257

Closed
wants to merge 22 commits into from
Closed

Trusted Setup client binaries #257

wants to merge 22 commits into from

Conversation

SupremoUGH
Copy link
Contributor

@SupremoUGH SupremoUGH commented Sep 23, 2022 •
edited
Loading

New:

Before we can merge this PR, please make sure that all the following items have been checked off:

  • Linked to an issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Added one line describing your change in CHANGELOG.md and added the appropriate changelog label to the PR.
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Checked that changes and commits conform to the standards outlined in CONTRIBUTING.md.

@SupremoUGH
wip: commit save point
14dc682 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
Config struct created implementing Ceremony
d104285 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
now it should work
b80229f 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
Features sorted
fc794dd 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
changelog + minor changes
10ed04e 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH SupremoUGH added changelog:added Changelog: add these changes to the `added` section of the changelog A-cryptography Area: Issues and PRs related to Cryptography A-trusted-setup Area: Issues and PRs related to Trusted Setup labels Sep 26, 2022
@SupremoUGH SupremoUGH self-assigned this Sep 26, 2022
@SupremoUGH SupremoUGH marked this pull request as ready for review September 26, 2022 15:00
@SupremoUGH
fmt
2e10dbf 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
Cleaner features. Improved error handling
7a84fb5 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
bhgomes
bhgomes suggested changes Sep 26, 2022
View reviewed changes
Copy link
Contributor

@bhgomes bhgomes left a comment

Choose a reason for hiding this comment

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

Also, please add a GH issue for removing the duplicate code when writing a configuration.

@SupremoUGH
Comments addressed
bdf93a3 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH SupremoUGH requested a review from bhgomes September 27, 2022 10:27
@SupremoUGH SupremoUGH mentioned this pull request Sep 27, 2022
Open
GhostOfGauss
GhostOfGauss reviewed Sep 27, 2022
View reviewed changes
Copy link
Contributor

@GhostOfGauss GhostOfGauss left a comment

Choose a reason for hiding this comment

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

Are there any interesting tests we can add to this?

bhgomes and others added 6 commits September 27, 2022 11:57
@bhgomes
feat: add basic ContributeResponse mechanism
6e56204 
Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
@bhgomes
wip: commit save point
a1130f6 
Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
@bhgomes
feat: add basic challenge hash response
f9f69a2 
Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
@GhostOfGauss
feat: deserialize state unchecked
bfeb7f6
@GhostOfGauss
merge commits
3c12c7a
@GhostOfGauss
fix: missing docs
0f28444
@SupremoUGH
Loop in Client::build
e1ebf1c 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
Todd's comments addressed
03221a5 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
contribution_hash in terms of response
72e3f17 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
comments addressed
4814cf3 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
Better messages for waiting clients
57d3b6f 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH
clippy issues
1ae6d7f 
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
GhostOfGauss
GhostOfGauss reviewed Sep 28, 2022
View reviewed changes
Copy link
Contributor

@GhostOfGauss GhostOfGauss left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think that since we don't have any sort of progress bars we need to at least number our messages like in here so that the user feels like they're making progress.

Maybe good intervals to mark would be:

  1. /5 "Connecting to server for Metadata" and "waiting for registry to update"
  2. /5 "Waiting in Queue, you are number ... "
  3. /5 "Receiving data from Coordinator"
  4. /5 "Computing contribution"
  5. /5 "Waiting for confirmation from Coordinator"

Then Finished, show tweet prompt

@bhgomes @GhostOfGauss
Apply suggestions from code review
b6f2f12 
Co-authored-by: Todd Norton <93260651+GhostOfGauss@users.noreply.github.com>
Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
@SupremoUGH @GhostOfGauss
Update manta-trusted-setup/src/groth16/ceremony/client.rs
13c92a2 
Co-authored-by: Todd Norton <93260651+GhostOfGauss@users.noreply.github.com>
Signed-off-by: Francisco Hernández Iglesias <38819712+SupremoUGH@users.noreply.github.com>
bhgomes
bhgomes suggested changes Sep 29, 2022
View reviewed changes
manta-trusted-setup/src/groth16/ceremony/config/ppot.rs
Comment on lines +424 to +426
println!(
"Success! You have contributed to the security of Manta Pay! \n Now set your contribution in stone! Tweet:\n\"I made contribution number {} to the #MantaNetworkTrustedSetup! My contribution's hash is {:?} \"",
response.index, C::contribution_hash(&response)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should return the hexstring representation. Use https://docs.rs/hex/latest/hex/ hex::encode.

manta-trusted-setup/src/groth16/mpc.rs
Comment on lines +75 to +77
P: Pairing<G1 = GroupAffine<R1>, G2 = GroupAffine<R2>> + ?Sized,
R1: SWModelParameters,
R2: SWModelParameters,
Copy link
Contributor

Choose a reason for hiding this comment

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

These constraints should go on the check function.

manta-trusted-setup/src/groth16/mpc.rs
}
}

fn curve_point_checks<P>(p: &GroupAffine<P>) -> Result<(), SerializationError>
Copy link
Contributor

Choose a reason for hiding this comment

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

Add documentation and #[inline] marker, and probably move to manta_crypto::arkworks::serialize.

@bhgomes bhgomes mentioned this pull request Sep 29, 2022
Closed
@bhgomes
Copy link
Contributor

bhgomes commented Sep 29, 2022

Closed in favor of #259.

@bhgomes bhgomes closed this Sep 29, 2022
@bhgomes bhgomes deleted the feat/client-server-bin branch January 10, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhgomes bhgomes bhgomes requested changes

@GhostOfGauss GhostOfGauss Awaiting requested review from GhostOfGauss

Assignees

@SupremoUGH SupremoUGH

Labels
A-cryptography Area: Issues and PRs related to Cryptography A-trusted-setup Area: Issues and PRs related to Trusted Setup changelog:added Changelog: add these changes to the `added` section of the changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SupremoUGH @bhgomes @GhostOfGauss