💬 fanquake commented on issue "Linux download needs installation instructions":
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-3225097225)
Yea I'm not sure. For master, the binaries still have a `libxcb-cursor` dependency, that may need to be installed. It's not clear if #33238 will work for `29.x`, so that will still have a `libxcb-xinerama` dependency, that may need to be installed, (same for `28.x`). There's a note about `libxcb-cursor0` in build-unix, but that isn't shipped with release binaries, or likely read by anyone who isn't compiling from source, so it seems like any of this could benefit from additional documentation.
...
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-3225097225)
Yea I'm not sure. For master, the binaries still have a `libxcb-cursor` dependency, that may need to be installed. It's not clear if #33238 will work for `29.x`, so that will still have a `libxcb-xinerama` dependency, that may need to be installed, (same for `28.x`). There's a note about `libxcb-cursor0` in build-unix, but that isn't shipped with release binaries, or likely read by anyone who isn't compiling from source, so it seems like any of this could benefit from additional documentation.
...
💬 maflcko commented on issue "Linux download needs installation instructions":
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-3225106768)
> There's a note about `libxcb-cursor0` in build-unix, but that isn't shipped with release binaries, or likely read by anyone who isn't compiling from source, so it seems like any of this could benefit from additional documentation.
I see, so it seems a bit related to https://github.com/bitcoin/bitcoin/issues/32565
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-3225106768)
> There's a note about `libxcb-cursor0` in build-unix, but that isn't shipped with release binaries, or likely read by anyone who isn't compiling from source, so it seems like any of this could benefit from additional documentation.
I see, so it seems a bit related to https://github.com/bitcoin/bitcoin/issues/32565
💬 ajtowns commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2301680659)
Added an assert and comment.
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2301680659)
Added an assert and comment.
💬 maflcko commented on issue "`feature_bind_extra.py` test fails in `test_runner` if new nodes are added":
(https://github.com/bitcoin/bitcoin/issues/33250#issuecomment-3225111710)
The assert is not hit, because you are just setting `port+=2`, not by calling `p2p_port(node_number+N)`.
My recommendation would be to call `p2p_port`, so that the assert is hit in your code.
(https://github.com/bitcoin/bitcoin/issues/33250#issuecomment-3225111710)
The assert is not hit, because you are just setting `port+=2`, not by calling `p2p_port(node_number+N)`.
My recommendation would be to call `p2p_port`, so that the assert is hit in your code.
⚠️ maflcko reopened an issue: "`feature_bind_extra.py` test fails in `test_runner` if new nodes are added"
(https://github.com/bitcoin/bitcoin/issues/33250)
When new nodes are added to the `feature_bind_extra.py` test, it succeeds if run individually, but fails if run via `test_runner` with the error message:
```
TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/home/node/test/functional/test_framework/test_framework.py", line 191, in main
self.setup()
File "/home/node/test/functional/test_framework/test_framework.py", line 349, in setup
self.setup_network()
File "/home/node/build/test/functional/fe
...
(https://github.com/bitcoin/bitcoin/issues/33250)
When new nodes are added to the `feature_bind_extra.py` test, it succeeds if run individually, but fails if run via `test_runner` with the error message:
```
TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/home/node/test/functional/test_framework/test_framework.py", line 191, in main
self.setup()
File "/home/node/test/functional/test_framework/test_framework.py", line 349, in setup
self.setup_network()
File "/home/node/build/test/functional/fe
...
💬 achow101 commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3225121798)
ACK b7b249d3adfbd3c7b0c4d0499a86300f57982972
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3225121798)
ACK b7b249d3adfbd3c7b0c4d0499a86300f57982972
💬 ajtowns commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3225128619)
> Edit: what is the expected memory increase because of the change?
32 bytes per mempool transaction and per extra txn (of which there are 100 by default).
A 300MB mempool might fill up with about ~100k txs (at about 440vb each on average), which would be 3.2MB total. (The 3.2MB would not be an increase overall, but would rather cause earlier eviction from the mempool, ie a ~1% reduction in capacity)
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3225128619)
> Edit: what is the expected memory increase because of the change?
32 bytes per mempool transaction and per extra txn (of which there are 100 by default).
A 300MB mempool might fill up with about ~100k txs (at about 440vb each on average), which would be 3.2MB total. (The 3.2MB would not be an increase overall, but would rather cause earlier eviction from the mempool, ie a ~1% reduction in capacity)
✅ enirox001 closed a pull request: "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py"
(https://github.com/bitcoin/bitcoin/pull/33184)
(https://github.com/bitcoin/bitcoin/pull/33184)
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2301702270)
According to my analysis, there is nothing more than specified criteria applied inconsistently in the codebase. Can you point out something?
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2301702270)
According to my analysis, there is nothing more than specified criteria applied inconsistently in the codebase. Can you point out something?
📝 enirox001 reopened a pull request: "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py"
(https://github.com/bitcoin/bitcoin/pull/33184)
The `rpc_getblockstats.py` test was failing with `--gen-test-data` because it used legacy wallet operations that could be affected by wallet changes, making the test data inconsistent.
I replaced the legacy wallet with MiniWallet to fix this. This makes the test data generation deterministic and reliable.
**Changes:**
- Switched from `wallet_importprivkey`/`sendtoaddress` to `MiniWallet.send_to()`
- Added helper methods for OP_RETURN creation and block retrieval
- Made assertions dynami
...
(https://github.com/bitcoin/bitcoin/pull/33184)
The `rpc_getblockstats.py` test was failing with `--gen-test-data` because it used legacy wallet operations that could be affected by wallet changes, making the test data inconsistent.
I replaced the legacy wallet with MiniWallet to fix this. This makes the test data generation deterministic and reliable.
**Changes:**
- Switched from `wallet_importprivkey`/`sendtoaddress` to `MiniWallet.send_to()`
- Added helper methods for OP_RETURN creation and block retrieval
- Made assertions dynami
...
💬 achow101 commented on pull request "[29.x] backport #33212":
(https://github.com/bitcoin/bitcoin/pull/33251#issuecomment-3225147396)
ACK fcac8022d839572f5d8781096eec14ca7ea2e0dd
(https://github.com/bitcoin/bitcoin/pull/33251#issuecomment-3225147396)
ACK fcac8022d839572f5d8781096eec14ca7ea2e0dd
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2301715335)
The meaning is totally different, and it's easier and much more maintainable right now. This class doesn't depend on any other class/object. It's an independent representation.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2301715335)
The meaning is totally different, and it's easier and much more maintainable right now. This class doesn't depend on any other class/object. It's an independent representation.
📝 polespinasa reopened a pull request: "wallet, rpc: remove settxfee and paytxfee"
(https://github.com/bitcoin/bitcoin/pull/32138)
**Summary**
This PR removes the settxfee RPC and paytxfee setting (Bitcoin Core 31.0).
These two features were deprecated in https://github.com/bitcoin/bitcoin/pull/31278.
This PR does not remove the internal implementation of the default value of paytxfee=0 but removes the option for users to modify it.
(https://github.com/bitcoin/bitcoin/pull/32138)
**Summary**
This PR removes the settxfee RPC and paytxfee setting (Bitcoin Core 31.0).
These two features were deprecated in https://github.com/bitcoin/bitcoin/pull/31278.
This PR does not remove the internal implementation of the default value of paytxfee=0 but removes the option for users to modify it.
👋 polespinasa's pull request is ready for review: "wallet, rpc: remove settxfee and paytxfee"
(https://github.com/bitcoin/bitcoin/pull/32138)
(https://github.com/bitcoin/bitcoin/pull/32138)
💬 enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2301723616)
Good catch! You're right - since `create_self_transfer()` defaults to `fee_rate=0.003`, explicitly setting it here is redundant and potentially confusing. I've removed the parameter to use the default.
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2301723616)
Good catch! You're right - since `create_self_transfer()` defaults to `fee_rate=0.003`, explicitly setting it here is redundant and potentially confusing. I've removed the parameter to use the default.
💬 enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2301724711)
Good idea, but `send_self_transfer()` only creates self-transfers (sending back to the wallet's own address). For this test, we need external outputs to properly test `getblockstats` UTXO statistics. The `send_to()` method is necessary here to create transactions with external recipients.
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2301724711)
Good idea, but `send_self_transfer()` only creates self-transfers (sending back to the wallet's own address). For this test, we need external outputs to properly test `getblockstats` UTXO statistics. The `send_to()` method is necessary here to create transactions with external recipients.
💬 enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2301728062)
This mining pattern is intentional - it creates a specific block structure for testing:
- Block 1: 1 transaction (first 10 BTC send)
- Block 2: 4 transactions (remaining sends + OP_RETURN)
This allows testing `getblockstats` across different block scenarios (single vs multi-transaction blocks) rather than having all transactions in one block.
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2301728062)
This mining pattern is intentional - it creates a specific block structure for testing:
- Block 1: 1 transaction (first 10 BTC send)
- Block 2: 4 transactions (remaining sends + OP_RETURN)
This allows testing `getblockstats` across different block scenarios (single vs multi-transaction blocks) rather than having all transactions in one block.
💬 enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2301731981)
You're right - we need to re-serialize after modifying the transaction. The `tx["hex"]` contains the original transaction, but we're modifying `tx["tx"]` by adding the OP_RETURN output. Without re-serializing, the OP_RETURN wouldn't actually be included in the sent transaction.
I've applied your suggested fix.
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2301731981)
You're right - we need to re-serialize after modifying the transaction. The `tx["hex"]` contains the original transaction, but we're modifying `tx["tx"]` by adding the OP_RETURN output. Without re-serializing, the OP_RETURN wouldn't actually be included in the sent transaction.
I've applied your suggested fix.
💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2301743800)
* `listsinceblock`'s `blockhash` is included in this list even though block hashes cannot include `=`. There are also a ton of other RPCs that take a block hash in the same way but are not included in this list.
* `descriptorprocesspsbt`'s `sighashtype` is a parameter which has a list of fixed string values and none of them have an `=`. There are multiple RPCs that take a sighashtype but they are not included in this list.
* `sendmany`'s `dummy` must be an empty string so it is reasonable to e
...
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2301743800)
* `listsinceblock`'s `blockhash` is included in this list even though block hashes cannot include `=`. There are also a ton of other RPCs that take a block hash in the same way but are not included in this list.
* `descriptorprocesspsbt`'s `sighashtype` is a parameter which has a list of fixed string values and none of them have an `=`. There are multiple RPCs that take a sighashtype but they are not included in this list.
* `sendmany`'s `dummy` must be an empty string so it is reasonable to e
...
💬 frankomosh commented on pull request "fuzz: add target for `DifferenceFormatter` and p2p invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2301744219)
@brunoerg The PR title was already updated yesterday, to "fuzz: add target for DifferenceFormatter and p2p invariant check" (changed from the previous version that mentioned net_processing.cpp). Please let me know if you’d still like to see any wording tweaks?
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2301744219)
@brunoerg The PR title was already updated yesterday, to "fuzz: add target for DifferenceFormatter and p2p invariant check" (changed from the previous version that mentioned net_processing.cpp). Please let me know if you’d still like to see any wording tweaks?
💬 polespinasa commented on pull request "wallet, rpc: remove settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/32138#issuecomment-3225201197)
Rebased on top of master 6001029dfd21a7ddd8ecb906961ed16c8f074298
(https://github.com/bitcoin/bitcoin/pull/32138#issuecomment-3225201197)
Rebased on top of master 6001029dfd21a7ddd8ecb906961ed16c8f074298