💬 davidgumberg commented on pull request "depends: remove `NO_HARDEN` option":
(https://github.com/bitcoin/bitcoin/pull/32038#issuecomment-2719073609)
crACK 5dfef6b9b379f51e

Sanity tested that the following depends build works:

```bash
make -C depends/ HOST=x86_64-w64-mingw32
cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
cmake --build build
```

---

> for a project like this it's unreasonable to disable hardening features

Would it make sense to also drop `ENABLE_HARDENING`? Currently set `OFF` in the `clang-tidy` ci job, but I [flipped it `ON`](https://github.com/davidgumberg/bitcoin/commit/ce79bea14ae
...

💬 jonatack commented on pull request "refactor: CLI cleanups":
(https://github.com/bitcoin/bitcoin/pull/31887#issuecomment-2719074019)
Thanks for the reviews, everyone. Ran clang-format on the third commit and adjusted a few commit messages per the feedback. Should be trivial to re-ack (thanks!)

💬 yancyribbens commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2719074335)
cc @murchandamus

💬 jonatack commented on pull request "refactor: CLI cleanups":
(https://github.com/bitcoin/bitcoin/pull/31887#discussion_r1992261225)
I believe I already addressed the conventions comment with https://github.com/bitcoin/bitcoin/pull/31887#discussion_r1966566982.

📝 VolodymyrBg opened a pull request: "docs: Enhance TODO section in README.md with detailed explanations"
(https://github.com/bitcoin/bitcoin/pull/32048)
This pull request improves the TODO section in the Minisketch README.md by:
1. Adding detailed explanations for each planned improvement:
- Explicit formulas for higher-degree polynomials
- Subquadratic multiplication and modulus algorithms
- Half-GCD algorithm implementation
- Incremental decoding interface
- Platform-specific optimizations beyond x86
- 32-bit host optimizations
- IBLT/Hybrid approaches
2. Introducing implementation priorities to guide future development efforts:
- Pr
...

💬 yancyribbens commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2719110760)
I also just wanted to add that, even if there was the extremely unlikely event where two utxos had the same effective_value and different fee values, the optimization should only compare effective_values as I understand it.

🤔 pablomartin4btc reviewed a pull request: "refactor: CLI cleanups"
(https://github.com/bitcoin/bitcoin/pull/31887#pullrequestreview-2679864084)
re-ACK d423fd9ec83ae323ac29bdc1b677ed8260bd59c4

🤔 glozow reviewed a pull request: "mining: drop unused -nFees and sigops from CBlockTemplate"
(https://github.com/bitcoin/bitcoin/pull/31897#pullrequestreview-2679788820)
looks correct, untested ACK 6d7648795aa053f535510d11379fdd9d0399004c

💬 glozow commented on pull request "mining: drop unused -nFees and sigops from CBlockTemplate":
(https://github.com/bitcoin/bitcoin/pull/31897#discussion_r1992245751)
When I change this to `int index_in_template = 0;` or make changes to the values pushed for "fee" and "depends" none of the functional tests fail. Could be worth adding coverage?

💬 darosior commented on pull request "fuzz: Expand script verification flag testing to segwit v0 and tapscript":
(https://github.com/bitcoin/bitcoin/pull/31460#discussion_r1992326301)
Same here, you could just variate based on the value itself?

🤔 darosior reviewed a pull request: "fuzz: Expand script verification flag testing to segwit v0 and tapscript"
(https://github.com/bitcoin/bitcoin/pull/31460#pullrequestreview-2679919459)
Concept ACK.

💬 darosior commented on pull request "fuzz: Expand script verification flag testing to segwit v0 and tapscript":
(https://github.com/bitcoin/bitcoin/pull/31460#discussion_r1992325287)
What does hashing adds here? If you need more than one bit why not take it out of `sig` directly (and have a default value if it's too small)?

💬 darosior commented on pull request "fuzz: Expand script verification flag testing to segwit v0 and tapscript":
(https://github.com/bitcoin/bitcoin/pull/31460#discussion_r1992323288)
This seems unnecessary, none of that is used.

💬 hodlinator commented on pull request "[IBD] specialize CheckBlock's input & coinbase checks":
(https://github.com/bitcoin/bitcoin/pull/31682#discussion_r1992337395)
This comment made me wonder if one could remove the loop, I think we could:
```diff
diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index 0a471deda2..6aa8a73de6 100644
--- a/src/consensus/tx_check.cpp
+++ b/src/consensus/tx_check.cpp
@@ -57,16 +57,21 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state)
for (const auto& txin : tx.vin) {
sortedPrevouts.push_back(txin.prevout);
}
- std::sort(sortedPrevouts.be
...

💬 brunoerg commented on pull request "descriptor: check whitespace in keys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#issuecomment-2719220144)
rfm?

📝 davidgumberg opened a pull request: "contrib: Fix `gen-bitcoin-conf.sh`"
(https://github.com/bitcoin/bitcoin/pull/32049)
In #31118, the format of bitcoind's `--help` output changed slightly in a way that breaks `gen-bitcoin-conf.sh`, modify the script to accomodate the new format, by starting after the line that says "Options:" and strip the `-help` option and its description from the output.

Before this PR, all options above `-help` were excluded from the example bitcoin.conf.

💬 l0rinc commented on pull request "[IBD] specialize CheckBlock's input & coinbase checks":
(https://github.com/bitcoin/bitcoin/pull/31682#discussion_r1992367875)
Thanks for the hint, this is also a good alternative.

That combination is tested via:
https://github.com/bitcoin/bitcoin/blob/6421b986f6c043fd1266c399e542279ad373822f/src/test/transaction_tests.cpp#L504

I ran `CheckBlockBench` a few times to see if there's any difference and there's barely any (the current one seemed a tiny bit faster with Clang) - so I'd rather go for simpler code.

In reality though the loop exits immediately since we rarely have hashes of 0, so the two solutions are
...

💬 davidgumberg commented on pull request "[29.x] bump to v29.0rc1":
(https://github.com/bitcoin/bitcoin/pull/32046#issuecomment-2719259291)
It appears that https://github.com/bitcoin/bitcoin/pull/31118 broke the `bitcoin.conf` generating script, all options above `-help` in the `--help` output are missing, including e.g. `-dbcache`. I've opened a fix in #32049.

<details>

<summary>

### bitcoin.conf diff between 28.1 and #32046

</summary>

```console
$ git diff v28.1/share/examples/bitcoin.conf 32046/share/examples/bitcoin.conf
```

```diff
diff --git a/v28.1/share/examples/bitcoin.conf b/32046/share/examples/bitcoi
...

💬 Nouridin commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2719272219)
In summary, while the fee check may be redundant given the current relationships between effective value and fee, it was likely originally added to ensure that the shortcut is _only_ applied when both the effective value and the spending fee are identical. Since testing shows that removing it does not lead to failures, it appears safe to remove it, but any change in this well‐tuned coin selection code must be approached with caution given the subtle and sometimes unexpected interactions in coin
...

💬 davidgumberg commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2719278848)
> It's true that wine tests can't catch every bug that could happen on windows since Wine is another platform and "Is Not an Emulator." But calling the wine tests "fundamentally broken" would seem to imply that there were bugs in wine or in the wine test setup, and I haven't seen evidence for that. It seems more likely to me that some of our own tests are flaky and not reliable or portable. At least I didn't see any evidence that the wine setup was actually broken when I asked about this previou
...