Define Combin{ing, ed} Result #17293
Conversation
|
!ci-build-me |
| @@ -0,0 +1,5 @@ | |||
| open Mina_wire_types | |||
|
|
|||
| type t = | |||
There was a problem hiding this comment.
The reason for this is
type t = Mina_wire_types.Network_pool.Snark_pool.Diff_versioned.V2.t =
| Add_solved_work of Work.t * Ledger_proof.t One_or_two.t Priced_proof.t
| EmptyIs the final type in Snark_pool_diff
There was a problem hiding this comment.
Could we rewrite the type without use of Mina_wire_types, instead using XX.Stable.Latest.t types.
This will be less fragile. Let me know if it's troublesome for whatever reason.
There was a problem hiding this comment.
This is just runtime data, I don't see good reason versioning it as it's not serialized. I'll instead put non-wire-type definition here.
There was a problem hiding this comment.
It seems if I refer to network_pool directly there's going to be a dependency cycle
There was a problem hiding this comment.
Okay, let's leave it as is
It would be a lot of work to change it, and not much of benefit
glyh
force-pushed
the
corvo/work-partitioner-combing-result
branch
from
de3a8b8 to
d05cc6d
Compare
|
!ci-build-me |
1 similar comment
|
!ci-build-me |
glyh
force-pushed
the
corvo/work-partitioner-combing-result
branch
from
370109e to
4a6353a
Compare
|
!ci-build-me |
glyh
added
the
snark-worker-optim
georgeee
left a comment
georgeee
left a comment
There was a problem hiding this comment.
Please document the combining_result.mli: every type, constructor and the merge function
| open Mina_wire_types | ||
|
|
||
| type t = | ||
| Transaction_snark_work.Statement.V2.t |
There was a problem hiding this comment.
We could remove the .ml file and add a stanza to dune?
|
!ci-build-me |
glyh
force-pushed
the
corvo/work-partitioner-combing-result
branch
from
4e9ef56 to
10d8bf3
Compare
|
!ci-build-me |
| (** [Done r] indicates that the result [r] is completed, and we should | ||
| submit it to the work selector. *) | ||
| | HalfMismatch of { submitted_half : submitted_half; in_pool : half } | ||
| (** submitted work doesn't match what we have in pool. It happens when the |
There was a problem hiding this comment.
If this is the only reason why HalfMismatch may happen?
Maybe then we don't need the { submitted_half : submitted_half; in_pool : half }, just the constructor with no underlying fields?
There was a problem hiding this comment.
Let's name it HalfAlreadyInPool?
| (** submitted work doesn't match what we have in pool. It happens when the | ||
| submitting half is the same as the half in pool *) | ||
| | NoSuchHalf of | ||
| { submitted_half : submitted_half |
There was a problem hiding this comment.
Why we return the submitted half? It's an argument to the call, hence if the caller needs it, it has it.
| @@ -0,0 +1,5 @@ | |||
| open Mina_wire_types | |||
|
|
|||
| type t = | |||
There was a problem hiding this comment.
Could we rewrite the type without use of Mina_wire_types, instead using XX.Stable.Latest.t types.
This will be less fragile. Let me know if it's troublesome for whatever reason.
| @@ -0,0 +1,5 @@ | |||
| open Mina_wire_types | |||
|
|
|||
| type t = | |||
There was a problem hiding this comment.
Okay, let's leave it as is
It would be a lot of work to change it, and not much of benefit
| | HalfMismatch of { in_pool : half } | ||
| (** submitted work doesn't match what we have in pool. It happens when the | ||
| submitting half is the same as the half in pool *) | ||
| | NoSuchHalf of { spec : Snark_work_lib.Selector.Single.Spec.t One_or_two.t } |
There was a problem hiding this comment.
Suggest StructureMismatch for the name
| (** [Done r] indicates that the result [r] is completed, and we should | ||
| submit it to the work selector. *) | ||
| | HalfMismatch of { submitted_half : submitted_half; in_pool : half } | ||
| (** submitted work doesn't match what we have in pool. It happens when the |
There was a problem hiding this comment.
Let's name it HalfAlreadyInPool?
| ~(submitted_half : submitted_half) : merge_outcome = | ||
| match (current, submitted_half) with | ||
| | Spec_only { spec = `One spec; sok_message = { fee; prover } }, `One -> | ||
| let submitted_result = |
There was a problem hiding this comment.
Let's move this piece of code as finalize_one function?
| } | ||
| , ((`First | `Second) as submitted_half) ) | ||
| when not (equal_half in_pool_half submitted_half) -> | ||
| let submitted_result = |
There was a problem hiding this comment.
Let's move this piece of code as finalize_two function?
|
!ci-bypass-changelog |
|
!ci-build-me |
|
!ci-build-me |
Combining Result are those stored in Pairing Pool; Combined Result are those being submitted back to the Work Selector.