👍 hebasto approved a pull request: "build: remove duplicate `-lminiupnpc` linking"
(https://github.com/bitcoin/bitcoin/pull/28755#pullrequestreview-1706070279)
ACK b74e449ffa9965f18037f0322ea178e2fe1dbc18

`./configure` output:
- in the master branch:
```
...
checking for miniupnpc/miniupnpc.h... yes
checking for upnpDiscover in -lminiupnpc... yes
checking for miniupnpc/upnpcommands.h... yes
checking for upnpDiscover in -lminiupnpc... (cached) yes
checking for miniupnpc/upnperrors.h... yes
checking for upnpDiscover in -lminiupnpc... (cached) yes
checking whether miniUPnPc API version is supported... yes
checking for natpmp.h... yes
checki
...

📝 fanquake opened a pull request: "guix: update signapple to latest master"
(https://github.com/bitcoin/bitcoin/pull/28759)
Fixes #28449, and removes the need to boostrap Rust, by avoiding the `python-requests` dependency.

Draft for now, as we still need https://github.com/achow101/signapple/pull/11, but anyone should be able to verify the bootstrapping difference with this branch.

Comparing a `--no-substitutes` build of this PR, to master, signapple requires ~1350 _less_ packages to boostrap:
Master derivation - https://gist.github.com/fanquake/dbf69a62c9a78b7ae8c183a160e6d58d
PR derivation - https://gist.gi
...

💬 fanquake commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1377456646)
This should be including `compat.h`, rather than introducing the same networking #ifdef here?

💬 hebasto commented on pull request "guix: update signapple to latest master":
(https://github.com/bitcoin/bitcoin/pull/28759#issuecomment-1787057059)
Concept ACK.

💬 brunoerg commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1787069782)
Rebased and changed the approach in fuzz to call `Ban` only with valid net address/subnet.

💬 fanquake commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1787095966)
Updated to a newer hash. We've also upstreamed a GCC bump from 10.4.0 to 10.5.0: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2fbb5398a39bf18e41235891a0740fa0bc4d7a4d.

💬 glozow commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1377493306)
Ok, renamed this to `IsTopoSortedPackage`, i.e. "is a topologically sorted package" to be more descriptive.

💬 dergoegge commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1377497946)
can you do the same for `IsConsistent`? e.g. `IsPackageConsistent`

🤔 furszy reviewed a pull request: "gui: fix crash on selecting "Mask values" in transaction view"
(https://github.com/bitcoin-core/gui/pull/774#pullrequestreview-1706180484)
Could also:
```diff
diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp
@@ -656,7 +656,7 @@
{
// close all dialogs opened from this view
for (QDialog* dlg : m_opened_dialogs) {
- dlg->close();
+ if (dlg) dlg->close();
}
m_opened_dialogs.clear();
}
```

🤔 furszy reviewed a pull request: "gui: fix crash on selecting "Mask values" in transaction view"
(https://github.com/bitcoin-core/gui/pull/774#pullrequestreview-1706180943)
utACK e26e665f9

👋 vasild's pull request is ready for review: "ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL"
(https://github.com/bitcoin/bitcoin/pull/28736)

⚠️ naumenkogs opened an issue: "Evicting and filling attack for linking multiple network addresses"
(https://github.com/bitcoin/bitcoin/issues/28760)
I've [discovered a paper](https://cybersecurity.springeropen.com/articles/10.1186/s42400-023-00182-9) published earlier this month. I haven't found any discussion in the repo, so I will summarize the relevant parts here and share my thoughts.

Let's say we have a node that operates both over ipv4 and TOR. We don't want an observer to link these two addresses to the same node. For example, ADDR caching (#18991) was implemented for this reason.

The paper suggests the following attack:
1. Fil
...

💬 naumenkogs commented on issue "Evicting and filling attack for linking multiple network addresses":
(https://github.com/bitcoin/bitcoin/issues/28760#issuecomment-1787154709)
Recently, someone at the meetup told me that other projects usually consider this a lost cause. Apparently, it's easier to maintain two separate nodes and connect them if needed. This is probably not the case for Bitcoin, where the difference between *run one multi-homed node* and *run two single-homed nodes* is substantial enough for a regular user, so we better facilitate it (at least continue implementing basic privacy improvements).

At the same time, things like #27509 hopefully reduce th
...

💬 theStack commented on pull request "gui: fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1787177758)
> Could also:
>
> ```diff
> diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp
> @@ -656,7 +656,7 @@
> {
> // close all dialogs opened from this view
> for (QDialog* dlg : m_opened_dialogs) {
> - dlg->close();
> + if (dlg) dlg->close();
> }
> m_opened_dialogs.clear();
> }
> ```

This wouldn't change the behaviour, as a pointer in the `m_opened_dialogs` list is not set to NULL after the dialog it points to is destroyed. I think
...

⚠️ dergoegge opened an issue: "Undefined behavior in AutoFile::write (gcc only)"
(https://github.com/bitcoin/bitcoin/issues/28761)
```bash
$ CC=gcc CXX=g++ ./configure --with-sanitizers=undefined && make
$ export UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1:report_error_type=1
$ FUZZ=utxo_snapshot ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/utxo_snapshot/
streams.cpp:48:24: runtime error: null pointer passed as argument 1, which is declared to never be null
#0 0x5634737bcdc0 in AutoFile::write(Span<std::byte const>) src/streams.cpp:48
#1 0x5634749e553b in void Serialize<AutoFile, unsigned char const
...

👍 TheCharlatan approved a pull request: "build: remove duplicate `-lminiupnpc` linking"
(https://github.com/bitcoin/bitcoin/pull/28755#pullrequestreview-1706312043)
ACK b74e449ffa9965f18037f0322ea178e2fe1dbc18

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1377597617)
Thank you, Will update if there is need to re touch.

💬 fanquake commented on pull request "build: Bump `native_clang` up to 17.0.2":
(https://github.com/bitcoin/bitcoin/pull/28732#issuecomment-1787240987)
Can you explain this change? Using a newer Xcode is one of the last the blockers for C++20.

👍 laanwj approved a pull request: "wallet: Cleanup accidental encryption keys in watchonly wallets"
(https://github.com/bitcoin/bitcoin/pull/28724#pullrequestreview-1706334932)
Code review ACK aaa6b709d164052c8b2496cbedccf8ccf48c4aa2

💬 laanwj commented on pull request "guix: update signapple to latest master":
(https://github.com/bitcoin/bitcoin/pull/28759#issuecomment-1787250103)
> Fixes https://github.com/bitcoin/bitcoin/issues/28449, and removes the need to boostrap Rust, by avoiding the python-requests dependency.

Reducing the bootstrapping footprint is great. concept ACK.

💬 laanwj commented on pull request "depends: Allow PATH with spaces in directory names.":
(https://github.com/bitcoin/bitcoin/pull/28733#issuecomment-1787256148)
Concept ACK