💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1202123731)
I agree that harder to debug test failure messages are to be avoided, but given the nature of the statement, I also don't mind the current approach to avoid having to create a separate var for the `optional` and then the `CNetAddr`, even if it's just the one line.

It's a shame the `BOOST_` functions don't support pass-through of return values, which would have solved this elegantly. We could use our `Assert` function (requires `#include <util/check.h>`), which yields an equally useful error
...

💬 ajtowns commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1202129704)
We could; it'd mean adding an extra field to the mempool, and using both mockable time and steady time for dealing with next invs (mockable time so we can skip over delays in sending invs; steady time for this purpose).

👍 MarcoFalke approved a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1439540106)
can squash to avoid touching the same line twice?

💬 hebasto commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1202205772)
Why this change?

💬 MarcoFalke commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1202222611)
Might have to use `LossyChronoFormatter` to avoid the fuzz crash?

🚀 fanquake merged a pull request: "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`"
(https://github.com/bitcoin/bitcoin/pull/27717)

💬 willcl-ark commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1559240120)
> can squash to avoid touching the same line twice?

Now squashed :)

👍 hebasto approved a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1439642762)
ACK 59c89447499bd9d6202269879555b8bc37373aa2

💬 hebasto commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1202283522)
nit: Could split the long line?

💬 willcl-ark commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1202287841)
Sure, will do if I touch again :)

I did think about it at the time, but some CI tasks seem to be split on ~100 chars, and some just have long lines. So I just left this one as "long" (and made it longer).

💬 Crypt-iQ commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1559296260)
I think something like #19434 that uses `evhttp_connection_set_closecb` is needed to avoid this

💬 hebasto commented on pull request "depends: remove redundant stdlib option":
(https://github.com/bitcoin/bitcoin/pull/27721#issuecomment-1559341906)
> This should be a no-op.

I cannot confirm it, at least for `x86_64-apple-darwin`
```
$ git fetch origin pull/27721/head
$ git checkout FETCH_HEAD
$ make -C depends clean-all
$ make -C depends HOST=x86_64-apple-darwin
$ find depends/built/x86_64-apple-darwin -name '*.hash' | sort | xargs cat
afc3a7c266402a88b9c2bbbaa11a4dfceca5b1d4d33290080b9c31c8f5c65d25 bdb-4.8.30-d31fb2c14af.tar.gz
460b8455718a04f7d25d84179149c2d9b98b3318eedb3f89f50f93b6cff3d8ec boost-1.81.0-6596e037cc7.tar.gz
7
...

💬 theStack commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1559453982)
Thanks for reporting! I observed this issue sporadically, but were never able to reproduce it until now. With the help of your description (_"... interrupting bitcoin-cli before it has received the reply ..."_) I found it's actually quite trivial at least on my machine:

1. startup `bitcoind -debug=http` (and any other desired flags; note that the hangup also happens without the debug flag, but this allows us to see where it's hanging)
2. execute `bitcoin-cli waitforblockheight 1234567` (or a
...

💬 hebasto commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1559478702)
Retracting my ACK, as `FILE_ENV="./ci/test/00_setup_env_native_fuzz_with_msan.sh" ./ci/test_run_all.sh` fails locally for me.

💬 hebasto commented on pull request "depends: remove redundant stdlib option":
(https://github.com/bitcoin/bitcoin/pull/27721#issuecomment-1559505920)
> > This should be a no-op.
>
> I cannot confirm it...

Well, `diffoscope` shows only diffs in object files timestamps.

💬 MarcoFalke commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1559553912)
> Retracting my ACK, as FILE_ENV="./ci/test/00_setup_env_native_fuzz_with_msan.sh" ./ci/test_run_all.sh fails locally for me.

Seems unrelated, as this is failing on master. I agree it should be fixed, but I don't see why something unrelated should be holding back this pull.

💬 willcl-ark commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1559577176)
Perhaps I can drop the fuzz CPPFLAGS (introduced in this pull) for now, so that nothing is broken after this commit?

```diff
diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh
index dd694f818c..35a0de8034 100755
--- a/ci/test/00_setup_env_native_fuzz_with_msan.sh
+++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh
@@ -17,7 +17,7 @@ export PACKAGES="clang-16 llvm-16 libclang-rt-16-dev cmake"
# BDB generates false-positives and will
...

💬 MarcoFalke commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1559600221)
> ... so that nothing is broken after this commit?

Again, this is unrelated, because the issue happens in `master`. See also https://cirrus-ci.com/task/4517884630663168?logs=ci#L4910

🤔 furszy reviewed a pull request: "Debug Console implementation of generate method"
(https://github.com/bitcoin-core/gui/pull/692#pullrequestreview-1439888882)
Looking much better, thanks for applying the suggestions 👌🏼.

Left few comments, it's getting closer.

💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1202468603)
In ad5642ae:

This two declarations can be removed. Can just make the definition static.