chore: remove outdated TODO and stabilize proof roundtrip test

[Ocheretovich]

This PR includes two small, independent cleanups:

• Remove an outdated TODO comment in version resolution logic
• Make the proof roundtrip test robust when testdata is missing

No functional changes.

[coderabbitai]

📝 Walkthrough

Walkthrough

Removed an inline TODO comment from the coordinator version utility and added a guard in a zkp crate test to skip when testdata is missing; no production API changes.

Changes

Cohort / File(s)	Summary
Comment cleanup coordinator/internal/utils/version.go	Removed a two-line TODO comment above Version; no code, logic, or behavior changes.
Test control flow crates/libzkp/src/proofs.rs	In test_roundtrip, added a guard to detect a missing testdata directory, print a skip message, and return early (affects test flow only).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit nibbles at a TODO leaf,
One line gone — the code finds relief.
A test now checks for a missing trail,
Skips politely if it cannot prevail.
Hops onward, tidy and hale! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description lacks required sections from the template, including conventional commit type selection, deployment versioning information, and breaking change label declaration.	Add the missing template sections: select a conventional commit type (chore appears most appropriate), confirm deployment tag versioning status, and declare breaking change status.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title accurately captures both main changes: removing an outdated TODO comment and stabilizing the proof roundtrip test. It is concise, specific, and directly related to the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e88ba1c and 408389d.

📒 Files selected for processing (1)

• crates/libzkp/src/proofs.rs

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Check directory structure and git tracking
echo "=== Checking testdata directory structure ==="
if [ -d "crates/libzkp/testdata" ]; then
    echo "✓ testdata directory exists"
    ls -la crates/libzkp/testdata/
else
    echo "✗ testdata directory does not exist"
fi

echo -e "\n=== Checking git tracking of testdata ==="
git ls-files crates/libzkp/testdata/ 2>/dev/null | head -20

echo -e "\n=== Checking .gitignore for testdata ==="
if [ -f ".gitignore" ]; then
    grep -n "testdata" .gitignore || echo "No testdata entries in .gitignore"
fi

# Check if there's a crates/.gitignore
if [ -f "crates/.gitignore" ]; then
    echo "Checking crates/.gitignore:"
    grep -n "testdata" crates/.gitignore || echo "No testdata entries in crates/.gitignore"
fi

# Check libzkp directory
if [ -f "crates/libzkp/.gitignore" ]; then
    echo "Checking crates/libzkp/.gitignore:"
    grep -n "testdata" crates/libzkp/.gitignore || echo "No testdata entries in crates/libzkp/.gitignore"
fi

Repository: scroll-tech/scroll

Length of output: 262

---

🏁 Script executed:

#!/bin/bash

# Review the actual test implementation (lines 220-250 for context)
echo "=== Test implementation context (lines 220-250) ==="
sed -n '220,250p' crates/libzkp/src/proofs.rs

echo -e "\n=== Full test function signature and body ==="
# Find the test_roundtrip function
rg -A 30 "fn test_roundtrip" crates/libzkp/src/proofs.rs

Repository: scroll-tech/scroll

Length of output: 2286

---

Silent test skip allows tests to pass without running any assertions, hiding test coverage gaps.

The testdata directory does not exist in the repository, causing test_roundtrip to silently return Ok(()) without executing any assertions. This means tests pass in CI without actually verifying the roundtrip serialization behavior for ChunkProof, BatchProof, and BundleProof.

Replace the silent skip with one of these explicit patterns:

1. Fail the test if testdata is required (strictest approach):

   let testdata_dir = std::path::Path::new("./testdata");
   if !testdata_dir.exists() {
       panic!("testdata directory required for test_roundtrip - ensure testdata files are checked in or generated during setup");
   }

2. Use #[ignore] for optional test data:

   #[test]
   #[ignore = "requires testdata directory with chunk-proof.json, batch-proof.json, bundle-proof.json"]
   fn test_roundtrip() -> eyre::Result<()> {

3. Check for specific required files and fail explicitly:

   let files = ["chunk-proof.json", "batch-proof.json", "bundle-proof.json"];
   for file in files {
       let path = Path::new("./testdata").join(file);
       if !path.exists() {
           panic!("Missing required test file: {}", path.display());
       }
   }

The current implementation risks CI falsely reporting test pass when test coverage is missing.

🤖 Prompt for AI Agents

crates/libzkp/src/proofs.rs around lines 224 to 228: the test currently silently
returns Ok(()) when ./testdata is missing which hides missing coverage; replace
that silent skip with an explicit policy: either (A) panic when the testdata
directory is required (fail the test), (B) mark the test #[ignore] with a clear
reason if testdata is optional, or (C) explicitly check for the required files
(chunk-proof.json, batch-proof.json, bundle-proof.json) and panic naming any
missing file; implement one of these patterns so CI cannot pass without the test
data or an explicit ignore.