feat: prevent multiple block proposal evals #5453
Conversation
stacks-signer/src/v0/signer.rs
Outdated
| // Remove this block validation from the pending table | ||
| let signer_sig_hash = block_response.signer_signature_hash(); | ||
| self.signer_db | ||
| .remove_pending_block_validation(&signer_sig_hash) |
There was a problem hiding this comment.
I think you could maybe have mark_locally or mark_globally etc. functions to just auto update these pending lists because if we have come to a consensus or we are calling these functions, it doens't matter what the block response is/we should no longer consider these as pending block proposals. Does that make sense?
There was a problem hiding this comment.
Interesting, good call. That somewhat contrasts the ticket for always sending BlockResponses, but if the block already reached consensus, there's no point in adding load to our node with a new block response. Your suggestion would probably also help mitigate the issue that this PR tries to solve.
There was a problem hiding this comment.
I think neither of us noticed this was in handle_block_response, not handle_block_validation_response 🤦. I've moved it to handle_block_validation_response.
Regarding your point, though, I think we should remove the pending validation when a block is marked globally accepted/rejected. If the validation response comes in after it's marked as globally handled (the "typical" scenario), we'll still respond with a block response, which we want. But if our signer is "catching up", and the pending block validation is 1 or more blocks behind the tip, we should just drop it.
…ls' into feat/retry-pending-block-proposals
| if let Some(sighash) = self.get_pending_block_validation()? { | ||
| self.remove_pending_block_validation(&sighash)?; | ||
| return Ok(Some(sighash)); | ||
| } |
There was a problem hiding this comment.
Would it be more efficient to do the get and delete in one query, like a DELETE with a RETURNING signer_signature_hash?
This PR adds a new table to SignerDB, which is used to track block proposal validation submissions that received a 429 (which happens when the node is already evaluating a proposal).
When a block validation response is received, the signer checks if there are any pending block proposals. If so, it submits the proposal for validation.
In a "one-node-many-signers" scenario, such as many of our integration tests, this doesn't result in the same proposal being submitted many times. This is because, when a block validation response is received by the signer, the signer checks if that block was marked as "pending" and removes it from the DB.
Note: opening in draft while I write an integration test