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

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