Snark Worker Opt Protocol Update - dev #17720
Conversation
…y always casting field to its type when checking if null
…sks on a ledger mask
The Root.Config module defines how a Root ledger can be created. There are two independently varying options: 1. The directory_name to use for the database(s) backing the Root 2. The backing_type for the Root Some methods have been defined on the abstract Config.t type, to represent common operations on the locations of the databases backing root ledgers. The intent is to support existing patterns in the ledger handling code (deleting the backing of a root ledger, testing if a backing already exists, creating a root ledger based on a particular directory name) while abstracting the filesystem details of the root ledger backing. This is necessary for the future converting merkle tree backing for root ledgers; the migrated database will need to be created, moved, and deleted properly.
…er `finalize_zkapp_proof`
Fix merge of zkapp proof in work partitioner V3
…tead of Mina_ledger.Ledger.t
Remove debug_assert library and replace all use with native assert
[CI] Support for jammy
Push sig kind out of Network Pool > Txn Pool
…app_transition_time across codebase
…-root Dummy tests for Converting Ledger backed mina ledger root
…ill attach the spec when returning
|
@glyh please rebase your commits against |
| Mina_stdlib.Bounded_types.Wrapped_error.Stable.V1.t | ||
| * Selector.Spec.Stable.Latest.t | ||
| * Signature_lib.Public_key.Compressed.Stable.Latest.t | ||
| Error.t * (unit, unit, unit) Spec.Partitioned.Poly.Stable.Latest.t |
There was a problem hiding this comment.
Why Mina_stdlib.Bounded_types.Wrapped_error.Stable.V1.t was replaced with Error.t?
There was a problem hiding this comment.
Use of Vx is not good, but let's use the Wrapped_error, even if it's Error.t underneath
src/lib/snark_worker/rpc_get_work.ml
Outdated
| @@ -11,12 +11,9 @@ module Master = struct | |||
| let name = "get_work" | |||
|
|
|||
| module T = struct | |||
| type query = unit | |||
| type query = [ `V3 ] | |||
There was a problem hiding this comment.
In case we want to support multiple RPC versions in the future.
There was a problem hiding this comment.
But I guess unnecessary even if we want multi version, fixed
| @@ -288,7 +287,11 @@ module Impl = struct | |||
| ({ proof_level_snark; proof_cache_db; logger; signature_kind } : | |||
| Worker_state.t ) | |||
| ~spec:(partitioned_spec : Work.Spec.Partitioned.Stable.Latest.t) : | |||
| Work.Result.Partitioned.Stable.Latest.t Deferred.Or_error.t = | |||
| ( Work.Spec.Single.Stable.Latest.t | |||
There was a problem hiding this comment.
Do we have to specify the full type here? If yes, maybe an standalone type alias would help the code readibility?
| ; ("error", `String (Error.to_string_hum err)) | ||
| ] ; | ||
| let _spec = One_or_two.map ~f:Tuple2.get1 spec_with_proof in | ||
| failwith "TODO: bring back work to work selector here" ) )) |
There was a problem hiding this comment.
This TODO needs to be resolved, I guess.
What can be a cause of failure? Hard to judge about whether we need to put data back to selector w/o knowing that precisely.
There was a problem hiding this comment.
Maybe it's wiser just to log. This happens if the SNARK proof is invalid and get rejected by the pool. So it could indicate a bug in partitioner or snark worker.
| Mina_metrics.(Counter.inc_one Snark_work.snark_work_assigned_rpc) ; | ||
| Deferred.return (Some spec) | ||
| | Some (Error e) -> | ||
| [%log error] "Mina_lib.request_work failed" |
There was a problem hiding this comment.
Worth adding a comment on what might be a cause of such failure
glyh
added
the
snark-worker-optim
|
I realized I can target compatible for a reviewable version. And then construct the PR for develop, merge develop first then compatible. |
|
I'll open another PR targeting compatible addressing all review comments. |
From d8b55c4 is the real stuff in this PR
Need to
The reason this is very ugly is because there's a conflict on develop/compatible preventing a clean merge to compatible
If possible, I'd like to all change request on this PR go to follow-ups. Because it's a mess to work with
compatibleanddevelop