fa776e61cd Add importmempool RPC (MarcoFalke)
fa20d734a2 refactor: Add and use kernel::ImportMempoolOptions (MarcoFalke)
fa8866990d doc: Clarify the getmempoolinfo.loaded RPC field documentation (MarcoFalke)
6888886cec Remove Chainstate::LoadMempool (MarcoFalke)
Pull request description:
Currently it is possible to import a mempool by placing it in the datadir and starting the node. However this has many issues:
* Users aren't expected to fiddle with the datadir, possibly corrupting it
* An existing mempool file in the datadir may be overwritten
* The node needs to be restarted
* Importing an untrusted file this way is dangerous, because it can corrupt the mempool
Fix all issues by adding a new RPC.
ACKs for top commit:
ajtowns:
utACK fa776e61cd
achow101:
ACK fa776e61cd
glozow:
reACK fa776e61cd
Tree-SHA512: fcb1a92d6460839283c546c47a2d930c363ac1013c4c50dc5215ddf9fe5e51921d23fe0abfae0a5a7631983cfc7e2fff3788b70f95937d0a989a203be4d67546
5364dd8666 test: locked_wallet, skip default fee estimation (furszy)
Pull request description:
Coming from https://github.com/bitcoin/bitcoin/pull/28139#discussion_r1284563239.
No test case in this file is meant to exercise fee estimation. All default wallets have a
custom tx fee set [here](b7138252ac/test/functional/wallet_fundrawtransaction.py (L100)). The only one missing is the one created for `locked_wallet`.
ACKs for top commit:
theStack:
ACK 5364dd8666
Tree-SHA512: 514c02708081d18330d759d10e306cee16c6350de243c68f0973777d2582f5d81968a237393c1f59aba245297e03f3f98d3ae5249a042469d0d016255f568719
1c976c691c tidy: Integrate bicoin-tidy clang-tidy plugin (fanquake)
7de23cceb8 refactor: fix unterminated LogPrintf()s (fanquake)
0a1029aa29 lint: remove /* Continued */ markers from codebase (fanquake)
910007995d lint: remove lint-logs.py (fanquake)
d86a83d6b8 lint: drop DIR_IWYU global (fanquake)
Pull request description:
Demo of integrating the [bitcoin-tidy](https://github.com/theuni/bitcoin-tidy-plugin), [clang-tidy plugin](https://clang.llvm.org/extra/clang-tidy/) written by theuni into our tidy CI job.
The plugin currently has a single check, `bitcoin-unterminated-logprintf`. This would replace our current Python driven, `git-grep`-based, `.cpp` file only, lint-logs linter.
ACKs for top commit:
TheCharlatan:
ACK 1c976c691c
theuni:
ACK 1c976c691c
MarcoFalke:
re-ACK 1c976c691c👠
Tree-SHA512: 725b45c70e431d48e6f276671e05c694e10b6047cae1a31906ac3ee9093bc8105fb226b36a5bac6709557526ca6007222112d66aecec05a574434edc4897e4b8
6a7686b446 scripted-diff: Specify Python major version explicitly on Windows (Hennadii Stepanov)
Pull request description:
On Windows, it is the accepted practice to use `py.exe` launcher:
- https://learn.microsoft.com/en-us/windows/python/faqs#what-is-py-exe-
- https://docs.python.org/3/using/windows.html#python-launcher-for-windows
One of its features is the correct handling of shebang lines like the one we use: `#!/usr/bin/env python3`.
However, Windows OS app execution aliases might [interfere](https://learn.microsoft.com/en-us/windows/python/faqs#why-does-running-python-exe-open-the-microsoft-store-) with the launcher's behaviour. Such aliases are enabled on Windows 11 by default:
For example, on a fresh Windows 11 Pro installation with the Python installed from the [Chocolatey](https://community.chocolatey.org/packages/python/3.11.4) package manager, one will get the following error:
```
>py -3 test\functional\rpc_signer.py
2023-08-03T19:41:13.353000Z TestFramework (INFO): PRNG seed is: 2694758731106548661
2023-08-03T19:41:13.353000Z TestFramework (INFO): Initializing test directory C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3
2023-08-03T19:41:14.538000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\util.py", line 140, in try_rpc
fun(*args, **kwds)
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\authproxy.py", line 129, in __call__
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found; run without arguments to install from the Microsoft Store, or disable this shortcut from Settings > Manage App Execution Aliases.
(-1)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\test_framework.py", line 131, in main
self.run_test()
File "C:\Users\hebasto\bitcoin\test\functional\rpc_signer.py", line 72, in run_test
assert_raises_rpc_error(-1, 'fingerprint not found',
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\util.py", line 131, in assert_raises_rpc_error
assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\util.py", line 146, in try_rpc
raise AssertionError(
AssertionError: Expected substring not found in error message:
substring: 'fingerprint not found'
error message: 'RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found; run without arguments to install from the Microsoft Store, or disable this shortcut from Settings > Manage App Execution Aliases.
'.
2023-08-03T19:41:14.592000Z TestFramework (INFO): Stopping nodes
2023-08-03T19:41:14.799000Z TestFramework (WARNING): Not cleaning up dir C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3
2023-08-03T19:41:14.799000Z TestFramework (ERROR): Test failed. Test logging available at C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3/test_framework.log
2023-08-03T19:41:14.799000Z TestFramework (ERROR):
2023-08-03T19:41:14.799000Z TestFramework (ERROR): Hint: Call C:\Users\hebasto\bitcoin\test\functional\combine_logs.py 'C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3' to consolidate all logs
2023-08-03T19:41:14.799000Z TestFramework (ERROR):
2023-08-03T19:41:14.799000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-08-03T19:41:14.799000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2023-08-03T19:41:14.799000Z TestFramework (ERROR):
```
This PR resolves this issue by explicitly specifying the Python major version and makes testing of self-compiled binaries more straightforward.
ACKs for top commit:
MarcoFalke:
lgtm ACK 6a7686b446
stickies-v:
utACK 6a7686b446
Tree-SHA512: 5681141e222bc833c6250cb79fe3a1c8e02255eb2c86010bc0f8239afcdfed784ed7788c8579209d931bd357f58d5655cf33ffeb2f46b1879f37cdc30e7a7c91
faafc35a77 doc: Clarify that -datacarriersize applies to the full raw scriptPubKey, not the data push (MarcoFalke)
55550e7fe7 test: Add -datacarriersize=2 tests (MarcoFalke)
Pull request description:
Clarify with a test that `-datacarriersize` applies to the serialized size of the scriptPubKey, not the size of the pushed data. So for example,
* `-datacarriersize=2` will reject a `raw(6a01aa)`, even though only one byte is pushed
* `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a)`, even though no byte is pushed
* `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a00)`, even though zero bytes are pushed
ACKs for top commit:
ajtowns:
ACK faafc35a77
instagibbs:
ACK faafc35a77
Tree-SHA512: f01ace02798f596ac2a02461e9f2a6ef91b3b37c976ea0b3bc860e2d3efb0ace0fd8b779dd18249cee7f84ebbe5fd21d8506afd3a15edadc00b843ff3b4aacc7
Using `py.exe` launcher might by fragile depending on how Python was
installed. Specifying the Python version explicitly fixes test errors
like this:
```
RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found...
```
-BEGIN VERIFY SCRIPT-
sed -i 's|"py "|"py -3 "|g' $(git grep -l '"py "' -- test/functional)
-END VERIFY SCRIPT-
Connection object used as context manager only commits or rollbacks
transactions, so the connection object should be closed manually.
Fixes the following error on Windows:
```
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: ...
```
ba8ab4fc54 test: cover addrv2 support in anchors.dat with a TorV3 address (Matthew Zipkin)
b4bee4bbf4 test: add keep_alive option to socks5 proxy in test_framework (Matthew Zipkin)
5aaf988ccc test: cover TorV3 address in p2p_addrv2_relay (Matthew Zipkin)
80f64a3d40 test: add support for all networks in CAddress in messages.py (brunoerg)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/27140
Adds test coverage for https://github.com/bitcoin/bitcoin/pull/20516 to ensure that https://github.com/bitcoin/bitcoin/issues/20511 is completed and may be closed.
This PR adds a test case to `feature_anchors.py` where an onion v3 address is set as a blocks-only relay peer and then shutdown, ensuring that the address is saved to anchors.dat in addrv2 format. We then ensure that bitcoin attempts to reconnect to that anchor address on restart.
To compute the addrv2 serialization of the onion v3 address, I added logic to `CAddress` in `messages.py`. This new logic is covered by extending `p2p_addrv2_relay.py` to include an onion v3 address. Future work will be adding coverage for ipv6, torv2 and cjdns in these modules and also `feature_proxy.py`
Also includes de/serialization unit test for `CAddress` in test framework.
ACKs for top commit:
jonatack:
ACK ba8ab4fc54
brunoerg:
crACK ba8ab4fc54
willcl-ark:
ACK ba8ab4fc54
Tree-SHA512: 7220e30d7cb975903d9ac575a7215a08e8f784c24c5741561affcbde12fb92cbf8704cb42e66494b788ba6ed4bb255fb0cc327e4f2190fae50c0ed9f336c0ff0
2c0c6f4477 test: dedup file hashing using `sha256sum_file` helper (Sebastian Falbesoner)
Pull request description:
Rather than doing the open/read/hash-steps manually in the affected functional tests, we can just use the `sha256sum_file` helper from the utils module instead.
Note that for the tool_wallet.py test, the used hash is changed from sha1 to sha256, but as the only purpose is to detect file content changes, this doesn't matter. Also, the optimization using `memoryview` is overkill here, as the opened file has only a size of 24KiB and determining the hash via the helper doesn't take longer than a few hundred micro-seconds on my machine.
ACKs for top commit:
kristapsk:
ACK 2c0c6f4477
Tree-SHA512: 64fe21650b56a50e9f1a95f6ef27d88d8bfbb621e5be456f327ef8dbb5596b529d03976c200f3fd68da48cc427de9f257b403f3228e38cf1df918006674fac68
8a20f765cc test: drop duplicate getaddrs from p2p_getaddr_caching (Martin Zumsande)
feb0096139 test: fix intermittent failure in p2p_getaddr_caching (Martin Zumsande)
Pull request description:
Fixes#28133
In the consistency check, it's not enough to check that our address/port is unique, only the combination of source and target must be unique. Otherwise, the OS may reuse ports for connections to different `-addrbind`, which was happening in the failed runs.
While at it, the second commit cleans up duplicate `getaddr` messages in `p2p_getaddr_caching.py` that do nothing but generate `Ignoring repeated "getaddr"` log messages (and cleans up some whitespace the python linter complains about).
ACKs for top commit:
vasild:
ACK 8a20f765cc
Tree-SHA512: eabe4727d7887f729074076f6333a918bba8cb34b8e3baaa83f167b441b0daa24f7c4824abcf03a9538a2ef14b2d826ff19aeffcb93a6c20735253a9678aac9c
90c8f79e94 test: remove redundant test values (Jon Atack)
c3f203387d test: use common assert_signing_completed_successfully helper (Jon Atack)
647d95aae9 test: add coverage for passing an invalid sighashtype (Jon Atack)
Pull request description:
Add test coverage for passing an invalid sighashtype to RPCs signrawtransactionwithwallet, signrawtransactionwithkey, walletprocesspsbt, and descriptorprocesspsbt.
ACKs for top commit:
MarcoFalke:
lgtm ACK 90c8f79e94🎥
brunoerg:
light crACK 90c8f79e94
Tree-SHA512: 3861658487edd0d9a377390acf3d43f45c3dd9e324894f0fdb8f5312b618301a55479b1f70c61daee0b20785e768ffde6fa5abe6af190b73c0d0e017f3976704
bee2d57a65 script: update flake8 to 6.1.0 (Jon Atack)
38c3fd846b test: python E721 updates (Jon Atack)
Pull request description:
Update our functional tests per [E721](https://www.flake8rules.com/rules/E721.html) enforced by [flake8 6.1.0](https://flake8.pycqa.org/en/latest/release-notes/6.1.0.html), and update our CI lint task to use that release. This makes the following linter output on current master with flake8 6.1.0 green.
```
$ ./test/lint/lint-python.py ; ./test/lint/lint-spelling.py
test/functional/p2p_invalid_locator.py:35:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
test/functional/test_framework/siphash.py:34:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
test/functional/test_framework/siphash.py:64:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
src/test/fuzz/descriptor_parse.cpp:88: occurences ==> occurrences
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
ACKs for top commit:
MarcoFalke:
lgtm ACK bee2d57a65
Tree-SHA512: f3788a543ca98e44eeeba1d06c32f1b11eec95d4aef068aa1b6b5c401261adfa3fb6c6d6c769f3fe6839d78e74a310d5c926867e7c367d6513a53d580fd376f3
fafe43cb6c scripted-diff: Use blocks_path where possible (MarcoFalke)
fa060c15fb test: Add blocks_path property to TestNode (MarcoFalke)
faba4fc325 test: Drop 22.x node from TxindexCompatibilityTest (MarcoFalke)
fa7f65b0f8 test: Use clean chain in MempoolCompatibilityTest (MarcoFalke)
Pull request description:
The node in this test was never really needed, because the compatibility tests shouldn't be used to test previous releases. (The test suite of the previous release itself should be used for that). So remove it.
Also, other test changes. (See individual commits)
ACKs for top commit:
theStack:
Code-review ACK fafe43cb6c
Tree-SHA512: 289f54695bf5310663ab38ebf1aa457f53d0aafae56e6657be0e75bf96b303165bad417dc7eaf4c40f0639aa92ce139e5bacb318a2eabab1f8e23a811cabe0cc
In `wallet_fundrawtransaction`, `totalOut` is used in
some functions to check if the change is correct. In
other ones, it has been created but never used.
c648bdbda2 test: create wallet specific for test_locked_wallet case (furszy)
Pull request description:
Coming from https://github.com/bitcoin/bitcoin/pull/28089#discussion_r1265478128.
Several test cases are relying on the node1 default wallet, which thanks to 'test_locked_wallet' is encrypted.
And can be only accessed within a specific timeframe (100ms), a duration internally set by the same test.
This situation introduces a potential race condition, where other tests must complete their operations within
the specified 100ms window to pass (otherwise the wallet gets re-locked and they fail).
This can be seen running the test in valgrind (https://github.com/bitcoin/bitcoin/pull/28089), where other test cases fail due the wallet re-locking
itself after the 100ms.
ACKs for top commit:
MarcoFalke:
lgtm ACK c648bdbda2
ishaanam:
utACK c648bdbda2
Tree-SHA512: 01cde5a4a0cb3405adb9ea3c1f73841f3fa237d1162268ed06f0d49ca38541006b423a029e0b5e5955e1aa7e018c4600d894e555a68cf17ff60a4b8be58f4aa9
d0c6cc4abe suppressions: note that 'type:ClassName::MethodName' should be used (fanquake)
Pull request description:
Now that the symbolizer is back in play, suppressions can once-again be targeted to functions, rather than file-wide.
ACKs for top commit:
MarcoFalke:
lgtm ACK d0c6cc4abe
hebasto:
ACK d0c6cc4abe
Tree-SHA512: fb65398eae18a6ebc5f8414275c568cf2664ab5357c2b3160f3bf285b67bc3af788225c5dba3c824c0e098627789450bec775375f52529d71c6ef700a9632d65
53c990ad34 test: fix `feature_addrman.py` on big-endian systems (Sebastian Falbesoner)
Pull request description:
The test `feature_addrman.py` currently serializes the addrdb without specifying endianness for `int`s, so the machine's native byte order is used (see https://docs.python.org/3/library/struct.html#byte-order-size-and-alignment) and the generated `peers.dat` would be invalid on big-endian systems (our internal (de)serializers always use little-endian, see `ser_{read,write}data32`). Fix this by explicitly specifying little-endian serialization via the `<` character in `struct.pack(...)`.
This is not detected by CI as we unfortunately don't run functional tests on big-endian systems there (I think we definitely should!).
ACKs for top commit:
MarcoFalke:
lgtm ACK 53c990ad34🔚
Tree-SHA512: 513af6f1f785a713e7a8ef3a57fcd3fe2520a7d537f63a9c8e1f4bdea4c2f605fd4c35001623d6b13458883dbc256f24943684ab8f224055c22bf8d8eeee5fe2
Only the combined addr:port of source and destination
must be unique. If the destination is different, the same addr:port
for the source may be used by the OS.
* The node was only used to migrate the legacy txindex. But now that it
is known to be working and that 22.x is EOL, it can be dropped.
* Also, fix a typo to properly check the txindex of node [1], not [2].
Other tests are also relying on the node1 default wallet,
which thanks to 'test_locked_wallet' is encrypted.
And can only be accessed within a specific timeframe (100ms)
set internally by the same test.
This make other tests susceptible to races. They can only
perform their operations successfully within the specified
time.
This can be seen running the test in valgrind, where other
test cases fail due the wallet re-locking itself after the
100ms.
e667bd68a1 test: fix intermittent failure in wallet_resendwallettransactions.py (Martin Zumsande)
Pull request description:
Fixes#28094
The test bumps the mocktime for ~2 weeks and then triggers eviction from the mempool. But this bump will also cause a new resubmit, and if the timing is such that this resubmit happens right after the eviction and before the check that the tx was evicted, the test can fail as in #28094:
```
node0 2023-07-17T21:31:23.809483Z (mocktime: 2023-08-02T09:46:27Z) [httpworker.1] [validation.cpp:267] [LimitMempoolSize] [mempool] Expired 2 transactions from the memory pool
node0 2023-07-17T21:31:23.810079Z (mocktime: 2023-08-02T09:46:27Z) [scheduler] [wallet/wallet.h:895] [WalletLogPrintf] [default wallet] ResubmitWalletTransactions: resubmit 2 unconfirmed transactions
node0 2023-07-17T21:31:23.810474Z (mocktime: 2023-08-02T09:46:27Z) [httpworker.2] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getmempoolentry user=__cookie__
2023-07-17T21:31:23.811000Z TestFramework (ERROR): Assertion failed (...) AssertionError: No exception raised
```
Fix this by flushing out the current resubmit call before triggering mempool eviction.
ACKs for top commit:
MarcoFalke:
Nice. lgtm ACK e667bd68a1
achow101:
ACK e667bd68a1
jonatack:
Light "this looks like the other tests in this file" ACK e667bd68a1
Tree-SHA512: 027c2177ecd8bea80ec388ec2564f8fcbc717efd2722304b16fc0e9fa7ad216af61977c4e360b8135de68586cf13b0aa729ffa4fa27bad655092c3a55f73933c
dd9633b516 test: wallet, add coverage for watch-only raw sh script migration (furszy)
cc781a2180 descriptor: InferScript, do not return top-level only func as sub descriptor (furszy)
286e0c7d5e wallet: loading, log descriptor parsing error details (furszy)
Pull request description:
Linked to #28057.
Currently, the `InferScript` function returns an invalid descriptor when it tries to infer a p2sh-p2pkh script whose pubkey is not known by the wallet.
This behavior occurs because the inference process bypasses the `pkh` subscript when the pubkey is not contained by the wallet (no pubkey provider), interpreting it as a `sh(addr(ADDR))` descriptor. Then, the failure arises because the `addr()` function is restricted to being used only at the top level.
For reviewers, would recommend to start by examining the functional test to understand the context and the circumstances on which this can result in a fatal error (e.g. during the migration process).
ACKs for top commit:
achow101:
ACK dd9633b516
darosior:
utACK dd9633b516
Tree-SHA512: 61e763206c604c372019d2c36e31684f3dddf81f8b154eb9aba5cd66d8d61bda457ed4e591613eb6ce6c76cf7c3f11764abc6cd727a7c2b6414f1065783be032
bbbb89d238 test: miner: add coverage for `-blockmintxfee` setting (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the `-blockmintxfee` option, which can be used by miners to specify the lowest fee-rate for transactions to be included in blocks. The setting was introduced in PR #9380 (commit daec955fd6), with the rationale to decouple different minimum fees from `-minrelaytxfee`. According to the PR description it _"should be set by miners to reflect their marginal cost of transmitting extra bytes."_.
On each iteration, the test creates and submits two txs using MiniWallet: one with the the minimum fee-rate as specified for `-blockmintxfee` and a second one with a fee-rate a little below that (-0.01 sats/vbyte). Then it checks that only the first one is picked for the block template and accordingly also only exists in the block that is mined after. This is repeatedly done for a fixed (but obviously somewhat arbitrary) list of different `-blockmintxfee` settings on a single node, including the default and zero (i.e. no minimum fee a.k.a. "include even zero-fee txs") settings.
ACKs for top commit:
ryanofsky:
Code review ACK bbbb89d238, nice test
brunoerg:
reACK bbbb89d238
glozow:
ACK bbbb89d238, sorry for the late re-review!
Tree-SHA512: 7b72612971e6a1667b4b3913ec27109953fd17a1020a4bde6941a93666b2e10a23fb6fe7df23471a5671ffe31e42cd992d2efb8b31903915b3dfc1d6478df757
e.g. sh(addr(ADDR)) or sh(raw(HEX)) are invalid descriptors.
Making sh and wsh top level functions to return addr/raw descriptors when
the subscript inference fails.
fa6245da60 fuzz: Generate process_message targets individually (MarcoFalke)
fa1471e575 refactor: Remove duplicate allNetMessageTypesVec (MarcoFalke)
Pull request description:
Now that `LIMIT_TO_MESSAGE_TYPE` is a runtime setting after commit 927b001502, it shouldn't hurt to also generate each message type individually. Something similar was done for the `rpc` target in commit cf4da5ec29.
ACKs for top commit:
stickies-v:
re-crACK fa6245da60
brunoerg:
reACK fa6245da60
Tree-SHA512: 8f3ec71bab89781f10820a0e027fcde8949f3333eb19a30315aaad6f90f5167028113cea255b2d60b700da817c7eaac20b7b4c92f931052d7f5c2f148d33aa5a
e8c31f135c tests: Test for bumping single output transaction (Andrew Chow)
4f4d4407e3 test: Test bumpfee reduce_output (Andrew Chow)
7d83502d3d bumpfee: Allow original change position to be specified (Andrew Chow)
Pull request description:
When bumping the transaction fee, we will try to find the change output of the transaction in order to have an output whose value we can modify so that we can meet the target feerate. However we do not always find the change output which can cause us to unnecessarily add an additional output to the transaction. We can avoid this issue by outsourcing the determination of change to the user if they so desire.
This PR adds a `orig_change_pos` option to bumpfee which the user can use to specify the index of the change output.
Fixes#11233Fixes#20795
ACKs for top commit:
ismaelsadeeq:
re ACK e8c31f135c
pinheadmz:
re-ACK e8c31f135c
furszy:
Code review ACK e8c31f13
Tree-SHA512: 3a230655934af17f7c1a5953fafb5ef0d687c21355cf284d5e98fece411f589cd69ea505f06d6bdcf82836b08d268c366ad2dd30ae3d71541c9cdf94d1f698ee
This fixes a bug in the linter:
"""
Python's open(...) seems to be used to open text files without explicitly specifying encoding='utf8':
test/functional/test_framework/test_node.py: with open(self.debug_log_path, **kwargs) as dl:
"""
read() fails in text mode when the unicode hasn't been fully written
yet. Fixes: "wallet_importdescriptors.py: can't decode bytes in position
228861-228863: unexpected end of data"
(https://github.com/bitcoin/bitcoin/issues/28030)
