Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👋 fanquake's pull request is ready for review: "Musig2 tests"
(https://github.com/bitcoin/bitcoin/pull/32724)
👋 fanquake's pull request is ready for review: "wallet: Remove wallet version and several legacy related functions"
(https://github.com/bitcoin/bitcoin/pull/32977)
🤔 enirox001 reviewed a pull request: "test: refactor mempool_accept_wtxid"
(https://github.com/bitcoin/bitcoin/pull/33067#pullrequestreview-3079794265)
ACK

Solid test refactor with significant performance improvement:

- Using pre-mined chain reduces runtime from ~36s to ~7-10s (70%+ faster)
- MiniWallet integration eliminates manual signing and key management
- Class-to-function conversion appropriate for single-use case
- Removes unnecessary variable assignments, improves readability

Test coverage remains identical, CI passes.
💬 hebasto commented on pull request "refactor: Use immediate lambda to work around GCC bug 117966":
(https://github.com/bitcoin/bitcoin/pull/33113#issuecomment-3145104459)
> ```diff
> ci: Re-enable DEBUG=1 in centos task
>
> This reverts the workaround in commit fa079538e32d20aec6786c93e1117da1e8ea0cab, as it is no longer needed.
> ```

Thanks! Your changes have been added.
💬 hebasto commented on pull request "refactor: Use immediate lambda to work around GCC bug 117966":
(https://github.com/bitcoin/bitcoin/pull/33113#issuecomment-3145108301)
> Seems fine, but it can't hurt to enforce it in CI again?
>
> ```diff
> ci: Re-enable DEBUG=1 in centos task
>
> This reverts the workaround in commit fa079538e32d20aec6786c93e1117da1e8ea0cab, as it is no longer needed.
>
> diff --git a/ci/test/00_setup_env_native_centos.sh b/ci/test/00_setup_env_native_centos.sh
> index 25b9923dc7..94002b6349 100755
> --- a/ci/test/00_setup_env_native_centos.sh
> +++ b/ci/test/00_setup_env_native_centos.sh
> @@ -10,11 +10,11 @@ export
...
💬 kevkevinpal commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2248423750)
looks like this was fixed in https://github.com/bitcoin/bitcoin/pull/33104
💬 hMsats commented on issue "bitcoind 29.0 much slower than 28.0 on my system: cause found":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-3145315142)
Closing this issue concurrent with the 29.1rc1 release candidate. After upgrading Bitcoin Core to 29.0, the solution is to keep the system running for a few days until most LevelDB files have been converted to approximately 32 MB, or to run a `-reindex`. Thanks for all the help and support!
hMsats closed an issue: "bitcoind 29.0 much slower than 28.0 on my system: cause found"
(https://github.com/bitcoin/bitcoin/issues/32455)
🤔 caesrcd reviewed a pull request: "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee"
(https://github.com/bitcoin/bitcoin/pull/33106#pullrequestreview-3080167562)
tACK a43e1b28b2899e1707e7867fd46efe9fcc08f241

Compiled and tested on Arch Linux (kernel 6.15.8, gcc 15.1.1, clang 20.1.8).
Build performed with CMake in Debug mode:
```bash
cmake -B build_test -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang
cmake --build build_test -j$(nproc)
ctest --output-on-failure --stop-on-failure --test-dir build_test -j$(nproc)
build_test/test/functional/test_runner.py
```

All unit and functional tests passed successfully.
🤔 glozow reviewed a pull request: "validation: detect witness stripping early on"
(https://github.com/bitcoin/bitcoin/pull/33105#pullrequestreview-3080155389)
approach ACK - I'm not super confident in my script knowledge but I can't think of why you couldn't detect witness stripped this way.
💬 glozow commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2248582026)
I don't think this comment belongs here. It has too many details about net_processing logic that shouldn't really be mentioned in validation / are already covered in the code that does rejection caching.
```suggestion
// Detect a missing witness so that p2p code can handle rejection caching appropriately.
```
💬 Sjors commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#issuecomment-3145445209)
I plan to test this soon(tm).
💬 achow101 commented on issue "spendable is true for UTXO of private key disabled wallet":
(https://github.com/bitcoin/bitcoin/issues/33110#issuecomment-3145501388)
The docs need to be updated, `spendable` is always true for UTXOs belonging to a descriptor wallet, including ones without private keys.
👍 theStack approved a pull request: "p2p: TxOrphanage revamp cleanups"
(https://github.com/bitcoin/bitcoin/pull/32941#pullrequestreview-3080320302)
re-ACK 3d4d4f0d92d42809e74377e4380abdc70f74de5d
🤔 glozow reviewed a pull request: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896#pullrequestreview-3080238325)
a2c43c3b9447377d2f2635e23c9043e40b769dd4 looks pretty good to me! Minor comments mostly about docs
💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248644653)
nit 7f43b5559c5f508ac210350847bca6f0f0e091be

call this `parent_it` as well?
💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248642322)
7f43b5559c5f508ac210350847bca6f0f0e091be

nit: generally prefer comments to live on separate lines
```suggestion
// this unconfirmed v3 transaction already has a child
if (wtx.truc_child_in_mempool.has_value()) continue;
```
💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248646240)
```suggestion
// If this tx has a parent, unset its truc_child_in_mempool to make it possible
// to spend from the parent again. If this tx was replaced by another
```
💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248686896)
This comment can be confusing - we're not actually looking for transactions spending from *this* tx. We're looking for siblings of this tx, which spend from utxos of `wtx` (which is actually this transaction's parent). Might be helpful to rename `wallet_it` to `parent_it` and update this comment.
💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248640723)
1910d9b61f23f65335367f2f8dd021ac1ccd907a

Could be more specific. Also, this isn't only used by AvailableCoinsListUnspent. It's always checked in `AvailableCoins`, but `AvailableCoinsListUnspent` is the only caller that disables it because it's not building a transaction.

```suggestion
// When true, filter unconfirmed coins by whether their version's TRUCness matches what is set by CCoinControl.
bool check_version_trucness{true};
```