π€ spboy777 reviewed a pull request: "depends: Fix `CXXFLAGS` on NetBSD"
(https://github.com/bitcoin/bitcoin/pull/31502#pullrequestreview-2708018703)
F
(https://github.com/bitcoin/bitcoin/pull/31502#pullrequestreview-2708018703)
F
π¬ yancyribbens commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2745259917)
> Close, but there is a subtlety here..
Thanks, that's a good point I might have overlooked, that in a high fee environment, the tie break on weight is different than in a low fee environment.
> Unfortunately not. We explored sorting by descending value density while developing CoinGrinder..
Hmm that make sense now that you say it. sorting by density isn't strictly equivalent to sorting by value with weight as a tie break and as such would change the behavior of the search.
So, it sounds li
...
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2745259917)
> Close, but there is a subtlety here..
Thanks, that's a good point I might have overlooked, that in a high fee environment, the tie break on weight is different than in a low fee environment.
> Unfortunately not. We explored sorting by descending value density while developing CoinGrinder..
Hmm that make sense now that you say it. sorting by density isn't strictly equivalent to sorting by value with weight as a tie break and as such would change the behavior of the search.
So, it sounds li
...
π¬ laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745263261)
> Vulkan has been disabled for mingw32, which effectively makes it disabled for all platforms.
Right. It seems unnecessary to have Vulkan support on any platform right now, Windows has the DX stuff for rendering, MacOS has its own rendering thing, and i guess on Linux the common denominator is still something like GLES2.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745263261)
> Vulkan has been disabled for mingw32, which effectively makes it disabled for all platforms.
Right. It seems unnecessary to have Vulkan support on any platform right now, Windows has the DX stuff for rendering, MacOS has its own rendering thing, and i guess on Linux the common denominator is still something like GLES2.
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008776357)
It seems this patch is not enough...
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008776357)
It seems this patch is not enough...
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745316102)
Updated 5b32eb7acca176efa4d20aa093ff56b8545eab05 -> 505607ba705ae146b658ec792556056c4966585f ([pr30997.70](https://github.com/hebasto/bitcoin/commits/pr30997.70) -> [pr30997.71](https://github.com/hebasto/bitcoin/commits/pr30997.71), [diff](https://github.com/hebasto/bitcoin/compare/pr30997.70..pr30997.71)):
- Fixed compiling for macOS using CMake < 3.25, including cross-compiling in Guix.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745316102)
Updated 5b32eb7acca176efa4d20aa093ff56b8545eab05 -> 505607ba705ae146b658ec792556056c4966585f ([pr30997.70](https://github.com/hebasto/bitcoin/commits/pr30997.70) -> [pr30997.71](https://github.com/hebasto/bitcoin/commits/pr30997.71), [diff](https://github.com/hebasto/bitcoin/compare/pr30997.70..pr30997.71)):
- Fixed compiling for macOS using CMake < 3.25, including cross-compiling in Guix.
π¬ grubles commented on issue "bitcoind immediately segfaults on ppc64 openbsd 7.4":
(https://github.com/bitcoin/bitcoin/issues/29517#issuecomment-2745365443)
Same issue with v29.0rc2 using both clang and gcc.
`bitcoind`, `bitcoin-cli`, `bitcoin-util`, and `bitcoin-wallet` crash. `bitcoin-tx` does not seem to crash but I haven't tested it much.
`test_bitcoin` prints a ton of these errors: `terminate called recursively`
The "Hi" program works though.
```
bsdppc$ ./a.out
Hi
```
(https://github.com/bitcoin/bitcoin/issues/29517#issuecomment-2745365443)
Same issue with v29.0rc2 using both clang and gcc.
`bitcoind`, `bitcoin-cli`, `bitcoin-util`, and `bitcoin-wallet` crash. `bitcoin-tx` does not seem to crash but I haven't tested it much.
`test_bitcoin` prints a ton of these errors: `terminate called recursively`
The "Hi" program works though.
```
bsdppc$ ./a.out
Hi
```
π€ fjahr reviewed a pull request: "test: assumeutxo: add missing tests in wallet_assumeutxo.py"
(https://github.com/bitcoin/bitcoin/pull/30455#pullrequestreview-2708105981)
re-ACK 55c6a69f777ac08a14e61b98efea00e5c8f98a5f
Just moved the newly added tests into separate functions (which necessitated duplicating the error message).
(https://github.com/bitcoin/bitcoin/pull/30455#pullrequestreview-2708105981)
re-ACK 55c6a69f777ac08a14e61b98efea00e5c8f98a5f
Just moved the newly added tests into separate functions (which necessitated duplicating the error message).
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745397818)
My Guix builds for both `x86_64` and `aarch64`:
```
eb1c0f2a5125776756b15cbb1681a1226f4a16bcbb3a3d5e6c5c9dc22a70e25a guix-build-505607ba705a/output/aarch64-linux-gnu/SHA256SUMS.part
2fb423c1f3c1b52351967d2c4ef732dcf9b46ff214f109febdc1ed861c7a1765 guix-build-505607ba705a/output/aarch64-linux-gnu/bitcoin-505607ba705a-aarch64-linux-gnu-debug.tar.gz
48f21a4cb661ea23948f419ceb9a25d074b0cf010d5b7122a057333844f976a1 guix-build-505607ba705a/output/aarch64-linux-gnu/bitcoin-505607ba705a-aarch64-li
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745397818)
My Guix builds for both `x86_64` and `aarch64`:
```
eb1c0f2a5125776756b15cbb1681a1226f4a16bcbb3a3d5e6c5c9dc22a70e25a guix-build-505607ba705a/output/aarch64-linux-gnu/SHA256SUMS.part
2fb423c1f3c1b52351967d2c4ef732dcf9b46ff214f109febdc1ed861c7a1765 guix-build-505607ba705a/output/aarch64-linux-gnu/bitcoin-505607ba705a-aarch64-linux-gnu-debug.tar.gz
48f21a4cb661ea23948f419ceb9a25d074b0cf010d5b7122a057333844f976a1 guix-build-505607ba705a/output/aarch64-linux-gnu/bitcoin-505607ba705a-aarch64-li
...
π¬ murchandamus commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2745411250)
> So, it sounds like we are in agreement that the [fee comparison](https://github.com/bitcoin/bitcoin/blob/eb9730ab65887279309af338ad0201924f945ac5/src/wallet/coinselection.cpp#L177) in BnB can be removed.
Yes, it looks to me like it should actually be an optimization to remove it.
> I'm tempted to explore adding an iteration count to BnB like coin-grinder since that would make the change nicer. IE if we had an iteration count, then removing the parameter wouldn't require any test changes beca
...
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2745411250)
> So, it sounds like we are in agreement that the [fee comparison](https://github.com/bitcoin/bitcoin/blob/eb9730ab65887279309af338ad0201924f945ac5/src/wallet/coinselection.cpp#L177) in BnB can be removed.
Yes, it looks to me like it should actually be an optimization to remove it.
> I'm tempted to explore adding an iteration count to BnB like coin-grinder since that would make the change nicer. IE if we had an iteration count, then removing the parameter wouldn't require any test changes beca
...
π¬ yancyribbens commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#issuecomment-2745411813)
Interesting - I read the link above "name pipe" and it seems the main benefit to a named pipes is just that for performance, one can skip writing to disk and the churn of creating a new file. I'm not sure I see the perf benefit here in a named pipe though..
(https://github.com/bitcoin/bitcoin/pull/31560#issuecomment-2745411813)
Interesting - I read the link above "name pipe" and it seems the main benefit to a named pipes is just that for performance, one can skip writing to disk and the churn of creating a new file. I'm not sure I see the perf benefit here in a named pipe though..
π¬ yancyribbens commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#issuecomment-2745413053)
I guess a follow commit is how big is the dump. If it's very large then that would be a good motivation for a named pipe.
(https://github.com/bitcoin/bitcoin/pull/31560#issuecomment-2745413053)
I guess a follow commit is how big is the dump. If it's very large then that would be a good motivation for a named pipe.
π luisschwab opened a pull request: "wallet: make coinbase that will mature on the next block available for selection"
(https://github.com/bitcoin/bitcoin/pull/32123)
Closes #32098.
Currently, only coinbase UTXOs that are mature at height **_h_** are available for coin selection. This PR makes UTXOs that mature at height **_h+1_** available for selection.
Since a transaction spending this output will be accepted by the mempool, it should be available for selection.
BDK implements this logic<sup>[1]</sup>, which I believe to be the correct one.
[1]: https://github.com/bitcoindevkit/bdk/issues/1878
(https://github.com/bitcoin/bitcoin/pull/32123)
Closes #32098.
Currently, only coinbase UTXOs that are mature at height **_h_** are available for coin selection. This PR makes UTXOs that mature at height **_h+1_** available for selection.
Since a transaction spending this output will be accepted by the mempool, it should be available for selection.
BDK implements this logic<sup>[1]</sup>, which I believe to be the correct one.
[1]: https://github.com/bitcoindevkit/bdk/issues/1878
π¬ brunoerg commented on pull request "fuzz: wallet: fix crypter target":
(https://github.com/bitcoin/bitcoin/pull/32118#discussion_r2008953125)
Without it I get:
```sh
#47708 REDUCE cov: 475 ft: 1809 corp: 217/12024b lim: 98 exec/s: 3669 rss: 79Mb L: 77/98 MS: 2 PersAutoDict-EraseBytes- DE: "\034\000"-
The current fuzz target used the global random state.
This is acceptable, but requires the fuzz target to call
SeedRandomStateForTest(SeedRand::ZEROS) in the first line
of the FUZZ_TARGET function.
An alternative solution would be to avoid any use of globals.
Without a solution, fuzz instability and non-determinism can l
...
(https://github.com/bitcoin/bitcoin/pull/32118#discussion_r2008953125)
Without it I get:
```sh
#47708 REDUCE cov: 475 ft: 1809 corp: 217/12024b lim: 98 exec/s: 3669 rss: 79Mb L: 77/98 MS: 2 PersAutoDict-EraseBytes- DE: "\034\000"-
The current fuzz target used the global random state.
This is acceptable, but requires the fuzz target to call
SeedRandomStateForTest(SeedRand::ZEROS) in the first line
of the FUZZ_TARGET function.
An alternative solution would be to avoid any use of globals.
Without a solution, fuzz instability and non-determinism can l
...
π¬ kevkevinpal commented on pull request "doc: clarify that testnet min-difficulty is not optional":
(https://github.com/bitcoin/bitcoin/pull/32095#issuecomment-2745981739)
ACK [288481a](https://github.com/bitcoin/bitcoin/pull/32095/commits/288481aabd77b90876489e39403b6eab9d4ae74d)
(https://github.com/bitcoin/bitcoin/pull/32095#issuecomment-2745981739)
ACK [288481a](https://github.com/bitcoin/bitcoin/pull/32095/commits/288481aabd77b90876489e39403b6eab9d4ae74d)
π¬ kevkevinpal commented on pull request "fuzz: Fix off-by-one in package_rbf target":
(https://github.com/bitcoin/bitcoin/pull/32122#issuecomment-2745995758)
Concept ACK
I was able to reproduce the error using the input in https://github.com/bitcoin/bitcoin/issues/32121
---
I think your approach works but I was wondering
why not just modify `initialize_package_rbf` to use this statement
`for (int i = 0; i <= NUM_ITERS; ++i)`
instead of
`for (int i = 0; i < NUM_ITERS; ++i)`
(https://github.com/bitcoin/bitcoin/pull/32122#issuecomment-2745995758)
Concept ACK
I was able to reproduce the error using the input in https://github.com/bitcoin/bitcoin/issues/32121
---
I think your approach works but I was wondering
why not just modify `initialize_package_rbf` to use this statement
`for (int i = 0; i <= NUM_ITERS; ++i)`
instead of
`for (int i = 0; i < NUM_ITERS; ++i)`
π¬ luisschwab commented on pull request "wallet: make coinbase that will mature on the next block available for selection":
(https://github.com/bitcoin/bitcoin/pull/32123#issuecomment-2746002323)
I'm not sure how the balance on `test/functional/interface_bitcoin_cli.py`, L225 was calculated. Should I just replace it by what is returned now?
```py
amounts = [BALANCE + Decimal('9.999928'), Decimal(9), Decimal(31)]
```
(https://github.com/bitcoin/bitcoin/pull/32123#issuecomment-2746002323)
I'm not sure how the balance on `test/functional/interface_bitcoin_cli.py`, L225 was calculated. Should I just replace it by what is returned now?
```py
amounts = [BALANCE + Decimal('9.999928'), Decimal(9), Decimal(31)]
```
β οΈ 1440000bytes opened an issue: "bitcoind crash with corrupt wallet.dat"
(https://github.com/bitcoin/bitcoin/issues/32124)
```
2025-03-20T00:52:09Z Using SQLite Version 3.38.5
2025-03-20T00:52:09Z Using wallet w1
2025-03-20T00:52:09Z init message: Loading walletβ¦
2025-03-20T00:52:09Z SQLite Error. Code: 1. Message: no such column: minversion in "INSERT INTO main VALUES(?, ?)"
2025-03-20T00:52:09Z [w1] Releasing wallet w1..
terminate called after throwing an instance of 'std::runtime_error'
what(): SQLiteDatabase: Failed to close database: database is locked
```
I had observed `bitcoind` crash a few days back whi
...
(https://github.com/bitcoin/bitcoin/issues/32124)
```
2025-03-20T00:52:09Z Using SQLite Version 3.38.5
2025-03-20T00:52:09Z Using wallet w1
2025-03-20T00:52:09Z init message: Loading walletβ¦
2025-03-20T00:52:09Z SQLite Error. Code: 1. Message: no such column: minversion in "INSERT INTO main VALUES(?, ?)"
2025-03-20T00:52:09Z [w1] Releasing wallet w1..
terminate called after throwing an instance of 'std::runtime_error'
what(): SQLiteDatabase: Failed to close database: database is locked
```
I had observed `bitcoind` crash a few days back whi
...
β οΈ jsarenik opened an issue: "Failed transactions on importing mempool"
(https://github.com/bitcoin/bitcoin/issues/32125)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
Hello! This is rather a question. I am wondering: are the 10 failed related to package relaying?
29.0rc2
```
2025-03-23T05:24:22Z SetNetworkActive: false
2025-03-23T05:24:27Z Loading 20483 mempool transactions from file...
2025-03-23T05:24:27Z Progress loading mempool transactions from file: 10% (tried 2049, 18434 remaining)
2025-03-23T05:24:27Z Progress loading mempool transactions from
...
(https://github.com/bitcoin/bitcoin/issues/32125)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
Hello! This is rather a question. I am wondering: are the 10 failed related to package relaying?
29.0rc2
```
2025-03-23T05:24:22Z SetNetworkActive: false
2025-03-23T05:24:27Z Loading 20483 mempool transactions from file...
2025-03-23T05:24:27Z Progress loading mempool transactions from file: 10% (tried 2049, 18434 remaining)
2025-03-23T05:24:27Z Progress loading mempool transactions from
...
π¬ fjahr commented on pull request "wallet: make coinbase that will mature on the next block available for selection":
(https://github.com/bitcoin/bitcoin/pull/32123#issuecomment-2746173630)
I understand what you are saying about correctness but @sipa 's concerns with propagation issues are also hard to ignore. Bitcoin Core just tends to be on the more conservative side when it comes to protecting users and BDK might leave protections like this to be handled by the actual wallet implemeters that use BDK.
A potential middle ground could be that current behavior is maintained but users get useful hint in return why they can't spend their funds yet. Then there could be a flag added
...
(https://github.com/bitcoin/bitcoin/pull/32123#issuecomment-2746173630)
I understand what you are saying about correctness but @sipa 's concerns with propagation issues are also hard to ignore. Bitcoin Core just tends to be on the more conservative side when it comes to protecting users and BDK might leave protections like this to be handled by the actual wallet implemeters that use BDK.
A potential middle ground could be that current behavior is maintained but users get useful hint in return why they can't spend their funds yet. Then there could be a flag added
...
π¬ sipa commented on pull request "wallet: make coinbase that will mature on the next block available for selection":
(https://github.com/bitcoin/bitcoin/pull/32123#issuecomment-2746179956)
@luisschwab There are other examples of coins which the Bitcoin Core wallet won't let users spend, for example, unconfirmed coins (unless they are self-created/change). Just like here, that's not a protocol rule, but it's a bad idea, and it's perfectly possible to bypass by using raw transactions.
Now, given how unlikely it is in 2025 for any user to use the Bitcoin Core wallet directly for mining payouts, the odds that this is still actually protecting anyone are probably negligible. So @fjahr
...
(https://github.com/bitcoin/bitcoin/pull/32123#issuecomment-2746179956)
@luisschwab There are other examples of coins which the Bitcoin Core wallet won't let users spend, for example, unconfirmed coins (unless they are self-created/change). Just like here, that's not a protocol rule, but it's a bad idea, and it's perfectly possible to bypass by using raw transactions.
Now, given how unlikely it is in 2025 for any user to use the Bitcoin Core wallet directly for mining payouts, the odds that this is still actually protecting anyone are probably negligible. So @fjahr
...