-
Notifications
You must be signed in to change notification settings - Fork 92
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
feat(distributed_sp1): first working version of sp1 distributed prover #302
base: main
Are you sure you want to change the base?
Changes from all commits
d8738fd
d890b6a
76813bb
dcd6bc5
09fb775
e258346
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,9 +57,10 @@ risc0-build = { version = "1.0.1" } | |
risc0-binfmt = { version = "1.0.1" } | ||
|
||
# SP1 | ||
sp1-sdk = { git = "https://github.com/succinctlabs/sp1.git", branch = "main" } | ||
sp1-zkvm = { git = "https://github.com/succinctlabs/sp1.git", branch = "main" } | ||
sp1-helper = { git = "https://github.com/succinctlabs/sp1.git", branch = "main" } | ||
sp1-sdk = { git = "https://github.com/succinctlabs/sp1.git", rev = "dd032eb23949828d244d1ad1f1569aa78155837c" } | ||
sp1-zkvm = { git = "https://github.com/succinctlabs/sp1.git", rev = "dd032eb23949828d244d1ad1f1569aa78155837c" } | ||
sp1-helper = { git = "https://github.com/succinctlabs/sp1.git", rev = "dd032eb23949828d244d1ad1f1569aa78155837c" } | ||
|
||
|
||
# alloy | ||
alloy-rlp = { version = "0.3.4", default-features = false } | ||
|
@@ -189,3 +190,15 @@ revm-primitives = { git = "https://github.com/taikoxyz/revm.git", branch = "v36- | |
revm-precompile = { git = "https://github.com/taikoxyz/revm.git", branch = "v36-taiko" } | ||
secp256k1 = { git = "https://github.com/CeciliaZ030/rust-secp256k1", branch = "sp1-patch" } | ||
blst = { git = "https://github.com/CeciliaZ030/blst.git", branch = "v0.3.12-serialize" } | ||
|
||
# Patch Plonky3 for Serialize and Deserialize of DuplexChallenger | ||
p3-field = { git = "https://github.com/Champii/Plonky3.git", branch = "serde_patch" } | ||
p3-challenger = { git = "https://github.com/Champii/Plonky3.git", branch = "serde_patch" } | ||
p3-poseidon2 = { git = "https://github.com/Champii/Plonky3.git", branch = "serde_patch" } | ||
p3-baby-bear = { git = "https://github.com/Champii/Plonky3.git", branch = "serde_patch" } | ||
p3-symmetric = { git = "https://github.com/Champii/Plonky3.git", branch = "serde_patch" } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be added to the taikoxyz org, or maybe a taikoxyz-patches special org to avoid pollution if across Raiko and Gwyneth we expect lots of long-term patches. cc @Brechtpd There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Could anybody fork |
||
|
||
# Patch Plonky3 for Serialize and Deserialize of DuplexChallenger | ||
[patch."https://github.com/succinctlabs/sp1.git"] | ||
sp1-sdk = { path = "../sp1/sdk" } | ||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -35,6 +35,21 @@ pub struct Opts { | |||||||
/// [default: 0.0.0.0:8080] | ||||||||
address: String, | ||||||||
|
||||||||
#[arg(long, require_equals = true, default_value = "0.0.0.0:8081")] | ||||||||
#[serde(default = "Opts::default_sp1_worker_address")] | ||||||||
/// Distributed SP1 worker listening address | ||||||||
/// [default: 0.0.0.0:8081] | ||||||||
sp1_worker_address: String, | ||||||||
|
||||||||
#[arg(long, default_value = None)] | ||||||||
Champii marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
/// Distributed SP1 worker orchestrator address | ||||||||
/// | ||||||||
/// Setting this will enable the worker and restrict it to only accept requests from | ||||||||
/// this orchestrator | ||||||||
/// | ||||||||
/// [default: None] | ||||||||
sp1_orchestrator_address: Option<String>, | ||||||||
|
||||||||
#[arg(long, require_equals = true, default_value = "16")] | ||||||||
#[serde(default = "Opts::default_concurrency_limit")] | ||||||||
/// Limit the max number of in-flight requests | ||||||||
|
@@ -88,6 +103,10 @@ impl Opts { | |||||||
"0.0.0.0:8080".to_string() | ||||||||
} | ||||||||
|
||||||||
fn default_sp1_worker_address() -> String { | ||||||||
"0.0.0.0:8081".to_string() | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
AFAIK in some cases 0.0.0.0 is/was use for broadcasting so I think it should be clarified it's a placeholder until we have a valid address. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the listening address of the worker socket, I always saw this format to indicate to listen on every interfaces. I'll do some more research but that's a sensible default in my opinion. |
||||||||
} | ||||||||
|
||||||||
fn default_concurrency_limit() -> usize { | ||||||||
16 | ||||||||
} | ||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,6 +4,7 @@ use raiko_lib::{ | |||||
prover::{IdStore, IdWrite, Proof, ProofKey, Prover, ProverConfig, ProverError, ProverResult}, | ||||||
}; | ||||||
use serde::{Deserialize, Serialize}; | ||||||
pub use sp1_sdk::serve_worker; | ||||||
use sp1_sdk::{ | ||||||
network::client::NetworkClient, | ||||||
proto::network::{ProofMode, ProofStatus, UnclaimReason}, | ||||||
|
@@ -50,8 +51,9 @@ impl Prover for Sp1Prover { | |||||
let local = true; | ||||||
let proof = match local { | ||||||
true => { | ||||||
let proof = client | ||||||
.prove(&pk, stdin) | ||||||
let prove_action = client.prove(&pk, stdin); | ||||||
let proof = prove_action | ||||||
.run() | ||||||
.map_err(|_| ProverError::GuestError("Sp1: proving failed".to_owned()))?; | ||||||
Ok::<_, ProverError>(proof) | ||||||
} | ||||||
|
@@ -113,9 +115,9 @@ impl Prover for Sp1Prover { | |||||
}?; | ||||||
|
||||||
// Verify proof. | ||||||
client | ||||||
.verify(&proof, &vk) | ||||||
.map_err(|_| ProverError::GuestError("Sp1: verification failed".to_owned()))?; | ||||||
client.verify(&proof, &vk).map_err(|e| { | ||||||
ProverError::GuestError(format!("Sp1: verification failed: {:#?}", e).to_owned()) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
})?; | ||||||
|
||||||
// Save the proof. | ||||||
let proof_dir = | ||||||
|
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 should use the v1.0.1 tag instead of the commit hash, it would be easier for future maintenance as people won't try to hunt "what is the specific feature we depend on that was added in that commit?"
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 waiting for taikoxyz/sp1#1 to be merged in the
taiko
branch and I'll use that particular revision as a source instead, as the tag v1.0.1 will be obsolete. Should we add our own versioning system on top of sp1's and refer to that ?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
v1.0.1-taiko-1
, you're the SP1 maintainer now ;). cc @smtmfft @Brechtpd @petarvujovic98