💬 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
...
(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`
(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.
(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).
(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?
(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 :)
(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?
(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.
(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
...
(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
...
💬 achow101 commented on pull request "Update libmultiprocess subtree to fix build issues":
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3221668831)
ACK 323b3fd27283282f2f8eb1096f56f23d8230f2d6
Verified the subtree update and that it fixes the Ubuntu issue (after removing the 0.8.0 restriction).
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3221668831)
ACK 323b3fd27283282f2f8eb1096f56f23d8230f2d6
Verified the subtree update and that it fixes the Ubuntu issue (after removing the 0.8.0 restriction).
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299137075)
I don't think any of those are actually useful things to assert. The point of the offline wallet is that it doesn't have access to the blockchain, so actually `txcount`, and `lastprocessedblock` should be different. The keypool sizes may be different depending on the node startup options, and birthtime may be different if something was used before being imported to the offline wallet.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299137075)
I don't think any of those are actually useful things to assert. The point of the offline wallet is that it doesn't have access to the blockchain, so actually `txcount`, and `lastprocessedblock` should be different. The keypool sizes may be different depending on the node startup options, and birthtime may be different if something was used before being imported to the offline wallet.
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299144047)
Done
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299144047)
Done
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299144625)
Added extra text to the help.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299144625)
Added extra text to the help.
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145052)
Renamed to `funder`.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145052)
Renamed to `funder`.
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145315)
Done
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145315)
Done
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145463)
Done
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145463)
Done
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299146384)
Done, however there should not be an expectation that the topup will always work as it does not work for wallets that are encrypted.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299146384)
Done, however there should not be an expectation that the topup will always work as it does not work for wallets that are encrypted.
🚀 achow101 merged a pull request: "Update libmultiprocess subtree to fix build issues"
(https://github.com/bitcoin/bitcoin/pull/33241)
(https://github.com/bitcoin/bitcoin/pull/33241)
👍 l0rinc approved a pull request: "Revert compact block cache inefficiencies"
(https://github.com/bitcoin/bitcoin/pull/33253#pullrequestreview-3152574449)
Concept ACK, excellent find.
I redid the benchmarks, got more modest changes with GCC and Clang:
<details>
<summary>C++ compiler .......................... GNU 14.2.0</summary>
```bash
echo "> C++ compiler .......................... GNU $(gcc -dumpfullversion)" && rm -rf build && cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ >/dev/null 2>&1 && cmake --build build -j$(nproc) >/dev/null 2>&1 && build/bin/bench_bitcoin -filter
...
(https://github.com/bitcoin/bitcoin/pull/33253#pullrequestreview-3152574449)
Concept ACK, excellent find.
I redid the benchmarks, got more modest changes with GCC and Clang:
<details>
<summary>C++ compiler .......................... GNU 14.2.0</summary>
```bash
echo "> C++ compiler .......................... GNU $(gcc -dumpfullversion)" && rm -rf build && cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ >/dev/null 2>&1 && cmake --build build -j$(nproc) >/dev/null 2>&1 && build/bin/bench_bitcoin -filter
...
💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298811715)
nit: newline at the very end is customary for the linter and formatter to not complain
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298811715)
nit: newline at the very end is customary for the linter and formatter to not complain