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.

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

upgrade gnark/mpc tools #2579

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open

upgrade gnark/mpc tools #2579

wants to merge 38 commits into from

Conversation

hussein-aitlahcen
Copy link
Contributor

No description provided.

@hussein-aitlahcen hussein-aitlahcen changed the title Upgrade gnark mpc upgrade gnark/mpc tools Jul 26, 2024
@hussein-aitlahcen hussein-aitlahcen marked this pull request as ready for review July 29, 2024 21:16
Copy link

github-actions bot commented Jul 29, 2024

App 🤌

🌎 Deploying...
✨ Deployment complete! Take a peek over at https://37bfa110.app-1b1.pages.dev


tools/rust/rust.nix Outdated Show resolved Hide resolved
@hussein-aitlahcen
Copy link
Contributor Author

Awaiting Consensys/gnark#1230 to rebase and merge.

Copy link

vercel bot commented Sep 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
site ⬜️ Ignored (Inspect) Visit Preview Sep 21, 2024 11:07am


This project contains the client and coordinator to conduct Groth16 multi-party computation for the circuit SRS.
Three components are in play:
- Supabase : host the state machine in postgresql and exposes api and storage services to upload contributions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Supabase : host the state machine in postgresql and exposes api and storage services to upload contributions.
- Supabase: host the state machine in postgresql and exposes api and storage services to upload contributions.

- `POST /contribute` a `Contribute` object in body. Returns :
- a `202 Accepted` if the contribution started.
- a `503 Unavailable` if the client is busy (likely already contributing).
- `GET /contribute` returns :
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `GET /contribute` returns :
- `GET /contribute` returns:

if is_already_successful().await {
return Ok(());
}
let mut secret_key = if let Ok(_) = tokio::fs::metadata(CONTRIB_SK_PATH).await {
Copy link
Contributor

Choose a reason for hiding this comment

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

clippy will probably tell you to use .is_ok() here

Comment on lines +446 to +448
if lock
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
.is_ok() =>
Copy link
Contributor

Choose a reason for hiding this comment

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

writing to an atomic bool in an if guard is interesting, i would recommend pulling this out into the arm expression for clarity/ least surprise

Comment on lines +577 to +602
tokio::select! {
Ok((stream, _)) = listener.accept() => {
let io = TokioIo::new(stream);
let status_clone = status_clone.clone();
let tx_status_clone = tx_status_clone.clone();
let lock_clone = lock.clone();
let conn = hyper::server::conn::http1::Builder::new().serve_connection(
io,
service_fn(move |req| {
handle(
lock_clone.clone(),
tx_status_clone.clone(),
status_clone.clone(),
req,
)
}),
);
let fut = graceful.watch(conn);
tokio::task::spawn(async move {
let _ = fut.await;
});
}
_ = token_clone.cancelled() => {
break;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

match client
.current_contributor()
.await?
.ok_or(Error::ContributorNotFound)
Copy link
Contributor

Choose a reason for hiding this comment

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

why not match on the Option directly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants