💬 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.

💬 hebasto commented on pull request "msvc: Provide `ObjectFileName` explicitly":
(https://github.com/bitcoin/bitcoin/pull/27687#issuecomment-1554347352)
Friendly ping @sipsorcery :)

💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198791887)
Yeah, it may be odd in the meantime, but if it is done for all the lines that are touched anyway and if longer term the ui_interface lines are touched, they can also be switched over?

💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1198793373)
Ah sure. You didn't want to move them to common, so you moved them to util ahead of the other change.

💬 MarcoFalke commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1554355740)
> Here's a test commit that adds that functionality https://github.com/bitcoin/bitcoin/commit/f26a85059fb4eaf29f9c1652b2b5e14060d79a36

Thanks. Did you want me to push it here (removing the logging), or did you want to open a fresh pull yourself?

Also, I think we dropped support for non-witness compact block relay, so I guess you can drop `txid` from your map and only use `wtxid`?

💬 MarcoFalke commented on pull request "[WIP] wallet: standardize change output detection process":
(https://github.com/bitcoin/bitcoin/pull/25979#issuecomment-1554360733)
Needs rebase if still relevant

💬 MarcoFalke commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1554361678)
Looks like CI failed?