⚠️ fanquake opened an issue: "32-bit Linux: build flags lost with depends & overriden CC(X)"
(https://github.com/bitcoin/bitcoin/issues/28096)
Building depends for 32-bit Linux (i686-pc-linux-gnu) but setting `CC`/`CXX`, causes us to loose build flags, and ultimately build a 64-bit executable:
```bash
# make -C depends/ HOST=i686-pc-linux-gnu -j9 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_USDT=1 NO_NATPMP=1 CC=clang CXX=clang++
...
copying packages: boost libevent
to: /home/ubuntu/bitcoin/depends/i686-pc-linux-gnu

./autogen.sh
CONFIG_SITE=/home/ubuntu/bitcoin/depends/i686-pc-linux-gnu/share/config.site ./configure
make -9
...
...

📝 fanquake opened a pull request: "depends: xcb-proto 1.15.2"
(https://github.com/bitcoin/bitcoin/pull/28097)
Resolves build failures with Python 3.12, i.e building on rawhide:
```bash
make -C depends -j9
...
make[3]: Nothing to be done for 'install-exec-am'.
/usr/bin/mkdir -p '/bitcoin/depends/work/staging/aarch64-unknown-linux-gnu/xcb_proto/1.14.1-4a91ac9dc41/bitcoin/depends/aarch64-unknown-linux-gnu/lib/python3.12/site-packages/xcbgen'
/usr/bin/install -c -m 644 __init__.py error.py expr.py align.py matcher.py state.py xtypes.py '/bitcoin/depends/work/staging/aarch64-unknown-linux-gnu/xcb_pro
...

💬 vasild commented on pull request "I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed"":
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1639987591)
@fanquake, There is an issue with the I2P router - there is a discrepancy between the router behavior and the SAM specification. Then, this unexpected behavior causes Bitcoin Core to log way too many messages. It becomes a problem with Bitcoin Core too. I should report this upstream as per https://geti2p.net/en/faq#bug.

⚠️ MarcoFalke opened an issue: "ci: Future of macOS and Windows MSVC CI tasks"
(https://github.com/bitcoin/bitcoin/issues/28098)
Cirrus CI will cap the community cluster, see https://cirrus-ci.org/blog/2023/07/17/limiting-free-usage-of-cirrus-ci/ .

Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks. Not sure if this is worth it, given that appveyor CI used to cost 50 USD/mo. Also, I am not sure if a sponsor can be found. Personally, I don't use macOS nor Windows, and I can't recommend it to anyone, so I don't mind if the tasks are removed. Though, I am opening this issue to gather fee
...

💬 fanquake commented on pull request "ci: document that -Wreturn-type has been fixed upstream (mingw-w64)":
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1639995430)
> Looks like this triggers an OOM for some reason, when it shouldn't?

Looks like this is the same bug with overriding cxxflags dropping other flags. Will fix that.

💬 vasild commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#issuecomment-1639995493)
The tests from https://github.com/bitcoin/bitcoin/pull/27509 failed because of this which prompted me to open this PR.

💬 MarcoFalke commented on pull request "ci: document that -Wreturn-type has been fixed upstream (mingw-w64)":
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1640000922)
Ok, an alternative would be to embed the `-Wno-return-type` into the compiler `CC` and `CXX` env vars, like I did for the valgrind task.

💬 MarcoFalke commented on issue "32-bit Linux: build flags lost with depends & overriden CC(X)":
(https://github.com/bitcoin/bitcoin/issues/28096#issuecomment-1640003448)
Isn't `-m32` set by oss-fuzz already? Also, if you compile locally, I'd always assume you'd have to embed `-m32` into the `CC` and `CXX` env vars.

💬 fanquake commented on issue "32-bit Linux: build flags lost with depends & overriden CC(X)":
(https://github.com/bitcoin/bitcoin/issues/28096#issuecomment-1640008851)
> Isn't -m32 set by oss-fuzz already?

Yes. I think in the oss-fuzz case it's an additional interaction with `DEBUG=1`, which causes sqlite to loose the flags.

> Also, if you compile locally, I'd always assume you'd have to embed -m32 into the CC and CXX env vars.

Builds work as expected when not overriding CC & CXX, I don't think there's a reason to expect that not to work, without additional flags, when using a different compiler.

🤔 stickies-v reviewed a pull request: "net, refactor: remove unneeded exports, use helpers over low-level code, use optional"
(https://github.com/bitcoin/bitcoin/pull/28078#pullrequestreview-1534736352)
Approach ACK 5d7188ed92908237ce51bbe6ed0a97472825c6b8

💬 stickies-v commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1266591147)
As we're now touching all these LoC, would suggest to clean up even further. Slightly increases the diff but worth it for readability imo.

<details>
<summary>git diff on 5d7188ed92908237ce51bbe6ed0a97472825c6b8</summary>

```diff
diff --git a/src/net.cpp b/src/net.cpp
index eee179c8a..b35725ea2 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -154,21 +154,21 @@ uint16_t GetListenPort()
int nBestReachability = -1;
{
LOCK(g_maplocalhost_mutex);
- for (const au
...

💬 stickies-v commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1266584113)
nit: is "TODO" easier (i.e. more likely someone does it) to grep than "Note:"? Also, I think these last 2 lines are better placed near the definition of `IsPrivacyNet()`.

💬 stickies-v commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1266596572)
Yeah I think that's a good approach.

💬 stickies-v commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1266580999)
nit: would not include the ~classname in method name

```suggestion
[[nodiscard]] bool HasCJDNSPrefix() const { return m_addr[0] == 0xfc; }
```

💬 stickies-v commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1266621620)
Maybe let's not move all these lines? A bit unnecessary churn, is fine as is imo.

💬 pinheadmz commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1266640671)
Weird, this comment was posted yesterday but is dated over a month ago (Jun 9). I thought for sure I had tested this case locally and didn't see a missing output -- is this still the case here? Since we check `reduce_output.has_value()` in the if condition, doesn't that mean that if that option is set only the selected option will be designated as change?

💬 real-or-random commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1640091409)
Why are Linux tasks not affected? As far as I understand the blog post, there will be a monthly limit of 40 compute credits (= 40 USD) for all tasks, including Linux tasks. (Or are we already using a persistent worker for Linux?)

💬 tobtoht commented on pull request "depends: xcb-proto 1.15.2":
(https://github.com/bitcoin/bitcoin/pull/28097#issuecomment-1640111808)
Guix builds

```
6d1725c9346bdf04e1eeec2deffda90957bd081700ba896834a143756410fb0e guix-build-7cb88c8b4672/output/aarch64-linux-gnu/SHA256SUMS.part
34486c39daea8a3ce8d92e9c1501f26e5f5e300d9612a25bd56c48ab56353329 guix-build-7cb88c8b4672/output/aarch64-linux-gnu/bitcoin-7cb88c8b4672-aarch64-linux-gnu-debug.tar.gz
cb0080d75d0fc4fc5c7fc022b5771470c60609c35836e7b66c4f479a2edcd098 guix-build-7cb88c8b4672/output/aarch64-linux-gnu/bitcoin-7cb88c8b4672-aarch64-linux-gnu.tar.gz
395c332f8320612581
...

💬 furszy commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1266748837)
> I thought for sure I had tested this case locally and didn't see a missing output -- is this still the case here? Since we check `reduce_output.has_value()` in the if condition, doesn't that mean that if that option is set only the selected option will be designated as change?

Yeah. Seems that I was partially blind yesterday. This issue can still be triggered by a transaction with +2 change outputs and no `reduce_output` (`OutputIsChange()` will be true for them, which overwrites `destChang
...

💬 furszy commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1266754466)
> Let's document which part of CoinSelectionParams is modified and which is not.

@S3RK. `CoinSelectionParams` wasn't modified. Only change related code was moved from `CreateTransactionInternal` to `ComputeChangeParams`.