Conversation
ce13e50 to
04e295c
Compare
…he vendored crypto backend impl
…end impl and sync the patching
fc72b43 to
6b63a53
Compare
6b63a53 to
1d7b04d
Compare
| [patch.crates-io] | ||
| sha2-v0-10-9 = { git = "https://github.com/sp1-patches/RustCrypto-hashes", package = "sha2", tag = "patch-sha2-0.10.9-sp1-4.0.0" } | ||
| sha3-v0-10-8 = { git = "https://github.com/sp1-patches/RustCrypto-hashes", package = "sha3", tag = "patch-sha3-0.10.8-sp1-4.0.0" } | ||
| crypto-bigint = { git = "https://github.com/sp1-patches/RustCrypto-bigint", tag = "patch-0.5.5-sp1-4.0.0" } |
There was a problem hiding this comment.
I found that in rsp they didn't patch crypto-bigint, do we remember why we have this?
1d7b04d to
d067ef9
Compare
jsign
left a comment
There was a problem hiding this comment.
LGTM! Great PR, exciting to see more progress on crypto backends
| expected_public_values.resize(32, 0); | ||
| } | ||
|
|
||
| if matches!(zkvm_kind, zkVMKind::Zisk) && expected_public_values.len() < 256 { |
There was a problem hiding this comment.
I think these kind of padding we have to do today should probably go away after zkVMs comply the to the zkVM standards. Although I'm not entirely sure we are being very explicit about it as part of the standard.
Context (Han correct me if wrong): whatever the guest program wrote as public input isn't exactly the same thing we get from the SDK but padded. I think ideally we get exactly the same thing to not having to pad the expected output to what the SDK returns. cc @kevaundray @marcinbugaj
There was a problem hiding this comment.
whatever the guest program wrote as public input isn't exactly the same thing we get from the SDK but padded
Yes, even we only write less than 256 bytes, the output is alwasy fixed 256 bytes (same for Airbender and OpenVM, but for them it's 32 bytes), but the unwritten parts will remain zeros. Agree that ideally the output should be identical to what the guest writes.
There was a problem hiding this comment.
Moving to standards should improve the situation. Zisk is actively working on adopting IO and accelerators standards.
| @@ -1,43 +1,56 @@ | |||
| // Copied from https://github.com/axiom-crypto/openvm-reth-benchmark/blob/openvm-v1.4.1-reth-1.8.3/crates/revm-crypto/src/lib.rs | |||
|
|
|||
| //! Copied and modified from https://github.com/axiom-crypto/openvm-eth/blob/938d3c0/crates/revm-crypto/src/lib.rs. | |||
There was a problem hiding this comment.
I hope OpenVM can also start moving more into the C accelerators standard since this is hairy.
There was a problem hiding this comment.
Can't use it directly because the alloy-consensus vesion the upstream dpends on is older which doesn't implement the CryptoProvider::verify_and_compute_signer_unchecked :(
…rsion in `Cargo.lock`
8b056b2 to
2ba20af
Compare
Update
eretov0.5.0, which updates zkVMs:v1.4.2tov1.4.3v5.2.4tov6.0.1v0.15.0tov0.16.0Because the patching crates of ZisK seems not available for
v0.16.0and they are moving to use crypto backend for both Reth and Ethrex, so this PR also update Ethrex to an unreleased revision that has crypto backend supports. Also thelibsszhassha2that's now not patched for ZisK, I'd expect the cost ratio of it will increase a bit.