🤔 real-or-random reviewed a pull request: "contrib/init: Better systemd integration"
(https://github.com/bitcoin/bitcoin/pull/25975#pullrequestreview-1408834823)
ACK mod nits

This is very useful for systemd users. The changes match systemd docs and I tested that process management and logging works as intended.

@achow101 Can you reopen this?


One minor issue: This means what (with default config), we have log output both in systemd's journal and in the log file. In principle, it may make sense to also pass `-nodebuglogfile`, but I'm not sure if this is really what we want because it means that the user can't override this option in the config
...

💬 real-or-random commented on pull request "contrib/init: Better systemd integration":
(https://github.com/bitcoin/bitcoin/pull/25975#discussion_r1182373299)
```suggestion
-startupnotify='systemd-notify --ready' \
-shutdownnotify='systemd-notify --stopping'
```
Shutdown notifications are just "nice to have" and I don't think anyone will rely on this, but now that we have the command-line option, why not use it here.

💬 MarcoFalke commented on pull request "test: add ripemd160 to test framework modules list":
(https://github.com/bitcoin/bitcoin/pull/27542#issuecomment-1531251936)
lgtm ACK 768ae178affa5be5960dfff38f7167a13f99e774

💬 MarcoFalke commented on pull request "test: add ripemd160 to test framework modules list":
(https://github.com/bitcoin/bitcoin/pull/27542#discussion_r1182382247)
Maybe add a comment that this should be kept in sync with the output from `grep`:

```sh
git grep unittest.TestCase ./test/functional/test_frame

📝 MarcoFalke reopened a pull request: "contrib/init: Better systemd integration"
(https://github.com/bitcoin/bitcoin/pull/25975)
```
1. Make logs available to journalctl (systemd's logging system) by not
specifying -daemonwait, which rightfully has its own set of stdout
and stderr descriptors (a user invoking with -daemonwait on the
command line should not see any logs). It makes more sense not to
daemonize in the systemd context anyway.

2. Make systemd aware of when bitcoind is started and in steady state by
specifying -startupnotify='systemd-notify --ready' and Type=notify.
NotifyAccess=all i
...

💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in native_asan job":
(https://github.com/bitcoin/bitcoin/pull/27360#discussion_r1182388624)
```suggestion
compute_engine_instance: # https://cirrus-ci.org/guide/custom-vms/#custom-compute-engine-vms
disk: 100
```

💬 ajtowns commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#issuecomment-1531265734)
> Yeah I'm pretty optimistic that cluster mempool will provide a natural solution like this.

Probably, but I suspect ephemeral anchors or similar will be ready before we've got all the i's dotted and t's crossed on cluster mempool, so some stop gap patch will be worthwhile. Still, we can cross that bridge once we get there.

💬 willcl-ark commented on issue "bitcoin core crashes when too many rpc calls are made":
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-1531268374)
> My impression is that mostly these issues are caused by people spamming their own rest interface with their own scripts. If users know about the issue they should be able to accommodate it pretty easily and not be surprised by the issue.

Agreed.

> I am happy to investigate other ideas but I currently don't see a real fix other than waiting until `libevent` 2.2 is released which should include either [libevent/libevent#578](https://github.com/libevent/libevent/pull/578) and/or [libevent/l
...

📝 brunoerg converted_to_draft a pull request: "test: merge banning test from p2p_disconnect_ban to rpc_setban"
(https://github.com/bitcoin/bitcoin/pull/26863)
There are tests related to banning in both `p2p_disconnect_ban` and `rpc_setban`, some of them are testing same scenarios. So, this PR moves all banning test stuff to `rpc_setban` and leaves `p2p_disconnect_ban` only for disconnecting stuff. Since `p2p_disconnect_ban` doesn't have any banning stuff, this PR also renames that to `p2p_disconnect`.

📝 Bushstar opened a pull request: "Reduce calls to various SetNull methods"
(https://github.com/bitcoin/bitcoin/pull/27551)
This PR aims to defaults initialise various class members and make redundant several calls to SetNull methods for those classes. This makes the default constructor for a few classes without any other constructors redundant as well but rather than remove them I'll created them with default in case of compiler complaints.

- Default initialise m_data member in base_blob which makes calls to SetNull on uint256 redundant, create base_blob() with default rather than remove in case of compiler comp
...

💬 MarcoFalke commented on pull request "Wallet: Add foreign_outputs metadata to support CoinJoin transactions":
(https://github.com/bitcoin/bitcoin/pull/25991#discussion_r1182407309)
For new RPCs it might be good to return an empty dict to avoid breaking changes later on and confusing behavior now.

💬 MarcoFalke commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1531298012)
Would you mind submitting the test to test current `master` behavior for `-nodatacarrier`, or would you mind if someone else did that?

💬 fanquake commented on issue "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm.":
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1531299855)
This works for me on master. It's not clear how you're hitting that issue, because passing `--disable-asm` will skip the usage of the `crc32c::ExtendArm64` code entirely.

💬 MarcoFalke commented on issue "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm.":
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1531306934)
Also, make sure to type `make clean` before `make`

💬 liuyangc3 commented on issue "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm.":
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1531324382)
Thanks, after I switch to master and run `make clean` I got `configure: error: cannot find required auxiliary files: ar-lib`
not sure what is `ar-lib`, how can I install it? I googled but not much help

💬 kevkevinpal commented on pull request "test: added coverage to rpc_scantxoutset.py":
(https://github.com/bitcoin/bitcoin/pull/27453#issuecomment-1531332579)
> Concept ACK,
>
> and warm welcome as a new contributor!
>
> Looks good to me, just one nit: the commit subject line has a missing-character-typo in the functional test filename (rpc_cantxoutset.py -> should be rpc_scantxoutset.py), can you fix that please?

Thanks! Fixed [24d55fb](https://github.com/bitcoin/bitcoin/pull/27453/commits/24d55fb9cfab88f546df35be5c0069b9b645438c)

🤔 mzumsande reviewed a pull request: "fuzz: addrman, add coverage for `network` field in `Select()`"
(https://github.com/bitcoin/bitcoin/pull/27549#pullrequestreview-1408940288)
Concept ACK.
I noticed this too, so a recently added commit from #27213, https://github.com/bitcoin/bitcoin/pull/27213/commits/8f91e5898b3571e0802f2197981c54dda9ee71fa, also includes this - plus coverage for the other network-specific functions (`GetAddr()`, `Size()`). I think it's fine to do it in a separate PR too, but while at it, it would be good to also add coverage for these other functions.

💬 MarcoFalke commented on issue "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm.":
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1531332969)
What is the `config.log`, or output?

💬 MarcoFalke commented on pull request "test: added coverage to rpc_scantxoutset.py":
(https://github.com/bitcoin/bitcoin/pull/27453#issuecomment-1531333931)
lgtm ACK 24d55fb9cfab88f546df35be5c0069b9b645438c

💬 liuyangc3 commented on issue "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm.":
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1531336487)
> What is the `config.log`, or output?

```
./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined --disable-asm CC=/opt/homebrew/opt/llvm/bin/clang CXX=/opt/homebrew/opt/llvm/bin/clang++
checking for pkg-config... /opt/homebrew/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking build system type... aarch64-apple-darwin22.4.0
checking host system type... aarch64-apple-darwin22.4.0
checking for a BSD-compatible install... /opt/homebrew/bin/ginstall
...