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

💬 Eunovo commented on pull request "Ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-2708070578)
Thanks for the reviews @l0rinc and @mzumsande . Due to your feedback, I have opened the PR for general review.

🤔 musaHaruna reviewed a pull request: "test: Fix authproxy named args debug logging"
(https://github.com/bitcoin/bitcoin/pull/31955#pullrequestreview-2669050326)
Tested ACK [fac1dd9](https://github.com/bitcoin/bitcoin/commit/fac1dd9dffba1033245c283bc0468e801c14e910)

with change:
``DEBUG:BitcoinRPC:-1-> getblockhash [] {}
{
"jsonrpc": "2.0",
"method": "getblockhash",
"params": {},
"id": 1
}

without change:
`DEBUG:RPC Test:Testing Positional Arguments Only
DEBUG:BitcoinRPC:-1-> getblockhash {}
{
"jsonrpc": "2.0",
"method": "getblockhash",
"params": {},
"id": 1
}
`
Also the new helper function helped in makes the json
...

👍 hebasto approved a pull request: "doc: link to benchcoin over bitcoinperf"
(https://github.com/bitcoin/bitcoin/pull/31996#pullrequestreview-2669098867)
ACK 611999e09777716d1fa686254db20845aff3dffe. I agree. I've had a great experience using it.

🚀 hebasto merged a pull request: "doc: link to benchcoin over bitcoinperf"
(https://github.com/bitcoin/bitcoin/pull/31996)

🚀 hebasto merged a pull request: "doc: warn against having qt6 installed on macOS"
(https://github.com/bitcoin/bitcoin/pull/32017)