💬 maflcko commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#issuecomment-3031268246)
Just ensure enough random transactions have been created to reliably return a fee estimate in any run?
(https://github.com/bitcoin/bitcoin/pull/32615#issuecomment-3031268246)
Just ensure enough random transactions have been created to reliably return a fee estimate in any run?
💬 maflcko commented on issue "fuzz: Fix stability, determinism issues":
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-3031280019)
Just dropping an extremely hacky/ugly patch that could be useful to collect the combined debug log while iterating over all fuzz input files in one process and then split that debug log again for each fuzz input file and show the diff:
<details><summary>hacky patch</summary>
```diff
diff --git a/a.py b/a.py
new file mode 100644
index 0000000000..a28b602891
--- /dev/null
+++ b/a.py
@@ -0,0 +1,27 @@
+import os
+import sys
+
+in_a=sys.argv[1]
+out_a=[]
+for basename_a in [in_a,in_a.replace('.a.',
...
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-3031280019)
Just dropping an extremely hacky/ugly patch that could be useful to collect the combined debug log while iterating over all fuzz input files in one process and then split that debug log again for each fuzz input file and show the diff:
<details><summary>hacky patch</summary>
```diff
diff --git a/a.py b/a.py
new file mode 100644
index 0000000000..a28b602891
--- /dev/null
+++ b/a.py
@@ -0,0 +1,27 @@
+import os
+import sys
+
+in_a=sys.argv[1]
+out_a=[]
+for basename_a in [in_a,in_a.replace('.a.',
...
💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2182139002)
hi @achow101 can you please review it again.
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2182139002)
hi @achow101 can you please review it again.
📝 fanquake converted_to_draft a pull request: "checkqueue: set MAX_SCRIPTCHECK_THREADS to nCores - 1"
(https://github.com/bitcoin/bitcoin/pull/32692)
A fixed MAX_SCRIPTCHECK_THREADS value is not flexible for users to leverage their cpu resources, and a value large than nCores - 1 doesn't make sense since it only adds some context switch overhead. Set it to nCores - 1. Assumption: A user who sets the number of script verification workers is aware of how this affects the system performance, otherwise he/she leaves it as default (which is 0)
(https://github.com/bitcoin/bitcoin/pull/32692)
A fixed MAX_SCRIPTCHECK_THREADS value is not flexible for users to leverage their cpu resources, and a value large than nCores - 1 doesn't make sense since it only adds some context switch overhead. Set it to nCores - 1. Assumption: A user who sets the number of script verification workers is aware of how this affects the system performance, otherwise he/she leaves it as default (which is 0)
🚀 fanquake merged a pull request: "test: check P2SH sigop count for coinbase tx"
(https://github.com/bitcoin/bitcoin/pull/32850)
(https://github.com/bitcoin/bitcoin/pull/32850)
💬 maflcko commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#discussion_r2182226882)
erroneous rebase?
(https://github.com/bitcoin/bitcoin/pull/31665#discussion_r2182226882)
erroneous rebase?
💬 rkrux commented on pull request "wallet, test: best block locator matches scan state follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32580#discussion_r2182241246)
I recall spending some time to come up with a log format specifically for this but I was not satisfied with the ones I came up, and thus defaulted to the regular one.
This suggestion looks better to me, I will use it if I end up retouching.
(https://github.com/bitcoin/bitcoin/pull/32580#discussion_r2182241246)
I recall spending some time to come up with a log format specifically for this but I was not satisfied with the ones I came up, and thus defaulted to the regular one.
This suggestion looks better to me, I will use it if I end up retouching.
📝 Sjors opened a pull request: "Have createwalletdescriptor auto-detect an unused(KEY)"
(https://github.com/bitcoin/bitcoin/pull/32861)
The `createwalletdescriptor` was introduced in #29130 to let users add a `tr()` descriptor to an existing SegWit wallet. The new `addhdkey` method from #29136 introduces a new potential workflow: start from a blank wallet, generate an HD and then add only the descriptors you need, e.g.:
```sh
bitcoin rpc createwallet TaprootMaxi blank=true
bitcoin rpc addhdkey
bitcoin rpc createwalletdescriptor bech32m
```
Before this PR the last line would fail, requiring the user to call `gethdkeys`
...
(https://github.com/bitcoin/bitcoin/pull/32861)
The `createwalletdescriptor` was introduced in #29130 to let users add a `tr()` descriptor to an existing SegWit wallet. The new `addhdkey` method from #29136 introduces a new potential workflow: start from a blank wallet, generate an HD and then add only the descriptors you need, e.g.:
```sh
bitcoin rpc createwallet TaprootMaxi blank=true
bitcoin rpc addhdkey
bitcoin rpc createwalletdescriptor bech32m
```
Before this PR the last line would fail, requiring the user to call `gethdkeys`
...
💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3031449845)
> Another thing I noticed is that createwalletdescriptor is not smart enough to just use the only available unused(KEY) descriptor.
I opened a followup PR for this #32861.
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3031449845)
> Another thing I noticed is that createwalletdescriptor is not smart enough to just use the only available unused(KEY) descriptor.
I opened a followup PR for this #32861.
💬 rkrux commented on pull request "wallet: remove dead code in legacy wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32758#issuecomment-3031518930)
Rebased over master to incorporate changes from #31423.
(https://github.com/bitcoin/bitcoin/pull/32758#issuecomment-3031518930)
Rebased over master to incorporate changes from #31423.
✅ fanquake closed an issue: "test: functional test failures under `--usecli`"
(https://github.com/bitcoin/bitcoin/issues/32264)
(https://github.com/bitcoin/bitcoin/issues/32264)
🚀 fanquake merged a pull request: "test: allow all functional tests to be run or skipped with --usecli"
(https://github.com/bitcoin/bitcoin/pull/32290)
(https://github.com/bitcoin/bitcoin/pull/32290)
👋 fanquake's pull request is ready for review: "[29.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/32810)
(https://github.com/bitcoin/bitcoin/pull/32810)
🚀 fanquake merged a pull request: "cmake: Improve Python robustness and test usability"
(https://github.com/bitcoin/bitcoin/pull/31233)
(https://github.com/bitcoin/bitcoin/pull/31233)
💬 fanquake commented on pull request "depends: Override host compilers for FreeBSD and OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/32716#issuecomment-3031623366)
> This also is related: https://codeberg.org/guix/guix/issues/556.
Reading the discussion there, it seems like the behaviour is intentional.
(https://github.com/bitcoin/bitcoin/pull/32716#issuecomment-3031623366)
> This also is related: https://codeberg.org/guix/guix/issues/556.
Reading the discussion there, it seems like the behaviour is intentional.
💬 fanquake commented on pull request "depends: Override host compilers for FreeBSD and OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/32716#issuecomment-3031631149)
There are more improvements we could make here, but we can take, and backport, the (simpler) current change as-is. Anything more will at least require some Guix changes.
(https://github.com/bitcoin/bitcoin/pull/32716#issuecomment-3031631149)
There are more improvements we could make here, but we can take, and backport, the (simpler) current change as-is. Anything more will at least require some Guix changes.
✅ fanquake closed an issue: "depends: OpenBSD (aarch64) needs gcc (instruction) for libevent"
(https://github.com/bitcoin/bitcoin/issues/32691)
(https://github.com/bitcoin/bitcoin/issues/32691)
🚀 fanquake merged a pull request: "depends: Override host compilers for FreeBSD and OpenBSD"
(https://github.com/bitcoin/bitcoin/pull/32716)
(https://github.com/bitcoin/bitcoin/pull/32716)
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3031698536)
Rebased after trivial conflict with #32290 (it touches `ci/test/00_setup_env_i686_multiprocess.sh` which this PR renames).
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3031698536)
Rebased after trivial conflict with #32290 (it touches `ci/test/00_setup_env_i686_multiprocess.sh` which this PR renames).
💬 maflcko commented on pull request "cmake: Improve Python robustness and test usability":
(https://github.com/bitcoin/bitcoin/pull/31233#discussion_r2182425599)
i guess unrelated to the changes here, but i still don't think this is the correct fix. it seems odd to special-case a single python test and silently disable it when cmake fails to detect python (in the ci or otherwise). This is brittle, inconsistent and just adds maintenance overhead.
the correct fix would be to just remove the cmake stuff here and move this test to the python test runner, so that it is run like all other python unit/util/functional tests.
(https://github.com/bitcoin/bitcoin/pull/31233#discussion_r2182425599)
i guess unrelated to the changes here, but i still don't think this is the correct fix. it seems odd to special-case a single python test and silently disable it when cmake fails to detect python (in the ci or otherwise). This is brittle, inconsistent and just adds maintenance overhead.
the correct fix would be to just remove the cmake stuff here and move this test to the python test runner, so that it is run like all other python unit/util/functional tests.
💬 fanquake commented on pull request "cmake: Improve Python robustness and test usability":
(https://github.com/bitcoin/bitcoin/pull/31233#discussion_r2182429044)
Yes I agree.
(https://github.com/bitcoin/bitcoin/pull/31233#discussion_r2182429044)
Yes I agree.