💬 fanquake commented on pull request "[POC] build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-2876119294)
```bash
test_bitcoin.vcxproj -> D:\a\bitcoin\bitcoin\build\bin\Release\test_bitcoin.exe
lld-link : warning : found both wWinMain and WinMain; using latter [D:\a\bitcoin\bitcoin\build\src\qt\bitcoin-qt.vcxproj]
lld-link : error : undefined symbol: __declspec(dllimport) PathRemoveFileSpecW [D:\a\bitcoin\bitcoin\build\src\qt\bitcoin-qt.vcxproj]
>>> referenced by bitcoinqt.lib(guiutil.obj):(bool __cdecl GUIUtil::SetStartOnSystemStartup(bool))
lld-link : error : undefined symbol: __declspec
...

👍 stickies-v approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2836196063)
ACK c164064c266c518588d9d9175f9b14140ee751b6. I've left a few nits, but nothing blocking, especially in a PR with this much activity.

With the new default, I think users disabling or lowering `datacarriersize` are mostly reducing the performance of their own node, so I am in support of removing it completely. However, some people seem really keen on continuing to be able to do so, so pragmatically I think the approach taken in this PR is sensible. Changing the default is what's really importa
...

💬 stickies-v commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086524579)
nit: would prefer avoiding the literal by keeping `MAX_OP_RETURN_RELAY`, or at least document why you picked 99800.

<details>
<summary>git diff on c164064c26</summary>

```diff
diff --git a/test/functional/mempool_datacarrier.py b/test/functional/mempool_datacarrier.py
index 6347215b86..8eab4ce6b2 100755
--- a/test/functional/mempool_datacarrier.py
+++ b/test/functional/mempool_datacarrier.py
@@ -5,6 +5,7 @@
"""Test datacarrier functionality"""
from test_framework.messages import
...

💬 stickies-v commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086497989)
nit: no strong view, and not important, but my preference would be to just remove the `std::optional` altogether. Simplifies fn signature and defines what zero means in a single location.

<details>
<summary>git diff on c164064c26</summary>

```diff
diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h
index d57dbb393f..4786aca67d 100644
--- a/src/kernel/mempool_options.h
+++ b/src/kernel/mempool_options.h
@@ -48,9 +48,9 @@ struct MemPoolOptions {
* type is de
...

💬 stickies-v commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086553680)
nit: I think it's strange to let one test handle another test's pre's, would just keep this in `test_block_max_weight`?

<details>
<summary>git diff on c164064c26</summary>

```diff
diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py
index 21451a9aaf..415a370b06 100755
--- a/test/functional/mining_basic.py
+++ b/test/functional/mining_basic.py
@@ -186,9 +186,6 @@ class MiningTest(BitcoinTestFramework):
assert tx_below_min_feerate['txid'] not in
...

💬 stickies-v commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086563850)
nit: would make sense to actually use `test_framework.script.MAX_SCRIPT_ELEMENT_SIZE` here?

🚀 fanquake merged a pull request: "refactor: Remove unused HaveKey and HaveWatchOnly"
(https://github.com/bitcoin/bitcoin/pull/32476)

💬 fanquake commented on pull request "doc: Add hint about avoiding spaces in paths when building on Windows":
(https://github.com/bitcoin/bitcoin/pull/32397#issuecomment-2876150177)
What is the status of this? Do we need to add something to the docs?

💬 fanquake commented on pull request "doc: update unix build doc with build flags":
(https://github.com/bitcoin/bitcoin/pull/32269#issuecomment-2876162598)
As mentioned, what you're adding is already present in this file, so if you're re-ordering things, you'll need to remove the second occurence.

💬 bufo24 commented on pull request "doc: update unix build doc with build flags":
(https://github.com/bitcoin/bitcoin/pull/32269#issuecomment-2876171103)
@fanquake Just did that!

💬 fanquake commented on pull request "doc: update unix build doc with build flags":
(https://github.com/bitcoin/bitcoin/pull/32269#issuecomment-2876174166)
Thanks. You'll need to squash your commits.

👍 l0rinc approved a pull request: "lint: Check for missing trailing newline"
(https://github.com/bitcoin/bitcoin/pull/32477#pullrequestreview-2836345824)
Concept ACK - thanks for automating yet another nit that I hate being brought up during review!

Should we apply this to a few other file types as well - and take advantage of the naturally independent and parallelizable nature of the problem?

💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2086617492)
would it help if we used `.par_iter()` here instead (not sure if that would require adding a new lib or not, but seems like a naturally parallelizable task)?

💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2086587961)
Should we add any other ones here?
```bash
$ find . -type f -exec grep -Il . {} + | rev | cut -d. -f1 | rev | sort | uniq -c | sort -nr
1266 json
721 cpp
704 h
461 d
344 py
275 cmake
228 md
162 ts
145 make
89 cc
73 txt
49 sh
43 mk
32 patch
32 hex
28 c
27 in
26 internal
20 svg
19 ui
16 yml
14 sample
14 gitignore
13 marks
11 m4
10 capnp
9 sage
8 xml
8 1
6 rc
6 ninja
6 include
6 fish
6 bash
5 x
...

💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2086594083)
haven't tried it, but was wondering it this would also work with [chaining](https://doc.rust-lang.org/std/iter/struct.Chain.html#examples) the iterators?
```suggestion
get_subtrees()
.into_iter()
.chain(["doc/release-notes/release-notes-*"]) // archived notes
```

💬 bufo24 commented on pull request "doc: update unix build doc with build flags":
(https://github.com/bitcoin/bitcoin/pull/32269#issuecomment-2876181111)
Done

💬 Sjors commented on pull request "doc: update CWallet::SignTransaction doc to mention SIGHASH_DEFAULT":
(https://github.com/bitcoin/bitcoin/pull/32411#issuecomment-2876195871)
They're only the same for Taproot inputs.

cc @achow101

💬 fanquake commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2086633989)
I agree. We might be best changing this documentation to something like: "If you have issues fuzzing on macOS, try using Linux".

💬 fanquake commented on pull request "doc: corrected lockunspent rpc quoting":
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-2876201952)
@Talej Can you rebase this?

👍 willcl-ark approved a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32299#pullrequestreview-2836453276)
crACK a325ca34653872534529e7934d07fe4e2ca29cae

Backports all look clean to me.

Would only check that #32299 was left out of the release notes intentionally (it's not really "notable")?