💬 achow101 commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1985900547)
A lot of extra newlines, and the 4o makes me think this was AI generated.

💬 achow101 commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1985895574)
This seems like an excessive number of new lines, probably spam?

🤔 jonatack reviewed a pull request: "Docs: fix typos in documentation files"
(https://github.com/bitcoin/bitcoin/pull/32011#pullrequestreview-2668719859)
ACK, modulo the feedback below.

(Friendly suggestion: I believe you have opened multiple spelling changes recently and suggest ramping up to more valuable contributions, including reviewing other pull requests. See https://jonatack.github.io/articles for more info. Cheers.)

💬 jonatack commented on pull request "Docs: fix typos in documentation files":
(https://github.com/bitcoin/bitcoin/pull/32011#discussion_r1985907602)
I think the active voice makes more sense here (the original text). The faucet is doing the limiting, not the IP. Suggest dropping this change.

💬 achow101 commented on pull request "doc: warn against having qt6 installed on macOS":
(https://github.com/bitcoin/bitcoin/pull/32017#issuecomment-2707860813)
ACK d79dab0fa999002a0c5b70c1688240e2a5032ce1

Did not test whether it actually is an issue, but seems plausible.

💬 wgyt commented on pull request "Docs: fix typos in documentation files":
(https://github.com/bitcoin/bitcoin/pull/32011#discussion_r1985917257)
Thanks, I have revert it.

💬 jaybny commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2707892337)
Before proceeding with a full technical review, I’d like to clarify whether this pull request represents a complete rewrite or simply a refactor of the original implementation from three years ago.

----------------------------

Off-topic

**Concept ACK**

CTV is an objectively well-reviewed, fundamental upgrade to Bitcoin, and it has gained broad consensus among subject matter experts over the past three years. There will only ever be a handful of legitimate, code-complete upgrades to B
...

💬 jonatack commented on pull request "Docs: fix typos in documentation files":
(https://github.com/bitcoin/bitcoin/pull/32011#issuecomment-2707893667)
ACK 5601bab4f8b01fdef7a54c9e397d513217ab1c1f

💬 rot13maxi commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2707907662)
simple implementation, helpful comments, good test coverage, no backwards compatibility issues, passes all CI.

LGTM!

📝 abguermez opened a pull request: "Improve Clarity of Commentary by Removing Redundant Word"
(https://github.com/bitcoin/bitcoin/pull/32020)
This PR removes a redundant word in a comment to improve clarity and readability. The change does not affect functionality but makes the documentation more precise.

🤔 jonatack reviewed a pull request: "Improve Clarity of Commentary by Removing Redundant Word"
(https://github.com/bitcoin/bitcoin/pull/32020#pullrequestreview-2668876485)
NACK, proposed change is incorrect/worse.

💬 laanwj commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2707990682)
The problem still exists in the latest binutils built from git (d07a59a5ca830bf74705471f6bea6db1a47da2b5 as of 2025-03-07).

i've been digging a bit as to what causes some weak symbols (but not all) to end up in the binary: overall, if a weak function symbol is in both `libstdc++.a` and in the `.o` file, it will end up exported in the binary. This behavior exists on riscv64 but not x86_64.

The only exception is the monstriosity `_ZZSt8to_arrayINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE
...

👍 laanwj approved a pull request: "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/31960#pullrequestreview-2668897481)
re-ACK f0b659716bd455dca02053df573d888b5a115aa4

💬 laanwj commented on pull request "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985978659)
> I don't see why that would be better than this approach.

The only possible advantage i can think of to use a straight-up list and a fixed regexp, would be that it is less prone to subtle errors in the regexp like https://github.com/bitcoin/bitcoin/pull/31960#pullrequestreview-2654030620 .

💬 laanwj commented on pull request "Improve Clarity of Commentary by Removing Redundant Word":
(https://github.com/bitcoin/bitcoin/pull/32020#issuecomment-2708015200)
NACK

This is not an improvement. The double "is" looks strange but it is intentional, and works.

Maybe inserting a comme inbetween would be a slight improvement? i don't think it's worth spending a lot of time on tbh.

And yes, this needs to go upstream into the minisketch repository.

✅ laanwj closed a pull request: "Improve Clarity of Commentary by Removing Redundant Word"
(https://github.com/bitcoin/bitcoin/pull/32020)

👍 laanwj approved a pull request: "doc: link to benchcoin over bitcoinperf"
(https://github.com/bitcoin/bitcoin/pull/31996#pullrequestreview-2668925537)
ACK 611999e09777716d1fa686254db20845aff3dffe

👍 laanwj approved a pull request: "cmake: Check for `makensis` tool before using it"
(https://github.com/bitcoin/bitcoin/pull/32019#pullrequestreview-2668929872)
Code review ACK deea0988720aab65b3cc6d80b50bbf0d537adb24

💬 laanwj commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-2708037553)
Concept ACK, imo determinism is good because it makes it possible to check if mistakes have been made in the process at earlier steps.

One comment i was about to give is "why use a .tar.gz instead of .tar, that's the only way to be sure to remove dependency on zlib", but that's a much more spread out code change. This is most likely fine.

👋 Eunovo's pull request is ready for review: "Ensure assumevalid is always used during reindex"
(https://github.com/bitcoin/bitcoin/pull/31615)