👍 vasild approved a pull request: "addrman: Enable selecting addresses by network"
(https://github.com/bitcoin/bitcoin/pull/27214)
ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6

Maybe other reviewers would be interested in the performance discussion:
https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1130844842

💬 petertodd commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1483728729)
@ajtowns
> > this PR proposes deleting an option that .. (2) does no harm to the node operator.
>
> This isn't true. Running a non-default mempool makes it easier to put different txs in your mempool compared to the majority of the network, which:
>
> * makes it easier to fingerprint your node and its network peers

How specifically does turning *on* full-rbf enable that attack?

> * creates pinning vectors

Can you explain what exactly you mean by this in the context of fu
...

💬 vasild commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1148307887)
@brunoerg, yes, that should still bring some improvement. But only push it into the vector if we have visited all addresses in that bucket and found 0 from the requested network because it may happen that we visit a bucket with an address from the requested network, but only pick it up on some of the subsequent iterations, not from the first one. Change it to `std::unordered_set` so that lookup is fast (`O(1)`).

💬 petertodd commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1483735264)
@ariard
> * I think the conciliation of the 0confs use-case with new policy regime like nversion=3 has not been very discussed (e.g I believe 0conf software might have to upgrade themselves to reject future nversion=v3 transactions).

To be clear, 0conf users have to "upgrade" every single time mempool policies/conditions change for any non-trivial amount of hashing power. Pretty much any mempool policy change can be exploited to double-spend unconfirmed transactions. That's why the only ent
...

💬 kyranjamie commented on pull request "BIP-322 basic support":
(https://github.com/bitcoin/bitcoin/pull/24058#discussion_r1148338733)
I get this value in my implementation, yet it is not reflected in the BIP

💬 hebasto commented on pull request "depends: fix osx build with clang 16":
(https://github.com/bitcoin/bitcoin/pull/27328#issuecomment-1483792947)
> Current build (using forced system clang as a test) results in:
>
> > error: unknown argument: '-internal-externc-isystem/opt/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/lib/clang/16/include'

Mind providing steps to reproduce it, including your build environment details?

💬 fanquake commented on pull request "guix: combine and document `enable_werror`":
(https://github.com/bitcoin/bitcoin/pull/27326#issuecomment-1483802220)
> as it was when you introduced those "multiple funcs", they will be useful again.

It was needed when we were building 2 different glibcs, in two different ways. Now we're building a single glibc, in one way, hence consolidating. We can speculate about what might happen in future, but I can't see a reason (looking at current releases), to return to multiple builds. The only reason it was done this way initially, was for RISC-V support. I also don't know why we'd turn -Werror back on again, it's
...

💬 Ayms commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1483821598)
@1440000bytes

> dont get mad else some people from this repo might write things about you. Getting mad is only reserved for a few people here

What do you mean and what is your problem? Write things about me?

💬 1440000bytes commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1483822456)
@Ayms Edited. Sorry it wasn't for you.

💬 vincenzopalazzo commented on issue "core stops to run with `Failed to read block` error":
(https://github.com/bitcoin/bitcoin/issues/27142#issuecomment-1483839533)
yeah I will try asap

💬 hebasto commented on pull request "depends: fix osx build with clang 16":
(https://github.com/bitcoin/bitcoin/pull/27328#issuecomment-1483841839)
> I'm seeing an additional unrelated problem with linking with system clang, but I'll PR the solution to that separately as it's not as straightforward as this.

FWIW, with the following diff:
```diff
--- a/depends/hosts/darwin.mk
+++ b/depends/hosts/darwin.mk
@@ -38,7 +38,9 @@ clangxx_prog=$(shell $(SHELL) $(.SHELLFLAGS) "command -v clang++")
llvm_config_prog=$(shell $(SHELL) $(.SHELLFLAGS) "command -v llvm-config")

clang_resource_dir=$(shell clang -print-resource-dir)
+llvm_bin_d
...

📝 martinus opened a pull request: "refactor: extract CCheckQueue's data handling into a separate container "Bag""
(https://github.com/bitcoin/bitcoin/pull/27331)
`CCheckQueue` has stored its work items in a `queue`, but made no guarantee about the order of elements in that container. This PR extracts that data storage handling into a separate container class `Bag`. This is pure refactoring, the result should have a better separation of concerns, adds tests for the new container, and makes it now easier to separately test and optimize the container.

👍 hebasto approved a pull request: "guix: combine and document `enable_werror`"
(https://github.com/bitcoin/bitcoin/pull/27326)
ACK 4becee396f3bda40832138dd1aaa90368ed31857, the diff is correct.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1483882249)
Regarding CI failures,` ./test/functional/wallet_create_tx.py` passes locally, actioned manually re-run and I'm checking the fuzzer one `cirrus-ci-build/ci/scratch/qa-assets/fuzz_seed_corpus/http_request" failed with exit code 77`.

💬 Ayush170-Future commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1483895345)
Hello @MarcoFalke!
I'm new to Bitcoin Core development as well as Bash scripting. But, based on the requirements and the script, I believe I can tackle this.

I only have a few questions:
1) I discovered other files in the test directory, such as ci/test/04_install, also contain the CI_EXEC_ROOT and CI_EXEC alias. So, are they also to be modified?

2) After reading your comment in the script, I believe I can just remove the CI_EXEC_ROOT and CI_EXEC parts of different commands in the script
...

💬 theStack commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1148418379)
nit: unused variable, can be removed
```suggestion
```

💬 theStack commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1148418808)
pedantic-nit:
```suggestion
// Note that each transaction's ancestor size is 1 or 3, and each descendant size is 1, 2 or 3.
```
The descendant count of 2 is the rare case here, only applying for parent-txs txp0 and txp49
(for the fun of it, created a functional test for verifying this: https://github.com/theStack/bitcoin/blob/functional_test_mempool_zigzag_playground/test/functional/mempool_zigzag.py).

👍 TheCharlatan approved a pull request: "guix: combine and document `enable_werror`"
(https://github.com/bitcoin/bitcoin/pull/27326)
ACK 4becee396f3bda40832138dd1aaa90368ed31857

👍 Ayaan7211 approved a pull request: "MiniTapscript: port Miniscript to Tapscript"
(https://github.com/bitcoin/bitcoin/pull/27255)

👍 aureleoules approved a pull request: "wallet: Replace use of purpose strings with an enum"
(https://github.com/bitcoin/bitcoin/pull/27217)
ACK a48010f1fd6d8a430b8234789ac3c91ec9d06972, looks good to me.