Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ 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
...
🚀 fanquake merged a pull request: "build: Bump minimum supported GCC to g++-9"
(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.
🚀 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)
💬 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.
💬 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.
🚀 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)
👍 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
...
💬 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`)
💬 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.
💬 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.
💬 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?
💬 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?
💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198758293)
nit in https://github.com/bitcoin/bitcoin/commit/e850319d5336335a9efbf04307261fb565b7bc68: Wrong section
💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198784478)
brainstorming nit in d96c82a76775b1a41c098e6af60130fbdbba9975: Not sure about adding test-only code to real code. Would it be a lot more effort to add a test-only derived `MockNotif : node::KernelNotifications` struct that no-ops the fatalError method and use it in the one test?
💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198753949)
Same for `AnyPtr`?
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198788990)
Just seemed weird to go from `CBlockIndex*` to `CBlockIndex&` back to `CBlockIndex*`
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198790430)
Not strictly required, but I decided this makes sense, since these are currently in `namespace util` and are just a small amount of header-only code. See the discussion we had here: https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196623448.
💬 fanquake commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198790811)
Yea. In general, it's fine for those comments to just be dropped if touching the line.