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.
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
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
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
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.
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
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
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
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#11233Fixes#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)
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 fa2f18ad8e
Tree-SHA512: 804b384904ad753845667998841cc7825f4229933ca2c42af021384713486ec3cca80ba58612d37557fba7ee1921439dacca5e1236aac0557dd75bd9a2f1875d
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
faf8be7c32be00f660eba90d3f07313fb25d5d1c test: Disable known broken USDT test (MarcoFalke)
Pull request description:
The failure is known and running into more failures doesn't help anyone. Not disabling the test would be a waste of CPU and developer time.
https://github.com/bitcoin/bitcoin/issues/27380
Top commit has no ACKs.
Tree-SHA512: d0469153b00d6b30e10a21bcd52d508fcf9f796ff2468f59aff75020a82c718bcae85caf4b58397dea6fd9e210b501353fd51567f979c6b57d3b1bb23d318216
5cf44275c8ca8c32d238f37f717d78e9823f44c2 test: refactor: deduplicate legacy ECDSA signing for tx inputs (Sebastian Falbesoner)
Pull request description:
There are several instances in functional tests and the framework (MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy ECDSA signature for a certain transaction's input by doing the following steps:
1. calculate the `LegacySignatureHash` with the desired sighash type
2. create the actual digital signature by calling `ECKey.sign_ecdsa` on the signature message hash calculated above
3. put the DER-encoded result as CScript data push into tx input's scriptSig
Create a new helper `sign_input_legacy` which hides those details and takes only the necessary parameters (tx, input index, relevant scriptPubKey, private key, sighash type [SIGHASH_ALL by default]). For further convenience, the signature is prepended to already existing data-pushes in scriptSig, in order to avoid rehashing the transaction after calling the new signing function.
ACKs for top commit:
dimitaracev:
ACK `5cf4427`
achow101:
ACK 5cf44275c8ca8c32d238f37f717d78e9823f44c2
pinheadmz:
ACK 5cf44275c8ca8c32d238f37f717d78e9823f44c2
Tree-SHA512: 8f0e4fb2c3e0f84fac5dbc4dda87973276242b0f628034272a7f3e45434c1e17dd1b26a37edfb302dcaf380dbfe98b0417391ace5e0ac9720155d8fba702031e
faf902858d38150caa8991b0ab9d7cfee2905684 test: Check expected_stderr after stop (MarcoFalke)
Pull request description:
This fixes a bug where stderr wasn't checked for the shutdown sequence.
Fix that by waiting for the shutdown to finish and then check stderr.
ACKs for top commit:
theStack:
ACK faf902858d38150caa8991b0ab9d7cfee2905684
Tree-SHA512: a70cd1e6cda84d542782e41e8b59741dbcd472c0d0575bcef5cbfd1418473ce94efe921481d557bae3fbbdd78f1c49c09c48872883c052d87c5c9a9a51492692
By moving the 'StartIndexes()' call into the 'initload'
thread, we can remove the threads active wait. Optimizing
the available resources.
The only difference with the current state is that now the
indexes threads will only be started when they can process
work and not before it.
The thread does not only load blocks, it loads the mempool and,
in a future commit, will start the indexes as well.
Also, renamed the 'ThreadImport' function to 'ImportBlocks'
And the 'm_load_block' class member to 'm_thread_load'.
-BEGIN VERIFY SCRIPT-
sed -i "s/ThreadImport/ImportBlocks/g" $(git grep -l ThreadImport -- ':!/doc/')
sed -i "s/loadblk/initload/g" $(git grep -l loadblk -- ':!/doc/release-notes/')
sed -i "s/m_load_block/m_thread_load/g" $(git grep -l m_load_block)
-END VERIFY SCRIPT-
7ecc29a0b7a23d8f5d3c1e6a0dad29b3ad839eb9 test: wallet, add coverage for addressbook migration (furszy)
a277f8357ad8b0eb26f33fc36f919d868c06847b wallet: migration bugfix, persist empty labels (furszy)
1b64f6498c394a143df196172a14204fe3b8a744 wallet: migration bugfix, clone 'send' record label to all wallets (furszy)
Pull request description:
Addressing two specific bugs encountered during the wallet migration process, related to the address book, and improves the test coverage for it.
Bug 1: Non-Cloning of External 'Send' Records
The external 'send' records were not being correctly cloned to all wallets.
Bug 2: Persistence of Empty Labels
As address book entries without associated db label records can be treated as change (the `label` field inside the `CAddressBookData` class is optional, `nullopt` labels make `CAddressBookData ::IsChange()` return true), we must persist empty labels during the migration process.
The user might have called `setlabel` with an "" string for an external address and that must be retained during migration.
ACKs for top commit:
achow101:
ACK 7ecc29a0b7a23d8f5d3c1e6a0dad29b3ad839eb9
Tree-SHA512: b8a8483a4178a37c49af11eb7ba8a82ca95e54a6cd799e155e33f9fbe7f37b259e28372c77d6944d46b6765f9eaca6b8ca8d1cdd9d223120a3653e4e41d0b6b7
fa1e27fe8ec42764d0250c82a83d774c15798c4a fuzz: Generate rpc fuzz targets individually (MarcoFalke)
Pull request description:
The `rpc` fuzz target was added more than two years ago in e45863166f5e44cc2c380f4667812fcd3cddc73b. However, the bug https://github.com/bitcoin/bitcoin/issues/27913 was only found recently. Thus, it is pretty clear that fuzz engines can't deal with a search space that is too broad and can be extended in too many directions.
Fix that by limiting the search space to each RPC method name and then iterate over all names, instead of letting the fuzz engine do the iteration.
With this, the bug can be found in seconds, as opposed to years of CPU time (or never).
ACKs for top commit:
brunoerg:
ACK fa1e27fe8ec42764d0250c82a83d774c15798c4a
dergoegge:
ACK fa1e27fe8ec42764d0250c82a83d774c15798c4a
Tree-SHA512: 45ccba842367650d010320603153276b1b303deda9ba8c6bb31a4d2473b00aa5bca866db95f541485d65efd8276e2575026968c037872ef344fa33cf45bcdcd7
6eb33bd0c21b3e075fbab596351cacafdc947472 kernel: Add fatalError method to notifications (TheCharlatan)
7320db96f8d2aeff0bc5dc67d8b7b37f5f808990 kernel: Add flushError method to notifications (TheCharlatan)
3fa9094b92c5d37f486b0f8265062d3456796a50 scripted-diff: Rename FatalError to FatalErrorf (TheCharlatan)
edb55e2777063dfeba0a52bbd0b92af8b4688501 kernel: Pass interrupt reference to chainman (TheCharlatan)
e2d680a32d757de0ef8eb836047a0daa1d82e3c4 util: Add SignalInterrupt class and use in shutdown.cpp (TheCharlatan)
Pull request description:
Get rid of all `ShutdownRequested` calls in validation code by introducing an interrupt object that applications can use to cancel long-running kernel operations.
Replace all `AbortNode` calls in validation code with new fatal error and flush error notifications so kernel applications can be notified about failures and choose how to handle them.
---
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587https://github.com/orgs/bitcoin/projects/3 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel".
The pull request mostly allows dropping the kernel dependency on shutdown.cpp. The only dependency left after this is a `StartShutdown` call which will be removed in followup PR https://github.com/bitcoin/bitcoin/pull/27711. This PR also drops the last reference to the `uiInterface` global in kernel code. The process of moving the `uiInterface` out of the kernel was started in https://github.com/bitcoin/bitcoin/pull/27636.
This pull request contains a subset of patches originally proposed in #27711. It will be part of a series of changes required to make handling of interrupts (or in other words the current shutdown procedure) in the kernel library more transparent and less reliable on global mutable state. The set of patches contained here was originally proposed by @ryanofsky [here](https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1580779869).
ACKs for top commit:
achow101:
light ACK 6eb33bd0c21b3e075fbab596351cacafdc947472
hebasto:
ACK 6eb33bd0c21b3e075fbab596351cacafdc947472, I have reviewed the code and it looks OK.
ryanofsky:
Code review ACK 6eb33bd0c21b3e075fbab596351cacafdc947472. No changes since last review other than rebase.
Tree-SHA512: 7d2d05fa4805428a09466d43c11ae32946cbb25aa5e741b1eec9cd142e4de4bb311e13ebf1bb125ae490c9d08274f2d56c93314e10f3d69e7fec7445e504987c
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.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Co-authored-by: Jon Atack <jon@atack.com>
8fbb6e99bfc85a1b9003cae402a7335843a86abd wallet: Give deprecation warning when loading a legacy wallet (Andrew Chow)
Pull request description:
Next step in legacy wallet deprecation.
ACKs for top commit:
S3RK:
reACK 8fbb6e99bfc85a1b9003cae402a7335843a86abd
jonatack:
re-ACK 8fbb6e99bfc85a1b9003cae402a7335843a86abd
Tree-SHA512: 902984b09452926cf199f06e5fb56e4985325cdd5e0dcc829992158488f42d5fbc33e9a30a29303feac24c8315193e8d31712022e2a0503abd6b67169a0027f4
There are several instances in functional tests and the framework
(MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy
ECDSA signature for a certain transaction's input by doing the following
steps:
1) calculate the `LegacySignatureHash` with the desired sighash type
2) create the actual digital signature by calling `ECKey.sign_ecdsa`
on the signature message hash calculated above
3) put the DER-encoded result as CScript data push into
tx input's scriptSig
Create a new helper `sign_input_legacy` which hides those details and
takes only the necessary parameters (tx, input index, relevant
scriptPubKey, private key, sighash type [SIGHASH_ALL by default]). For
further convenience, the signature is prepended to already existing
data-pushes in scriptSig, in order to avoid rehashing the transaction
after calling the new signing function.
4f4d039a98370a91e3cd5977352a9a4b260aa06b test: add ellswift test vectors from BIP324 (stratospher)
a31287718aebad847b232eff59adc16c166c99e8 test: Add ellswift unit tests (stratospher)
714fb2c02ab4bfdd8a5a4f420036ece217c8b474 test: Add python ellswift implementation to test framework (stratospher)
Pull request description:
Built on top of https://github.com/bitcoin/bitcoin/pull/26222.
This PR introduces Elligator swift encoding and decoding in the functional test framework. It's used in #24748 for writing p2p encryption tests.
ACKs for top commit:
sipa:
ACK 4f4d039a98370a91e3cd5977352a9a4b260aa06b
theStack:
ACK 4f4d039a98370a91e3cd5977352a9a4b260aa06b 🐊
Tree-SHA512: 32bc8e88f715f2cd67dc04cd38db92680872072cb3775478e2c30da89aa2da2742992779ea14da2f1faca09228942cfbd86d6957402b24bf560244b389e03540
6c97757a480b6e71a0750330d69ff18ac7cc6127 script: appease spelling linter (Jon Atack)
1316119ce7ba3de4135bbf1e5ac28c9ea26f62e1 script: update ignored-words.txt (Jon Atack)
146c861da2e4236997bee3eed6110a5016a8b86b script: update linter dependencies (Jon Atack)
92408224a4cb2f454465d5ff8445c247f2c4318a test: fix PEP484 no implicit optional argument types errors (Jon Atack)
f86a3014338de6a2204bbdda10795b75ef6654c0 script, test: add missing python type annotations (Jon Atack)
Pull request description:
With these updates, `./test/lint/lint-python.py` and `./test/lint/lint-spelling.py` should be green again for developers using relatively recent Python dependencies, in particular mypy 0.991 (released 11/2022) and later. Please see the commit messages for details.
ACKs for top commit:
fanquake:
ACK 6c97757a480b6e71a0750330d69ff18ac7cc6127
Tree-SHA512: 8a46a4d36d5978affdcecf4f2ace20ca1b52d483e098304911a2169afe60ccb9b042fa90c04b762d94f3ce53d2cafe6f24476ae839867a770c7f31e7e7242d99
Fix warnings for these files when ./test/lint/lint-python.py is run using
mypy 0.991 (released 11/2022) and later:
$ test/lint/lint-python.py
test/functional/test_framework/coverage.py:23: error: Incompatible default for argument "coverage_logfile" (default has type "None", argument has type "str") [assignment]
test/functional/test_framework/coverage.py:23: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
test/functional/test_framework/util.py:318: error: Incompatible default for argument "timeout" (default has type "None", argument has type "int") [assignment]
test/functional/test_framework/util.py:318: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
test/functional/test_framework/util.py:318: error: Incompatible default for argument "coveragedir" (default has type "None", argument has type "str") [assignment]
test/functional/interface_rest.py:67: error: Incompatible default for argument "query_params" (default has type "None", argument has type "dict[str, Any]") [assignment]
test/functional/interface_rest.py:67: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
Verified using https://github.com/hauntsaninja/no_implicit_optional
For details, see:
https://mypy-lang.blogspot.com/2022/11/mypy-0990-released.html
Fix warnings for these files when ./test/lint/lint-python.py is run using
mypy 0.991 (released 11/2022) and later:
"By default the bodies of untyped functions are not checked, consider using
--check-untyped-defs [annotation-unchecked]"
For details, see:
https://mypy-lang.blogspot.com/2022/11/mypy-0990-released.html