🤔 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

💬 laanwj commented on pull request "miniscript: convert non-critical asserts to Assumes":
(https://github.com/bitcoin/bitcoin/pull/28678#issuecomment-1787266550)
Concept ACK. Not having these potentially crash the application is an improvement.
Also agree with @darosior though, we don't want error conditions that can be caused by user input to go by silently, either.

📝 fanquake converted_to_draft a pull request: "build: Bump `native_clang` up to 17.0.2"
(https://github.com/bitcoin/bitcoin/pull/28732)
This PR makes it possible to build the `qt` package with macOS 14 SDK (Xcode 15.0).

For more details, please refer to https://github.com/bitcoin/bitcoin/pull/28622.

💬 fanquake commented on pull request "build: Bump `native_clang` up to 17.0.2":
(https://github.com/bitcoin/bitcoin/pull/28732#issuecomment-1787275533)
Discussed further offline. Moving this to draft for now. As mentioned, bumping Clang isn't required to use C++20, because bitcoind and other utils can already be compiled using C++20, against the newer SDK, so the assumption is that this is something that will need to be patched in Qt?

💬 fanquake commented on pull request "guix: Zip needs to include all files and set time to SOURCE_DATE_EPOCH":
(https://github.com/bitcoin/bitcoin/pull/28757#issuecomment-1787279028)
Yea, we should nuke the `dist/` here, otherwise, this looks good to me.

🤔 TheCharlatan reviewed a pull request: "wallet: Fix wallet directory initialization"
(https://github.com/bitcoin/bitcoin/pull/28514#pullrequestreview-1706431124)
Concept ACK

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1377694801)
> In that case, i'd probably say that explicitly in the function's name. Like SeedsServiceFlags.

Sounds good. Will rename it to `SeedsServiceFlags`.
I was thinking we would use this function for other cases in the future, but agree that this can be simplified for now.

> Then, a comment saying something like these flags should probably be in sync with GetDesirableServiceFlags, but not strictly.

I'm not sure about this comment. We could have a much more complex logic at the PeerManager l
...

💬 glozow commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1377697411)
Renamed to `IsConsistentPackage`

💬 laanwj commented on pull request "Do the SOCKS5 handshake reliably":
(https://github.com/bitcoin/bitcoin/pull/28649#issuecomment-1787362292)
Concept ACK. Good catch!