| Age | Commit message (Collapse) | Author |
|---|
|
1c976c691cc4b20f43071aabf36c7afed1571057 tidy: Integrate bicoin-tidy clang-tidy plugin (fanquake)
7de23cceb8ac13fcc709453ef0fa14fb93c460b0 refactor: fix unterminated LogPrintf()s (fanquake)
0a1029aa2946787bfe91a84de882c2dd0de076e9 lint: remove /* Continued */ markers from codebase (fanquake)
910007995d8603ffc466878856227153a638caff lint: remove lint-logs.py (fanquake)
d86a83d6b8587b0971e66c6910af23dd8c042969 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 1c976c691cc4b20f43071aabf36c7afed1571057
theuni:
ACK 1c976c691cc4b20f43071aabf36c7afed1571057
MarcoFalke:
re-ACK 1c976c691cc4b20f43071aabf36c7afed1571057 ðŸ‘
Tree-SHA512: 725b45c70e431d48e6f276671e05c694e10b6047cae1a31906ac3ee9093bc8105fb226b36a5bac6709557526ca6007222112d66aecec05a574434edc4897e4b8
|
|
explicitly on Windows
6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91 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 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91
stickies-v:
utACK 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91
Tree-SHA512: 5681141e222bc833c6250cb79fe3a1c8e02255eb2c86010bc0f8239afcdfed784ed7788c8579209d931bd357f58d5655cf33ffeb2f46b1879f37cdc30e7a7c91
|
|
Enable `bitcoin-unterminated-logprintf`.
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
|
|
|
|
-datacarriersize=2 tests
faafc35a779745d59fdb0e88698b579215f42b08 doc: Clarify that -datacarriersize applies to the full raw scriptPubKey, not the data push (MarcoFalke)
55550e7fe7e4ffe14637a901b568d1d7e1c716d4 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 faafc35a779745d59fdb0e88698b579215f42b08
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/27832/commits/faafc35a779745d59fdb0e88698b579215f42b08
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: ...
```
|
|
CAddress in messages.py
ba8ab4fc545800c4fb283a5ff0b19138a0451aba test: cover addrv2 support in anchors.dat with a TorV3 address (Matthew Zipkin)
b4bee4bbf45785fbbb355194ccb23c70acd19d27 test: add keep_alive option to socks5 proxy in test_framework (Matthew Zipkin)
5aaf988ccca210228c5a41ea0a18b0c85a85cf71 test: cover TorV3 address in p2p_addrv2_relay (Matthew Zipkin)
80f64a3d40779d342b740fbc57474e170b102678 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 ba8ab4fc545800c4fb283a5ff0b19138a0451aba
brunoerg:
crACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba
willcl-ark:
ACK ba8ab4fc54
Tree-SHA512: 7220e30d7cb975903d9ac575a7215a08e8f784c24c5741561affcbde12fb92cbf8704cb42e66494b788ba6ed4bb255fb0cc327e4f2190fae50c0ed9f336c0ff0
|
|
helper
2c0c6f44770403899bd8514ad7343356853bf38c 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 2c0c6f44770403899bd8514ad7343356853bf38c
Tree-SHA512: 64fe21650b56a50e9f1a95f6ef27d88d8bfbb621e5be456f327ef8dbb5596b529d03976c200f3fd68da48cc427de9f257b403f3228e38cf1df918006674fac68
|
|
p2p_getaddr_caching.py
8a20f765cce2fc0fadf1a2b66b843b67f2d2ae12 test: drop duplicate getaddrs from p2p_getaddr_caching (Martin Zumsande)
feb0096139e9e864632d2826d2e213b26146fff1 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 8a20f765cce2fc0fadf1a2b66b843b67f2d2ae12
Tree-SHA512: eabe4727d7887f729074076f6333a918bba8cb34b8e3baaa83f167b441b0daa24f7c4824abcf03a9538a2ef14b2d826ff19aeffcb93a6c20735253a9678aac9c
|
|
90c8f79e945863f3818748b86572948d1558aec3 test: remove redundant test values (Jon Atack)
c3f203387df60c596a1e416658d87d68b85adbf2 test: use common assert_signing_completed_successfully helper (Jon Atack)
647d95aae9720543c2c9c46c70191e6f9f775d66 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 90c8f79e945863f3818748b86572948d1558aec3 🎥
brunoerg:
light crACK 90c8f79e945863f3818748b86572948d1558aec3
Tree-SHA512: 3861658487edd0d9a377390acf3d43f45c3dd9e324894f0fdb8f5312b618301a55479b1f70c61daee0b20785e768ffde6fa5abe6af190b73c0d0e017f3976704
|
|
bee2d57a655645dbfaf0242e85c5af034023a2fb script: update flake8 to 6.1.0 (Jon Atack)
38c3fd846bff163eb7c50bd77efcdcf8fcbc7f43 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 bee2d57a655645dbfaf0242e85c5af034023a2fb
Tree-SHA512: f3788a543ca98e44eeeba1d06c32f1b11eec95d4aef068aa1b6b5c401261adfa3fb6c6d6c769f3fe6839d78e74a310d5c926867e7c367d6513a53d580fd376f3
|
|
suppressions for `src/secp256k1` subtree
a7477744c5e1df56d3a1e9ab9fc400bfb0ef6ec3 Add UBSan `-fsanitize=integer` suppressions for `src/secp256k1` subtree (Hennadii Stepanov)
Pull request description:
Required for https://github.com/bitcoin/bitcoin/pull/27991 (see the [comment](https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1611472816)) and for the upcoming CMake-based build system.
ACKs for top commit:
MarcoFalke:
lgtm ACK a7477744c5e1df56d3a1e9ab9fc400bfb0ef6ec3
Tree-SHA512: 602fa3ad22d3b0f6981a51358677d2347c92c4c9f59626b497af10f7ba828ede37227d8ee717f089bf33bde5efe0854d53acc89bea46f0955e62b7f22c454d05
|
|
fafe43cb6c76a5f60194be128a40baf161d39920 scripted-diff: Use blocks_path where possible (MarcoFalke)
fa060c15fb5081e66ed1ebe05dca6e8026f32c4f test: Add blocks_path property to TestNode (MarcoFalke)
faba4fc3257c0e7d7bcb5146dab07ffe3193744b test: Drop 22.x node from TxindexCompatibilityTest (MarcoFalke)
fa7f65b0f81f3a2899c8eb540182c7d9a90fff00 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 fafe43cb6c76a5f60194be128a40baf161d39920
Tree-SHA512: 289f54695bf5310663ab38ebf1aa457f53d0aafae56e6657be0e75bf96b303165bad417dc7eaf4c40f0639aa92ce139e5bacb318a2eabab1f8e23a811cabe0cc
|
|
|
|
This change is a follow-up for:
- https://github.com/bitcoin/bitcoin/pull/9956
- https://github.com/bitcoin/bitcoin/pull/10096
|
|
as they are parsed identically.
See AmountFromValue() / ParseFixedPoint() / UniValue#getValStr()
|
|
|
|
in RPCs descriptorprocesspbst, walletprocesspbst, signrawtransactionwithkey,
and signrawtransactionwithwallet.
|
|
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.
|
|
In `wallet_fundrawtransaction`, there are some unecessary
variables/calls to `decoderawtransaction`. They have not
been used.
|
|
test_locked_wallet case
c648bdbda21c7ae90c6b40e506ca4ed62b1dbb6c 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 c648bdbda21c7ae90c6b40e506ca4ed62b1dbb6c
ishaanam:
utACK c648bdbda21c7ae90c6b40e506ca4ed62b1dbb6c
Tree-SHA512: 01cde5a4a0cb3405adb9ea3c1f73841f3fa237d1162268ed06f0d49ca38541006b423a029e0b5e5955e1aa7e018c4600d894e555a68cf17ff60a4b8be58f4aa9
|
|
`type:ClassName::MethodName` should be used
d0c6cc4abe42163aaf081a969d2c449785563ba2 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 d0c6cc4abe42163aaf081a969d2c449785563ba2
hebasto:
ACK d0c6cc4abe42163aaf081a969d2c449785563ba2
Tree-SHA512: fb65398eae18a6ebc5f8414275c568cf2664ab5357c2b3160f3bf285b67bc3af788225c5dba3c824c0e098627789450bec775375f52529d71c6ef700a9632d65
|
|
systems
53c990ad3406ee945305af84af98d2f020e5f316 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 53c990ad3406ee945305af84af98d2f020e5f316 🔚
Tree-SHA512: 513af6f1f785a713e7a8ef3a57fcd3fe2520a7d537f63a9c8e1f4bdea4c2f605fd4c35001623d6b13458883dbc256f24943684ab8f224055c22bf8d8eeee5fe2
|
|
fa3d72960bc86319aa8b838e3df4e833f845c71f lint: Ignore check_fileopens failure on **kwargs (MarcoFalke)
fa6bb85cd264adf2e1f0c71fa97b650f9a62745f test: Ignore UTF-8 errors in assert_debug_log (MarcoFalke)
fa63326fbc1634ef2816f79c54db9266e56f4b3b test: Fix debug_log_size helper (MarcoFalke)
Pull request description:
Fix two bugs, see commit messages.
ACKs for top commit:
theStack:
utACK fa3d72960bc86319aa8b838e3df4e833f845c71f
Tree-SHA512: 4a29bdf954bf62bb7676c2a41b03ad017bc86d535b2bd912c96bd41d1621beb06d840b53c211480ad51974e8b293bbae620060d2528d269159f32c0b44e47712
|
|
python p2p instances will automatically send a getaddr msg after
connecting, the explicit message was a duplicate that was being ignored.
|
|
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.
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's|].chain_path / "blocks"|].blocks_path|g' $(git grep -l chain_path)
-END VERIFY SCRIPT-
|
|
|
|
* 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].
|
|
The test creates enough blocks itself, so there is no need to have more.
Also, remove os import.
|
|
Now that the symbolizer is back in play, suppressions can once-again be
targeted to functions, rather than file-wide.
|
|
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.
|
|
|
|
|
|
wallet_resendwallettransactions.py
e667bd68a10512ddc784df44bdcb63ee441e5551 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 e667bd68a10512ddc784df44bdcb63ee441e5551
achow101:
ACK e667bd68a10512ddc784df44bdcb63ee441e5551
jonatack:
Light "this looks like the other tests in this file" ACK e667bd68a10512ddc784df44bdcb63ee441e5551
Tree-SHA512: 027c2177ecd8bea80ec388ec2564f8fcbc717efd2722304b16fc0e9fa7ad216af61977c4e360b8135de68586cf13b0aa729ffa4fa27bad655092c3a55f73933c
|
|
as sub descriptors
dd9633b516d6936ac4e23a40f9b0bea120117d35 test: wallet, add coverage for watch-only raw sh script migration (furszy)
cc781a21800a6ce13875feefd0cb14ab0a84524c descriptor: InferScript, do not return top-level only func as sub descriptor (furszy)
286e0c7d5e9538198b28b792c5168b8fafa1534f 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 dd9633b516d6936ac4e23a40f9b0bea120117d35
darosior:
utACK dd9633b516d6936ac4e23a40f9b0bea120117d35
Tree-SHA512: 61e763206c604c372019d2c36e31684f3dddf81f8b154eb9aba5cd66d8d61bda457ed4e591613eb6ce6c76cf7c3f11764abc6cd727a7c2b6414f1065783be032
|
|
setting
bbbb89d238e9bdaa9f426d55b0a3b714dac1d39b 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 daec955fd68bd0da036a5b446b54ffb01108adcd), 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 bbbb89d238e9bdaa9f426d55b0a3b714dac1d39b, nice test
brunoerg:
reACK bbbb89d238e9bdaa9f426d55b0a3b714dac1d39b
glozow:
ACK bbbb89d238e9bdaa9f426d55b0a3b714dac1d39b, 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.
|
|
fa6245da6061050eb77ad07cd4caf8c596d89dc6 fuzz: Generate process_message targets individually (MarcoFalke)
fa1471e5754484f997ddf9db70888679dcd1d64a refactor: Remove duplicate allNetMessageTypesVec (MarcoFalke)
Pull request description:
Now that `LIMIT_TO_MESSAGE_TYPE` is a runtime setting after commit 927b001502a74a7224f04cfe6ffddc9a59409ba1, it shouldn't hurt to also generate each message type individually. Something similar was done for the `rpc` target in commit cf4da5ec29f9e8cd6cc6577e5ecbd87174edba62.
ACKs for top commit:
stickies-v:
re-crACK fa6245da6061050eb77ad07cd4caf8c596d89dc6
brunoerg:
reACK fa6245da6061050eb77ad07cd4caf8c596d89dc6
Tree-SHA512: 8f3ec71bab89781f10820a0e027fcde8949f3333eb19a30315aaad6f90f5167028113cea255b2d60b700da817c7eaac20b7b4c92f931052d7f5c2f148d33aa5a
|
|
is change
e8c31f135c6e9a5f57325dbf4feceafd384f7762 tests: Test for bumping single output transaction (Andrew Chow)
4f4d4407e3d2cc5ac784524c0cb0602837dc7860 test: Test bumpfee reduce_output (Andrew Chow)
7d83502d3d52218e7b0b0634cff2a9aba9cc77ef 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 #11233
Fixes #20795
ACKs for top commit:
ismaelsadeeq:
re ACK e8c31f135c6e9a5f57325dbf4feceafd384f7762
pinheadmz:
re-ACK e8c31f135c6e9a5f57325dbf4feceafd384f7762
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)
|
|
debug_log_bytes returned "an opaque number when in text mode"
(https://docs.python.org/3/tutorial/inputoutput.html#methods-of-file-objects),
not the number of bytes.
Fix this by using binary mode or text mode (with the same encoding)
consistently when opening the file for ftell() and read().
|
|
Before, it was possible that a resend was triggered right between
eviction the txns from the mempool and the check that they were evicted.
|
|
|
|
20b49460b35268db59f7efcb02736b0e31191a74 test: remove race in the user-agent reception check (Vasil Dimov)
Pull request description:
In `add_p2p_connection()` we connect to `bitcoind` from the Python client and check that it has received our version string.
This check looked up the last/newest entry from `getpeerinfo` RPC, assuming that it must be the connection we have just opened. But this will not be the case if a new inbound or outbound connection is made to/from `bitcoind` in the meantime.
Instead of the last entry in `getpeerinfo`, check all and find the one which corresponds to our connection using our outgoing address:port tuple which is unique.
ACKs for top commit:
jonatack:
re-ACK 20b49460b35268db59f7efcb02736b0e31191a74
MarcoFalke:
Concept ACK 20b49460b35268db59f7efcb02736b0e31191a74
Tree-SHA512: 61fd3359ef11ea830021ede0e745497a7b60690c32d21c47cd12ff79f78907bb45e922c9d01e5d7ff614a8cd5e4560d39a3fc86b01b200429773a23ace3917e4
|
|
fa2f18ad8e386f067c2d7d5362f6b4dd84c295a0 ci: Use DOCKER_BUILDKIT for lint image (MarcoFalke)
Pull request description:
Currently the lint docker/podman image has many issues:
* It relies on an EOL debian version.
* It relies on a debian version different from the one used in the CI lint task.
* It relies on the legacy docker build command, which requires the user to make `cd ./ci/lint/` before the build step.
* It doesn't use the `.python-version` file, but a hardcoded version.
Fix all issues by using the recommended `DOCKER_BUILDKIT=1` to generate the image.
Also:
* Rename `/tmp/python` to `/python_build`.
* Compress all `pip install` commands into one.
* Bump `.python-version`.
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/28083/commits/fa2f18ad8e386f067c2d7d5362f6b4dd84c295a0
Tree-SHA512: 804b384904ad753845667998841cc7825f4229933ca2c42af021384713486ec3cca80ba58612d37557fba7ee1921439dacca5e1236aac0557dd75bd9a2f1875d
|
|
descriptors
c7db88af71b3204171f33399aa4f33b40a4f7cd9 descriptor: assert we never parse a sane miniscript with no pubkey (Antoine Poinsot)
a49402a9ec7431c286139b76f8759719a99a8551 qa: make sure we don't let unspendable Miniscript descriptors be imported (Antoine Poinsot)
639e3b6c9759a7a582c5c86fdbfa5ea99cb7bb16 descriptor: refuse to parse unspendable miniscript descriptors (Antoine Poinsot)
e3280eae1b53006d74d11f3cf9d7a9dc7ff2c39e miniscript: make GetStackSize() and GetOps() return optionals (Antoine Poinsot)
Pull request description:
`IsSane()` in Miniscript does not ensure a Script is actually spendable. This is an issue as we would accept any sane Miniscript when parsing a descriptor. Fix this by explicitly checking a Miniscript descriptor is both sane and spendable when parsing it.
This bug was exposed due to a check added in #22838 (https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1226859880) that triggered a fuzz crash (https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057).
ACKs for top commit:
sipa:
utACK c7db88af71b3204171f33399aa4f33b40a4f7cd9
achow101:
ACK c7db88af71b3204171f33399aa4f33b40a4f7cd9
Tree-SHA512: e79bc9f7842e98a4e8f358f05811fca51b15b4b80a171c0d2b17cf4bb1f578a18e4397bc2ece9817d392e0de0196ee6a054b7318441fd3566dd22e1f03eb64a5
|
