💬 rkrux commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2298155422)
In cf588607fa36964597a6d6acc853eeb26e51b0ea "test: Test for exportwatchonlywallet"

This `getnewaddress` fails as expected because 10 pre-generated keys were there in the offline before export. I understand this seems to be a caveat for the hardened ranged descriptors that few pre-generated keys must be there before export otherwise the online wallet can't create new addresses - I feel this should be mentioned in the RPC Help section wrt the usability of the online wallet where the user might
...

💬 rkrux commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2298140555)
In cf588607fa36964597a6d6acc853eeb26e51b0ea "test: Test for exportwatchonlywallet"

In this particular subtest, there are 5 inactive and 8 active descriptors in the `listdescriptors` RPC response after importing few descriptors. This subtest can also check that the active and inactive descriptors remain the same after export.

💬 rkrux commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2298283741)
In cf588607fa36964597a6d6acc853eeb26e51b0ea "test: Test for exportwatchonlywallet"

Nit for clarity in the subtests. Otherwise it takes a while to figure out that these funds are not part of the wallet(s) under consideration.

```diff
- self.funds = self.online.get_wallet_rpc(self.default_wallet_name)
+ self.outside_wallet_funds = self.online.get_wallet_rpc(self.default_wallet_name)
```

💬 rkrux commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2298309162)
In https://github.com/bitcoin/bitcoin/commit/cf588607fa36964597a6d6acc853eeb26e51b0ea "test: Test for exportwatchonlywallet"

While calling this RPC here is fine, maybe add 10 as the default value for keypool in the node extra_args argument? That would represent a scenario that's more likely to happen in the real world.

💬 rkrux commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2298311703)
In https://github.com/bitcoin/bitcoin/commit/cf588607fa36964597a6d6acc853eeb26e51b0ea "test: Test for exportwatchonlywallet"

Nit: would be nice to tie this `9` to the `10` above in the `keypoolrefill` RPC - `KEYPOOL_SIZE - 1`

💬 rkrux commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2298304988)
In https://github.com/bitcoin/bitcoin/commit/cf588607fa36964597a6d6acc853eeb26e51b0ea "test: Test for exportwatchonlywallet"

In this subtest, can also assert for few `getwalletinfo` RPC response values being equal as well such as `txcount, keypoolsize, keypoolsize_hd_internal, birthtime, lastprocessedblock`. For the `flags` RPC response key, it can be asserted that for the watch-only one, the value is the union of the original flags and `disable_private_keys`.

Might as well put all these a
...

💬 rkrux commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2298327208)
In cdaed2bab95870f77efb3cbca1dc98e5d4f5695c "wallet, rpc: Add exportwatchonlywallet RPC"

Based on some threshold value of remaining unused keys available, should this RPC also pre-generate keys before exporting the watch-only wallet so that the user doesn't end up with not being able to generate new addresses quickly after exporting and has to export again?

💬 instagibbs commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3220639788)
Seems like a03aef9cec35b0d03aa63d7e8093f0420cd4b40b is the problematic commit for my machine? Doubles the runtime.

Both reverted:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1,955,234.00 | 511.45 | 2.1% | 0.02 | `BlockEncoding`

No reversions:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--
...

🤔 jonatack reviewed a pull request: "Revert compact block cache inefficiencies"
(https://github.com/bitcoin/bitcoin/pull/33253#pullrequestreview-3152074460)
ARM64 M1 Max (2022), macOS 15.6.1

benchmark only

```
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 2,643,000.00 | 378.36 | 1.5% | 0.03 | `BlockEncoding`
```

benchmark with reversions

```
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:--------
...

💬 cedwies commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#discussion_r2298491922)
This test also passes on master (MacOS environment with ARG_MAX = 1048576). It does not fail with the test_framework changes reverted.
However, the following test only passes on this PR, not on my master:
```python
self.log.info("test many small args (sum-size trigger)")
tiny = "a" * 120000
args = [tiny] * 10
assert_equal(args, node.echo(*args))
```

💬 cedwies commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#issuecomment-3220904717)
ACK fabe214

This PR fixes how the functional test framework estimates command-line argument size. Previously it checked only the longest argument, but the OS limit is on the total argv/env size, so it now sums all args. It also adds a functional test using very large echo args, which exercises the `-stdin` fallback. The test passes for me.

I appreciate lowering `CLI_MAX_ARG_SIZE` to `1024`. It’s a very safe, conservative threshold that avoids platform-dependent `ARG_MAX` issues, and since
...

🤔 enirox001 reviewed a pull request: "test: Fix CLI_MAX_ARG_SIZE issues"
(https://github.com/bitcoin/bitcoin/pull/33243#pullrequestreview-3152161767)
Concept ACK fa18c82 — The fix is a welcome improvement: summing arg sizes better reflects kernel ARG_MAX behavior. Good use of >= to match inclusive boundary semantics. Left a few non-blocking clarifier nits inline.

💬 enirox001 commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#discussion_r2298536682)
```
2025-08-25T15:54:00.131311Z TestFramework.bitcoincli (DEBUG): Cli: Command size 131140 too large, using stdin
2025-08-25T15:54:00.148742Z TestFramework.bitcoincli (DEBUG): Cli: Command size 262212 too large, using stdin
2025-08-25T15:54:00.179312Z TestFramework.bitcoincli (DEBUG): Cli: Command size 4194372 too large, using stdin
2025-08-25T15:54:00.418554Z TestFramework.bitcoincli (DEBUG): Cli: Command size 8388676 too large, using stdin
```

fa18c82 — The debug logs show byte-based s
...

💬 enirox001 commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#discussion_r2298540591)
fa18c82 — Does this test use the system `ARG_MAX` at runtime or intentionally use a conservative value? Could we add a short comment that `1024` is a test-only, conservative value to ensure the stdin fallback is exercised, and that production behavior still relies on the system `ARG_MAX`

💬 enirox001 commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#discussion_r2298546674)
fa18c82 — Perhaps add a short note that these sizes target historical/modern `ARG_MAX` boundaries (64KiB, ~128KiB, ~2MiB, ~4MiB) and that `-1` tests just-below the power-of-two to catch off-by-ones? Took me a while to understand the intent.

💬 l0rinc commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2298621307)
These are extremely fast tests, keeping the usual test format has more advantages in my opinion.
If you have a string preference or if other reviewers prefer that, I don't mind changing, but I don't think the current one has measurable disadvantages compared to the suggestion - while being more in-line with how we usually test, making `GetDbBatchSize` easily debuggable (often helps with understanding if you can step through it).

💬 Galoretka commented on pull request "doc: add Linux GUI runtime instructions to doc/README.md":
(https://github.com/bitcoin/bitcoin/pull/33197#issuecomment-3221188022)
> The dependency on `libxcb-xinerama0` has been removed in #33217 (which will also be backported to the `29.x` branch) so I think pretty much all of the text being added here, is no-longer needed?

so i should close pr?

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3221463982)
Ping :)

💬 achow101 commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3221526194)
> _aj_> instagibbs: re: 33253, the benchmark doesn't exercise [a8203e9](https://github.com/bitcoin/bitcoin/commit/a8203e94123b6ea6e4f4a6320e3ad20457f44a28) -- it just passes an empty vector of extra transactions

Can we bench that?

💬 martinatime commented on issue "bitcoind 29.0 much slower than 28.0 on my system: cause found":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-3221628972)
As an update, my bitcoin instance has finally finished the initial verification. I started it on June 1st and it has finished between last night (August 24) and today.

📝 151henry151 opened a pull request: "wallet: Replace fee magic numbers with named constants"
(https://github.com/bitcoin/bitcoin/pull/33254)
## Summary

This PR addresses a TODO item in the wallet fee handling code by replacing magic numbers with descriptive named constants. It also adds `constexpr` constructors to `CFeeRate` to support compile-time constants.

## Changes

- **Add `constexpr` CFeeRate constructors** to support compile-time constants
- **Replace magic `CFeeRate(0)` values** with descriptive named constants:
- `DEFAULT_FEERATE` for default/unset fee rates
- `NO_FEE_DATA` for fee estimation failures
- `FEERATE_D
...