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

🤔 ismaelsadeeq reviewed a pull request: "test: Add test coverage for rpcwhitelistdefault when unset"
(https://github.com/bitcoin/bitcoin/pull/32079#pullrequestreview-2690555367)
Concept ACK, nice follow-up!
Why is it in draft?

🚀 hebasto merged a pull request: "test: Check datadir cleanup after assumeutxo was successful"
(https://github.com/bitcoin/bitcoin/pull/32033)

💬 Prabhat1308 commented on pull request "fuzz: Speed up *_package_eval fuzz targets a bit":
(https://github.com/bitcoin/bitcoin/pull/31457#issuecomment-2729581927)
> Is this a correct way to measure the speed difference (on a Mac)?
>
> ```shell
> git clone --depth=1 https://github.com/bitcoin-core/qa-assets
> cmake --preset=libfuzzer\
> -DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
> -DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
> -DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld"
> cmake --build build_fuzz
> time FUZZ=tx_package_eval build_fuzz/bin/fuzz qa-assets/fuzz_corpora/tx_package_eval
> ```

Running this I run into
...

💬 furszy commented on issue "listdescriptors true fails with 'Can't get descriptor string' in non-watch-only descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/32078#issuecomment-2729618600)
This occurs when the wallet does not contain all key/script paths key material (e.g. taproot descriptor with an internal pubkey). Test exercising the behavior and explaining the issue further: https://github.com/furszy/bitcoin-core/commit/970efeb0fb2632d714e0f444b4ac89278eb80b28.

💬 suiyuan1314 commented on pull request "docs: improve development guidelines for PR merging":
(https://github.com/bitcoin/bitcoin/pull/32006#discussion_r1998815319)
I have reworded the guidelines, is it okay?

💬 willcl-ark commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#discussion_r1998797323)
```suggestion
Install Xcode Command Line Tools, Homebrew Package Manager, and
the Required Dependencies from [build-osx.md](../doc/build-osx.md).
```

👍 willcl-ark approved a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2690626911)
ACK 3ad36fce4be0aa413639468118d6dbd7379a2106

I confirm that the `make` brew formula [symlinks itself](https://github.com/Homebrew/homebrew-core/blob/5c4e0f50fa0cee5ae512f09ae887e3f5d0c05318/Formula/m/make.rb#L52-L55) to `gmake`.

Did a successful (`NO_QT=1`) depends build using `gmake` on macOS 15 and no longer see the errors from #30978.

I think it would be good to keep #30978 open if this is merged, to address https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849

💬 fanquake commented on issue "build: status code 503 for freetype":
(https://github.com/bitcoin/bitcoin/issues/32085#issuecomment-2729687435)
It'd be good if the Windows builds didn't compile all the Qt deps (which we don't even use) from source all the time, which would bypass this issue entirely, however outside of that there's no much we can do here.

✅ fanquake closed an issue: "build: status code 503 for freetype"
(https://github.com/bitcoin/bitcoin/issues/32085)

🤔 furszy reviewed a pull request: "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan"
(https://github.com/bitcoin/bitcoin/pull/32023#pullrequestreview-2690742860)
Code review ACK c986b43f0c45388b87106bcbf2534c75eebe7b72.

💬 furszy commented on pull request "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#discussion_r1998864194)
could make this more descriptive so that the user is aware of which specific descriptor failed when multiple are being imported.
```c++
throw std::runtime_error(strprintf("Can't update descriptor %s, error: %s", descriptor.descriptor->ToString(), error))
```
This will need modifications in the test side too.

🤔 rkrux reviewed a pull request: "wallet, rpc: deprecate settxfee and paytxfee"
(https://github.com/bitcoin/bitcoin/pull/31278#pullrequestreview-2690764016)
Concept and utACK 2f2ab47bf74f4da37aad75a186cb0bb16e8af579

I am in favour of having both a consistent fee rate unit to specify across RPCs (sat/vB over BTC/kvB), and relying on fee estimation most of the times while creating the transaction that the user can override by setting the `fee_rate` argument in the transaction creation RPCs (including PSBT ones), which is expressed in `sat/vB`.

As a user, deprecating this RPC and startup option streamlines the overall flows for me by not having t
...

💬 rkrux commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1998876768)
Having reviewed this PR https://github.com/bitcoin/bitcoin/pull/31977 earlier, I feel ideally this log should not be removed but commented at this stage and the above comment should also generally mention to uncomment this log later when the RPCs tested below are fully removed from the codebase.
But we can get this done in a separate PR now after this one is merged.

💬 Sjors commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#discussion_r1998898227)
The Required Dependencies would defeat the purpose of depends. at least `boost` and `libevent`. But maybe `pkgconf` and `cmake` should be added to the brew install incantation?

💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1998903473)
I agree. If it is not done in a separate PR we can simply add it back into the PR where we remove the RPC call. We will have to leave the test to its default form again anyway.

👍 darosior approved a pull request: "refactor: Make node_id a const& in RemoveBlockRequest"
(https://github.com/bitcoin/bitcoin/pull/31282#pullrequestreview-2690856800)
utACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc

💬 laanwj commented on pull request "build: Drop option to disable hardening.":
(https://github.com/bitcoin/bitcoin/pull/32071#issuecomment-2729857146)
> The only case I'm aware of where some hardening flags cause an issue is on NetBSD 10, and this is expected to be fixed in NetBSD 11.

Right. Not sure if it's worth it in this specific case, as it's going to be fixed upstream soon, but imo in general if some flags (hardening or otherwise) are a problem on some platform this should be taken into account by the build system automatically instead of having the user disable hardening entirely, which is a hassle and creates a bigger risk than nece
...

💬 yancyribbens commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2729873842)
I did some testing on this issue and I found that if the [parameter](https://github.com/bitcoin/bitcoin/blob/eb9730ab65887279309af338ad0201924f945ac5/src/wallet/coinselection.cpp#L177) is removed, one of [tests](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L427) finishes in 3 fewer iterations, from 38 iterations to 35. The reason the test is more efficient without the added parameter is that there are two UTXOs with the same effective value of `5000000 s
...

🤔 ismaelsadeeq reviewed a pull request: "[29.x] backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/32062#pullrequestreview-2690931188)
Code review ACK 74df31cb0bdef9cce31ae62ed71a1e386cba0274

I've verified that the patch-ids are identical. Only 4e438d326ea55ac0f98f89e41e69b56354e801e7 has a different patch-id from the original commit, and that's due to line number differences.

I've verified that the backported commits is identical to the PR commits.