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

💬 aureleoules commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1148451397)
Could use `purpose.value_or(is_mine ? AddressPurpose::RECEIVE : AddressPurpose::SEND)` instead.
Same for other instances.

💬 aureleoules commented on pull request "wallet: Use defined purposes instead of inlining":
(https://github.com/bitcoin/bitcoin/pull/26858#issuecomment-1483953418)
Closing in favor of #27217.

✅ aureleoules closed a pull request: "wallet: Use defined purposes instead of inlining"
(https://github.com/bitcoin/bitcoin/pull/26858)

⚠️ EthanHeilman opened an issue: "Bitcoin fails to build on MSVC: Libevent and BCryptGenRandom errors "
(https://github.com/bitcoin/bitcoin/issues/27332)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

When running `msbuild build_msvc\bitcoin.sln -property:Configuration=Release -maxCpuCount -verbosity:minimal` I get the following two errors in the build.

`C:\Users\e0\Documents\GitHub\bitcoin\src\httpserver.cpp(637,9): error C2664: 'void evhttp_connection_get_peer(evhttp_connection *,const char **,uint16_t *)': cannot convert argument 2 from 'char **' to 'const char **' [C:\Users\e0\Do
...

👍 TheCharlatan approved a pull request: "build, qt: Fix handling of `CXX=clang++` when building `qt` package"
(https://github.com/bitcoin/bitcoin/pull/27314)
tACK 25e8fe70c6e65c07cf602568f5c2cf5535fca995

```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum

daa94946aae7d4826d6f329337791a6bda0f01ac73f27003d1677dc8de031071 guix-build-25e8fe70c6e6/output/aarch64-linux-gnu/SHA256SUMS.part
38e9dcb99e329ad02837fd08559a8408bd7ba698f65163d0834d52dc9eefceae guix-build-25e8fe70c6e6/output/aarch64-linux-gnu/bitcoin-25e8fe70c6e6-aarch64-linux-gnu-debug.tar.gz
36d48b93724b112e5553a8d04
...

📝 vstoyanov opened a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)"
(https://github.com/bitcoin/bitcoin/pull/27333)
Basically it removes the above-mentioned env-vars as per @MarcoFalke's instructions. The only liberty I took was in double-quoting the last instance of $ANDROID_HOME for the sake of consistency and future-proofing.

References #27321

💬 vstoyanov commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484085076)
Hello,

I think I just completed this one. My understanding was that it only covers 01_base_install.sh and not instances after test/04_install.sh as from there on it also redefines $PATH and current directory.

Best,
Vasil Stoyanov