The server isn't running out of memory when the private broadcast transaction queue is full. Add and use a new -37 (RPC_LIMIT_EXCEEDED) code that can be used whenever a resource is bound and currently at capacity.
Addresses #35406 (review)
Also includes commits to address other outstanding suggestions/nits from #35406:
[[nodiscard]]: #35406 (review)BOOST_CHECK_EQUAL: #35406 (review)
The server isn't running out of memory when the private broadcast
transaction queue is full. Add a new RPC_LIMIT_EXCEEDED code that
can be used whenever a resource is bound and currently at capacity.
Makes the failure output more useful.
Add the tx that was just removed, instead of an earlier one, for
improved clarity.
The header file already properly uses the attribution.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35678.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | instagibbs, andrewtoth, sedited |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
ACK 8ac222484c92f320f56273035bc66dbc14c0b9a1
ACK 8ac222484c92f320f56273035bc66dbc14c0b9a1
ACK 8ac222484c92f320f56273035bc66dbc14c0b9a1
395 | @@ -396,7 +396,7 @@ RPCErrorCode RPCErrorFromTransactionError(TransactionError terr) 396 | case TransactionError::ALREADY_IN_UTXO_SET: 397 | return RPC_VERIFY_ALREADY_IN_UTXO_SET; 398 | case TransactionError::PRIVATE_BROADCAST_FULL: 399 | - return RPC_OUT_OF_MEMORY; 400 | + return RPC_LIMIT_EXCEEDED;
it would be nice to use a more descriptive/specific constant name here. RPC_LIMIT_EXCEEDED reads like an http queue capacity error, and not something related to the private broadcast queue.
extra note: we use HTTP_SERVICE_UNAVAILABLE for the http queue return error code.
I think that is the idea,no? The description indicates that this can be re-used for other purposes.
I think that is the idea,no? The description indicates that this can be re-used for other purposes.
Ups, I jumped straight to the code. Forget what I said.
I thought this was suggesting to use the introduced RPCErrorCode at the http queue level, but that is not the case. I'm just not fan of the http <--> json-rpc coupling we have.
