💬 Sjors commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#discussion_r1116142978)
That seems like a good idea, also because `GetSnapshotBaseHeight` will deference the result to get a height. (Though it won't crash because it checks if the result is a `nullptr`.)

💬 Sjors commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#discussion_r1116123753)
Neither are used here, because `GetSnapshotBaseBlock` is only used by `GetSnapshotBaseHeight`. But they are used in the final PR, so I'm fine with introducing them here.

💬 TheCharlatan commented on pull request "Fix various libbitcoinkernel DLL build problems":
(https://github.com/bitcoin/bitcoin/pull/27146#discussion_r1116185034)
Nice, I applied theuni@1733d01 and:
```
objdump -p install_dir/bin/bitcoin-chainstate.exe | grep dll
DLL Name: libbitcoinkernel-0.dll
DLL Name: ADVAPI32.dll
DLL Name: KERNEL32.dll
DLL Name: msvcrt.dll
```
:smile:

⚠️ Sjors opened an issue: "test: p2p_message_capture.py fails with undefined sanitizer"
(https://github.com/bitcoin/bitcoin/issues/27149)
On Ubuntu 22.10

Relevant configure variable: `--with-sanitizers=undefined`

```
$ test/functional/p2p_message_capture.py
2023-02-23T19:55:57.251000Z TestFramework (INFO): PRNG seed is: 8604348948412116696
2023-02-23T19:55:57.251000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_v2mn0yad
2023-02-23T19:55:57.666000Z TestFramework (INFO): Stopping nodes
Traceback (most recent call last):
File "test/functional/p2p_message_capture.py", line 72, in <module>

...

💬 Sjors commented on issue " Document how to run bitcoin unittests + functional tests with sanitizers":
(https://github.com/bitcoin/bitcoin/issues/17834#issuecomment-1442364207)
Anything left to do here?

Seems mostly a matter of `./configure with-sanitizers=…` and then the test suite runs as normal, albeit slower and more memory hungry.

💬 MarcoFalke commented on issue "test: p2p_message_capture.py fails with undefined sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27149#issuecomment-1442374172)
Did you set the suppression file? (`env|grep UBSAN`?)

💬 Sjors commented on issue "test: p2p_message_capture.py fails with undefined sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27149#issuecomment-1442383883)
Which suppression file?

Aha: https://github.com/bitcoin/bitcoin/blob/master/test/sanitizer_suppressions/ubsan

We should point to it in https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#sanitizers

💬 MarcoFalke commented on issue "RPC: gettxoutsetinfo correctly flushes transactions to the coindb, but then does not return any RPC reply, and keeps running":
(https://github.com/bitcoin/bitcoin/issues/25912#issuecomment-1442387196)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

💬 MarcoFalke commented on issue "test: p2p_message_capture.py fails with undefined sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27149#issuecomment-1442391173)
> It's not clear to me how to use it.

https://github.com/bitcoin/bitcoin/issues/17834#issuecomment-1230498583

💬 achow101 commented on pull request "util: Use void* throughout support/lockedpool.h":
(https://github.com/bitcoin/bitcoin/pull/16195#issuecomment-1442393931)
ACK f36d1d5b8934aac60d3097047ecedeb58bae2185

💬 jkczyz commented on pull request "util: Use void* throughout support/lockedpool.h":
(https://github.com/bitcoin/bitcoin/pull/16195#issuecomment-1442407067)
> This PR has had a couple of acks for a while now and could be ready to merge. Is this change still relevant and valid?

I'm not longer employed with the organization in questions, FWIW. This patch was required to compile in their environment as noted in https://github.com/bitcoin/bitcoin/pull/16195#issuecomment-501370908.

🚀 achow101 merged a pull request: "util: Use void* throughout support/lockedpool.h"
(https://github.com/bitcoin/bitcoin/pull/16195)

💬 TheCharlatan commented on pull request "Fix various libbitcoinkernel DLL build problems":
(https://github.com/bitcoin/bitcoin/pull/27146#issuecomment-1442433720)
ACK 5da7c0b3e34626ca57d1f0773db61e7d8351d8c7

💬 mzumsande commented on pull request "p2p, rpc: Manual block-relay-only connections with addnode":
(https://github.com/bitcoin/bitcoin/pull/24170#issuecomment-1442450925)
> in the very unlikely scenario that everyone likes this connection and all bitcoin nodes on tor connect with clearnet only using this option - that wouldn't be good for the network right? (assuming an average tor user wouldn’t want their addresses/transactions relayed to their clearnet peers)

We'd need enough inbound capacity on clearnet to accomodate the additional manual connections to it, and also some users relaying transactions on both networks, otherwise txns submitted through tor woul
...

💬 achow101 commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1442452427)
ACK ce882ef0cf338d390733dd85e339a1c0da73072c

📝 ryanofsky opened a pull request: "Deduplicate bitcoind and bitcoin-qt init code"
(https://github.com/bitcoin/bitcoin/pull/27150)
Add common `InitConfig` function to deduplicate `bitcoind` and `bitcoin-qt` code parsing the config file, creating the datadir, and selecting the network.

Noticed the duplicate code while reviewing #27073 and want to remove it because difference in bitcoin-qt and bitcoind behavior make it hard to evaluate changes like #27073

There is a slight change of behavior for `bitcoin-qt`: When there is a problem reading the configuration file, the error prefix is changed from "Error: Cannot parse co
...

💬 ryanofsky commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#issuecomment-1442453898)
Leaving as draft because #27073 could be merged first, and if it is more duplicate code can be moved to the common function.

👍 furszy approved a pull request: "prune: scan and unlink already pruned block files on startup"
(https://github.com/bitcoin/bitcoin/pull/26533)
Code review ACK 3141eab9
Left a non-blocking nit. No need to do it.

💬 furszy commented on pull request "prune: scan and unlink already pruned block files on startup":
(https://github.com/bitcoin/bitcoin/pull/26533#discussion_r1116165449)
nit: could make use of the std::error_code:
e.g.
```c++
else {
LogPrint(BCLog::BLOCKSTORE, "Prune: failed to removed block/undo file, error %s", ec.message());
}
```

💬 achow101 commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1442456504)
ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387