👍 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?
(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?
(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?
(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)
(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 :)
(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
(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?
(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).
(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
(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
...
(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
...
(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.
(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.
(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.
(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
...
(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
(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.
(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.
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1202468603)
In ad5642ae:
This two declarations can be removed. Can just make the definition static.
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1202477990)
In ad5642ae:
Can remove the "\n" separator by providing the `command` variable instead of the `executableCommand` string at line 428. The code adds the "\n" few lines before calling `executeConsoleOnlyCommand`.
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1202477990)
In ad5642ae:
Can remove the "\n" separator by providing the `command` variable instead of the `executableCommand` string at line 428. The code adds the "\n" few lines before calling `executeConsoleOnlyCommand`.
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1202493790)
In ad5642ae:
Wrong Indentation here.
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1202493790)
In ad5642ae:
Wrong Indentation here.
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1202482937)
In ad5642ae:
Can remove the conditional statement entirely and always return true.
1) Functions are mapped in a "command name" -> function map structure. The code will never reach this point If the command wouldn't be a `help-console`.
2) The `!parsed_command.empty()` statement is not needed. It will never be empty at this point, the parent dispatcher function already checks for emptiness.
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1202482937)
In ad5642ae:
Can remove the conditional statement entirely and always return true.
1) Functions are mapped in a "command name" -> function map structure. The code will never reach this point If the command wouldn't be a `help-console`.
2) The `!parsed_command.empty()` statement is not needed. It will never be empty at this point, the parent dispatcher function already checks for emptiness.