[Optimize] Add guards for processing unconfirmed solutions #3225
Conversation
| @@ -338,9 +338,27 @@ impl<N: Network, C: ConsensusStorage<N>, R: Routing<N>> Rest<N, C, R> { | |||
| Json(solution): Json<Solution<N>>, | |||
| ) -> Result<ErasedJson, RestError> { | |||
| // If the consensus module is enabled, add the unconfirmed solution to the memory pool. | |||
| if let Some(consensus) = rest.consensus { | |||
| // Otherwise, verify it prior to broadcasting. | |||
There was a problem hiding this comment.
I can imagine spamming a publicly accessible REST endpoint with solutions could halt a client due to the amount of verifications required. But we should test (cc @iamalwaysuncomfortable).
If it turns out to be an issue, we can:
- accept the risk and tell everyone not to expose their REST endpoint...
- add a separate transmission rate limit using a second
.layer(GovernorLayer { }. - queue transmissions to verify, takes more work but seems more robust. See my comment below for an example implementation
| consensus.add_unconfirmed_solution(solution).await?; | ||
| Some(consensus) => consensus.add_unconfirmed_solution(solution).await?, | ||
| // Verify the solution. | ||
| None => { |
There was a problem hiding this comment.
If clients receive transactions/solutions via P2P, they are already verified before propagation in fn unconfirmed_{transaction, solution}. So I suggest we just call that function from here. This is what the validator also does.
For consistency and clarity, we should also do this for transactions. A solution takes longer to verify than an execution, but less than a deployment, so there's no difference justifying a different verification approach.
Recall this PR was triggered in part because recently we did a large invalid solution spam, but @iamalwaysuncomfortable and I just realized we never tried spamming very large amounts of invalid transactions.
There was a problem hiding this comment.
This PR also remains relevant in the face of solution or deployment spam: #2970
|
The following measures are being done to test this
The devnet run has completed, and analysis of it is forthcoming. |
Motivation
This PR adds a few changes to improve syncing and reduce network spam:
POST /broadcast/solutionprior to solution propagation.