[Refactor] For universal prover task (the update on coordinator) #1682
Conversation
WalkthroughThis update introduces a new universal task lifecycle for zero-knowledge proofs (ZKP), expanding the FFI and Go interfaces to support universal task generation, metadata management, and proof wrapping. The coordinator's logic, types, and database schema are updated to handle universal tasks, metadata, and wrapped proofs. Build and workflow scripts are adjusted to accommodate the new library structure. Changes
Sequence Diagram(s)sequenceDiagram
participant GoApp as Go Coordinator
participant GoLib as libzkp Go Bindings
participant CLib as C/Rust FFI (libzkp)
participant DB as Database
GoApp->>GoLib: GenerateUniversalTask(taskType, taskJSON, forkName)
GoLib->>CLib: gen_universal_task(...)
CLib-->>GoLib: HandlingResult {ok, universal_task, metadata, hash}
GoLib-->>GoApp: (ok, universal_task, metadata, hash)
GoApp->>DB: Store ProverTask (with metadata)
Note right of GoApp: Later, when proof is submitted:
GoApp->>GoLib: GenerateWrappedProof(proofJSON, metadata, vkData)
GoLib->>CLib: gen_wrapped_proof(...)
CLib-->>GoLib: wrapped_proof
GoLib-->>GoApp: wrapped_proof
GoApp->>GoLib: Verify*Proof(wrapped_proof, forkName)
GoLib->>CLib: verify_*_proof(...)
CLib-->>GoLib: verification result
GoLib-->>GoApp: verification result
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1682 +/- ##
===========================================
- Coverage 40.16% 40.09% -0.08%
===========================================
Files 232 232
Lines 18447 18502 +55
===========================================
+ Hits 7409 7418 +9
- Misses 10315 10357 +42
- Partials 723 727 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (3)
coordinator/internal/logic/verifier/verifier.go (2)
111-122: File descriptor leak inReadVK
os.Openis invoked but the returned file handle is never closed, leaking an FD every time the method is used.func (v *Verifier) ReadVK(filePat string) (string, error) { - f, err := os.Open(filepath.Clean(filePat)) + f, err := os.Open(filepath.Clean(filePat)) if err != nil { return "", err } + defer f.Close()
124-147:loadOpenVMVksleaves temp files & leaks FDs
os.Openresult is not closed.- The temporary file is never deleted, so every Coordinator start will litter
$TMPDIR.- f, err := os.Open(filepath.Clean(tempFile)) + f, err := os.Open(filepath.Clean(tempFile)) if err != nil { return err } + defer func() { + f.Close() + _ = os.Remove(tempFile) // ignore error – best-effort cleanup +}()common/libzkp/impl/src/lib.rs (1)
14-18: Add null-check & robust deserialisation
init_verifierdereferencesconfigblindly andunwrap()s JSON parsing. Any null pointer or malformed JSON will cause UB or panic across the FFI boundary.pub unsafe extern "C" fn init_verifier(config: *const c_char) { - let config_str = c_char_to_str(config); - let verifier_config = serde_json::from_str::<VerifierConfig>(config_str).unwrap(); + if config.is_null() { + log::error!("init_verifier called with NULL pointer"); + return; + } + let config_str = match std::str::from_utf8(std::ffi::CStr::from_ptr(config).to_bytes()) { + Ok(s) => s, + Err(e) => { + log::error!("Invalid UTF-8 in config: {:#}", e); + return; + } + }; + let verifier_config = match serde_json::from_str::<VerifierConfig>(config_str) { + Ok(cfg) => cfg, + Err(e) => { + log::error!("Failed to parse verifier config: {:#}", e); + return; + } + };
🧹 Nitpick comments (16)
common/libzkp/impl/src/utils.rs (1)
6-8: Add null-pointer guard to avoid UB in FFI helper
CStr::from_ptrtriggers undefined behaviour whencis null. A minimal guard prevents crashes coming from buggy callers.pub(crate) fn c_char_to_str(c: *const c_char) -> &'static str { - let cstr = unsafe { CStr::from_ptr(c) }; - cstr.to_str().unwrap() + if c.is_null() { + return ""; + } + let cstr = unsafe { CStr::from_ptr(c) }; + cstr.to_str().unwrap() }common/libzkp/impl/Makefile (2)
5-6: Hard-coded path: make it a variable for portabilityBaking
../libinto the recipe ties the Makefile to the current directory layout. Consider:LIB_DIR ?= $(CURDIR)/../lib ... @mkdir -p $(LIB_DIR) @cp -f target/release/libzkp.so $(LIB_DIR)/
11-13:cleantarget leaves Cargo artefacts behindRemoving only the copied
.socan still leave stale binaries intarget/.
Addcargo cleanorrm -rf targetfor a truly fresh build.coordinator/internal/types/submit_proof.go (1)
10-10: Explicitly document & validate the “universal” flagThe new field is optional, but its semantics ( e.g. allowed only for certain
TaskTypes ) are not enforced. Consider adding validation logic or at least a comment so downstream handlers don’t silently ignore malformed requests (for instanceuniversal=truewith a non-universal task).coordinator/internal/config/config.go (2)
31-35: Name exported fieldURL, notUrlGo uses initialisms in all-caps (
URL) to follow the standard library’s style (url.URL,http.Handler, etc.). Renaming improves readability and tooling auto-completion.-type L2Endpoint struct { - Url string `json:"endpoint"` +type L2Endpoint struct { + URL string `json:"endpoint"` }
39-41: Pointer indirection may be unnecessary
Endpoint *L2Endpointforces nil-checking everywhere (as seen in the controller). Unless hot-swapping endpoints is expected, embed the struct directly to simplify access.coordinator/internal/controller/api/controller.go (1)
34-37: “l2geth is not specified” panic happens lateYou dereference
cfg.L2.Endpointonly during controller initialisation. Mis-configuration would be caught earlier and more cleanly if validated inconfig.NewConfig, preventing partial startup.coordinator/internal/logic/provertask/batch_prover_task.go (1)
197-228: Heavy path inGetTaskMetaData& logging mismatchesMinor but noisy: log messages reference
"chunk"while executing batch logic, which confuses ops.- log.Error("failed to get chunk by hash", "task_id", taskID, "err", err) + log.Error("failed to get batch by hash", "task_id", taskID, "err", err) ... - log.Error("Generate universal prover task failure", "task_id", taskID, "type", "chunk") + log.Error("Generate universal prover task failure", "task_id", taskID, "type", "batch")Additionally, consider short-circuiting when
cached == nilafterapplyUniversalinstead of a second lookup.coordinator/internal/logic/provertask/chunk_prover_task.go (1)
46-47: Use a named constant for cache size
newCache(1024)is a magic number. Define aconst defaultChunkTaskCacheSize = 1024near the top of the file (or intask_cache.go) for clarity and easier tuning.coordinator/internal/logic/provertask/bundle_prover_task.go (1)
48-49: Introduce a named constant for cache sizeReplace bare
128with a descriptive constant (e.g.defaultBundleTaskCacheSize).coordinator/Makefile (2)
19-26: Declarelibzkp(andclean_libzkp) as.PHONYtargetsGNU Make will incorrectly treat them as real files and may skip rebuilding.
-.PHONY: lint docker clean coordinator coordinator_skip_libzkp mock_coordinator +.PHONY: lint docker clean coordinator coordinator_skip_libzkp mock_coordinator \ + libzkp clean_libzkp
45-52: Consider making shared-library path a variable
../common/libzkp/lib/libzkp.soappears in three targets; pull it into aLIBZKP_SOvariable for maintainability.coordinator/internal/logic/provertask/task_cache.go (2)
14-18: Renamedropping_cachetodroppingCacheGo style prefers mixed-caps identifiers; improves readability without changing behaviour.
34-38: Avoid discarding the entire active map on single overflowPurging the whole
cachewhen it hitslimitcauses a bursty eviction pattern and defeats LRU intent.
Consider incremental eviction or a real LRU (list orcontainer/list) for smoother hit ratio.common/libzkp/lib.go (1)
35-42: Docstring copy-paste – function isn’t a verifier
InitL2gethcomment duplicates “Initialize the verifier”; update comment to reflect L2 geth init.common/libzkp/impl/src/lib.rs (1)
125-129: Consider idempotentrelease_string
release_stringcurrently UB-panics if passed twice. After freeing, set the pointer to null on the caller side or accept*mut *mut c_charto null-out internally to make accidental double frees harmless.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
common/libzkp/impl/Cargo.lockis excluded by!**/*.lockgo.workis excluded by!**/*.work
📒 Files selected for processing (23)
common/libzkp/go.mod(1 hunks)common/libzkp/impl/Cargo.toml(0 hunks)common/libzkp/impl/Makefile(1 hunks)common/libzkp/impl/src/lib.rs(2 hunks)common/libzkp/impl/src/utils.rs(1 hunks)common/libzkp/interface/libzkp.h(1 hunks)common/libzkp/lib.go(1 hunks)coordinator/.gitignore(1 hunks)coordinator/Makefile(2 hunks)coordinator/internal/config/config.go(1 hunks)coordinator/internal/controller/api/controller.go(2 hunks)coordinator/internal/controller/api/get_task.go(1 hunks)coordinator/internal/controller/api/submit_proof.go(2 hunks)coordinator/internal/logic/provertask/batch_prover_task.go(3 hunks)coordinator/internal/logic/provertask/bundle_prover_task.go(3 hunks)coordinator/internal/logic/provertask/chunk_prover_task.go(3 hunks)coordinator/internal/logic/provertask/prover_task.go(4 hunks)coordinator/internal/logic/provertask/task_cache.go(1 hunks)coordinator/internal/logic/submitproof/proof_receiver.go(3 hunks)coordinator/internal/logic/verifier/verifier.go(6 hunks)coordinator/internal/logic/verifier/verifier_test.go(2 hunks)coordinator/internal/types/get_task.go(1 hunks)coordinator/internal/types/submit_proof.go(1 hunks)
💤 Files with no reviewable changes (1)
- common/libzkp/impl/Cargo.toml
🧰 Additional context used
🧬 Code Graph Analysis (8)
coordinator/internal/controller/api/get_task.go (3)
common/types/message/message.go (1)
ProofType(20-20)coordinator/internal/logic/provertask/prover_task.go (1)
ProverTask(35-38)coordinator/internal/orm/prover_task.go (2)
ProverTask(20-46)ProverTask(54-56)
coordinator/internal/logic/verifier/verifier_test.go (1)
common/types/message/message.go (2)
OpenVMBatchProof(189-198)OpenVMChunkProof(157-165)
coordinator/internal/controller/api/submit_proof.go (3)
coordinator/internal/controller/api/get_task.go (1)
GetTaskController(24-28)coordinator/internal/logic/submitproof/proof_receiver.go (1)
NewSubmitProofReceiverLogic(81-137)common/types/message/message.go (3)
ProofTypeChunk(39-39)ProofTypeBatch(41-41)ProofTypeBundle(43-43)
coordinator/internal/logic/provertask/prover_task.go (4)
coordinator/internal/types/auth.go (1)
HardForkName(24-24)coordinator/internal/logic/provertask/task_cache.go (2)
TaskCache(13-18)CachedTaskData(7-10)coordinator/internal/types/get_task.go (1)
GetTaskSchema(12-18)common/libzkp/lib.go (1)
GenerateUniversalTask(77-102)
coordinator/internal/controller/api/controller.go (4)
coordinator/internal/config/config.go (1)
L2(37-41)common/libzkp/lib.go (1)
InitL2geth(36-41)coordinator/internal/controller/api/get_task.go (1)
NewGetTaskController(31-48)coordinator/internal/controller/api/submit_proof.go (1)
NewSubmitProofController(26-35)
coordinator/internal/logic/provertask/chunk_prover_task.go (3)
coordinator/internal/logic/provertask/prover_task.go (2)
ErrCoordinatorInternalFailure(26-26)ProverTask(35-38)coordinator/internal/logic/submitproof/proof_receiver.go (1)
ErrCoordinatorInternalFailure(47-47)coordinator/internal/orm/prover_task.go (2)
ProverTask(20-46)ProverTask(54-56)
common/libzkp/impl/src/lib.rs (3)
common/libzkp/impl/src/utils.rs (2)
c_char_to_str(6-9)c_char_to_vec(11-14)coordinator/internal/config/config.go (1)
VerifierConfig(66-68)common/libzkp/interface/libzkp.h (6)
init_verifier(12-12)gen_universal_task(35-35)release_task_result(38-38)gen_wrapped_proof(43-43)release_string(46-46)init_l2geth(15-15)
coordinator/internal/logic/verifier/verifier.go (2)
common/libzkp/lib.go (5)
InitVerifier(28-33)VerifyBatchProof(55-63)VerifyChunkProof(44-52)VerifyBundleProof(66-74)DumpVk(127-143)coordinator/internal/logic/verifier/types.go (1)
Verifier(11-14)
🪛 checkmake (0.2.2)
coordinator/Makefile
[warning] 25-25: Target "libzkp" should be declared PHONY.
(phonydeclared)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: tests
🔇 Additional comments (8)
common/libzkp/go.mod (2)
1-3: Go 1.22 requirement: double-check CI & prod images
go 1.22is still very new. Make sure all build environments (CI, docker images, release pipelines) have been upgraded, otherwise the build will fail silently.
1-3: Module declaration looks good
No other issues spotted.coordinator/.gitignore (1)
4-4: Ignore pattern may point to an obsolete locationThe library is now produced at
common/libzkp/lib/libzkp.so, but the ignore entry iscoordinator/internal/libzkp/lib/libzkp.so.
Verify the path and update if the artefact is no longer copied there, otherwise the new binary could be added to git accidentally.coordinator/internal/types/get_task.go (1)
7-8: ```shell
#!/bin/bash
rg -n "TaskTypes" coordinator/internal/types/get_task.go || echo "No TaskTypes in get_task.go"
rg -n "task_types" coordinator/internal/types/get_task.go || echo "No task_types in get_task.go"</details> <details> <summary>coordinator/internal/logic/verifier/verifier_test.go (2)</summary> `14-14`: **Import path update looks correct** Switching to `scroll-tech/common/types/message` aligns with the refactor; compilation should proceed without additional changes. --- `61-83`: **Helper functions updated consistently** Return types, unmarshalling and callers all reflect the new message package. Tests should remain green. </details> <details> <summary>coordinator/internal/controller/api/submit_proof.go (1)</summary> `26-34`: **Nil-pointer danger when `ProverTasks()` is incomplete** If `getTaskController` is `nil` or its map lacks one of the three proof-type keys, the assignments below will panic. Guard upfront and fail fast with a clear error. ```diff -proverTaskIf := getTaskController.ProverTasks() -ifc.submitProofReceiverLogic.ChunkTask = proverTaskIf[message.ProofTypeChunk] +if getTaskController == nil { + log.Crit("GetTaskController is nil – cannot wire prover tasks") +} +proverTaskIf := getTaskController.ProverTasks() +if proverTaskIf == nil { + log.Crit("GetTaskController returned nil task map") +} +if controller.submitProofReceiverLogic.ChunkTask = proverTaskIf[message.ProofTypeChunk]; controller.submitProofReceiverLogic.ChunkTask == nil { + log.Crit("ChunkTask not registered in GetTaskController") +}common/libzkp/lib.go (1)
76-103: Double-check lifetime ofresultstrings afterrelease_task_result
defer C.release_task_result(result)frees C-allocated strings at function exit – safe. Ensure the C side does not retain Go-allocated memory after the call. (Heads-up only.)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
coordinator/internal/logic/submitproof/proof_receiver.go (1)
177-185: Still passing an emptypiHashtoGenerateWrappedProofA previous review already pointed out that the third parameter must be the
piHashobtained fromGenerateUniversalTask.
The call is unchanged –[]byte{}is supplied, making the wrapped proof invalid.- proofParameter.Proof = libzkp.GenerateWrappedProof(proofParameter.Proof, string(proverTask.Metadata), []byte{}) + piHash := proverTask.PiHash // retrieve the value stored at assignment time + proofParameter.Proof = libzkp.GenerateWrappedProof(proofParameter.Proof, string(proverTask.Metadata), piHash)Please store
piHashalongsidemetadatawhen assigning the task (see comment inapplyUniversal) and propagate it here.#!/bin/bash # Show all GenerateWrappedProof calls and the argument used for piHash rg --line-number 'GenerateWrappedProof\(' | grep -v '[]byte{}'
🧹 Nitpick comments (2)
.github/workflows/integration.yml (1)
41-41: Validate ordering and optimize the new libzkp build step
The addedmake -C common/libzkp/impl buildis required for the refactored module, but please verify:
- That
make dev_docker(line 38) does not depend on the.soproduced here—if it does, consider moving this command up.- The produced
libzkp.sois correctly placed incommon/libzkp/libbefore running tests.- Whether caching
common/libzkp/impl/buildviaactions/cache@v3could speed up CI on subsequent runs.Optionally, split into a dedicated “Build libzkp” step for clearer logging and caching setup.
coordinator/internal/logic/submitproof/proof_receiver.go (1)
75-78: Struct now owns three task helpers that are never referenced
ChunkTask,BundleTask, andBatchTaskwere added but are not used anywhere inProofReceiverLogic.
Dead fields complicate the struct, trigger linter warnings, and mislead future maintainers.If they are meant for future work, add a TODO explaining their purpose; otherwise remove them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/coordinator.yml(1 hunks).github/workflows/integration.yml(1 hunks)common/libzkp/impl/src/lib.rs(2 hunks)coordinator/internal/controller/api/controller.go(1 hunks)coordinator/internal/logic/provertask/batch_prover_task.go(2 hunks)coordinator/internal/logic/provertask/bundle_prover_task.go(1 hunks)coordinator/internal/logic/provertask/chunk_prover_task.go(1 hunks)coordinator/internal/logic/provertask/prover_task.go(2 hunks)coordinator/internal/logic/submitproof/proof_receiver.go(3 hunks)coordinator/internal/orm/prover_task.go(1 hunks)coordinator/internal/types/get_task.go(1 hunks)database/migrate/migrations/00028_add_metadata_to_prover_task.sql(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- coordinator/internal/orm/prover_task.go
- database/migrate/migrations/00028_add_metadata_to_prover_task.sql
🚧 Files skipped from review as they are similar to previous changes (7)
- .github/workflows/coordinator.yml
- coordinator/internal/controller/api/controller.go
- coordinator/internal/types/get_task.go
- coordinator/internal/logic/provertask/batch_prover_task.go
- coordinator/internal/logic/provertask/chunk_prover_task.go
- coordinator/internal/logic/provertask/bundle_prover_task.go
- common/libzkp/impl/src/lib.rs
🧰 Additional context used
🪛 golangci-lint (1.64.8)
coordinator/internal/logic/provertask/prover_task.go
16-16: could not import scroll-tech/common/libzkp (-: build constraints exclude all Go files in /home/jailuser/git/common/libzkp)
(typecheck)
coordinator/internal/logic/submitproof/proof_receiver.go
18-18: could not import scroll-tech/common/libzkp (-: build constraints exclude all Go files in /home/jailuser/git/common/libzkp)
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: check
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
common/libzkp/impl/src/verifier/euclidv2.rs (1)
48-50: Minor lifetime / allocation nit – extract the conversion into a local.
&proof.into_evm_proof()creates a temporaryEvmProofthat is immediately borrowed.
While legal, extracting it into a named variable avoids the temporary-lifetime dance, makes the code easier to step through in a debugger, and prevents an accidental double conversion in future edits.- self.bundle_verifier - .verify_proof_evm(&proof.into_evm_proof()) + let evm_proof = proof.into_evm_proof(); + self.bundle_verifier.verify_proof_evm(&evm_proof)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/libzkp/impl/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
common/libzkp/impl/Cargo.toml(2 hunks)common/libzkp/impl/src/verifier/euclidv2.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- common/libzkp/impl/Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: check
- GitHub Check: check
🔇 Additional comments (1)
common/libzkp/impl/src/verifier/euclidv2.rs (1)
6-6: Import looks correct – no issues found.
IntoEvmProofis required for the new conversion call below; the added import is appropriate and keeps the dependency list explicit.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
coordinator/internal/logic/provertask/prover_task.go (1)
189-197:piHashis still dropped – universal proof wrapping will break
libzkp.GenerateUniversalTaskreturns four values, the fourth beingpiHash, which is required later when callinglibzkp.GenerateWrappedProof.
The current implementation keeps ignoring it, so downstream code will continue to supply an empty hash and wrapped-proof verification will fail once the library enforces the check.-func (b *BaseProverTask) applyUniversal(schema *coordinatorType.GetTaskSchema) (*coordinatorType.GetTaskSchema, []byte, error) { - ok, uTaskData, metadata, _ := libzkp.GenerateUniversalTask(schema.TaskType, schema.TaskData, schema.HardForkName) +func (b *BaseProverTask) applyUniversal(schema *coordinatorType.GetTaskSchema) (*coordinatorType.GetTaskSchema, []byte, []byte, error) { + ok, uTaskData, metadata, piHash := libzkp.GenerateUniversalTask(schema.TaskType, schema.TaskData, schema.HardForkName) if !ok { - return nil, nil, fmt.Errorf("can not generate universal task, see coordinator log for the reason") + return nil, nil, nil, fmt.Errorf("can not generate universal task, see coordinator log for the reason") } schema.TaskData = uTaskData - return schema, []byte(metadata), nil + return schema, []byte(metadata), piHash, nil }Make sure the extra
piHashslice is persisted (e.g., inCachedTaskData) and later fed intoGenerateWrappedProof.
This was raised in a previous review but has not been addressed yet.
🧹 Nitpick comments (1)
database/migrate/migrate_test.go (1)
57-76: Avoid repetition of the migration version magic-number
The literal28is now duplicated in three assertions. The next migration will force another mechanical update. Define a single constant to keep tests DRY and reduce maintenance friction.- // total number of tables. - assert.Equal(t, int64(28), cur) + const expectedVersion = int64(28) // update when a new migration is added + // total number of tables. + assert.Equal(t, expectedVersion, cur) @@ - assert.Equal(t, int64(28), cur) + assert.Equal(t, expectedVersion, cur) @@ - assert.Equal(t, int64(28), version) + assert.Equal(t, expectedVersion, version)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
coordinator/internal/logic/provertask/bundle_prover_task.go(1 hunks)coordinator/internal/logic/provertask/prover_task.go(2 hunks)database/migrate/migrate_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- coordinator/internal/logic/provertask/bundle_prover_task.go
🧰 Additional context used
🪛 golangci-lint (1.64.8)
coordinator/internal/logic/provertask/prover_task.go
16-16: could not import scroll-tech/common/libzkp (-: build constraints exclude all Go files in /home/jailuser/git/common/libzkp)
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: check
- GitHub Check: tests
noel2004
dismissed stale reviews from lispc, georgehao, and colinlyguo
via
d844890
Background: this is one of several PRs to support "Universal Prover", which can help prove circuits(zk programs) of different hardforks in one single binary.
This PR is part of #1680 and include required updates for coordinator.
There are changes as follows:
The packing of libzkp is moved from coordinator and become a separated go module, with corresponding updates in Make routine
The arguments of
get_taskandsubmit_proofhas been updated to support the "universal task" proverTo support universal prover, some translating work is moved into coordinator, they are generated on-the-fly with a
get_taskrequest from new prover.We use a simple cache for saving the new universal tasks, no updated is required to the data in dbAnd themetadatadata is now separated before task is sent to prover and persisted in coordinator sideSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores