📝 kevkevinpal opened a pull request: "test: locking -testdatadir when not specified and then deleting lock and dir at end of test"
(https://github.com/bitcoin/bitcoin/pull/31328)
### Description
Motivated by this comment https://github.com/bitcoin/bitcoin/pull/31317#discussion_r1846842029
I wanted to lock the temp directory like we do when we specify the `-testdatadir` param

### Changes
Now when we run `BasicTestingSetup::~BasicTestingSetup()` we unlock the directory no matter what and we always delete unless `-testdatadir` is used

### Testing
- I've validated by both running the fuzz test runner and individual fuzz tests like so
- `FUZZ=utxo_total_supply ./bu
...

💬 kevkevinpal commented on pull request "test: Revert to random path element":
(https://github.com/bitcoin/bitcoin/pull/31317#discussion_r1849400734)
>> Probably we might also need to lock the directory like we do for the custom datadir path.

> Maybe a follow-up can do this? The goal of this pull request is to unbreak OSS-Fuzz (not sure if any other deployment is affected) by simply restoring what has been done for all previous releases in the unit tests

Created this PR, let me know if you would prefer any changes or if you think my approach is incorrect!
https://github.com/bitcoin/bitcoin/pull/31328

💬 adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-2487172554)
It seems like, with https://github.com/bitcoin/bitcoin/pull/31249, I should migrate these test changes as well. WIP

💬 kevkevinpal commented on pull request "test: Revert to random path element":
(https://github.com/bitcoin/bitcoin/pull/31317#issuecomment-2487182033)
reACK https://github.com/bitcoin/bitcoin/commit/faaaf59f71ede057b2c1d369ef8db973c2f2dbc2

Ran fuzz tests with 8 jobs and 8 workers `FUZZ=utxo_total_supply ./build_fuzz/src/test/fuzz/fuzz -jobs=8 -workers=8`
and validated that the temp directorys at `/tmp/test_common bitcoin/<random value>` got created and deleted for each fuzz test.

💬 kevkevinpal commented on pull request "doc: Correct PR Review Club frequency from weekly to monthly":
(https://github.com/bitcoin/bitcoin/pull/31327#issuecomment-2487192730)
Concept ACK

I agree with tbd3 and jonatack, drop the "monthly" and "meeting" and even if the frequency changes we won't need to touch this again. Unless the link is dead

🤔 tdb3 reviewed a pull request: "kernel: make m_tip_block std::optional"
(https://github.com/bitcoin/bitcoin/pull/31325#pullrequestreview-2447212379)
Concept ACK

Using `optional` seems much nicer than using a magic value. Planning to circle back to take another look.

⚠️ Klzklzklz opened an issue: "[{Tatra Bank Xvery}]</>"
(https://github.com/bitcoin/bitcoin/issues/31329)
<h1>TATRSKBX</h1>Xx (())<h2>

_[sell&withdraw]</h2>TME 05:52(BTCs)_<5>;
(/wallet@ID:klevismetani@NWO949.microsoft.com~Transferieren_dinero)atach€€@

4405 7783 4587 1093 /fin 09/29 178
1910&@plustomorrow/affecion_tired/sad/dissapoint/dontcry€
[ASAP]()_

✅ achow101 closed an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31329)

:lock: achow101 locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31329)

💬 naumenkogs commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#issuecomment-2487609939)
ACK 1807df3d9fb0135057a33e01173080ea14b0403d

💬 maflcko commented on pull request "test: locking -testdatadir when not specified and then deleting lock and dir at end of test":
(https://github.com/bitcoin/bitcoin/pull/31328#discussion_r1849669093)
I think the error message was a bit incorrect. `"The test executable is probably already running."` may not be accurate, because a lockfile may be present when the test executable crashed, no?

Also, it may be confusing for the default case, which should never have a colliding test dir. The issue in that case would be the path collision, not that `"The test executable is probably already running."`.

💬 maflcko commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2487689810)
> Edit: Fixed in #31317

My understanding is that with the current state of that pull, all outstanding observable issues mentioned in this thread are fixed? If not, please leave a comment.

There may or may not be some small style-fixups/follow-ups such as https://github.com/bitcoin/bitcoin/pull/31317#discussion_r1849149324, but no real issue to be fixed.

💬 maflcko commented on pull request "test: Revert to random path element":
(https://github.com/bitcoin/bitcoin/pull/31317#discussion_r1849675654)
> Having `g_rng_temp_path_init` be called as soon as this compilation unit is included in a binary isn't ideal but still acceptable (`BasicTestingSetup` may not be used in a given run).

Yeah, I guess it should be fine to move the init inside the `BasicTestingSetup` constructor, as long as it is before the `SeedRandomForTest`. I am happy to review a follow-up doing that, but I'll leave this as-is for now, due to the three acks.

💬 maflcko commented on issue "Feature request: Backup of datadir state":
(https://github.com/bitcoin/bitcoin/issues/31324#issuecomment-2487740107)
> Not a software problem, but its the environment. Many events can corrupt data inside datadir, to name some: power outages, out of resources (RAM), network fail (in case of network storage).

From a Bitcoin Core perspective, none of these should lead to corruption, as the state is flushed atomically. If a flush were to fail, it will be re-done on the next startup (which could then take a long time).

If you are observing issues, it is likely hardware issues, or software issues somewhere els
...

💬 maflcko commented on issue "Feature request: Backup of datadir state":
(https://github.com/bitcoin/bitcoin/issues/31324#issuecomment-2487763524)
> The creation of this backup also can be automated using scripts in bash, python, etc. The script need to stop bitcoind, write chainstate, blocks/index and some last .blk and .rev files. The restoration can be done manually, jsut replace chainstate, blocks/index and exclude .blk/.rev files down to current in the backup.


I am sure most people using Bitcoin Core in production will be doing backups already. However, I am also sure that everyone is using their preferred method of backing up. A
...

💬 maflcko commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#issuecomment-2487825932)
Only trivial style-only changes

re-ACK 55347a5018b2c252c56548f0a6dc1e88e42f66b6 🥊

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted c
...

💬 GabrieleBocchi commented on pull request "doc: Correct PR Review Club frequency from weekly to monthly":
(https://github.com/bitcoin/bitcoin/pull/31327#issuecomment-2488005127)
Thank you for the suggestion! I completely agree. I've updated the PR accordingly.

✅ fanquake closed an issue: "Feature request: Backup of datadir state"
(https://github.com/bitcoin/bitcoin/issues/31324)

💬 fanquake commented on issue "Feature request: Backup of datadir state":
(https://github.com/bitcoin/bitcoin/issues/31324#issuecomment-2488095819)
Going to close this. It's unlikely we are going to introduce additional complexity to our software, to work around issues in your hardware / environment.

👍 dergoegge approved a pull request: "test: Revert to random path element"
(https://github.com/bitcoin/bitcoin/pull/31317#pullrequestreview-2448100194)
ACK faaaf59f71ede057b2c1d369ef8db973c2f2dbc2