-
Notifications
You must be signed in to change notification settings - Fork 105
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(core): Plonky2 proofs onchain verification (part 1) #4442
base: master
Are you sure you want to change the base?
Conversation
fa3b359
to
971d106
Compare
utils/wasm-proc/src/main.rs
Outdated
@@ -114,6 +114,8 @@ const RT_ALLOWED_IMPORTS: [&str; 76] = [ | |||
// From `GearBls12_381` | |||
"ext_gear_bls_12_381_aggregate_g1_version_1", | |||
"ext_gear_bls_12_381_map_to_g2affine_version_1", | |||
// From `PoseidonHash` | |||
"ext_poseidon_hash_poseidon_version_1", |
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.
mb rename to ext_gear_poseidon_hash_poseidon_version_1
? like GearBls12_381
does
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
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.
Generally, LGTM
fn order() -> BigUint { | ||
Self::ORDER.into() | ||
} | ||
fn characteristic() -> BigUint { | ||
Self::order() | ||
} |
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.
fn order() -> BigUint { | |
Self::ORDER.into() | |
} | |
fn characteristic() -> BigUint { | |
Self::order() | |
} | |
fn order() -> BigUint { | |
GoldilocksField::order() | |
} | |
fn characteristic() -> BigUint { | |
GoldilocksField::characteristic() | |
} |
/// | ||
/// The following code has been adapted from | ||
/// winterfell/math/src/field/f64/mod.rs located at <https://github.com/facebook/winterfell>. | ||
fn try_inverse(&self) -> Option<Self> { |
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.
Cannot GoldilocksField::try_inverse
be reused here?
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.
Absolutely! Good catch
// Using the fact that `GoldilocksFieldWrapper` is a newtype around `u64`. | ||
let data: [u64; 12] = unsafe { core::mem::transmute(input) }; |
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 input.map(...)
? I'm sure that it will be no-op in the compiled code but will not require unsafe
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.
Clearly, unsafe
is safe in this case.
If we replace type transmutation with a map, some overhead will definitely be there (at least in terms of the code size): compare. And in a contract the size is crucial.
Not sure if it has any runtime impact, but definitely this doesn't make anything better, in my opinion.
vec![SyscallName::PoseidonPermute].into_iter().for_each(|syscall| { | ||
// SyscallName::all().for_each(|syscall| { |
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.
Seems like previous version is correct
{ | ||
run_tester::<T, _, _, T::AccountId>(|_, _| { | ||
let input = [1_u64; 12]; | ||
// TODO: uncomment when the actual host implementatoin of Poseidon permute is ready |
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.
Will it be implemented in the following PR?
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.
Yes. It has already been implemented and tested. Goes in a separate PR for the sake of the changes readability.
gcore/src/exec.rs
Outdated
/// Perform input data transformation and return data in the same format. | ||
/// Primarily designed for the Poseidon permutation of Goldilocks field arrays, |
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.
Not very clear - isn't this fn only
for poseidon permutation?
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.
You're right. The description must be a leftover from an earlier version, apparently.
gcore/src/utils.rs
Outdated
// pub(crate) trait AsRawPtr: AsRef<[u8]> + AsMut<[u8]> { | ||
// fn as_ptr(&self) -> *const [u8; 32] { | ||
// self.as_ref().as_ptr() as *const _ | ||
// } | ||
|
||
// fn as_mut_ptr(&mut self) -> *mut [u8; 32] { | ||
// self.as_mut().as_mut_ptr() as *mut _ | ||
// } | ||
// } |
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.
// pub(crate) trait AsRawPtr: AsRef<[u8]> + AsMut<[u8]> { | |
// fn as_ptr(&self) -> *const [u8; 32] { | |
// self.as_ref().as_ptr() as *const _ | |
// } | |
// fn as_mut_ptr(&mut self) -> *mut [u8; 32] { | |
// self.as_mut().as_mut_ptr() as *mut _ | |
// } | |
// } |
35ab836
to
0e854c9
Compare
0e854c9
to
ff75a21
Compare
Overview
This is the first part of a bigger change (#4393 ) in order to break down the original PR into several more or less independent and tractable parts.
The ultimate goal is to provide a framework for zk-related apps on top of (currently) Vara blockchain. Specifically, it supports on-chain verification of Plonky2 proofs for arbitrary circuits. This would require speeding up of computationally greedy tasks in order to ensure any
verify
operation fits in a single block.The big picture looks like so:
Most of the workflow happens in the user-space. We define a set of contracts with (potentially) well-known addresses:
Application contract
contains the main app logic, and, therefore, stores the circuit-specific data; every proof sent to it for verification is prepended with the encoded circuit data;Verifier contract
(can be a single instance for the entire chain) wraps the original Plonky2 code for proofs verification; however, the underlying primitives (GoldilocksField
) are custom-implemented in such a way that heavy operations are offloaded to the host (through a chain of a syscall->host call).The runtime part includes a syscall implementation to export heavy computing outside of WASM environment, as well as a
runtime-interface
to proxy a call to the host.On the host we only maintain a self-sustained set of primitives (independent of the Plonky implementation) that ensure a client will always be able to carry out the Poseidon permute operation without requiring additional dependencies.
Changes
This PR only concerns the runtime part and the “user-space” part for the sake of keeping the PR size under control:
GoldilocksField
implementation to plug into the out-of-the-boxplonky2
verifier;The host call is replaced with a mock - the actual host call comes in a separate PR.