Bound parallel verification of transactions #2387
Conversation
Signed-off-by: Raymond Chu <14917648+raychu86@users.noreply.github.com>
synthesizer/src/vm/verify.rs
Outdated
| )); | ||
| } | ||
| // Verify the transaction. | ||
| match self.check_transaction(transaction, None, &mut rng) { |
There was a problem hiding this comment.
I should have noticed this earlier, but be aware: if a malicious validator inserts a deployment into their Batch Proposal with a malformed verifying key, this thread may panic, causing all transmissions to be sent back into the mempool queue for verification.
The only way I see us preventing this is to perform validation on transactions of validator Batch Proposals before signing them.
I will make sure we have a consensus test case for this to reproduce.
There was a problem hiding this comment.
Good point, we should wrap the call with a handle_halting macro, like we do for other functions that have the capability of panicking.
There was a problem hiding this comment.
I added panic catchers in check_transaction and updated the test - fbe604d
Signed-off-by: Raymond Chu <14917648+raychu86@users.noreply.github.com>
Motivation
This PR bounds the memory usage during transactions verification by introducing
MAX_PARALLEL_DEPLOY_VERIFICATIONSandMAX_PARALLEL_EXECUTE_VERIFICATIONS, where we verify a limited number of transactions at once instead of all at once. Previously the unbounded verification could lead to OOM errors when a node is forced to verify many large/expensive transactions at once.Profiling memory usage of verification gives us some insights -
#2376 (comment)
For an upper bound of 10GB of memory dedicated to transaction verification, we set the following values to:
MAX_PARALLEL_DEPLOY_VERIFICATIONS: 5MAX_PARALLEL_EXECUTE_VERIFICATIONS: 1000This means we verify deployments first (in chunks of 5), then executions after (in chunks of 1000).