💬 Christewart commented on pull request "tests: Remove hardcoded addresstype in `rpc_psbt.py`":
(https://github.com/bitcoin/bitcoin/pull/32701#issuecomment-3006580686)
@danielabrozzoni thank you for the kick ass review! You are exactly right.
I made half hearted attempt to take yours and @achow101 's suggestions to make this test case work, but I think this is a bit more work that I would like to invest in this currently: 874064c781e822c8f7d49873e275e77f63acde86
AFAICT, the test suite _only_ works for bech32 address types and is broken for all other address types - so a large overhaul for this test suite is needed to thoroughly test PSBT operations for
...
(https://github.com/bitcoin/bitcoin/pull/32701#issuecomment-3006580686)
@danielabrozzoni thank you for the kick ass review! You are exactly right.
I made half hearted attempt to take yours and @achow101 's suggestions to make this test case work, but I think this is a bit more work that I would like to invest in this currently: 874064c781e822c8f7d49873e275e77f63acde86
AFAICT, the test suite _only_ works for bech32 address types and is broken for all other address types - so a large overhaul for this test suite is needed to thoroughly test PSBT operations for
...
💬 ismaelsadeeq commented on pull request "test: fix an incorrect `feature_fee_estimation.py` subtest":
(https://github.com/bitcoin/bitcoin/pull/32463#issuecomment-3007287022)
Thanks for you review @glozow
> Did you consider baking this logic into check_smart_estimates instead? It feels a bit hacky to just append it to the fees seen, given that it isn't actually a seen fee.
I hadn’t considered that before, your suggestion is definitely more elegant and cleaner.
I’ve updated the PR to implement your approach, and I also added a follow-up improvement I had in mind as a separate commit.
[22a36e8..9b75cfd](https://github.com/bitcoin/bitcoin/compare/2c5f26bc6a
...
(https://github.com/bitcoin/bitcoin/pull/32463#issuecomment-3007287022)
Thanks for you review @glozow
> Did you consider baking this logic into check_smart_estimates instead? It feels a bit hacky to just append it to the fees seen, given that it isn't actually a seen fee.
I hadn’t considered that before, your suggestion is definitely more elegant and cleaner.
I’ve updated the PR to implement your approach, and I also added a follow-up improvement I had in mind as a separate commit.
[22a36e8..9b75cfd](https://github.com/bitcoin/bitcoin/compare/2c5f26bc6a
...
👍 vasild approved a pull request: "rpc: use CScheduler for HTTPRPCTimer"
(https://github.com/bitcoin/bitcoin/pull/32796#pullrequestreview-2961018364)
ACK d06942c6731d5db7326bc565655b33a379a5d9b0
(https://github.com/bitcoin/bitcoin/pull/32796#pullrequestreview-2961018364)
ACK d06942c6731d5db7326bc565655b33a379a5d9b0
🤔 Bitchryankilledme reviewed a pull request: "test: Use the correct node for doubled keypath test"
(https://github.com/bitcoin/bitcoin/pull/32369#pullrequestreview-2961066413)
approved
(https://github.com/bitcoin/bitcoin/pull/32369#pullrequestreview-2961066413)
approved
💬 willcl-ark commented on issue "Cleanup CFeeRate constructor (sat/vB vs BTC/kvB)":
(https://github.com/bitcoin/bitcoin/issues/23129#issuecomment-3007508973)
@jonatack @murchandamus is this issue effectively solved via previous changes + #24305
(https://github.com/bitcoin/bitcoin/issues/23129#issuecomment-3007508973)
@jonatack @murchandamus is this issue effectively solved via previous changes + #24305
🤔 janb84 reviewed a pull request: "rpc: use CScheduler for HTTPRPCTimer"
(https://github.com/bitcoin/bitcoin/pull/32796#pullrequestreview-2961081158)
re ACK d06942c6731d5db7326bc565655b33a379a5d9b0
Changes sinds last ACK:
Included suggestions (thanks) to change:
- moved away from `std::any` abstraction in the `HTTPRPCTimerInterface`
- changed testing
(https://github.com/bitcoin/bitcoin/pull/32796#pullrequestreview-2961081158)
re ACK d06942c6731d5db7326bc565655b33a379a5d9b0
Changes sinds last ACK:
Included suggestions (thanks) to change:
- moved away from `std::any` abstraction in the `HTTPRPCTimerInterface`
- changed testing
💬 polespinasa commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-3007553572)
Rebased on top of master e5f9218b6a
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-3007553572)
Rebased on top of master e5f9218b6a
📝 VolodymyrBg opened a pull request: "Fix broken navigation link to files documentation in README_doxygen.md"
(https://github.com/bitcoin-core/gui/pull/878)
Replaced the outdated link to files.html with the correct files.md in the navigation section of README_doxygen.md, ensuring users are directed to the current documentation format.
(https://github.com/bitcoin-core/gui/pull/878)
Replaced the outdated link to files.html with the correct files.md in the navigation section of README_doxygen.md, ensuring users are directed to the current documentation format.
💬 fanquake commented on pull request "depends: Bump boost to 1.88.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/32665#issuecomment-3007781988)
Guix Build:
```bash
dcedbdafa5857dbbb7748e27bbbfadc57a5dc70eff9b565c1d30061e64027ca4 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/SHA256SUMS.part
ccf56c678924e933ef46f306d0be509251d1920f17529e11f007a5c0b74a5282 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/bitcoin-6c2538d5bfea-aarch64-linux-gnu-debug.tar.gz
550fa33f9e19c06cf4651abd4e69d813db6702461b6b59b717a719821c8b6495 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/bitcoin-6c2538d5bfea-aarch64-linux-gnu.tar.gz
aab708c38da0b9e0
...
(https://github.com/bitcoin/bitcoin/pull/32665#issuecomment-3007781988)
Guix Build:
```bash
dcedbdafa5857dbbb7748e27bbbfadc57a5dc70eff9b565c1d30061e64027ca4 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/SHA256SUMS.part
ccf56c678924e933ef46f306d0be509251d1920f17529e11f007a5c0b74a5282 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/bitcoin-6c2538d5bfea-aarch64-linux-gnu-debug.tar.gz
550fa33f9e19c06cf4651abd4e69d813db6702461b6b59b717a719821c8b6495 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/bitcoin-6c2538d5bfea-aarch64-linux-gnu.tar.gz
aab708c38da0b9e0
...
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2168643875)
I prefer to be on the safe side and handle `fclose()` errors, it is just a few extra lines.
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2168643875)
I prefer to be on the safe side and handle `fclose()` errors, it is just a few extra lines.
📝 leopardracer opened a pull request: "Update p2p_monitor.py"
(https://github.com/bitcoin/bitcoin/pull/32816)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/32816)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 hebasto commented on pull request "Fix broken navigation link to files documentation in README_doxygen.md":
(https://github.com/bitcoin-core/gui/pull/878#issuecomment-3007988414)
It works fine as is in https://doxygen.bitcoincore.org/.
In the future, please open non-GUI-specific PRs in the main [repo](https://github.com/bitcoin/bitcoin).
(https://github.com/bitcoin-core/gui/pull/878#issuecomment-3007988414)
It works fine as is in https://doxygen.bitcoincore.org/.
In the future, please open non-GUI-specific PRs in the main [repo](https://github.com/bitcoin/bitcoin).
✅ hebasto closed a pull request: "Fix broken navigation link to files documentation in README_doxygen.md"
(https://github.com/bitcoin-core/gui/pull/878)
(https://github.com/bitcoin-core/gui/pull/878)
💬 fanquake commented on pull request "test: fix catchup loop in outbound eviction functional test":
(https://github.com/bitcoin/bitcoin/pull/32742#issuecomment-3007993519)
Backported to 29.x in #32810.
(https://github.com/bitcoin/bitcoin/pull/32742#issuecomment-3007993519)
Backported to 29.x in #32810.
💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3008004914)
@maflcko addressed nits and force pushed
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3008004914)
@maflcko addressed nits and force pushed
💬 josibake commented on pull request "[29.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/32810#issuecomment-3008071033)
ACK [e96d360](https://github.com/bitcoin/bitcoin/commit/e96d360767e7dd2e3fc215c48384fadd3f2da82d)
Verified that the correct commits are being pulled in and the release notes.
(https://github.com/bitcoin/bitcoin/pull/32810#issuecomment-3008071033)
ACK [e96d360](https://github.com/bitcoin/bitcoin/commit/e96d360767e7dd2e3fc215c48384fadd3f2da82d)
Verified that the correct commits are being pulled in and the release notes.
🤔 fanquake reviewed a pull request: "cmake: Use `HINTS` instead of `PATHS` in `find_*` commands"
(https://github.com/bitcoin/bitcoin/pull/32805#pullrequestreview-2961702133)
ACK ead44687483e9c936ba970de890c01d5e7ad3485
(https://github.com/bitcoin/bitcoin/pull/32805#pullrequestreview-2961702133)
ACK ead44687483e9c936ba970de890c01d5e7ad3485
🚀 fanquake merged a pull request: "cmake: Use `HINTS` instead of `PATHS` in `find_*` commands"
(https://github.com/bitcoin/bitcoin/pull/32805)
(https://github.com/bitcoin/bitcoin/pull/32805)
💬 fanquake commented on pull request "Fix build on macOS when `qt@6` is installed":
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3008113882)
Do we need this open? Rebased it's the same as #32814.
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3008113882)
Do we need this open? Rebased it's the same as #32814.
💬 fanquake commented on pull request "cmake: Use `HINTS` instead of `PATHS` in `find_*` commands":
(https://github.com/bitcoin/bitcoin/pull/32805#issuecomment-3008115336)
Backported to 29.x in #32810.
(https://github.com/bitcoin/bitcoin/pull/32805#issuecomment-3008115336)
Backported to 29.x in #32810.