✅ willcl-ark closed an issue: ""netinfo" doesn't show IPv6 "Local addresses""
(https://github.com/bitcoin/bitcoin/issues/30165)

💬 willcl-ark commented on issue ""netinfo" doesn't show IPv6 "Local addresses"":
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2202562859)
OK I'm going to close this for now.

Feel free to open a new issue or comment here if you are still experiencing this problem.

💬 glozow commented on pull request "26.2 final changes":
(https://github.com/bitcoin/bitcoin/pull/30376#discussion_r1662211978)
thanks, taken

💬 glozow commented on pull request "26.2 final changes":
(https://github.com/bitcoin/bitcoin/pull/30376#issuecomment-2202578548)
Rebased and made copyright year update, thanks @stickies-v!

👍 stickies-v approved a pull request: "26.2 final changes"
(https://github.com/bitcoin/bitcoin/pull/30376#pullrequestreview-2153323242)
ACK eef5dbc21b3fd52069f2f0855fb76a13234ebbf3

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1662227439)
Yes, moved to a constant and added a comment to it. Placed the constant in the `.cpp` file since it is only used in that `.cpp` file, to limit its scope. It is not part of the "public" `private_broadcast.h/cpp` interface

👍 willcl-ark approved a pull request: "Fix SSE4.1-related issues"
(https://github.com/bitcoin/bitcoin/pull/28893#pullrequestreview-2153336754)
tACK d440f13db02c82c842000abe4fe4d0c721a4ad3b

These changes fix the issues correctly in my testing on Ubuntu.

I do slightly prefer how my changeset printed a configure-time [warning message](https://github.com/bitcoin/bitcoin/commit/20b1fe92c87a86c8df4f1c7a0529a7d9e6710571#diff-49473dca262eeab3b4a43002adb08b4db31020d190caaad1594b47f1d5daa810R537) for the user that SHANI support would be disabled if SSE4.1 was not enabled, but overall prefer the approach of this fix.

👍 alfonsoromanz approved a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2153338081)
Re ACK 8ec24bdad89e2a72c394060ba5661a91f374b874

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1662247336)
The Tor connectivity could be "off" during startup and become "on" later at runtime, when we connect to the Tor daemon and it tells us the Tor proxy address. This is why the startup condition is a bit different, it has the `onion_may_become_reachable` variable.

👍 stickies-v approved a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2153373722)
re-ACK 064a401e10f2b2da8f31f682929431d06c671d93 , main change is using a single nonce for both caches

I think the last commit 064a401e10f2b2da8f31f682929431d06c671d93 is unnecessary scope creep for this PR, and although it looks safe to me, I am not 100% confident I am grasping the full extent of the implications, and I think a separate PR would've been more suitable.

💬 vasild commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1662270745)
from comment https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1659087156:
> Why didn't we need to do this on master?
> The old log message was `Connected & Listening: 127.0.0.1:15689` not `Connected & Listening: 0:0`

When accepting connections (from the python node point of view), then indeed 0:0 is printed (on `master`):

```
$ ./test/functional/p2p_addr_relay.py -l debug --nocleanup
$ ./test/functional/combine_logs.py |grep 'Connected & L'
...
test 2024-07-02T10:18:18.494
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1662271368)
I replied to that comment to keep the discussion in one place.

💬 Sjors commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-2202687388)
Nice! Will try this out soon.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2202707437)
`7a7e7d189d...3bd618a91f`: rebase and address suggestions

📝 hodlinator opened a pull request: "refactor: Make uint256S(const char*) consteval"
(https://github.com/bitcoin/bitcoin/pull/30377)
`uint256S()` calls taking C-string literals are littered throughout the code and were executed at runtime to perform parsing unless a given optimizer was surprisingly efficient. While this may not be a hot spot, it's better hygiene *in C++20* to store the parsed data blob directly in the binary, without any parsing at runtime.

**transaction_identifier.h** - Fixed dormant bug in `TxidFromString()` where the `string_view` length wasn't respected(!).

Added runtime overload taking `std::string
...

💬 vasild commented on issue "ci: failure in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2202728919)
In case you settle with adding to `m_nodes` before `PushNodeVersion()` and I do not manage to review, it is good to ensure that no code will be upset by an existence of an (oubound) entry in `m_nodes` that does not have version pushed.

💬 maflcko commented on pull request "fuzz: Mutate -max_len= during generation phase":
(https://github.com/bitcoin/bitcoin/pull/30371#issuecomment-2202734365)
> I don't know how to evaluate if this is a good idea but I'm also not using the test runner, so wouldn't mind. Also, I don't think oss-fuzz is doing this, so how come they found the bug but we didn't?

Good points. Possibly, the single run on my laptop was just extremely lucky and it is highly variant to find the bug regardless of max_len. The high variance will probably make this hard to benchmark.

🤔 glozow reviewed a pull request: "doc: use TRUC instead of v3 and add release note"
(https://github.com/bitcoin/bitcoin/pull/30272#pullrequestreview-2153448506)
thanks @ismaelsadeeq! Took your suggestions and fixed docs in a few more places

💬 glozow commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1662304753)
added

💬 glozow commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1662304855)
added