💬 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
```
(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.
(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
...
(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`.
(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
...
(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.
(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?
(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.
(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`
(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
(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)
(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.
(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?
(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
(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
...
(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
...
👍 theStack approved a pull request: "test: added coverage to rpc_scantxoutset.py"
(https://github.com/bitcoin/bitcoin/pull/27453#pullrequestreview-1408953706)
ACK 24d55fb9cfab88f546df35be5c0069b9b645438c
(https://github.com/bitcoin/bitcoin/pull/27453#pullrequestreview-1408953706)
ACK 24d55fb9cfab88f546df35be5c0069b9b645438c
💬 fjahr commented on issue "bitcoin core crashes when too many rpc calls are made":
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-1531357390)
> [libevent/libevent#592](https://github.com/libevent/libevent/pull/592) has now been merged into libevent master and should provide by far the cleanest solution to our issue here. The last release was cut 5th July 2020, so perhaps a new one is due soon, although they do not seem to follow any particular schedule that I can see. I have likewise reached out to them (but on their matrix channel).
>
> I also set up a notification for new relases with a reminder to fix this issue when it lands.
...
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-1531357390)
> [libevent/libevent#592](https://github.com/libevent/libevent/pull/592) has now been merged into libevent master and should provide by far the cleanest solution to our issue here. The last release was cut 5th July 2020, so perhaps a new one is due soon, although they do not seem to follow any particular schedule that I can see. I have likewise reached out to them (but on their matrix channel).
>
> I also set up a notification for new relases with a reminder to fix this issue when it lands.
...
💬 theStack commented on pull request "test: add ripemd160 to test framework modules list":
(https://github.com/bitcoin/bitcoin/pull/27542#discussion_r1182465178)
Good idea, done.
(https://github.com/bitcoin/bitcoin/pull/27542#discussion_r1182465178)
Good idea, done.
📝 sipa opened a pull request: "Add support for "partial" fuzzers that indicate usefulness"
(https://github.com/bitcoin/bitcoin/pull/27552)
This adds supports for fuzz targets that return a boolean: `true` is the normal case, while `false` indicates the input was uninteresting and should not under any circumstances be added to the corpus. This is intended for fuzz targets that have some early bail-out criteria, so that the fuzzer doesn't continue to iterate on them.
(https://github.com/bitcoin/bitcoin/pull/27552)
This adds supports for fuzz targets that return a boolean: `true` is the normal case, while `false` indicates the input was uninteresting and should not under any circumstances be added to the corpus. This is intended for fuzz targets that have some early bail-out criteria, so that the fuzzer doesn't continue to iterate on them.
💬 MarcoFalke commented on pull request "test: add ripemd160 to test framework modules list":
(https://github.com/bitcoin/bitcoin/pull/27542#issuecomment-1531367642)
lgtm ACK 82e6e3cae50d30b28b98f0cb7789c28e68170d5e
(https://github.com/bitcoin/bitcoin/pull/27542#issuecomment-1531367642)
lgtm ACK 82e6e3cae50d30b28b98f0cb7789c28e68170d5e