-
Notifications
You must be signed in to change notification settings - Fork 4
powdr: end-to-end prover #7
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
Conversation
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.
Pull Request Overview
This PR creates a new library to establish bindings between powdr and zeam, allowing block data to be passed as private inputs.
- Added a Cargo.toml for the new host library.
- Introduced an FFI entry point (prove) in the host library that wraps session setup and execution.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
pkgs/state-transition-runtime/src/powdr/host/Cargo.toml | New package configuration for the zeam-prover-host-powdr library |
pkgs/state-transition-runtime/src/powdr/host/src/lib.rs | Implementation of the FFI prove function linking powdr Sessions |
if !serialized.is_null() { | ||
std::slice::from_raw_parts(output, len) |
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.
The _output_slice initialization is using the 'serialized' pointer instead of 'output'. Update the condition to check if 'output' is null, and use 'output_len' in place of 'len' to correctly reflect the intended slice length.
if !serialized.is_null() { | |
std::slice::from_raw_parts(output, len) | |
if !output.is_null() { | |
std::slice::from_raw_parts(output, output_len) |
Copilot uses AI. Check for mistakes.
78b1a59
to
5131f55
Compare
616e553
to
a187862
Compare
Note that backtraces report both rust and zig code !
|
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.
Pull Request Overview
This PR implements an end-to-end prover that creates the integration layer between powdr and zeam, allowing block data to be passed as private inputs (required for #3).
- Corrects a typo in the zeam documentation.
- Introduces a new FFI function (powdr_prove) to initialize and run the powdr session.
- Adds the Cargo.toml configuration for building the host as a static library.
Reviewed Changes
Copilot reviewed 5 out of 11 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
resources/zeam.md | Corrected spelling of "latest_block_header" |
pkgs/state-transition-runtime/src/powdr/host/src/lib.rs | Added FFI function powdr_prove for session initialization and proving |
pkgs/state-transition-runtime/src/powdr/host/Cargo.toml | Added host configuration for building the static library |
Files not reviewed (6)
- build.zig: Language not supported
- pkgs/cli/src/main.zig: Language not supported
- pkgs/state-proving-manager/src/manager.zig: Language not supported
- pkgs/state-transition/src/transition.zig: Language not supported
- pkgs/state-transition/src/utils.zig: Language not supported
- pkgs/types/src/lib.zig: Language not supported
Comments suppressed due to low confidence (1)
resources/zeam.md:31
- The typo 'lastest' has been corrected to 'latest' to reflect the correct property name. Verify that all references now match the intended naming.
- only block's parent_root matched against state's `latest_block_header` hash tree root
if !serialized.is_null() { | ||
std::slice::from_raw_parts(output, len) |
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.
The check for the output slice incorrectly verifies 'serialized' rather than 'output', and uses 'len' instead of 'output_len'. Consider updating the condition to 'if !output.is_null()' and using 'output_len' when creating the slice.
if !serialized.is_null() { | |
std::slice::from_raw_parts(output, len) | |
if !output.is_null() { | |
std::slice::from_raw_parts(output, output_len) |
Copilot uses AI. Check for mistakes.
de25aac
to
082e10c
Compare
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.
Pull Request Overview
This PR introduces an end-to-end prover that bridges powdr and zeam by passing block data as private inputs, addressing part of issue #3.
- Corrected a typo in the zeam documentation regarding the state header name.
- Added a new FFI function (powdr_prove) in Rust for invoking the prover with byte slices sourced from external pointers.
- Introduced a dedicated Cargo.toml for the zeam-prover-host-powdr package and updated the README build instructions accordingly.
Reviewed Changes
Copilot reviewed 6 out of 12 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
resources/zeam.md | Fixed typo in the protocol description regarding state header |
pkgs/state-transition-runtime/src/powdr/host/src/lib.rs | Introduced the powdr_prove function with pointer handling |
pkgs/state-transition-runtime/src/powdr/host/Cargo.toml | Added package configuration and dependency details |
README.md | Updated build instructions and added a demo run section |
Files not reviewed (6)
- build.zig: Language not supported
- pkgs/cli/src/main.zig: Language not supported
- pkgs/state-proving-manager/src/manager.zig: Language not supported
- pkgs/state-transition/src/transition.zig: Language not supported
- pkgs/state-transition/src/utils.zig: Language not supported
- pkgs/types/src/lib.zig: Language not supported
Blocked on powdr-labs/powdr#2622 and powdr-labs/powdr#2620 for merging, but I can keep working on it in the meantime. |
082e10c
to
0be6f69
Compare
e4419e2
to
ab53efe
Compare
ab53efe
to
06d7133
Compare
@@ -28,5 +42,34 @@ pub fn main() !void { | |||
|
|||
try clock.run(); | |||
}, | |||
.prove => |provecmd| { | |||
const state = types.BeamState{ |
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 guess i can replace it by genMockChain
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.
feel free to
@@ -64,6 +64,7 @@ pub fn verify_signatures(signedBlock: types.SignedBeamBlock) !void { | |||
_ = signedBlock; | |||
} | |||
|
|||
// TODO(gballet) check if beam block needs to be a pointer |
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.
it doesn't need to be but zig must be memcping it so i guess pointer would be better
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.
let's do the optimization in another PR, right now it works
} | ||
extern fn powdr_prove(serialized: [*]const u8, len: usize, output: [*]u8, output_len: usize, binary_path: [*]const u8, binary_path_length: usize, result_path: [*]const u8, result_path_len: usize) void; | ||
|
||
pub const zkvm_configs: []const ZKVMContext = &.{ |
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.
we can probably transition it to a enum union
struct
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, but since there's only one right now, let's leave it as is and change it in a future PR when we support proving more than one zkvm
try ssz.serialize(types.BeamSTFProverInput, prover_input, &serialized); | ||
|
||
var output: [256]u8 = undefined; | ||
powdr_prove(serialized.items.ptr, serialized.items.len, @ptrCast(&output), 256, zkvm_configs[0].program_path.ptr, zkvm_configs[0].program_path.len, zkvm_configs[0].output_dir.ptr, zkvm_configs[0].output_dir.len); | ||
return types.BeamSTFProof{}; |
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.
so proof is still somewhere in the files and not pickedup yet?
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.
yeah atm, this comes with the verifier PR, which will probably cause some changes in what is returned so I'll leave it at that for now
06d7133
to
40f94ed
Compare
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.
lgtm
This creates glue between powdr and zeam, so that the block data can be passed as private inputs. The publics extraction and verification require more work on the powdr side, so I suggest we merge this PR first.
TODO before merging:
prove
also rebuilds the guest programs