🤔 glozow reviewed a pull request: "test: remove duplicate `WITNESS_SCALE_FACTOR` constant defination"
(https://github.com/bitcoin/bitcoin/pull/30029#pullrequestreview-2037852552)
lgtm ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4

💬 maflcko commented on issue "qa: Support git worktrees when running the linters locally via Docker":
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2092726109)
> (as part of a general python revamp in [this branch](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:fixup-lint-docker-image)).

Not sure about complicating the cirrus yml. I think it should be considered to be set in stone, because any changes will break re-running the lint task on old pull requests, IIRC. At this point, if you want to change it substantially, it could make sense to move it to GHA, along with an update to DrahtBot to be able to re-run GHA tasks. :swea
...

🤔 glozow reviewed a pull request: "doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE"
(https://github.com/bitcoin/bitcoin/pull/30024#pullrequestreview-2037856873)
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number

💬 fanquake commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-2092737922)
Guix Build (aarch64):
```bash
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
90a786
...

👋 fanquake's pull request is ready for review: "depends: swap some cctools tools for LLVM tools"
(https://github.com/bitcoin/bitcoin/pull/29739)

👍 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)?