💬 dergoegge commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1198645480)
```suggestion
CNodeState& nodestate = *Assert(State(nodeid));
```
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1198645480)
```suggestion
CNodeState& nodestate = *Assert(State(nodeid));
```
⚠️ kallerosenbaum opened an issue: "Sign PSBT: Can't verify change output"
(https://github.com/bitcoin-core/gui/issues/732)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When I load a PSBT from file to sign it, the dialog that appears looks like this:

There's no way for me to know that the second output belongs to me (it does). Without knowing this I'd feel really uncomfortable signing the PSBT.
### Expected behaviour
The
...
(https://github.com/bitcoin-core/gui/issues/732)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When I load a PSBT from file to sign it, the dialog that appears looks like this:

There's no way for me to know that the second output belongs to me (it does). Without knowing this I'd feel really uncomfortable signing the PSBT.
### Expected behaviour
The
...
💬 willcl-ark commented on issue "Enable consistency checks by default with `--enable-debug`":
(https://github.com/bitcoin/bitcoin/issues/24709#issuecomment-1554227886)
Before I actually push any more changes, I wonder if the following would work and feel "expected" to developers:
1. `--enable-debug`: behaviour change to disable lockorder/lockcontention consistency checks + behaviour change to disable `boost_multi_index_safe_mode`
2. `--enable-debug-checks`: implies 1. plus enables consistency checks (addrman, lockorder, lockcontention)
3. `--enable-debug-all`?: implies 2. plus enables `boost_multi_index_safe_mode`
The way I had been approaching this wa
...
(https://github.com/bitcoin/bitcoin/issues/24709#issuecomment-1554227886)
Before I actually push any more changes, I wonder if the following would work and feel "expected" to developers:
1. `--enable-debug`: behaviour change to disable lockorder/lockcontention consistency checks + behaviour change to disable `boost_multi_index_safe_mode`
2. `--enable-debug-checks`: implies 1. plus enables consistency checks (addrman, lockorder, lockcontention)
3. `--enable-debug-all`?: implies 2. plus enables `boost_multi_index_safe_mode`
The way I had been approaching this wa
...
💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1554246293)
Rebased because `<key_io.h>` moved (Kernel related?) and to add an explicit `include <univalue.h>` (macOS / clang doesn't care, but CI does and it makes sense anyway).
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1554246293)
Rebased because `<key_io.h>` moved (Kernel related?) and to add an explicit `include <univalue.h>` (macOS / clang doesn't care, but CI does and it makes sense anyway).
💬 fanquake commented on issue "ci: failure in Docker build step":
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1554257095)
Closing for now.
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1554257095)
Closing for now.
✅ fanquake closed an issue: "ci: failure in Docker build step"
(https://github.com/bitcoin/bitcoin/issues/27492)
(https://github.com/bitcoin/bitcoin/issues/27492)
⚠️ fanquake reopened an issue: "ci: failure in Docker build step"
(https://github.com/bitcoin/bitcoin/issues/27492)
Failure here: https://cirrus-ci.com/task/4581145136857088 in #27479. Does this just need a rebase? cc @MarcoFalke
```bash
docker build --tag gcr.io/cirrus-ci-community/bitcoin/bitcoin/ci/test_imagefile:d050f5b1ebc8c0ebdf779e9eda85ea03bfd0fca46391a14e57fbe478652e6623 --file ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG="ubuntu:focal" --build-arg FILE_ENV="./ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh" ${CIRRUS_DOCKER_CONTEXT:-$CIRRUS_WORKING_DIR}
Sending build context to Docker
...
(https://github.com/bitcoin/bitcoin/issues/27492)
Failure here: https://cirrus-ci.com/task/4581145136857088 in #27479. Does this just need a rebase? cc @MarcoFalke
```bash
docker build --tag gcr.io/cirrus-ci-community/bitcoin/bitcoin/ci/test_imagefile:d050f5b1ebc8c0ebdf779e9eda85ea03bfd0fca46391a14e57fbe478652e6623 --file ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG="ubuntu:focal" --build-arg FILE_ENV="./ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh" ${CIRRUS_DOCKER_CONTEXT:-$CIRRUS_WORKING_DIR}
Sending build context to Docker
...
🚀 fanquake merged a pull request: "build: Bump minimum supported GCC to g++-9"
(https://github.com/bitcoin/bitcoin/pull/27662)
(https://github.com/bitcoin/bitcoin/pull/27662)
💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1554266029)
Oops, I didn't rebase all the way. Specifically not past #27605. Will fix.
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1554266029)
Oops, I didn't rebase all the way. Specifically not past #27605. Will fix.
🚀 fanquake merged a pull request: "build: Detect USDT the same way how it is used in the code"
(https://github.com/bitcoin/bitcoin/pull/27458)
(https://github.com/bitcoin/bitcoin/pull/27458)
💬 MarcoFalke commented on issue "ci: failure in Docker build step":
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1554285090)
Note for reviewers if they encounter a rebase: https://github.com/bitcoin/bitcoin/blob/master/doc/productivity.md#diff-the-diffs-with-git-range-diff
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1554285090)
Note for reviewers if they encounter a rebase: https://github.com/bitcoin/bitcoin/blob/master/doc/productivity.md#diff-the-diffs-with-git-range-diff
💬 fanquake commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1198751058)
Added `[[maybe_unused]]` to GetDevURandom.
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1198751058)
Added `[[maybe_unused]]` to GetDevURandom.
💬 fanquake commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#issuecomment-1554300778)
Added a commit to use `[[maybe_unused]]` on `GetDevURandom`, rather than adding another instance of `(void)GetDevURandom`, to silence a warning.
Added a commit to slightly refactor `getentropy()` detection on macOS, so that we can combine the #ifdefs for the `sys/random.h` include.
(https://github.com/bitcoin/bitcoin/pull/27699#issuecomment-1554300778)
Added a commit to use `[[maybe_unused]]` on `GetDevURandom`, rather than adding another instance of `(void)GetDevURandom`, to silence a warning.
Added a commit to slightly refactor `getentropy()` detection on macOS, so that we can combine the #ifdefs for the `sys/random.h` include.
🚀 fanquake merged a pull request: "test: Add test to check tx in the last block can be downloaded"
(https://github.com/bitcoin/bitcoin/pull/27695)
(https://github.com/bitcoin/bitcoin/pull/27695)
👍 MarcoFalke approved a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1433987014)
nice, lgtm ACK d96c82a76775b1a41c098e6af60130fbdbba9975 🏬
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: nice, lgtm ACK d96c
...
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1433987014)
nice, lgtm ACK d96c82a76775b1a41c098e6af60130fbdbba9975 🏬
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: nice, lgtm ACK d96c
...
💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198713287)
1be1c16b70edc5def98eb1903cee08b5d546ba88: Not sure about passing only the translated string here. I think it should be up to the receiving code to decide whether to use `.original` (`bitcoin-chainstate.cpp`) or `.translated` (`node/kernel_notif.cpp`)
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198713287)
1be1c16b70edc5def98eb1903cee08b5d546ba88: Not sure about passing only the translated string here. I think it should be up to the receiving code to decide whether to use `.original` (`bitcoin-chainstate.cpp`) or `.translated` (`node/kernel_notif.cpp`)
💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198680321)
974b4293a5f2fe01302c608bf0063e3b52721d0b: Any reason to accept a nullptr for something that can never be null and would lead to UB if it was null? Might be better to use a reference.
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198680321)
974b4293a5f2fe01302c608bf0063e3b52721d0b: Any reason to accept a nullptr for something that can never be null and would lead to UB if it was null? Might be better to use a reference.
💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198753425)
c05c15cc7b4fb9052b87bc2f8ea0500cd5218f4e: question for clarity: This commit is not needed and unrelated to kernel? Seems fine to do, just asking.
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198753425)
c05c15cc7b4fb9052b87bc2f8ea0500cd5218f4e: question for clarity: This commit is not needed and unrelated to kernel? Seems fine to do, just asking.
💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198757020)
nit in e850319d5336335a9efbf04307261fb565b7bc68: Would be good to either not touch it or remove the comment?
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198757020)
nit in e850319d5336335a9efbf04307261fb565b7bc68: Would be good to either not touch it or remove the comment?
💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198767568)
d96c82a76775b1a41c098e6af60130fbdbba9975: I think you forgot the call to `std::abort`, otherwise it seems that the error isn't treated fatal?
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198767568)
d96c82a76775b1a41c098e6af60130fbdbba9975: I think you forgot the call to `std::abort`, otherwise it seems that the error isn't treated fatal?