:lock: achow101 locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/32120)
(https://github.com/bitcoin/bitcoin/issues/32120)
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008659388)
Ok, so it looks like we should instead apply this patch: https://codereview.qt-project.org/c/qt/qtbase/+/633612 ?
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008659388)
Ok, so it looks like we should instead apply this patch: https://codereview.qt-project.org/c/qt/qtbase/+/633612 ?
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008662782)
> ... or a poor design of Qt's internal find modules.
Possibly, have we reported this (regression?) upstream? From my reading of the Qt docs, I can't seem to find `pkg-config` listed as a build-time dependency.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2008662782)
> ... or a poor design of Qt's internal find modules.
Possibly, have we reported this (regression?) upstream? From my reading of the Qt docs, I can't seem to find `pkg-config` listed as a build-time dependency.
💬 davidgumberg commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-2745031877)
I don't have an explanation for this yet, but I am able to reproduce the provided hash on systems with python versions >= 3.12.0, with various `zlib` and `zlib-ng` versions.
`pkgdiff` again reports the mismatching archives as identical. But, now that the archives are uncompressed, the binary diff output is a little more revealing, here's the start of the diff:
<details>
<summary>
`git diff <(xxd rocky9.3/Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers.tar.gz) <(xxd fedora41/Xcode-
...
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-2745031877)
I don't have an explanation for this yet, but I am able to reproduce the provided hash on systems with python versions >= 3.12.0, with various `zlib` and `zlib-ng` versions.
`pkgdiff` again reports the mismatching archives as identical. But, now that the archives are uncompressed, the binary diff output is a little more revealing, here's the start of the diff:
<details>
<summary>
`git diff <(xxd rocky9.3/Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers.tar.gz) <(xxd fedora41/Xcode-
...
💬 ajtowns commented on pull request "fuzz: enable running fuzz test cases in Debug mode":
(https://github.com/bitcoin/bitcoin/pull/32113#issuecomment-2745039599)
> I don't think the current patch will work. `G_FUZZING` influences more than just the behavior of the asserts/assumes. For example:
>
> * POW checks are different
> * The task runner is different
> * The random seeding is different
Yes -- the same fuzz seed will in many cases execute different code paths for the fuzzer when compiled in Debug mode without BUILD_FOR_FUZZING set. But that's a feature not a bug -- the idea is to be able to still run the fuzz tests with a relativ
...
(https://github.com/bitcoin/bitcoin/pull/32113#issuecomment-2745039599)
> I don't think the current patch will work. `G_FUZZING` influences more than just the behavior of the asserts/assumes. For example:
>
> * POW checks are different
> * The task runner is different
> * The random seeding is different
Yes -- the same fuzz seed will in many cases execute different code paths for the fuzzer when compiled in Debug mode without BUILD_FOR_FUZZING set. But that's a feature not a bug -- the idea is to be able to still run the fuzz tests with a relativ
...
💬 maflcko commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-2745042271)
> I don't have an explanation for this yet, but I am able to reproduce the provided hash on systems with python versions >= 3.12.0, with various `zlib` and `zlib-ng` versions, but python < 3.12.0 I get varying hashes.
A wild guess: Could this be related to https://github.com/python/cpython/issues/95385?
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-2745042271)
> I don't have an explanation for this yet, but I am able to reproduce the provided hash on systems with python versions >= 3.12.0, with various `zlib` and `zlib-ng` versions, but python < 3.12.0 I get varying hashes.
A wild guess: Could this be related to https://github.com/python/cpython/issues/95385?
📝 achow101 locked a pull request: "[28.x] 28.1 backports and final changes"
(https://github.com/bitcoin/bitcoin/pull/31594)
Backports:
- #31502
- #31563
(https://github.com/bitcoin/bitcoin/pull/31594)
Backports:
- #31502
- #31563
💬 maflcko commented on pull request "fuzz: enable running fuzz test cases in Debug mode":
(https://github.com/bitcoin/bitcoin/pull/32113#issuecomment-2745046158)
> But that's a feature not a bug -- the idea is to be able to still run the fuzz tests with a relatively normal production-like build that doesn't completely special-case out POW checks, random seeding, etc.
My fear is that some bugs are not reproducible. (I know this is mostly theoretical, because we haven't seen such a case yet, but still it would be a bit disappointing if someone spent hours trying to reproduce a fuzz run, when it is impossible in a normal debug build)
> `inline bool Fu
...
(https://github.com/bitcoin/bitcoin/pull/32113#issuecomment-2745046158)
> But that's a feature not a bug -- the idea is to be able to still run the fuzz tests with a relatively normal production-like build that doesn't completely special-case out POW checks, random seeding, etc.
My fear is that some bugs are not reproducible. (I know this is mostly theoretical, because we haven't seen such a case yet, but still it would be a bit disappointing if someone spent hours trying to reproduce a fuzz run, when it is impossible in a normal debug build)
> `inline bool Fu
...
⚠️ maflcko opened an issue: "fuzz: package_rbf crashes after out-of-range memory read"
(https://github.com/bitcoin/bitcoin/issues/32121)
Diff to reproduce (turns UB into a runtime exception):
```diff
diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp
index 3e5b361186..74099f770d 100644
--- a/src/test/fuzz/rbf.cpp
+++ b/src/test/fuzz/rbf.cpp
@@ -118,7 +118,7 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
}
assert(iter <= g_outpoints.size());
replacement_tx->vin.resize(1);
- replacement_tx->vin[0].prevout = g_outpoints[iter++];
+ replacement_tx->vin[0].prevout = g_outpoints.at(iter++);
...
(https://github.com/bitcoin/bitcoin/issues/32121)
Diff to reproduce (turns UB into a runtime exception):
```diff
diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp
index 3e5b361186..74099f770d 100644
--- a/src/test/fuzz/rbf.cpp
+++ b/src/test/fuzz/rbf.cpp
@@ -118,7 +118,7 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
}
assert(iter <= g_outpoints.size());
replacement_tx->vin.resize(1);
- replacement_tx->vin[0].prevout = g_outpoints[iter++];
+ replacement_tx->vin[0].prevout = g_outpoints.at(iter++);
...
📝 maflcko opened a pull request: "fuzz: Fix off-by-one in package_rbf target"
(https://github.com/bitcoin/bitcoin/pull/32122)
Running the while loop up to `NUM_ITERS` times may set `iter` to `g_outpoints.size()`, which will then lead to an out-of-bounds read.
There was an assert, which I guess tried to catch this, but the condition in the assert was wrong as well.
Fix all issues by replacing the broken assert with the internal and correct check inside `std::vector::at` and by limiting `iter` to `NUM_ITERS` in the while loop.
(https://github.com/bitcoin/bitcoin/pull/32122)
Running the while loop up to `NUM_ITERS` times may set `iter` to `g_outpoints.size()`, which will then lead to an out-of-bounds read.
There was an assert, which I guess tried to catch this, but the condition in the assert was wrong as well.
Fix all issues by replacing the broken assert with the internal and correct check inside `std::vector::at` and by limiting `iter` to `NUM_ITERS` in the while loop.
💬 maflcko commented on pull request "fuzz: Fix off-by-one in package_rbf target":
(https://github.com/bitcoin/bitcoin/pull/32122#issuecomment-2745083344)
Can be tested with the input and steps from https://github.com/bitcoin/bitcoin/issues/32121
(https://github.com/bitcoin/bitcoin/pull/32122#issuecomment-2745083344)
Can be tested with the input and steps from https://github.com/bitcoin/bitcoin/issues/32121
💬 ajtowns commented on pull request "fuzz: enable running fuzz test cases in Debug mode":
(https://github.com/bitcoin/bitcoin/pull/32113#issuecomment-2745105618)
> lgtm
Okay, done. How's that look?
(https://github.com/bitcoin/bitcoin/pull/32113#issuecomment-2745105618)
> lgtm
Okay, done. How's that look?
💬 maflcko commented on pull request "fuzz: enable running fuzz test cases in Debug mode":
(https://github.com/bitcoin/bitcoin/pull/32113#discussion_r2008700665)
nit: Forgot to mention this, but to me it seems simpler to have the program automatically set this. Otherwise, it is just busy work for the dev.
in `src/test/fuzz/fuzz.cpp` you can set it to true, and default it to false, no?
This is also the code prior to the changes in https://github.com/bitcoin/bitcoin/pull/31191/files
(https://github.com/bitcoin/bitcoin/pull/32113#discussion_r2008700665)
nit: Forgot to mention this, but to me it seems simpler to have the program automatically set this. Otherwise, it is just busy work for the dev.
in `src/test/fuzz/fuzz.cpp` you can set it to true, and default it to false, no?
This is also the code prior to the changes in https://github.com/bitcoin/bitcoin/pull/31191/files
🤔 maflcko reviewed a pull request: "Rust tool to import bip39 mnemonic"
(https://github.com/bitcoin/bitcoin/pull/32115#pullrequestreview-2707937328)
> Or perhaps Rust Bitcoin could ship it as an example.
Yeah, right now it looks like this is just a few lines of docs and example code with the bulk being several imported rust-bitcoin dependencies. So I wonder what the benefits are to include it here, instead of somewhere closer where the majority of its (implicit) code sits.
(https://github.com/bitcoin/bitcoin/pull/32115#pullrequestreview-2707937328)
> Or perhaps Rust Bitcoin could ship it as an example.
Yeah, right now it looks like this is just a few lines of docs and example code with the bulk being several imported rust-bitcoin dependencies. So I wonder what the benefits are to include it here, instead of somewhere closer where the majority of its (implicit) code sits.
💬 maflcko commented on pull request "Rust tool to import bip39 mnemonic":
(https://github.com/bitcoin/bitcoin/pull/32115#discussion_r2008706369)
This is a bit confusing. What does "back it up" mean? The wallet, the mnemonic? Also, I am not sure about the risks that users are exposed to now. I've only read the first section of https://github.com/bitcoin/bips/wiki/Comments:BIP-0039 and it mentions loss of funds. So I think it would be better to educate the users about how wallet incompatibilities can silently lead to loss of funds, instead of implying that a backup of the mnemonic is sufficient for recovery.
(https://github.com/bitcoin/bitcoin/pull/32115#discussion_r2008706369)
This is a bit confusing. What does "back it up" mean? The wallet, the mnemonic? Also, I am not sure about the risks that users are exposed to now. I've only read the first section of https://github.com/bitcoin/bips/wiki/Comments:BIP-0039 and it mentions loss of funds. So I think it would be better to educate the users about how wallet incompatibilities can silently lead to loss of funds, instead of implying that a backup of the mnemonic is sufficient for recovery.
🤔 maflcko reviewed a pull request: "fuzz: wallet: fix crypter target"
(https://github.com/bitcoin/bitcoin/pull/32118#pullrequestreview-2707939655)
Would be nice to add a short line on how to test/observe this fix
(https://github.com/bitcoin/bitcoin/pull/32118#pullrequestreview-2707939655)
Would be nice to add a short line on how to test/observe this fix
💬 maflcko commented on pull request "fuzz: wallet: fix crypter target":
(https://github.com/bitcoin/bitcoin/pull/32118#discussion_r2008708494)
Why is this needed?
(https://github.com/bitcoin/bitcoin/pull/32118#discussion_r2008708494)
Why is this needed?
💬 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)