Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 l0rinc commented on pull request "clang-format: modernize and realign clang-format configuration":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2167702132)
I've added all of them, this was the simplest change with fewest commits and explanations
🤔 pablomartin4btc reviewed a pull request: "test: fix catchup loop in outbound eviction functional test"
(https://github.com/bitcoin/bitcoin/pull/32742#pullrequestreview-2959781264)
cr-ACK dd8447f70faf6419b4617da3c1b57098e9cd66a6

As I verified, `tip_header` was never updated since the top of the test and `prev_prev_hash` had the wrong value assigned (`tip_header.hash` which was `None`), but the test was passing due to `wait_for_getheaders` when the `block_hash` is `None` (same behaviour as if nothing is passed to the function), [checks](https://github.com/bitcoin/bitcoin/blob/f27898c8bfe339e2cad09d42e133a05602b1a030/test/functional/test_framework/p2p.py#L667-L669) for la
...
🤔 glozow reviewed a pull request: "test: fix an incorrect `feature_fee_estimation.py` subtest"
(https://github.com/bitcoin/bitcoin/pull/32463#pullrequestreview-2959775440)
Nice work finding the issue. For some reason, I couldn't reproduce using `--random=3450808900320758527`. But `--random=4130785039185616282` from https://cirrus-ci.com/task/5487636053229568 and `--random=6565824249277094228` from https://github.com/bitcoin/bitcoin/runs/43526123006 worked for me.

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.

```
diff --git a/test/func
...
💬 glozow commented on pull request "test: fix an incorrect `feature_fee_estimation.py` subtest":
(https://github.com/bitcoin/bitcoin/pull/32463#discussion_r2167700984)
Agree this naming is better 👍
🚀 glozow merged a pull request: "test: fix catchup loop in outbound eviction functional test"
(https://github.com/bitcoin/bitcoin/pull/32742)
💬 achow101 commented on pull request "wallet: remove dead code in legacy wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32758#issuecomment-3006481159)
ACK 0635f107c4e1442ce6b953308723b58044c558df
💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#issuecomment-3006508302)
untested reACK 215e599


https://github.com/bitcoin/bitcoin/pull/32768/commits/9eb2c82e7c911a066781d67e6846cf6bbbaba6e9 removed the unused `upgraded_txs` field from `LoadTxRecords()` and the latest rebase just drops this parameter from the places where it calls `LoadTxRecords()`

<details><summary>

```console
$ git range-diff 4b1d48a..ec25de3 f27898c..215e599
```
</summary>


```diff
1: a39074c52a = 1: e02f2d331c bench: Have AvailableCoins benchmark include a lot of unrelated
...
Christewart closed a pull request: "tests: Remove hardcoded addresstype in `rpc_psbt.py`"
(https://github.com/bitcoin/bitcoin/pull/32701)
💬 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
...
💬 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
...
👍 vasild approved a pull request: "rpc: use CScheduler for HTTPRPCTimer"
(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
💬 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
🤔 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
💬 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
📝 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.
💬 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
...
💬 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.
📝 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
...
💬 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).