💬 hebasto commented on pull request "net processing: #26140 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27379#issuecomment-1492418189)
Concept ACK.

👍 jarolrod approved a pull request: "Update translation source file for v25.0 string freeze"
(https://github.com/bitcoin-core/gui/pull/724)
ACK e414edd8fc88b55a91f446dcc4d55cc4cd6d32e7

Went through the translation process again, confirming that plurals requires manual intervention in the translation source file.

I have a zero-diff with this branch after following the process steps.

💬 hebasto commented on pull request "net processing: #26140 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27379#discussion_r1154779794)
Nice!

👍 hebasto approved a pull request: "net processing: #26140 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/27379)
ACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3, I have reviewed the code and it looks OK, I agree it can be merged.

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1492439997)
Could someone remove the `Needs rebase` label and add a v25 milestone? Thanks.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154783411)
Yeah this one may be tricky to work around. Some of the CI temp dir paths are crazy long like that. I'm going to add an init check & test to throw on socket paths longer than 92 characters. For the sake of passing the tests, I'll try using a temp path that is in the parent of `test_runner_...` and we'll see if that is short enough to pass on the CI VMs.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154738130)
To literally answer your question: I was not clear on what `-onion` actually does! Now I do, adding this in next update.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154739870)
agreed, updated.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154739487)
I only added this for my own debugging, we could even just take it out. There is also a "proxy failed to initialize" error message in `Socks5()` which will get thrown by `ConnectThroughProxy()` below.

💬 LarryRuane commented on pull request "reduce cs_main scope, guard block index 'nFile' under a local mutex":
(https://github.com/bitcoin/bitcoin/pull/27006#discussion_r1154827107)
Are you sure these are protected by `cs_main`, is it? Running `gdb test_bitcoin` on the PR branch (leaving some irrelevant stuff out):
```
(gdb) b MakeBlockInfo
(gdb) run
Thread 29 "b-basic block f" hit Breakpoint 1, kernel::MakeBlockInfo (index=0x55555749b018, data=0x0) at kernel/chain.cpp:14
#0 kernel::MakeBlockInfo (index=0x55555749b018, data=0x0) at kernel/chain.cpp:14
#1 0x000055555623fa81 in BaseIndex::ThreadSync (this=0x7fffffffad00) at index/base.cpp:201
(gdb) p cs_main
$1 = {
...

💬 pinheadmz commented on issue "anchors.dat doesn't support V2 addresses":
(https://github.com/bitcoin/bitcoin/issues/20511#issuecomment-1492512141)
@sipa was this closed by https://github.com/bitcoin/bitcoin/pull/20516 ?

💬 pinheadmz commented on issue "Avoid pruning blocks with transactions in wallets (even after wallets sync)":
(https://github.com/bitcoin/bitcoin/issues/20384#issuecomment-1492535735)
This looks like another issue that could be addressed by https://github.com/bitcoin/bitcoin/issues/21267

💬 hebasto commented on pull request "depends: harden libevent":
(https://github.com/bitcoin/bitcoin/pull/27118#issuecomment-1492606272)
This PR introduced a regression when building with depends and disabled hardening:
```
$ make -C depends HOST=x86_64-w64-mingw32 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1
$ ./configure --disable-hardening CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site
$ make
...
/usr/bin/x86_64-w64-mingw32-ld: /home/hebasto/git/gui/depends/x86_64-w64-mingw32/lib/libevent.a(evutil.o):evutil.c:(.text+0x2975): undefined reference to `__memcpy_chk'
/usr/bin/x86_64-w64-mingw32
...

💬 ishaanam commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1154926275)
If I'm understanding this correctly, this means that this could result in the creation of a transaction with 2 outputs that our wallet would identify as change. What would happen if we wanted to bump this transaction again, but keep the outputs the same? Though I haven't tested this, I think that the following will happen:

1. The first output is added as `new_coin_control.destChange`
2. Then we find the second output is also change and over-write the original `destChange` with the second out
...

📝 jonatack opened a pull request: "net: extract Network and BIP155Network logic to node/network"
(https://github.com/bitcoin/bitcoin/pull/27385)
This extracts the `Network` and `BIP155Network` logic to `node/network`. The code has been living between `netaddress` and `netbase`, and some compilation units include these large files when they only use a `Network` enum or related method. Separating the code to a standalone unit in `node` improves code separation, helps with using only what is needed, and may speed up build times.

In draft to check CI/builds/IWYU/assumptions, e.g. perhaps common instead of node.

Possible to-do for a f
...

💬 ishaanam commented on pull request "bumpfee: avoid making bumped transactions with too low fee when replacing outputs":
(https://github.com/bitcoin/bitcoin/pull/27308#issuecomment-1492717965)
ACK 26b4c66418f97b4d13989a5c6cf2a7827af4fc1e

💬 DevAgrawal1112 commented on issue "net: signed-integer-overflow in LocalServiceInfo":
(https://github.com/bitcoin/bitcoin/issues/24049#issuecomment-1492838178)
Since I am new to open source, I would like to know if I need to change int to int64_t for nScore or if I have to work on something else.

📝 DevAgrawal1112 opened a pull request: "#24049 Issue: Update nScore datatype"
(https://github.com/bitcoin/bitcoin/pull/27386)
I had just changed the datatype of nScore from int to int64_t as the issue suggest.

💬 hebasto commented on pull request "#24049 Issue: Update nScore datatype":
(https://github.com/bitcoin/bitcoin/pull/27386#issuecomment-1492877236)
@DevAgrawal1112 Thank you for your contribution!

Did you check the discussion in the previous [attempt](https://github.com/bitcoin/bitcoin/pull/24090) to resolve the issue?

📝 hebasto opened a pull request: "depends, doc: Document `NO_USDT` option"
(https://github.com/bitcoin/bitcoin/pull/27387)
A follow-up for https://github.com/bitcoin/bitcoin/pull/23724.