π¬ jsarenik commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2745135513)
One more RPC worth mentioning here is `getblockstats` which is currently showing sat/vB in its `feerate_percentiles` JSON array.
I would find it much more helpful if it printed the integer values in sat/kvB as mentioned by @murchandamus .
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2745135513)
One more RPC worth mentioning here is `getblockstats` which is currently showing sat/vB in its `feerate_percentiles` JSON array.
I would find it much more helpful if it printed the integer values in sat/kvB as mentioned by @murchandamus .
π¬ jsarenik commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2745145771)
utACK 6913904
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2745145771)
utACK 6913904
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745146334)
Updated 507c593f18fbd6635dbc29a8f67f6e277c2b33b8 -> 5b32eb7acca176efa4d20aa093ff56b8545eab05 ([pr30997.69](https://github.com/hebasto/bitcoin/commits/pr30997.69) -> [pr30997.70](https://github.com/hebasto/bitcoin/commits/pr30997.70), [diff](https://github.com/hebasto/bitcoin/compare/pr30997.69..pr30997.70)):
- addressed @fanquake's [comment](https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008659388)
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745146334)
Updated 507c593f18fbd6635dbc29a8f67f6e277c2b33b8 -> 5b32eb7acca176efa4d20aa093ff56b8545eab05 ([pr30997.69](https://github.com/hebasto/bitcoin/commits/pr30997.69) -> [pr30997.70](https://github.com/hebasto/bitcoin/commits/pr30997.70), [diff](https://github.com/hebasto/bitcoin/compare/pr30997.69..pr30997.70)):
- addressed @fanquake's [comment](https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008659388)
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008716515)
Thanks! The patch has been [applied](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745146334).
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008716515)
Thanks! The patch has been [applied](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2745146334).
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008720963)
These issues are somewhat related:
- https://bugreports.qt.io/browse/QTBUG-86283
- https://bugreports.qt.io/browse/QTBUG-114570
@fanquake
As a buildsystem maintainer, do you think this is a blocker for this PR?
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008720963)
These issues are somewhat related:
- https://bugreports.qt.io/browse/QTBUG-86283
- https://bugreports.qt.io/browse/QTBUG-114570
@fanquake
As a buildsystem maintainer, do you think this is a blocker for this PR?
π¬ ajtowns commented on pull request "fuzz: enable running fuzz test cases in Debug mode":
(https://github.com/bitcoin/bitcoin/pull/32113#discussion_r2008731010)
Oh yeah, that's obviously better.
(https://github.com/bitcoin/bitcoin/pull/32113#discussion_r2008731010)
Oh yeah, that's obviously better.
π€ hodlinator reviewed a pull request: "doc: clarify the documentation of `Assume` assertion"
(https://github.com/bitcoin/bitcoin/pull/32100#pullrequestreview-2707994339)
Concept ACK
Good to document useful aspects of `Assume`.
(https://github.com/bitcoin/bitcoin/pull/32100#pullrequestreview-2707994339)
Concept ACK
Good to document useful aspects of `Assume`.
π¬ hodlinator commented on pull request "doc: clarify the documentation of `Assume` assertion":
(https://github.com/bitcoin/bitcoin/pull/32100#discussion_r2008737842)
This seems sufficient to me:
```suggestion
expression is always evaluated. However, if the compiler can prove that a
statement inside `Assume` is side-effect-free, it may optimize the call away,
skipping its evaluation in production. This enables a lower-cost way of
making explicit statements about the code, aiding review.
```
Motivation:
> `Assume` can also act as a lightweight debugging assertion, ensuring statements are tested e.g., during fuzzingβto catch violations.
E
...
(https://github.com/bitcoin/bitcoin/pull/32100#discussion_r2008737842)
This seems sufficient to me:
```suggestion
expression is always evaluated. However, if the compiler can prove that a
statement inside `Assume` is side-effect-free, it may optimize the call away,
skipping its evaluation in production. This enables a lower-cost way of
making explicit statements about the code, aiding review.
```
Motivation:
> `Assume` can also act as a lightweight debugging assertion, ensuring statements are tested e.g., during fuzzingβto catch violations.
E
...
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008739707)
> Possibly, have we reported this (regression?) upstream?
Sure! Please see https://bugreports.qt.io/browse/QTBUG-135042.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008739707)
> Possibly, have we reported this (regression?) upstream?
Sure! Please see https://bugreports.qt.io/browse/QTBUG-135042.
π¬ stringintech commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2008742566)
Thanks a lot! Yes the email is okay.
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2008742566)
Thanks a lot! Yes the email is okay.
π€ spboy777 reviewed a pull request: "contrib: Make deterministic-coverage error messages more readable"
(https://github.com/bitcoin/bitcoin/pull/32074#pullrequestreview-2708009681)
A
(https://github.com/bitcoin/bitcoin/pull/32074#pullrequestreview-2708009681)
A
π€ 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
...