💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#issuecomment-2727342830)
re-ACK 2f2ab47bf74f4da37aad75a186cb0bb16e8af579

💬 instagibbs commented on pull request "test: fix intermittent failure in p2p_orphan_handling.py":
(https://github.com/bitcoin/bitcoin/pull/32063#issuecomment-2727344887)
ACK https://github.com/bitcoin/bitcoin/pull/32063/commits/02942056fd861581503a8a35a06dcf22d4ba1473

💬 maflcko commented on pull request "test: Update coverage.cpp to drop linux restriction":
(https://github.com/bitcoin/bitcoin/pull/32059#issuecomment-2727364835)
review-only ACK 54e6eacc1fccd602897d9e3025c62f83194ffd5b

💬 Prabhat1308 commented on pull request "doc: Update documentation to include Clang/llvm based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#issuecomment-2727372085)
Addressed the comments to fix nits. On the use of `-DEBUG` , I haven't been able to do a comparison from lcov generated reports as I don't have any other system . Since `-02` introduces optimisation , I have chosen to keep the `-DEBUG` flag to use `-00` as mentioned here to keep it close to gcov reports.
https://stackoverflow.com/questions/36930207/code-coverage-with-optimization
> it seems if I want to collect coverage information with tools like gcov, any optimization flag must be disabled

👍 TheCharlatan approved a pull request: "test: Check datadir cleanup after assumeutxo was successful"
(https://github.com/bitcoin/bitcoin/pull/32033#pullrequestreview-2688633947)
Re-ACK 0d10f1a66436cd2ddab6b04247bcd6c4747cccc3

💬 TheCharlatan commented on pull request "test: Check datadir cleanup after assumeutxo was successful":
(https://github.com/bitcoin/bitcoin/pull/32033#discussion_r1997579018)
Nit: I think you can just pass in `i==1` here, without the `if` `else` (and similarly below).

💬 TheCharlatan commented on pull request "test: Check datadir cleanup after assumeutxo was successful":
(https://github.com/bitcoin/bitcoin/pull/32033#discussion_r1997578384)
Nit: `s/assumutxo/assumeutxo/`.

💬 Chand-ra commented on pull request "test: fix intermittent failure in wallet_reorgsrestore.py":
(https://github.com/bitcoin/bitcoin/pull/32069#issuecomment-2727416972)
tACK [36b0713](https://github.com/bitcoin/bitcoin/commit/36b0713edc4655f6e0c291975d6d280fbc89cf2e)

Tested by pulling the PR locally and verifying that the behavior of the changed test (or any other test) doesn't change with the proposed change.

💬 maflcko commented on pull request "doc: Update documentation to include Clang/llvm based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#issuecomment-2727426434)
> gcov

This pull isn't about gcov from GCC, but about clang/llvm-based coverage reports. It seems odd to apply a pessimising workaround from a different tool without documentation. If there is an issue that requires the workaround, it should be trivial to find exact steps to reproduce, or at least a note in the official docs: https://clang.llvm.org/docs/SourceBasedCodeCoverage.html

📝 naiyoma opened a pull request: "test: Add test coverage for rpcwhitelistdefault when unset"
(https://github.com/bitcoin/bitcoin/pull/32079)
This is a follow-up PR to address review feedback from [https://github.com/bitcoin/bitcoin/pull/29858](https://github.com/bitcoin/bitcoin/pull/29858)

- [x] add case where rpcwhitelistdefault setting is [unset](https://github.com/bitcoin/bitcoin/pull/29858#pullrequestreview-2532726241)
- [x] Code [cleanup](https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1927238617) , change password and f-string formatting
- [x] [Combine](https://github.com/bitcoin/bitcoin/pull/29858#discussion_r
...

💬 maflcko commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2727453653)
> > Users could remove necessary dependencies after their existence has been checked and cached. Trying to protect the project against those cases means questioning the concept of a cache as such.
>
> FWIW, the `find_package()` command creates cache variables when it succeeds. If the dependency package is later removed from the system, a subsequent `cmake` re-run won't detect the change.

If the cache is known to possibly result in a (silent) break on any change to installed packages, it just d
...

✅ fanquake closed an issue: "intermittent issue in p2p_orphan_handling.py"
(https://github.com/bitcoin/bitcoin/issues/31700)

🚀 fanquake merged a pull request: "test: fix intermittent failure in p2p_orphan_handling.py"
(https://github.com/bitcoin/bitcoin/pull/32063)

🚀 fanquake merged a pull request: "test: fix intermittent failure in wallet_reorgsrestore.py"
(https://github.com/bitcoin/bitcoin/pull/32069)

💬 maflcko commented on pull request "refactor: Remove redundant and confusing calls to IsArgSet":
(https://github.com/bitcoin/bitcoin/pull/31896#discussion_r1997633914)
removed it here. Should be trivial to re-review

💬 maflcko commented on pull request "test: replace hardcoded fee with node relay fee based calculation":
(https://github.com/bitcoin/bitcoin/pull/32058#discussion_r1997654688)
not sure about using float and then int again. Why is this needed?

Also, changing `FEE` may change the test logic, so it seems better to adjust manually

It would be best to just add an `assert_equal(FEE, minrelaytxfee)`

📝 theStack opened a pull request: "qt: doc: adapt outdated binary paths to CMake changes"
(https://github.com/bitcoin-core/gui/pull/858)
Adapt the qt-related instances of outdated binary paths to `./build/bin/...` (see [#30454](https://github.com/bitcoin/bitcoin/pull/30454) and the more recently merged [#31161](https://github.com/bitcoin/bitcoin/pull/31161)). According to `$ git grep src/qt.*bitcoin` there should be no more left to address.

✅ maflcko closed an issue: "intermittent issue in wallet_reorgsrestore.py in test_reorg_handling_during_unclean_shutdown: FailedToStartError: [node 0] bitcoind exited with status 1 during initialization. Error: Cannot obtain a lock on directory"
(https://github.com/bitcoin/bitcoin/issues/32066)

💬 Chand-ra commented on pull request "test: replace hardcoded fee with node relay fee based calculation":
(https://github.com/bitcoin/bitcoin/pull/32058#discussion_r1997662972)
> not sure about using float and then int again. Why is this needed?

The fee in satoshis must be integer, so I added those as a precautionary measure. Looking back at it, they do seem redundant.

> Also, changing FEE may change the test logic, so it seems better to adjust manually

Isn't that the point of the change suggested by the `TODO` tag though? To make `FEE` adapt dynamically to a change in `minRelayTxFee`?

💬 fjahr commented on pull request "test: Check datadir cleanup after assumeutxo was successful":
(https://github.com/bitcoin/bitcoin/pull/32033#discussion_r1997671044)
fixed