snark pool batching #5579
snark pool batching #5579
Conversation
| () ) ) | ||
| |> don't_wait_for ; | ||
| r | ||
| in |
There was a problem hiding this comment.
we should do this for local_diffs pipe as well. These come from the snark workers
| (* TODO: For now, we must not punish since we batch across messages received from | ||
| different senders and we don't isolate the bad proof in a batch, so we cannot | ||
| properly attribute blame. | ||
| *) |
| Error (`Locally_generated (diff, ())) ) | ||
| else Error (`Other (Error.of_string reason)) | ||
| in | ||
| let check_and_add () = |
There was a problem hiding this comment.
nit: rename it to add_to_pool or something?
src/lib/network_pool/snark_pool.ml
Outdated
| @@ -330,15 +330,21 @@ module Make (Transition_frontier : Transition_frontier_intf) : | |||
| work ) ] ; | |||
| `Statement_not_referenced ) | |||
|
|
|||
| (* At this point, verification already has happened *) | |||
There was a problem hiding this comment.
You mean after the function is called?
There was a problem hiding this comment.
I was confused about what the function was doing when I wrote the comment :)
We should prioritize full implementation of #4882 before mainnet. It should address the concern around blaming, and mitigate obvious DDoS attacks that levy batched verification of incoming snark work. |
deepthiskumar
added
the
ci-build-me
…a into fix/snark-pool-batching
@deepthiskumar pointed out that we do not currently batch snark verification across different snark pool diffs, which currently means we only do batches of size 2. This puts too much load on nodes in general and the snark worker coordinators in particular.
This PR makes it so that network pool diffs are asynchronously verified before being synchronously applied. For the transaction pool, no change is made. For the snark pool, we now use the batcher to verify across diffs. This has the effect that we can no longer properly attribute blame for punishments, so we will eventually have to figure something out there.