💬 fanquake commented on pull request "wallet, refactor: Remove Legacy wallet unused warnings and errors":
(https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2087016856)
You can drop the changes in here, this file will be updated in a translations update. @hebasto might be worth doing one shortly just to cleanup these strings, since they keep being removed in various PRs.

💬 hebasto commented on pull request "Update `secp256k1` subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2876906638)
My Guix build:
```
aarch64
24986a84cc15356c2761a8d05898d33d60ff10431eecdf68d9fb758432a87f79 guix-build-915c1fa72c07/output/aarch64-linux-gnu/SHA256SUMS.part
be7bba66e8e84ba8428aed395a296e397d07f5d961bb8ab80bdd19d2c48589ba guix-build-915c1fa72c07/output/aarch64-linux-gnu/bitcoin-915c1fa72c07-aarch64-linux-gnu-debug.tar.gz
a77553ea1b9eca35f6b9bc00f0be22f3016b8fdf1425b71ec93a6fdbcd8e154c guix-build-915c1fa72c07/output/aarch64-linux-gnu/bitcoin-915c1fa72c07-aarch64-linux-gnu.tar.gz
4f1a82c8
...

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r2087061842)
Dropped and added `OP_2` as an example for inspiration.

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r2087066202)
I switched to using a range, where it makes more sense to use a code comment.

💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2087066691)
Any accounting inconsistency could've caused it to become negative and fail, so instead of removing these suppressions in the last commit that fixed the problem (giving the impression that it was just one change that enabled us removing it), I did it separately instead.
Is this important, is this holding back the ACK from your part?

💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876927264)
I agree with you, but I also prefer the current code; seems easier to understand.

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876927696)
I added `OP_2` through `OP_16`, with a new test case for the latter. It's not practical to catch _all_ possible trivial scripts though.

📝 fanquake opened a pull request: "build: add `-W[leading|trailing]-whitespace`"
(https://github.com/bitcoin/bitcoin/pull/32482)
GCC 15 now has options to turn these into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable these options. Unfortunately, CMake/Qt will generate code that contains trailing whitespace:
```bash
[ 98%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/EJRQKI7XPS/qrc_bitcoin_locale.cpp.o
/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:5603:2: warning: trailing
...

💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876932472)
Code Review ACK 6adec89e3961508875e79240a19bc981e5addf1a (untested)

💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2087077335)
Did a git clean -fxd before running it so I think it should've covered only the tracked files.

------

Will let you decide of course which ones makes sense to include, just pointing out that we may want to extend this list.

💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876941545)
Code Review reACK 2be93f7ba3866892d98795384779ce2e965753a1 (untested)

💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2087083142)
I haven't tested it, if you say it's fast enough, I believe you

💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#issuecomment-2876944750)
ACK fa9198af55df74b0c19c9125d256ad4df83cf005

🤔 caesrcd reviewed a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2837204950)
reACK e98c51fcce

💬 hebasto commented on pull request "build: add `-W[leading|trailing]-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-2876950405)
> GCC 15 now has options to turn these into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable these options.

Concept ACK on that.

> Unfortunately, CMake/Qt will generate code that contains trailing whitespace:
>
> ```shell
> [ 98%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/EJRQKI7XPS/qrc_bitcoin_locale.cpp.o
> /root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW
...

💬 ismaelsadeeq commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#discussion_r2087092911)
Done

💬 ismaelsadeeq commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#discussion_r2087093235)
Done!

💬 ismaelsadeeq commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#discussion_r2087093815)
Added the comment as requested.

💬 fanquake commented on pull request "Update `secp256k1` subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2876976229)
> Here are the identified typographic and grammatical errors that impact comprehension in the provided git diff:
>
> CHECK(secp256k1_ec_seckey_tweak_add(CTX, privkey, rnd); -> CHECK(secp256k1_ec_seckey_tweak_add(CTX, privkey, rnd)); [missing closing parenthesis]

I can't seem to find `CHECK(secp256k1_ec_seckey_tweak_add(CTX, privkey, rnd)` in either our, or libsecp256k1s codebase.

💬 ismaelsadeeq commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#issuecomment-2876983186)
Force-pushed to address recent comments by @Sjors:

1. Added an explanation for why we ignore the result of `WaitTipChanged`.
2. Clarified the comment for `AddMerkleRootAndCoinbase`.
3. Shortened line lengths for improved readability.

Additionally, in the recent push, I added a commit that skips the fee check during block template total fees computation in `waitNext`. This check is redundant after #31897.

🤔 ismaelsadeeq reviewed a pull request: "Add checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31981#pullrequestreview-2837265952)
Concept ACK after https://bitcoincore.reviews/31981