Skip to content

upgrade 0809: use feynman guest rkyv #1718

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

upgrade 0809: use feynman guest rkyv #1718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1f4c7f
use feynman guest rkyv
lispc Aug 8, 2025
25e67b3
upgrade zkvm-prover
lispc Aug 9, 2025
059a467
fix bundle
lispc Aug 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ repository = "https://github.com/scroll-tech/scroll"
version = "4.5.8"

[workspace.dependencies]
scroll-zkvm-prover-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", branch = "feat/0.5.1", package = "scroll-zkvm-prover" }
scroll-zkvm-verifier-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", branch = "feat/0.5.1", package = "scroll-zkvm-verifier" }
scroll-zkvm-types = { git = "https://github.com/scroll-tech/zkvm-prover", branch = "feat/0.5.1" }
scroll-zkvm-prover-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4", package = "scroll-zkvm-prover" }
scroll-zkvm-verifier-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4", package = "scroll-zkvm-verifier" }
scroll-zkvm-types = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4" }
Comment on lines +20 to +22
Copy link

@coderabbitai coderabbitai bot Aug 10, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Pin git dependencies to the exact commit for reproducibility and supply-chain safety

Using a tag is better than a branch, but tags can be retagged. Since the PR references commit 72f9abb00f0ef7c5e46b7856d58bb86cbb332509, prefer rev to guarantee determinism.

Apply this diff:

-scroll-zkvm-prover-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4", package = "scroll-zkvm-prover" }
-scroll-zkvm-verifier-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4", package = "scroll-zkvm-verifier" }
-scroll-zkvm-types = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4" }
+scroll-zkvm-prover-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509", package = "scroll-zkvm-prover" }
+scroll-zkvm-verifier-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509", package = "scroll-zkvm-verifier" }
+scroll-zkvm-types = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509" }

Verification script to confirm the tag’s commit (optional):

🏁 Script executed:

#!/bin/bash
set -euo pipefail
REPO="https://github.com/scroll-tech/zkvm-prover"
TAG="0.5.4"
EXPECTED="72f9abb00f0ef7c5e46b7856d58bb86cbb332509"

echo "Tag -> commit:"
git ls-remote "$REPO" "refs/tags/$TAG" | awk '{print $1, $2}'
echo "Expected: $EXPECTED"

if git ls-remote "$REPO" "refs/tags/$TAG" | grep -q "^$EXPECTED"; then
  echo "OK: Tag $TAG points to expected commit."
else
  echo "WARN: Tag $TAG does not match expected commit."
fi

Length of output: 658

Pin git dependencies to an exact commit using rev

Confirmed that tag 0.5.4 currently points to commit 72f9abb00f0ef7c5e46b7856d58bb86cbb332509. To guard against future retagging and ensure full reproducibility, switch from tag to rev.

• Update in Cargo.toml (around lines 20–22):

-scroll-zkvm-prover-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4", package = "scroll-zkvm-prover" }
-scroll-zkvm-verifier-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4", package = "scroll-zkvm-verifier" }
-scroll-zkvm-types           = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4" }
+scroll-zkvm-prover-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509", package = "scroll-zkvm-prover" }
+scroll-zkvm-verifier-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509", package = "scroll-zkvm-verifier" }
+scroll-zkvm-types           = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509" }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
scroll-zkvm-prover-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4", package = "scroll-zkvm-prover" }
scroll-zkvm-verifier-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4", package = "scroll-zkvm-verifier" }
scroll-zkvm-types = { git = "https://github.com/scroll-tech/zkvm-prover", tag = "0.5.4" }
scroll-zkvm-prover-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509", package = "scroll-zkvm-prover" }
scroll-zkvm-verifier-euclid = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509", package = "scroll-zkvm-verifier" }
scroll-zkvm-types = { git = "https://github.com/scroll-tech/zkvm-prover", rev = "72f9abb00f0ef7c5e46b7856d58bb86cbb332509" }
🤖 Prompt for AI Agents 
In Cargo.toml around lines 20 to 22, the git dependencies use the tag "0.5.4"
which can be moved to point to a specific commit for better reproducibility.
Replace the "tag" field with "rev" and set its value to the commit hash
"72f9abb00f0ef7c5e46b7856d58bb86cbb332509" for all three dependencies to pin
them to that exact commit.


sbv-primitives = { git = "https://github.com/scroll-tech/stateless-block-verifier", branch = "chore/openvm-1.3", features = ["scroll"] }
sbv-utils = { git = "https://github.com/scroll-tech/stateless-block-verifier", branch = "chore/openvm-1.3" }
Expand Down
2 changes: 1 addition & 1 deletion crates/libzkp/src/tasks/bundle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl TryFrom<BundleProvingTask> for ProvingTask {
.into_iter()
.map(|w_proof| w_proof.proof.into_root_proof().expect("expect root proof"))
.collect(),
serialized_witness: vec![witness.rkyv_serialize(None)?.to_vec()],
serialized_witness: vec![witness.rkyv_serialize(Some(ForkName::Feynman))?.to_vec()],
Copy link

@coderabbitai coderabbitai bot Aug 10, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Hard-coding Feynman here ignores the task’s provided fork_name; use the actual fork to avoid mismatches

Serialization should respect the task’s fork to keep witness and metadata consistent. Hard-coding Feynman can produce invalid or unverifiable artifacts when fork_name != "feynman".

Apply this diff within the selected line:

-            serialized_witness: vec![witness.rkyv_serialize(Some(ForkName::Feynman))?.to_vec()],
+            serialized_witness: vec![witness.rkyv_serialize(Some(fork))?.to_vec()],

And add this line before constructing ProvingTask (outside the selected range):

let fork = ForkName::from(value.fork_name.as_str());

Optional repo-wide sanity check:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "Calls to rkyv_serialize and their fork arg:"
rg --line-number 'rkyv_serialize\(' -A 1

echo
echo "Places using Some(ForkName::Feynman):"
rg --line-number 'Some\(ForkName::Feynman\)'

echo
echo "Places constructing ForkName dynamically:"
rg --line-number 'ForkName::from\('

Length of output: 1308

Replace hard-coded Feynman with the task’s actual fork_name

Serialization must use the configured fork to keep witness and metadata in sync. Update both occurrences where ForkName::Feynman is passed to rkyv_serialize:

Files needing changes:

  • crates/libzkp/src/tasks/bundle.rs (line 86)
  • crates/libzkp/src/tasks/chunk.rs (line 75)

Apply this diff in each file:

- serialized_witness: vec![witness.rkyv_serialize(Some(ForkName::Feynman))?.to_vec()],
+ serialized_witness: vec![witness.rkyv_serialize(Some(fork_name))?.to_vec()],

Both modules already bind fork_name earlier (e.g. let fork_name = ForkName::from(self.fork_name.as_str());), so no new variable is needed.

Optional sanity check to confirm all instances:

rg --line-number 'rkyv_serialize\(' -A 1
rg --line-number 'Some\(ForkName::Feynman\)'
rg --line-number 'ForkName::from\('
🤖 Prompt for AI Agents 
In crates/libzkp/src/tasks/bundle.rs at line 86, replace the hard-coded
ForkName::Feynman argument passed to rkyv_serialize with the existing fork_name
variable to ensure serialization uses the configured fork. Locate the
rkyv_serialize calls and substitute Some(ForkName::Feynman) with
Some(fork_name). Repeat the same change in crates/libzkp/src/tasks/chunk.rs at
line 75. No new variable declaration is needed since fork_name is already
defined earlier in both files.

vk: Vec::new(),
})
}
Expand Down
3 changes: 2 additions & 1 deletion crates/libzkp/src/tasks/chunk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use eyre::Result;
use sbv_primitives::{types::BlockWitness, B256};
use scroll_zkvm_types::{
chunk::{execute, ChunkInfo, ChunkWitness, ToArchievedWitness},
public_inputs::ForkName,
task::ProvingTask,
};

Expand Down Expand Up @@ -71,7 +72,7 @@ impl TryFrom<ChunkProvingTask> for ProvingTask {
identifier: value.identifier(),
fork_name: value.fork_name,
aggregated_proofs: Vec::new(),
serialized_witness: vec![witness.rkyv_serialize(None)?.to_vec()],
serialized_witness: vec![witness.rkyv_serialize(Some(ForkName::Feynman))?.to_vec()],
Copy link

@coderabbitai coderabbitai bot Aug 10, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Same issue: serialization must use the task’s fork, not a hard-coded Feynman

Align serialization with the provided fork_name to prevent cross-fork inconsistencies and verification failures.

Apply this diff within the selected line:

-            serialized_witness: vec![witness.rkyv_serialize(Some(ForkName::Feynman))?.to_vec()],
+            serialized_witness: vec![witness.rkyv_serialize(Some(fork))?.to_vec()],

And add this line before constructing ProvingTask (outside the selected range):

let fork = ForkName::from(value.fork_name.as_str());

Optional check for consistency across the repo:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "All rkyv_serialize calls and fork argument:"
rg --line-number 'rkyv_serialize\(' -A 1

Length of output: 558

Use the provided fork name for serialization instead of hard-coding Feynman

Both crate/libzkp/src/tasks/chunk.rs and crate/libzkp/src/tasks/bundle.rs currently serialize the witness with ForkName::Feynman. This will break cross-fork proofs if a different fork is requested. You need to:

• Extract the fork from value.fork_name before building the task
• Pass that fork into rkyv_serialize instead of ForkName::Feynman

Locations to update:

  • crates/libzkp/src/tasks/chunk.rs (around line 75)
  • crates/libzkp/src/tasks/bundle.rs (around line 86)

Example diff for both files:

@@ before constructing the ProvingTask
-            serialized_witness: vec![witness.rkyv_serialize(Some(ForkName::Feynman))?.to_vec()],
+            serialized_witness: vec![witness.rkyv_serialize(Some(fork))?.to_vec()],

Make sure you add, just above the task construction in each file:

let fork = ForkName::from(value.fork_name.as_str());

This ensures serialization always uses the fork specified by the caller.

🤖 Prompt for AI Agents 
In crates/libzkp/src/tasks/chunk.rs at line 75, replace the hard-coded
ForkName::Feynman in the witness serialization with a dynamic fork value. First,
extract the fork by adding let fork = ForkName::from(value.fork_name.as_str());
just above the task construction, then pass this fork variable into
rkyv_serialize instead of ForkName::Feynman. This change ensures the
serialization respects the requested fork and prevents cross-fork proof issues.

vk: Vec::new(),
})
}
Expand Down