👍 hebasto approved a pull request: "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set"
(https://github.com/bitcoin/bitcoin/pull/25972#pullrequestreview-2037913769)
ACK f0e22be69a15248c42964d57f44ce8c37a36081d.

My Guix builds:
```
x86_64
a416b91cfcc2ed18250021e67b5130e46d407998a642cdaed49996c0be79dd08 guix-build-f0e22be69a15/output/aarch64-linux-gnu/SHA256SUMS.part
569dc4cf491fe4f07009ce8f9d202c05f29a18eec6963fa3e7286a944dd93a82 guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu-debug.tar.gz
126a3b9712bb7685e89e84f6b0d18cce6389bf580e3e66136a976e1f2f50f76f guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoi
...

💬 willcl-ark commented on issue "qa: Support git worktrees when running the linters locally via Docker":
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2092847287)
Thanks, that's useful feedback.

TBH I really don't enjoy

💬 hebasto commented on pull request "miniscript: make operator""_mst consteval":
(https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-2092864754)
> Could rebase, as CI on master should now be passing with clang-15 in.

As we now build the `fuzz.exe` on MSVC, this PR seems require a little adjustment while rebasing:
```diff
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -391,6 +391,7 @@ std::optional<NodeInfo> ConsumeNodeStable(MsCtx script_ctx, FuzzedDataProvider&
bool allow_K = (type_needed == ""_mst) || (type_needed << "K"_mst);
bool allow_V = (type_needed == ""_mst) || (type_needed << "V"_mst
...

💬 sipa commented on pull request "miniscript: make operator""_mst consteval":
(https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-2092882889)
@hebasto Thanks, rebased and applied.

👋 sipa's pull request is ready for review: "miniscript: make operator""_mst consteval"
(https://github.com/bitcoin/bitcoin/pull/28657)

👍 brunoerg approved a pull request: "test: remove duplicate `WITNESS_SCALE_FACTOR` constant defination"
(https://github.com/bitcoin/bitcoin/pull/30029#pullrequestreview-2038003175)
ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4

📝 hebasto opened a pull request: "msvc: Compile `test\fuzz\miniscript.cpp`"
(https://github.com/bitcoin/bitcoin/pull/30031)
Based on https://github.com/bitcoin/bitcoin/pull/28657.

💬 hebasto commented on pull request "miniscript: make operator""_mst consteval":
(https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-2092904582)
> @hebasto Thanks, rebased and applied.

Sorry. It doesn't work -- https://github.com/bitcoin/bitcoin/pull/30031.

🤔 ismaelsadeeq reviewed a pull request: "Fix waste calculation in SelectionResult"
(https://github.com/bitcoin/bitcoin/pull/28366#pullrequestreview-2038028445)
Concept ACK

This is a nice cleanup 👍🏾

💬 ismaelsadeeq commented on pull request "Fix waste calculation in SelectionResult":
(https://github.com/bitcoin/bitcoin/pull/28366#discussion_r1589133578)
Why are you reducing 0.1 here?

And above why are `effective_feerate`,` long_term_feerate`, and `discard_feerate` now having values?

💬 ismaelsadeeq commented on pull request "Fix waste calculation in SelectionResult":
(https://github.com/bitcoin/bitcoin/pull/28366#discussion_r1589130802)
From the commit message you mention we can have change_cost of 0 and also a change output, so this is no longer the case?
```suggestion
* @param[in] change_cost The cost of creating change and spending it in the future.
* Only used if there is change.
```

👍 ismaelsadeeq approved a pull request: "functional test: ensure confirmed utxo being sourced for 2nd chain"
(https://github.com/bitcoin/bitcoin/pull/29998#pullrequestreview-2038053777)
utACK 07aba8dd215b23b06853b1a9fe04ac8b08f62932

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2092955734)
@sipa
> What is the issue with the ... miniscript fuzz tests?

The miniscript issue is resolved with your https://github.com/bitcoin/bitcoin/pull/28657 :)

💬 ajtowns commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2092959394)
> I'd be curious to see what that patch look like.

Something along the lines of https://github.com/bitcoin/bitcoin/commit/970c0c31e99bebe2b60ade930c96600d1d82f7ca maybe? Essentially untested.

The main advantage of this is that block 0 in a retarget period could also be min difficulty, allowing you to keep mining min-difficulty blocks with 20 minutes between them, halving the difficulty every 4 weeks / 2016 blocks, until you get to something that the network can mine on top of. The disadvan
...

👍 hebasto approved a pull request: "miniscript: make operator""_mst consteval"
(https://github.com/bitcoin/bitcoin/pull/28657#pullrequestreview-2038082509)
ACK 73619b819c37e98bd9a100bea14a3366aebfabb8.

I apologise for messing up.

The https://github.com/bitcoin/bitcoin/pull/30031, which is built on top of this PR, succeeds. It includes additional code changes in `src/test/fuzz/miniscript.cpp` (with some inconsistency in local variable naming).

@sipa feel free to pull changes from #30031 into this PR.

💬 GregTonoski commented on pull request "doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE":
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2092984943)
What is the meaning and the purpose of the both the "520" and the "MAX_SCRIPT_ELEMENT_SIZE"? In particular, is the "520" and "MAX_SCRIPT_ELEMENT_SIZE" meant to describe limit of size of any element in order to disregard transactions that would have overused resources (similarily to MAX_SCRIPT_SIZE albeit on a level deeper)?

💬 TheCharlatan commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-2092997971)
Guix builds (aarch64)
```
a416b91cfcc2ed18250021e67b5130e46d407998a642cdaed49996c0be79dd08 guix-build-f0e22be69a15/output/aarch64-linux-gnu/SHA256SUMS.part
569dc4cf491fe4f07009ce8f9d202c05f29a18eec6963fa3e7286a944dd93a82 guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu-debug.tar.gz
126a3b9712bb7685e89e84f6b0d18cce6389bf580e3e66136a976e1f2f50f76f guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu.tar.gz
90a7866982
...

💬 willcl-ark commented on pull request "doc: fix broken relative md links":
(https://github.com/bitcoin/bitcoin/pull/30025#issuecomment-2093006666)
> Is there an easy way to find those?

I was using this https://github.com/becheran/mlc/ in offline mode to find broken internal links. I'm sure it wouldn't take much to modify it to surface relative links, if we want them all absolute?

💬 theuni commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-2093030865)
I can't repro the `-Wmaybe-uninitialized` issue. Might it be possible to just add the initializations (even if they're unnecessary) rather than disabling?

Like for the example provided, making it:
```c++
T result{};
```

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2093037407)
Here's a commit that makes `getblocktemplate` reorg low difficulty testnet4 blocks: https://github.com/Sjors/bitcoin/commit/2125fc56163ceaddfadbc78ad0d0da5b1a99a8fa It adds some complexity, but not the validation code.

Untested, other than the the template looks correct to me. I still need to figure out an easy way to setup a local stratum v1 pool (to CPU mine).

By the way, I noticed that `src/bitcoin-cli -testnet4 -generate 1 1000000000` does not find a block if called a dozen times, even
...