📝 MarcoFalke opened a pull request: "XOR blocksdir *.dat files"
(https://github.com/bitcoin/bitcoin/pull/28052)
Currently the *.dat files in the blocksdir store the data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan them and move them into quarantine, or delete them, or corrupt them. This may cause Bitcoin Core to fail a reorg, or fail to reply to P2P or RPC block requests.

Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a ran
...

💬 MarcoFalke commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#discussion_r1257196740)
Looks like this is the only option that is required by the fuzz target, so maybe remove the others for now to hopefully reduce the number of conflicting pulls and remove the dependency on the other pull?

💬 MarcoFalke commented on pull request "test: indexes, fix on error infinite loop":
(https://github.com/bitcoin/bitcoin/pull/28044#issuecomment-1627051897)
Can be tested on current master with:

```diff
diff --git a/src/index/base.cpp b/src/index/base.cpp
index cf07cae286..2987eeb97b 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -234,6 +234,7 @@ void BaseIndex::ThreadSync()
__func__, pindex->GetBlockHash().ToString());
return;
}
+ return FatalErrorf("foobar");
}
}

```

Result on master:

```
$ ./src/test/test_bitcoin -t blockfilter
...

📝 TheCharlatan opened a pull request: "refactor: Move stopafterblockimport option out of blockstorage"
(https://github.com/bitcoin/bitcoin/pull/28053)
This has the benefit of moving this StartShutdown call out of the blockstorage file and thus out of the kernel's responsibility. The user can now decide if he wants to start shutdown / interrupt after a block import or not.

Once https://github.com/bitcoin/bitcoin/pull/27607 is merged (splitting up the block import and `LoadMempool` steps), the `return_after_import` bool can be removed.

This also simplifies https://github.com/bitcoin/bitcoin/pull/28048, making it one fewer shutdown call to
...

💬 MarcoFalke commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#issuecomment-1627094953)
I have the same question here: https://github.com/bitcoin/bitcoin/pull/27861#discussion_r1255496707

💬 TheCharlatan commented on pull request "kernel: Remove shutdown globals from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1627096093)
Closing this PR in favor of:
* #28053 Handling shutdown with `-stopafterblockimport`
* https://github.com/bitcoin/bitcoin/pull/28048 Handling shutdown with `-stopatheight`
* https://github.com/bitcoin/bitcoin/pull/27866 Handling blockstorage flush errors
* https://github.com/bitcoin/bitcoin/pull/28051 Getting rid of shutdown code

✅ TheCharlatan closed a pull request: "kernel: Remove shutdown globals from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27711)

💬 TheCharlatan commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#issuecomment-1627142023)
Re https://github.com/bitcoin/bitcoin/pull/28048#pullrequestreview-1520042712
> Maybe the simplest path to not conflict much could be to provide ThreadImport with a "return_after_import" bool arg (true when -stopafterblockimport is set)

Thanks @furszy, opened https://github.com/bitcoin/bitcoin/pull/28053.

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1627212544)
@achow101 I added `IsNotification()` and a test. Also rebased the PR with more atomic commits for hopefully easier reviewing. As far as 500, that should only be returned by a "legacy" json rpc request. The legacy protocol should be unchanged by this PR. Unfortunately if the request is unparseable we can not read the `jsonrpc:"2.0"` identifier so the default is to assume legacy.

⚠️ TNTBOMBOM opened an issue: "Bitcoin core hidden services link down/doesnt work"
(https://github.com/bitcoin/bitcoin/issues/28054)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Bitcoin core hidden services website entry not functional.

### Expected behaviour

It should work/connect.

### Steps to reproduce

Just visit the URL with your TB. or you can test the [onion URL](http://6hasakffvppilxgehrswmffqurlcjjjhd76jgvaqmsg6ul25s7t3rzyd.onion/) here:

https://onions.danwin1210.de/test.php

Output: `No, the service is offline!`

### Relevant log output

_No resp
...

💬 furszy commented on pull request "test: make assumeUTXO test capture the expected fatal error":
(https://github.com/bitcoin/bitcoin/pull/28050#discussion_r1257284611)
Didn't know that the macro existed. Thanks.

> Finally, it may be good to add steps to reproduce/test for reviewers. Otherwise it is unclear what your goal is. Reading the description you are trying to avoid that some error string is output?

Yeah. The `chainstatemanager_snapshot_completion_hash_mismatch` unit test manually triggers a fatal error, and the test framework by default prints the error message to the console ([this one](https://github.com/bitcoin/bitcoin/blob/79e8247ddb166f9b980f
...

🤔 furszy reviewed a pull request: "test: make assumeUTXO test capture the expected fatal error"
(https://github.com/bitcoin/bitcoin/pull/28050#pullrequestreview-1520722077)
updated per feedback, thanks @MarcoFalke.

* Moved `DebugLogHelper` usage to `ASSERT_DEBUG_LOG`.
* Added reproduction steps to the PR description.

👍 furszy approved a pull request: "refactor: Move stopafterblockimport option out of blockstorage"
(https://github.com/bitcoin/bitcoin/pull/28053#pullrequestreview-1520725353)
ACK 5867bb1a

💬 furszy commented on pull request "refactor: Move stopafterblockimport option out of blockstorage":
(https://github.com/bitcoin/bitcoin/pull/28053#discussion_r1257289914)
tiny nit:
A bit more readable if it uses `bool` instead of `auto`. (and also, maybe a better name for the variable would be `stop_after_import`)

💬 WaynePerth74 commented on issue "Improving fee estimation accuracy":
(https://github.com/bitcoin/bitcoin/issues/27995#issuecomment-1627397920)
free rate meed to be in union or better tet need to be unifited i would recomend dev team implament someting simulor to mempool.spoce

all so on asidenote i request dev team to reintraduce mining funchion to bitcoin core but have it mainly used for asic miners as cpu and gpu mining of bitcoin isnt safisent

💬 jamesob commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1627431981)
Concept ACK. Certainly worth doing.

👋 sipa's pull request is ready for review: "blockstorage: XOR blocksdir *.dat files"
(https://github.com/bitcoin/bitcoin/pull/28052)

💬 sipa commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1627435086)
Oops, I clocked wrong.

I just meant to Concept ACK.

🤔 TheCharlatan reviewed a pull request: "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly"
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1520702348)
Concept ACK

💬 TheCharlatan commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1257291194)
I was looking at the initialization order here and noticed that I might have introduced more unsafe behavior here in e2d680a32d757de0ef8eb836047a0daa1d82e3c4. These handlers are registered in `AppInitBasicSetup`. But if you look in bicoind.cpp, the kernel context is created after `AppInitBasicSetup` is called.

💬 TheCharlatan commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1257259643)
Why not use `EnsureAnyNodeContext` here as done in many other places?