💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1900345308)
Note that this pattern is copied from a similar testnet3 test.

💬 philmb3487 commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1900345499)
just one nit, the OS is named 'macOS'

💬 zaidmstrr commented on pull request "Execute Discover() when bind=0.0.0.0 or :: is set":
(https://github.com/bitcoin/bitcoin/pull/31492#discussion_r1894636030)
Hey, I'm reviewing this PR; can you please explain to me the thoughts behind removing this line of code? 

👍 tdb3 approved a pull request: "rpc: add gettarget , target getmininginfo field and show next block info"
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2526895295)
ACK 7f34802fd726b259f7df6f605e1d488faf251127


<details><summary>Tested on mainnet for 876960 difficulty adjustment</summary>

Expected: same diff/target in next block
```
$ build/src/bitcoin-cli getblockhash 876959
000000000000000000002428ab473fa8570115d4a000e637814b86248d35d370
$ build/src/bitcoin-cli invalidateblock 000000000000000000002428ab473fa8570115d4a000e637814b86248d35d370
$ build/src/bitcoin-cli getdifficulty
108522647629298.2
$ build/src/bitcoin-cli getdifficulty true
10
...

💬 maflcko commented on issue " Cannot figure out how to use std::atomic error for MacOS Sequoia 15.2":
(https://github.com/bitcoin/bitcoin/issues/31579#issuecomment-2567097930)
What is your version of XCode? Ref: https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#1-xcode-command-line-tools

👍 tdb3 approved a pull request: "doc: Update NetBSD Build Guide"
(https://github.com/bitcoin/bitcoin/pull/31586#pullrequestreview-2526910770)
ACK 2bdaf52ed1259fd3bec22b680e12563fcee0a8b3

Dry ran the guide with a fresh NetBSD installation, built with zmq, tested zmqpubhashblock with an adhoc python client (saw the topic, block hash, and sequence arrive when generating blocks in regtest with `-generate`).

Also successfully ran the functional test suite (python). Had to create a symlink (`python3` --> `python3.10`).

💬 yancyribbens commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2567108046)
> Right, but I think the question is whether negative fee rates should be rounded away from or towards zero

`ceil` rounds towards zero: `ceil(-2.4) = -2.000000`. Rounding toward zero when negative is correct imo. Rounding up is done to make sure the fee is adequate instead of falling short.

> Also, I'm not sure what kind of scenario a negative fee rate would be used.

Good question. frameworks like rust-bitcoin use an unsigned int for FeeRate so it can't be negative.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2567112162)
@darosior @Sjors @pinheadmz

I've recently analyzed the blockchain for the last two years from 4th December 2022 to 23rd December 2024 to move this forward.

I will cross-write my findings here, but more seen here details can be seen here https://delvingbitcoin.org/t/analyzing-mining-pool-behavior-to-address-bitcoin-cores-double-coinbase-reservation-issue/1351

- Most mining pools adhere to Bitcoin Core’s default setup, keeping coinbase transaction weights well below **4000 WU**. However,
...

📝 theStack opened a pull request: "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds"
(https://github.com/bitcoin/bitcoin/pull/31588)
The determinstic line coverage checking script `test_deterministic_coverage.sh` was seemingly forgotten to be adapted in the course of switching from autotools to CMake (only the error messages containing build instructions were updated in a9964c04447745435747d9cc557165c43902783b / #30875). Since `gcovr` needs to access both the source and build files, the path to the latter has to be passed explicitly as they are not in the same anymore (see e.g. https://github.com/gcovr/gcovr/issues/340).

C
...

💬 tcharding commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2567132185)
If I'm not mistaken (again) this line of code can never be hit.

`if (nSatoshisPerK > 0) nFee = CAmount(1);`

💬 theStack commented on pull request "test: raise explicit error if any of the needed release binaries is missing":
(https://github.com/bitcoin/bitcoin/pull/31462#issuecomment-2567137879)
Thanks for the reviews! I agree that skipping the test is a bad idea, switched to to an explicit error message instead as suggested, also added a log.info with instructions on how to obtain the binaries for previous releases in this case.

💬 sipa commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2567140752)
I believe the only way to hit negative feerates is when using the `prioritizetransaction` RPC with a (highly) negative fee_delta argument. I don't know why in this context rounding away from zero (as `CFeeRate` does) would matter.

Longer term, I think `CFeeRate` should be replaced with, or implemented using, `FeeFrac` which avoids divisions.

🤔 ariard reviewed a pull request: "p2p: Fill reconciliation sets (Erlay) attempt 2"
(https://github.com/bitcoin/bitcoin/pull/30116#pullrequestreview-2526935458)
Reviewed up to commit feb8c98db (“p2p: Add transactions to reconciliation sets…”).

💬 ariard commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1900472039)
If the peer is `m_wtxid_relay=false` should `m_tx_inven tory_known_filter` be checked by `txid` ? Note, given the parent identifiers are retrieved from `GetMemPoolParents()` we can have the identifiers easily. This could avoid some duplicated INV flooding.

💬 ariard commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1900468228)
I think the rationale for introducing `SortPeersByFewestParents` could be more fleshed out, i.e iiuc explaining that eligible peers with the lowest number of parents for a target transaction in their reconciliation sets are favored for fan-out announcements, as this can be a hint of better connectivity in the overall transaction-relay graph and we wish to propagate the target transaction faster (..?).

💬 yancyribbens commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2567174219)
> If I'm not mistaken (again) this line of code can never be hit.

If the goal is to prevent a fee of zero, it probably should be: `if (nSatoshisPerK == 0) nFee = CAmount(1);`

💬 sipa commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2567180312)
For context, support for negative feerates in `CFeeRate` was added in #7796.

✅ maflcko closed a pull request: "Enhance fee estimation logic and improve error handling in TxConfirmS…"
(https://github.com/bitcoin/bitcoin/pull/31587)

💬 tdb3 commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#issuecomment-2567204709)
Looks like we can re-use the new `DIFF_1_N_BITS` and `REGTEST_N_BITS` in a few other tests (https://github.com/tdb3/bitcoin/commits/2024/12/gettarget/). Could be added if there are other changes or left for future follow-up. The use of `REGTEST_N_BITS` might be borderline (the test is creating blocks by hand).

⚠️ Pttn opened an issue: "P2TR Spending Bug - Signing Transaction Failed"
(https://github.com/bitcoin/bitcoin/issues/31589)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Some Segwit Version 1 Addresses generated with [Achow101's Rust-Vanitygen](https://github.com/achow101/rust-vanitygen) cause an issue when spending coins using them. Generally, the "Signing transaction failed" error occurs and the transaction is not created. Though, a workaround still allows to spend the coins.

I don't know if the issue is exclusive to P2TR, but never encountered such p
...