Commit graph

5604 commits

Author SHA1 Message Date
fanquake
54fe963a53
Merge bitcoin/bitcoin#28035: test: Ignore UTF-8 errors in assert_debug_log
fa3d72960b lint: Ignore check_fileopens failure on **kwargs (MarcoFalke)
fa6bb85cd2 test: Ignore UTF-8 errors in assert_debug_log (MarcoFalke)
fa63326fbc test: Fix debug_log_size helper (MarcoFalke)

Pull request description:

  Fix two bugs, see commit messages.

ACKs for top commit:
  theStack:
    utACK fa3d72960b

Tree-SHA512: 4a29bdf954bf62bb7676c2a41b03ad017bc86d535b2bd912c96bd41d1621beb06d840b53c211480ad51974e8b293bbae620060d2528d269159f32c0b44e47712
2023-07-26 09:35:51 +01:00
MarcoFalke
faa8c1be26
fuzz: Re-enable symbolize=1 in ASAN_OPTIONS 2023-07-22 08:26:34 +02:00
Andrew Chow
b3022af0e2
Merge bitcoin/bitcoin#28108: test: fix intermittent failure in wallet_resendwallettransactions.py
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
2023-07-20 11:39:24 -04:00
Andrew Chow
7edce77ff3
Merge bitcoin/bitcoin#28067: descriptors: do not return top-level only funcs as sub descriptors
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
2023-07-20 11:16:45 -04:00
fanquake
79954903b2
Merge bitcoin/bitcoin#27620: test: miner: add coverage for -blockmintxfee setting
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
2023-07-20 15:33:54 +01:00
furszy
dd9633b516
test: wallet, add coverage for watch-only raw sh script migration 2023-07-20 11:04:52 -03:00
furszy
cc781a2180
descriptor: InferScript, do not return top-level only func as sub descriptor
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.
2023-07-20 11:04:52 -03:00
fanquake
355bbcba01
Merge bitcoin/bitcoin#28066: fuzz: Generate process_message targets individually
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
2023-07-20 10:17:08 +01:00
fanquake
04afe55e29
Merge bitcoin/bitcoin#26467: bumpfee: Allow the user to choose which output is change
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 #11233
  Fixes #20795

ACKs for top commit:
  ismaelsadeeq:
    re ACK e8c31f135c
  pinheadmz:
    re-ACK e8c31f135c
  furszy:
    Code review ACK e8c31f13

Tree-SHA512: 3a230655934af17f7c1a5953fafb5ef0d687c21355cf284d5e98fece411f589cd69ea505f06d6bdcf82836b08d268c366ad2dd30ae3d71541c9cdf94d1f698ee
2023-07-20 09:55:04 +01:00
MarcoFalke
fa3d72960b
lint: Ignore check_fileopens failure on **kwargs
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:
"""
2023-07-20 09:15:43 +02:00
MarcoFalke
fa6bb85cd2
test: Ignore UTF-8 errors in assert_debug_log
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)
2023-07-20 09:15:29 +02:00
MarcoFalke
fa63326fbc
test: Fix debug_log_size helper
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().
2023-07-20 09:15:04 +02:00
Martin Zumsande
e667bd68a1 test: fix intermittent failure in wallet_resendwallettransactions.py
Before, it was possible that a resend was triggered right between
eviction the txns from the mempool and the check that they were evicted.
2023-07-19 14:14:59 -04:00
fanquake
0be2f5481c
Merge bitcoin/bitcoin#27986: test: remove race in the user-agent reception check
20b49460b3 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 20b49460b3
  MarcoFalke:
    Concept ACK 20b49460b3

Tree-SHA512: 61fd3359ef11ea830021ede0e745497a7b60690c32d21c47cd12ff79f78907bb45e922c9d01e5d7ff614a8cd5e4560d39a3fc86b01b200429773a23ace3917e4
2023-07-19 12:32:30 +01:00
fanquake
c6a338b67e
Merge bitcoin/bitcoin#28083: ci: Use DOCKER_BUILDKIT for lint image
fa2f18ad8e 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
2023-07-18 16:40:39 +01:00
Andrew Chow
bc88f3ab90
Merge bitcoin/bitcoin#27997: Descriptors: rule out unspendable miniscript descriptors
c7db88af71 descriptor: assert we never parse a sane miniscript with no pubkey (Antoine Poinsot)
a49402a9ec qa: make sure we don't let unspendable Miniscript descriptors be imported (Antoine Poinsot)
639e3b6c97 descriptor: refuse to parse unspendable miniscript descriptors (Antoine Poinsot)
e3280eae1b 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 c7db88af71
  achow101:
    ACK c7db88af71

Tree-SHA512: e79bc9f7842e98a4e8f358f05811fca51b15b4b80a171c0d2b17cf4bb1f578a18e4397bc2ece9817d392e0de0196ee6a054b7318441fd3566dd22e1f03eb64a5
2023-07-17 19:16:09 -04:00
fanquake
d09c8bc730
Merge bitcoin/bitcoin#28088: test: Disable known broken USDT test
faf8be7c32 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
2023-07-17 15:04:49 +01:00
MarcoFalke
faf8be7c32
test: Disable known broken USDT test 2023-07-17 13:49:00 +02:00
MarcoFalke
fa367422ef
fuzz: Bump FuzzedDataProvider.h
From fa8401f9bf/compiler-rt/include/fuzzer/FuzzedDataProvider.h
2023-07-17 09:39:52 +02:00
MarcoFalke
fa2f18ad8e
ci: Use DOCKER_BUILDKIT for lint image
Can be reviewed with:
--color-moved=dimmed-zebra  --ignore-all-space
2023-07-16 13:18:18 +02:00
MarcoFalke
fa6245da60
fuzz: Generate process_message targets individually
Also, add an "rpc" target without LIMIT_TO_RPC_COMMAND set.
2023-07-12 15:52:14 +02:00
Andrew Chow
357e3f6aa4
Merge bitcoin/bitcoin#28025: test: refactor: deduplicate legacy ECDSA signing for tx inputs
5cf44275c8 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 5cf44275c8
  pinheadmz:
    ACK 5cf44275c8

Tree-SHA512: 8f0e4fb2c3e0f84fac5dbc4dda87973276242b0f628034272a7f3e45434c1e17dd1b26a37edfb302dcaf380dbfe98b0417391ace5e0ac9720155d8fba702031e
2023-07-11 17:25:40 -04:00
fanquake
21ed784614
Merge bitcoin/bitcoin#28028: test: Check expected_stderr after stop
faf902858d 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 faf902858d

Tree-SHA512: a70cd1e6cda84d542782e41e8b59741dbcd472c0d0575bcef5cbfd1418473ce94efe921481d557bae3fbbdd78f1c49c09c48872883c052d87c5c9a9a51492692
2023-07-11 10:14:48 +01:00
furszy
fcbdaeef4d
init: don't start indexes sync thread prematurely
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.
2023-07-10 10:50:50 -03:00
MarcoFalke
faf902858d
test: Check expected_stderr after stop 2023-07-10 13:45:50 +02:00
furszy
04575106b2
scripted-diff: rename 'loadblk' thread name to 'initload'
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-
2023-07-07 19:31:27 -03:00
fanquake
87e19b047c
Merge bitcoin/bitcoin#28038: wallet: address book migration bug fixes
7ecc29a0b7 test: wallet, add coverage for addressbook migration (furszy)
a277f8357a wallet: migration bugfix, persist empty labels (furszy)
1b64f6498c 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 7ecc29a0b7

Tree-SHA512: b8a8483a4178a37c49af11eb7ba8a82ca95e54a6cd799e155e33f9fbe7f37b259e28372c77d6944d46b6765f9eaca6b8ca8d1cdd9d223120a3653e4e41d0b6b7
2023-07-07 17:30:07 +01:00
Sebastian Falbesoner
bbbb89d238 test: miner: add coverage for -blockmintxfee setting
Co-authored-by: glozow <gloriajzhao@gmail.com>
2023-07-07 15:56:24 +02:00
fanquake
cf4da5ec29
Merge bitcoin/bitcoin#28015: fuzz: Generate rpc fuzz targets individually
fa1e27fe8e fuzz: Generate rpc fuzz targets individually (MarcoFalke)

Pull request description:

  The `rpc` fuzz target was added more than two years ago in e45863166f. 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 fa1e27fe8e
  dergoegge:
    ACK fa1e27fe8e

Tree-SHA512: 45ccba842367650d010320603153276b1b303deda9ba8c6bb31a4d2473b00aa5bca866db95f541485d65efd8276e2575026968c037872ef344fa33cf45bcdcd7
2023-07-07 11:26:22 +01:00
Ryan Ofsky
75135c673e
Merge bitcoin/bitcoin#27861: kernel: Rm ShutdownRequested and AbortNode from validation code.
6eb33bd0c2 kernel: Add fatalError method to notifications (TheCharlatan)
7320db96f8 kernel: Add flushError method to notifications (TheCharlatan)
3fa9094b92 scripted-diff: Rename FatalError to FatalErrorf (TheCharlatan)
edb55e2777 kernel: Pass interrupt reference to chainman (TheCharlatan)
e2d680a32d 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/27587 https://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 6eb33bd0c2
  hebasto:
    ACK 6eb33bd0c2, I have reviewed the code and it looks OK.
  ryanofsky:
    Code review ACK 6eb33bd0c2. No changes since last review other than rebase.

Tree-SHA512: 7d2d05fa4805428a09466d43c11ae32946cbb25aa5e741b1eec9cd142e4de4bb311e13ebf1bb125ae490c9d08274f2d56c93314e10f3d69e7fec7445e504987c
2023-07-06 17:07:27 -04:00
furszy
7ecc29a0b7
test: wallet, add coverage for addressbook migration 2023-07-06 16:11:55 -03:00
Vasil Dimov
20b49460b3
test: remove race in the user-agent reception check
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>
2023-07-06 17:48:15 +02:00
glozow
ecf3baffc0
Merge bitcoin/bitcoin#27869: wallet: Give deprecation warning when loading a legacy wallet
8fbb6e99bf 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 8fbb6e99bf
  jonatack:
    re-ACK 8fbb6e99bf

Tree-SHA512: 902984b09452926cf199f06e5fb56e4985325cdd5e0dcc829992158488f42d5fbc33e9a30a29303feac24c8315193e8d31712022e2a0503abd6b67169a0027f4
2023-07-06 10:47:41 +01:00
Sebastian Falbesoner
5cf44275c8 test: refactor: deduplicate legacy ECDSA signing for tx inputs
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.
2023-07-03 17:33:41 +02:00
Antoine Poinsot
a49402a9ec
qa: make sure we don't let unspendable Miniscript descriptors be imported 2023-07-01 12:12:26 +02:00
fanquake
61d59fed74
Merge bitcoin/bitcoin#24005: test: add python implementation of Elligator swift
4f4d039a98 test: add ellswift test vectors from BIP324 (stratospher)
a31287718a test: Add ellswift unit tests (stratospher)
714fb2c02a 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 4f4d039a98
  theStack:
    ACK 4f4d039a98 🐊

Tree-SHA512: 32bc8e88f715f2cd67dc04cd38db92680872072cb3775478e2c30da89aa2da2742992779ea14da2f1faca09228942cfbd86d6957402b24bf560244b389e03540
2023-06-30 19:30:49 +01:00
fanquake
3367e1c850
Merge bitcoin/bitcoin#28009: script, test: python typing and linter updates
6c97757a48 script: appease spelling linter (Jon Atack)
1316119ce7 script: update ignored-words.txt (Jon Atack)
146c861da2 script: update linter dependencies (Jon Atack)
92408224a4 test: fix PEP484 no implicit optional argument types errors (Jon Atack)
f86a301433 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 6c97757a48

Tree-SHA512: 8a46a4d36d5978affdcecf4f2ace20ca1b52d483e098304911a2169afe60ccb9b042fa90c04b762d94f3ce53d2cafe6f24476ae839867a770c7f31e7e7242d99
2023-06-30 16:20:37 +01:00
Jon Atack
1316119ce7 script: update ignored-words.txt 2023-06-29 16:14:07 -06:00
Jon Atack
92408224a4 test: fix PEP484 no implicit optional argument types errors
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
2023-06-29 16:14:07 -06:00
Jon Atack
f86a301433 script, test: add missing python type annotations
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
2023-06-29 16:13:51 -06:00
stratospher
4f4d039a98 test: add ellswift test vectors from BIP324
The test vector input file is taken from:
1. https://github.com/bitcoin/bips/blob/master/bip-0324/xswiftec_inv_test_vectors.csv
2. https://github.com/bitcoin/bips/blob/master/bip-0324/ellswift_decode_test_vectors.csv

Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
2023-06-29 23:32:56 +05:30
stratospher
a31287718a test: Add ellswift unit tests
remove util also since unit tests there were removed in #27538

Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
2023-06-29 23:32:56 +05:30
stratospher
714fb2c02a test: Add python ellswift implementation to test framework
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
2023-06-29 23:32:54 +05:30
MarcoFalke
fabd34873c
test: Rename EncodeDecimal to serialization_fallback
The new name better explains that the function handles fallbacks,
without listing all in the function name.
2023-06-29 19:51:43 +02:00
fanquake
3d51f7c9a8
Merge bitcoin/bitcoin#27932: test: Fuzz on macOS
fae7c50d20 test: Run fuzz tests on macOS (MarcoFalke)

Pull request description:

  Any reason not to?

ACKs for top commit:
  jamesob:
    Github ACK fae7c50d20
  dergoegge:
    utACK fae7c50d20

Tree-SHA512: e45122d73fafb17cea312258314b826cb0745e08daadd28465f687ec02d4c127d2f8cbe20179a9fff5712038850c02c968abb4838fa088b7555e28709317d3a3
2023-06-29 13:08:58 +01:00
fanquake
e8543629ae
Merge bitcoin/bitcoin#27884: test: Use TestNode datadir_path or chain_path where possible
aaaa3aefbd test: Use TestNode *_path properties where possible (MarcoFalke)
dddd89962b test: Allow pathlib.Path as RPC argument via authproxy (MarcoFalke)
fa41614a0a scripted-diff: Use wallets_path and chain_path where possible (MarcoFalke)
fa493fadfb test: Use wallet_dir lambda in wallet_multiwallet test where possible (MarcoFalke)

Pull request description:

  It seems inconsistent, fragile and verbose to:

  * Call `get_datadir_path` to recreate the path that already exists as field in TestNode
  * Call `os.path.join` with the hardcoded chain name or `self.chain` to recreate the TestNode `chain_path` property
  * Sometimes even use the hardcoded node dir name (`"node0"`)

  Fix all issues by using the TestNode properties.

ACKs for top commit:
  willcl-ark:
    re-ACK aaaa3aefbd
  theStack:
    Code-review ACK aaaa3aefbd 🌊

Tree-SHA512: e4720278085beb8164e1fe6c1aa18f601558a9263494ce69a83764c1487007de63ebb51d1b1151862dc4d5b49ded6162a5c1553cd30ea1c28627d447db4d8e72
2023-06-29 09:51:53 +01:00
Andrew Chow
626d346469
Merge bitcoin/bitcoin#26222: Introduce secp256k1 module with field and group classes to test framework
d4fb58ae8a test: EC: optimize scalar multiplication of G by using lookup table (Sebastian Falbesoner)
1830dd8820 test: add secp256k1 module with FE (field element) and GE (group element) classes (Pieter Wuille)

Pull request description:

  This PR rewrites a portion of `test_framework/key.py`, in a compatible way, by introducing classes that encapsulate field element and group element logic, in an attempt to be more readable and reusable.

  To maximize readability, the group element logic does not use Jacobian coordinates. Instead, group elements just store (affine) X and Y coordinates directly. To compensate for the performance loss this causes, field elements are represented as fractions. This undoes most, but not all, of the performance loss, and there is a few % slowdown (as measured in `feature_taproot.py`, which heavily uses this).

  The upside is that the implementation for group laws (point doubling, addition, subtraction, ...) is very close to the mathematical description of elliptic curves, and this extends to potential future extensions (e.g. ElligatorSwift as needed by #27479).

ACKs for top commit:
  achow101:
    ACK d4fb58ae8a
  theStack:
    re-ACK d4fb58ae8a
  stratospher:
    tested ACK d4fb58a. really liked how this PR makes the secp256k1 code in the tests more intuitive and easier to follow!

Tree-SHA512: 9e0d65d7de0d4fb35ad19a1c19da7f41e5e1db33631df898c6d18ea227258a8ba80c893dab862b0fa9b0fb2efd0406ad4a72229ee26d7d8d733dee1d56947f18
2023-06-28 16:27:55 -04:00
MarcoFalke
fa1e27fe8e
fuzz: Generate rpc fuzz targets individually 2023-06-28 17:50:45 +02:00
TheCharlatan
6eb33bd0c2
kernel: Add fatalError method to notifications
FatalError replaces what previously was the AbortNode function in
shutdown.cpp.

This commit is part of the libbitcoinkernel project and further removes
the shutdown's and, more generally, the kernel library's dependency on
interface_ui with a kernel notification method. By removing interface_ui
from the kernel library, its dependency on boost is reduced to just
boost::multi_index. At the same time it also takes a step towards
de-globalising the interrupt infrastructure.

Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-06-28 09:52:33 +02:00
TheCharlatan
3fa9094b92
scripted-diff: Rename FatalError to FatalErrorf
This is done in preparation for the next commit where a new FatalError
function is introduced. FatalErrorf follows common convention to append
'f' for functions accepting format arguments.

-BEGIN VERIFY SCRIPT-
sed -i 's/FatalError/FatalErrorf/g' $( git grep -l 'FatalError')
-END VERIFY SCRIPT-
2023-06-28 09:52:30 +02:00