💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262653604)
Ok thanks I added the preprocessor logic to the function, I'll look into adding afunix.h for windows, maybe in compat.h? also add it to the first commit to test for AF_UNIX in configure.ac

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262664857)
great catch thanks yeah I saw that in your commit but I was confused. makes sense now

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262667486)
got it thanks, fixed this when changing sockaddr_storage to sockaddr_un and it's now `addrun`

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262690201)
ok fixed this, just using default (with auth) for the test

🤔 ryanofsky reviewed a pull request: "kernel: Remove StartShutdown calls from validation code"
(https://github.com/bitcoin/bitcoin/pull/28048#pullrequestreview-1526920138)
Rebased 3e6b6877645f978b4999efdfb9e616f4dbf9dd19 -> cbea21da035d9dbcd38669d780611c9a05442f3d ([`pr/stopafter.4`](https://github.com/ryanofsky/bitcoin/commits/pr/stopafter.4) -> [`pr/stopafter.5`](https://github.com/ryanofsky/bitcoin/commits/pr/stopafter.5), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/stopafter.4-rebase..pr/stopafter.5)) fixing conflict with #28053 and also removing mentions about changed -stopafterheight behavior after Martin's comment https://github.com/bitcoin/bi
...

💬 ryanofsky commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1261536518)
re: https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1261480112

:man_facepalming: Thanks! I didn't notice the `StartShutdown` call was followed by a `if (m_chainman.m_interrupt) break` line. So it was already exiting the loop after the height check, and adding a new `break` would not change anything.

I think you are also right that there are race conditions where extra blocks could be attached, but I haven't looked into the details. Maybe it is worth opening an issue with your obs
...

💬 ryanofsky commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1262699057)
re: https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1259566308

I'm not too concerned about `blockTip` notification shutting down during an `invalidateblock` call, because it seems like a corner case to worry about someone using `-stopafterheight` and `invalidblock` at the same time. If someone is doing this, I think there is only a small change of behavior not worth documenting where the `-stopatheight` option might take effect a little earlier and the node would shut down a little
...

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262751862)
ah sorry

💬 sipa commented on pull request "[WIP] descriptors: do not return top-level only funcs as sub descriptors":
(https://github.com/bitcoin/bitcoin/pull/28067#issuecomment-1634494768)
@furszy I think there is only a downgrade issue, no? If someone were to import a `sh(raw(...))` as watch-only address in a descriptor wallet after allowing that, and then downgrades to older software which does not support it.

💬 Sjors commented on pull request "Bugfix: net_processing: Restore "Already requested" error for FetchBlock":
(https://github.com/bitcoin/bitcoin/pull/28055#issuecomment-1634495424)
I think @mzumsande's suggestion is better. But the current PR, with @instagibbs's tests to document the existing weirdness, is fine too.

👍 jamesob approved a pull request: "util: Teach AutoFile how to XOR"
(https://github.com/bitcoin/bitcoin/pull/28060#pullrequestreview-1528767782)
reACK faebf00

Trivial rephrasing to avoid `FILE` release looking like a multiplication:

```diff
diff --git a/src/streams.h b/src/streams.h
-index 03df20b5db..fc2fe4d9f8 100644
+index 03df20b5db..5ff952be76 100644
--- a/src/streams.h
+++ b/src/streams.h
@@ -13,6 +13,7 @@
@@ -315,7 +315,7 @@
- file = nullptr;
- }
- return retval;
-+ if (std::FILE * rel{release()}) return std::fclose(rel);
++ if (auto rel{release()}) return std::fclose
...

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1262766470)
Thanks! Updated.

👍 TheCharlatan approved a pull request: "kernel: Remove StartShutdown calls from validation code"
(https://github.com/bitcoin/bitcoin/pull/28048#pullrequestreview-1528801908)
Very happy with this change. Nice to see these last few PRs pushing configuration options out of the kernel and letting the user decide how to react to kernel actions.

ACK 31eca93a9eb8e54f856d3f558aa3c831ef181d37

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-1634533832)
Thanks, @MarcoFalke and @brunoerg for the reviews.

- Moved `Encrypt/Decrypt` variables outside of `LIMITED_WHILE` and added a function to update them regularly.

💬 achow101 commented on pull request "[WIP] descriptors: do not return top-level only funcs as sub descriptors":
(https://github.com/bitcoin/bitcoin/pull/28067#issuecomment-1634547609)
Allowing new types of descriptors only disallows downgrading of wallets that have imported them. Downgrade protection is automatic for such wallets since descriptor wallets were introduced.

💬 hebasto commented on pull request "multiprocess: Add basic spawn and IPC support":
(https://github.com/bitcoin/bitcoin/pull/19160#discussion_r1262872076)
I apologize for revisiting this old PR, but I couldn't resist asking to clarify things for me.

As far as I understand, `interfaces::MakeNodeInit` returns a `std::unique_ptr` that owns an instance of `init::BitcoindInit` or throws an exception. Given this understanding, it seems unclear how the `return exit_status;` statement could ever execute.

💬 achow101 commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1634647753)
ACK e7cf8657e1165ea5da3911a9e543837cd8938f97

🚀 achow101 merged a pull request: "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting"
(https://github.com/bitcoin/bitcoin/pull/27411)

💬 ryanofsky commented on pull request "multiprocess: Add basic spawn and IPC support":
(https://github.com/bitcoin/bitcoin/pull/19160#discussion_r1262895010)
> As far as I understand, `interfaces::MakeNodeInit` returns a `std::unique_ptr` that owns an instance of `init::BitcoindInit` or throws an exception. Given this understanding, it seems unclear how the `return exit_status;` statement could ever execute.

There are two `interfaces:MakeNodeInit` functions. The one in [`src/init/bitcoind.cpp`](https://github.com/bitcoin/bitcoin/blob/master/src/init/bitcoind.cpp#L41-L44) which is linked into `bitcoind` will leave exit_status unchanged, as you say.
...

💬 hebasto commented on pull request "multiprocess: Add basic spawn and IPC support":
(https://github.com/bitcoin/bitcoin/pull/19160#discussion_r1262899118)
Right. Thank you.