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

💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1262622915)
```suggestion
/*nDerivationMethod=*/ nDerivationMethod);

```

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

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

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

💬 kristapsk commented on pull request "RPC: Add universal options argument to listtransactions":
(https://github.com/bitcoin/bitcoin/pull/22807#issuecomment-1634333616)
Now it builds, but backwards compatibility with passing number instead of object as a third parameter is broken, will look at it later.

👍 brunoerg approved a pull request: "fuzz: wallet, add target for `Crypter`"
(https://github.com/bitcoin/bitcoin/pull/28074#pullrequestreview-1528586326)
ACK 6c98f4c137c9d557c78ebd52379711ebbd23e24a

non blocker, just an idea: Perhaps we could have encrypt/decrypt stuff outside of `LIMITED_WHILE` and then we could have a call just to fill them. I'm suppose this way we would have more chance of decrypting a previous encrypted thing. Just an example:

```diff
diff --git a/src/wallet/test/fuzz/crypter.cpp b/src/wallet/test/fuzz/crypter.cpp
index 7ba43821b..03ad97a35 100644
--- a/src/wallet/test/fuzz/crypter.cpp
+++ b/src/wallet/test/fuzz/cry
...

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262645801)
ah, thanks. I had a hard time with this one for some reason because casting to `sockaddr` kept truncating the unix socket path to 14 bytes before it got to `Connect()`. Anyway this works and I don't know why I didn't try it like this at first.

💬 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.