💬 sipsorcery commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588166620)
This check will fail on Windows. I'm guessing that's expected but just checking?

💬 vasild commented on issue "Rethink thread_local (take 2)":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2091280337)
This is indeed some problem in FreeBSD's libc which I reported upstream with a minimal, reproducable test case:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278701

Is anybody interested in reviewing the patch I posted above if I PR it:
https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2079776241
reworked to return `std::string_view` to the callers, but still storing the thread name in `char []`? That would work around the FreeBSD weird message printout because the `thread
...

💬 vasild commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#issuecomment-2091285732)
ping @maflcko

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588204550)
The goal is to detect that binaries, which are cross-compiled for Windows, are running in Wine.

💬 laanwj commented on issue "Rethink thread_local (take 2)":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2091354481)
> reworked to return std::string_view to the callers, but still storing the thread name in a thread_local char []?

Yes. i think (for this one very rare exception) it's acceptable to store a string in a fixed-size buffer. To not need a destructor and heap deallocation when a thread goes away, works around a large part of the complexity of handling thread-local data.

And making it use `string_view` throughout the changed functions instead of `char*` is a good idea, a lot less ugly.

💬 jonatack commented on pull request "doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE":
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2091356712)
> maybe just replace 520 with MAX_SCRIPT_ELEMENT_SIZE and nothing else?

Thanks @instagibbs, done.

💬 laanwj commented on issue "importaddress failed, Only legacy wallets are supported by this command.":
(https://github.com/bitcoin/bitcoin/issues/29772#issuecomment-2091363954)
Yes, adding an example would be useful.

💬 instagibbs commented on pull request "doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE":
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2091366440)
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860

💬 sipsorcery commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588252435)
Does that mean this PR is intended to be a fix for Windows or for wine?

🤔 mzumsande reviewed a pull request: "net: don't lock cs_main while reading blocks in net processing"
(https://github.com/bitcoin/bitcoin/pull/26326#pullrequestreview-2036711916)
Concept ACK

💬 mzumsande commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588242121)
As written somewhere above, we never prune blocks > 288 blocks from the tip, and if pruning, we should have disconnected any peer requesting older blocks - so it should be impossible to trigger this log even with pruning.
On the other hand, it could still be hit while not pruning due to disk problems - e.g. helping other peers with IBD. Searching for this assert gives me multiple matches, e.g.
* #6263
* #5668
* #12230
* https://bitcoin.stackexchange.com/questions/113218/how-to-fix-the-bitc
...

💬 sipsorcery commented on pull request "refactor, fuzz: Make 64-bit shift explicit":
(https://github.com/bitcoin/bitcoin/pull/30017#issuecomment-2091446475)
utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203

💬 willcl-ark commented on pull request "Fix misleading signmessage error with segwit":
(https://github.com/bitcoin-core/gui/pull/819#issuecomment-2091482504)
Sorry, I've addressed the linter now.

💬 achow101 commented on pull request "refactor: remove remaining unused code from cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/29961#issuecomment-2091483193)
ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62

💬 andrewtoth commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588303739)
> so it should be impossible to trigger this log even with pruning

Since we have a 2 block buffer, peers can request up to 290 blocks. So one way this can be triggered - a peer requests block 290 and we get the filepos, we release the lock, `pruneblockchain` RPC is called, rpc thread acquires the lock and block 290 is pruned before we can acquire the fd, read now fails.
However, `pruneblockchain` has a 2 hour buffer, so there must also be a long window of no new blocks.
I don't think this i
...

💬 willcl-ark commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2091500828)
I've been using this in a side-project and noticed some unintended behaviour, which could be my system; if I `make distclean` (removing the generated files) and then `make -j16` the build fails as it can't find the built artefacts. Running plain `make` works fine every time, and _sometimes_ a low job number works too.

The Makefile [appears](https://github.com/ryanofsky/bitcoin/blob/56ef459b573461087fbe4f39d9b0a7108936335f/src/Makefile.am#L1083-L1098) (to me) to have the dependencies listed co
...

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588314813)
For both. Wine is used to run test suite on Linux after cross-compiling for Windows. This need root as Wine and Windows runtime produce different error messages: https://github.com/bitcoin/bitcoin/blob/db73ac8c517f91ed5175b3635eb37ed56852ff91/src/test/system_tests.cpp#L80

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588317380)
FWIW, the same behaviour was before https://github.com/bitcoin/bitcoin/pull/29489.

🚀 achow101 merged a pull request: "refactor: remove remaining unused code from cpp-subprocess"
(https://github.com/bitcoin/bitcoin/pull/29961)

📝 willcl-ark opened a pull request: "doc: fix broken relative md links"
(https://github.com/bitcoin/bitcoin/pull/30025)
These relative links in our documentation are broken, fix them.