Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ’¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2167541369)
Agree. New attempt:
```C++
//! Only start outputting headers once this many headers have been received
//! and validated against commitments.
```
πŸš€ glozow merged a pull request: "wallet, rpc: Remove deprecated balances from getwalletinfo and getunconfirmedbalance"
(https://github.com/bitcoin/bitcoin/pull/32721)
πŸ‘ hodlinator approved a pull request: "test: enabling wallet migration functional test on windows"
(https://github.com/bitcoin/bitcoin/pull/32219#pullrequestreview-2959616863)
re-ACK 941b8f54c0d35d3243bb6083f3b52681d1b9a555

Fixed some nits (https://github.com/bitcoin/bitcoin/pull/32219#pullrequestreview-2959123050) since my first ACK (https://github.com/bitcoin/bitcoin/pull/32219#pullrequestreview-2921071060).
Passed linter locally.
πŸ’¬ l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#issuecomment-3006209711)
I've rebased the changed and adjusted the benchmark to be more similar to the other production usages in the first commit, rounded to even in the second (optimization) commit, so that we can realistically measure the speed difference before and after:

% build/bin/bench_bitcoin -filter='MerkleRoot' --min-time=1000

Before 7f620cffebee593e48434cf182cc2fd64a6d76be:

| ns/leaf | leaf/s | err% | total | benchmark
|--------------------:|--------------------:|---
...
πŸ“ solveforceapp opened a pull request: "Codex/explore paradoxes in language and coding"
(https://github.com/bitcoin/bitcoin/pull/32815)
<!--
*** 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
...
πŸ’¬ darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3006215185)
> I have a preference for breaking out the new checks into its own function since if we intend it to someday soon(TM) be a consensus rule it would have to be pulled out anyways. [instagibbs@5d63372](https://github.com/instagibbs/bitcoin/commit/5d63372f75c4a2403c4032b63e5c604ee96c5a40)

If we do this the separate function could be much simpler than that (no solver etc). I'll have a go at it tomorrow morning, inspired by the consensus implementation i have on a private branch.
βœ… achow101 closed a pull request: "Codex/explore paradoxes in language and coding"
(https://github.com/bitcoin/bitcoin/pull/32815)
πŸ‘ theStack approved a pull request: "doc: Add fetching single PRs from upstream to productivity.md"
(https://github.com/bitcoin/bitcoin/pull/32783#pullrequestreview-2959753896)
ACK 45b1d39757668939b03b27401c324a938ef0cd8d
πŸ’¬ 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