Bugfix: RPC/Mining: getblocktemplate: Delay updating nTransactionsUpdatedLast and time_start until after the new template is cached #30088

This is needed to avoid parallel calls from incorrectly using the stale cached template or (after an OOM) dereferencing a nullptr pblocktemplate

Considering memory overcommit, the worst case scenario likely never occurs, but that possibility is the clear bugfix here.

Aside from that, I wonder if using the stale cache in this race scenario (which is what could happen if this isn't fixed) is better than having N threads all making a new block (with this fix).

A middle ground (and perhaps cleaner than a bunch of static variables) might be to take a lock and have racing requests all wait on the same block creation, but that is not implemented here.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage

For detailed information about the code coverage, see the test coverage report.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

A unique lock on cs_main is taken on line 605. It is temporarily released if (!lpval.isNull()), but re-locked again before exiting that if-block, well before the block with the static variables around line 740 and the rest of the function body. So only one thread should be executing that section at a time.

That would mean having a null pindexPrev should be enough for the null pblocktemplate to be recalculated after an OOM. But maybe something less obvious is happening here?

Indeed, forgot about cs_main