Commit graph

4607 commits

Author SHA1 Message Date
Ryan Ofsky
3746f00be1 init: Error if ignored bitcoin.conf file is found
Show an error on startup if a bitcoin datadir that is being used contains a
`bitcoin.conf` file that is ignored. There are two cases where this could
happen:

- One case reported in
  https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043
  happens when a bitcoin.conf file in the default datadir (e.g.
  $HOME/.bitcoin/bitcoin.conf) has a "datadir=/path" line that sets different
  datadir containing a second bitcoin.conf file. Currently the second
  bitcoin.conf file is ignored with no warning.

- Another way this could happen is if a -conf= command line argument points
  to a configuration file with a "datadir=/path" line and that specified path
  contains a bitcoin.conf file, which is currently ignored.

This change only adds an error message and doesn't change anything about way
settings are applied. It also doesn't trigger errors if there are redundant
-datadir or -conf settings pointing at the same configuration file, only if
they are pointing at different files and one file is being ignored.
2023-04-21 06:53:23 -04:00
Sebastian Falbesoner
6a77d290da test: add regression tests for #27468 (invalid URI segfaults)
Prior to PR #27468 (commit 11422cc572) all
call-sites of `GetQueryParameter(...)` in the REST module could trigger
a crash. Add missing test cases for all possible code-paths as a
regression test, as a foundation for possible follow-up fixes (which aim
to resolve this issue in a more general and robust way).
2023-04-17 18:40:58 +02:00
fanquake
54e07a05b2
Merge bitcoin/bitcoin#27471: test: fix bumpfee 'spend_one_input' occasional failure
e07dd5fff9 test: fix bumpfee 'spend_one_input' occasional failure (furszy)

Pull request description:

  CI test failure, in master: https://cirrus-ci.com/task/5975232842825728.
  In #27469 https://cirrus-ci.com/task/6452468402356224

  Most of the subtests in `wallet_bumpfee.py` expect to find spendable UTXOs of 0.001 btc in the rbf wallet. They use the `spend_one_input()` method which fails if none of them exist.

  The sporadic failure comes from the recently added `test_feerate_checks_replaced_outputs` subtest that can spend all them. Leaving the next subtests with no 0.001 UTXOs to spend.

  To solve it, this PR moves the recently added case into a "context independent subtests" section.
  Which is placed at the end to not affect other cases.

ACKs for top commit:
  achow101:
    ACK e07dd5fff9
  pablomartin4btc:
    ACK e07dd5fff9.

Tree-SHA512: c150ed6fcfbb6f1502eaf6370a57aae0da991c0fc48e8bb3d446805f4336abba5d80ff0de26094914da95432dd0255030fe527001af4510dfdcefbc7230f14d6
2023-04-17 16:31:31 +01:00
fanquake
e054b7390c
Merge bitcoin/bitcoin#27468: bugfix: rest: avoid segfault for invalid URI
11422cc572 bugfix: rest: avoid segfault for invalid URI (pablomartin4btc)

Pull request description:

  Minimal fix to get it promptly into 25.0 release (suggested by  [stickies-v](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1385130381) and supported by [vasild](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1385842606)  )

  Please check #27253 for reviewers comments and acks regarding this PR and read the commit comment message body for more details about the fix.

ACKs for top commit:
  achow101:
    ACK 11422cc572
  stickies-v:
    re-ACK 11422cc

Tree-SHA512: 5af6b53fb266a12b463f960910556d5e97bc88b3c2a4f437ffa343886b38749e1eb058cf7bc64d62e82e1acf6232a186bddacd8f3b4500c87bf9e550a0153386
2023-04-17 15:11:15 +01:00
pablomartin4btc
11422cc572 bugfix: rest: avoid segfault for invalid URI
`evhttp_uri_parse` can return a nullptr, for example when the URI
contains invalid characters (e.g. "%").
`GetQueryParameterFromUri` passes the output of `evhttp_uri_parse`
straight into `evhttp_uri_get_query`, which means that anyone calling
a REST endpoint in which query parameters are used (e.g. `rest_headers`)
can cause a segfault.

This bugfix is designed to be minimal and without additional behaviour change.
Follow-up work should be done to resolve this in a more general and robust way,
so not every endpoint has to handle it individually.
2023-04-17 10:13:34 -03:00
furszy
e07dd5fff9
test: fix bumpfee 'spend_one_input' occasional failure
Most of the subtests in wallet_bumpfee.py expect to
find spendable UTXOs of 0.001 btc in the rbf wallet
(they use the 'spend_one_input()' method that tries
to spend one of them and if it doesn't find any, it
throws an exception).

The sporadic failure comes from the recently added
'test_feerate_checks_replaced_outputs' subtest that
can spend all them. Leaving the next subtests with
no 0.001 UTXOs to spend.

To solve it, this PR moves the recently added case
into a "context independent subtests" section.
Which is placed at the end to not affect other cases.
2023-04-15 23:01:45 -03:00
fanquake
90bfa9d2d7
Merge bitcoin/bitcoin#27308: bumpfee: avoid making bumped transactions with too low fee when replacing outputs
d52fa1b0a5 tests: Make sure that bumpfee feerate checks work when replacing outputs (Andrew Chow)
be177c15a4 bumpfee: Check the correct feerate when replacing outputs (Andrew Chow)

Pull request description:

  When replacing the outputs of a transaction during `bumpfee`, it is possible to accidentally create a transaction that will not be accepted into the mempool as it does not meet the incremental relay fee requirements. This occurs because the size estimation used for checking the provided feerate does not account for the replaced outputs; it instead uses the original outputs. When the replaced outputs is significantly different from the original, there can be a large difference in estimated transaction sizes that can make a transaction miss the absolute fee requirements for the incremental relay fee. Unfortunately we do not currently inform the user when the bumped transaction fails to relay, so they could use `bumpfee` and think the transaction has been bumped when it actually has not.

  This issue is resolved by replacing the outputs before doing the size estimation, and also updating the feerate checker to use the actual fee values when calculating the required minimum fee.

  Also added a test for this scenario.

ACKs for top commit:
  ishaanam:
    reACK d52fa1b0a5
  Xekyo:
    reACK d52fa1b0a5

Tree-SHA512: d18301b587465322dd3fb1bb86496c3675265a56072047576e2baa5cf907dd3b54778f30721f662f0c235709a5568427c18542eb7efbfb6fdd9f481fe676c66b
2023-04-15 12:55:10 +01:00
Andrew Chow
6a167325f0
Merge bitcoin/bitcoin#27279: Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet
7ccdd741fe test: fix importmulti/importdescriptors assertion (Jon Atack)
19d888ce40 rpc: move WALLET_FLAG_CAVEATS to the compilation unit of its caller (Jon Atack)
01df011ca2 doc: release note for wallet RPCs "warning" field deprecation (Jon Atack)
9ea8b3739a test: createwallet "warning" field deprecation test (Jon Atack)
645d7f75ac rpc: deprecate "warning" field in {create,load,unload,restore}wallet (Jon Atack)
2f4a926e95 test: add test coverage for "warnings" field in createwallet (Jon Atack)
4a1e479ca6 rpc: add "warnings" field to RPCs {create,load,unload,restore}wallet (Jon Atack)
079d8cdda8 rpc: extract wallet "warnings" fields to a util helper (Jon Atack)
f73782a903 doc: fix/improve warning helps in {create,load,unload,restore}wallet (Jon Atack)

Pull request description:

  Based on discussion and concept ACKed in #27138, add a `warnings` field to RPCs createwallet, loadwallet, unloadwallet, and restorewallet as a JSON array of strings to replace the `warning` string field in these 4 RPCs. The idea is to more gracefully handle multiple warning messages and for consistency with other wallet RPCs.  Then, deprecate the latter fields, which represent all the remaining RPC `warning` fields.

  The first commit f73782a903 implements https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1474789198 as an alternative to #27138. One of those two could potentially be backported to our currently supported releases.

ACKs for top commit:
  achow101:
    ACK 7ccdd741fe
  1440000bytes:
    utACK 7ccdd741fe
  vasild:
    ACK 7ccdd741fe
  pinheadmz:
    re-ACK 7ccdd741fe

Tree-SHA512: 314e0a4c41fa383d95e2817bfacf359d449e460529d235c3eb902851e2f4eacbabe646d9a5a4beabc4964cdfabf6397ed8301366a58d344a2f787f83b75e9d64
2023-04-12 13:09:23 -04:00
Andrew Chow
e83babe3b8 wallet: Replace use of purpose strings with an enum
Instead of storing and passing around fixed strings for the purpose of
an address, use an enum.

This also rationalizes the CAddressBookData struct, documenting all fields and
making them public, and simplifying the representation to avoid bugs like
https://github.com/bitcoin/bitcoin/pull/26761#discussion_r1134615114 and make
it not possible to invalid address data like change addresses with labels.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-04-11 15:55:31 -04:00
Jon Atack
7ccdd741fe test: fix importmulti/importdescriptors assertion
as these RPCs have a "warnings" field, not a "warning" one.
2023-04-10 10:42:25 -07:00
Jon Atack
9ea8b3739a test: createwallet "warning" field deprecation test 2023-04-10 10:41:56 -07:00
Jon Atack
645d7f75ac rpc: deprecate "warning" field in {create,load,unload,restore}wallet
This string field has been replaced in these four RPCs by a "warnings" field
returning a JSON array of strings.
2023-04-10 10:41:56 -07:00
Jon Atack
2f4a926e95 test: add test coverage for "warnings" field in createwallet
and clarify the "warning" field behavior.
2023-04-10 10:41:56 -07:00
Andrew Chow
d52fa1b0a5 tests: Make sure that bumpfee feerate checks work when replacing outputs
When replacing the outputs of a transaction, we can end up with
fees that are drastically different from the original. This tests that
the feerate checks we perform will properly detect when the bumping tx
will have an insufficient feerate.
2023-04-10 10:02:41 -04:00
MarcoFalke
fa584b4d01
test: Remove windows workaround in authproxy 2023-04-04 13:23:53 +02:00
fanquake
49b87bfe7e
Merge bitcoin/bitcoin#27389: test: refactor: replace unnecessary BytesIO uses
f842ed9a40 test: refactor: replace unnecessary `BytesIO` uses (Sebastian Falbesoner)

Pull request description:

  Rather than needing to create intermediate stream variables, we can use helper functions like `tx_from_hex` instead or access the result directly, leading both to increased readability and less code.

ACKs for top commit:
  stickies-v:
    ACK f842ed9a40
  brunoerg:
    crACK f842ed9a40
  aureleoules:
    ACK f842ed9a40 - It seems that these are the only instances that can be changed and it simplifies test code.

Tree-SHA512: 7f4fd7a26720d1988bf27f66c817ff6cd7060350a3be62d28e7848c768fd43578719ca475193d4057ccf4f2458af18564fd513fe3a1d458a11c799927c12bd65
2023-04-04 10:36:45 +01:00
Sebastian Falbesoner
f842ed9a40 test: refactor: replace unnecessary BytesIO uses
Rather than needing to create intermediate stream variables, we can use
helper functions like `tx_from_hex` instead or access the result
directly, leading both to increased readability and less code.
2023-04-01 14:15:17 +02:00
MarcoFalke
fae66fceb3
test: Remove python3.5 workaround in authproxy
Also, move the burden of checking for a timeout to the client and
disable the timeout on the server. This should avoid intermittent issues
in slow tests (for example mining_getblocktemplate_longpoll.py, or
feature_dbcrash.py), or possibly when the server is running slow (for
example in valgrind).  There shouldn't be any downside in tests caused
by a high rpcservertimeout.
2023-03-31 14:26:08 +02:00
glozow
328087d16f
Merge bitcoin/bitcoin#27350: test: refactor: dedup mempool_package_limits.py subtests via decorator
e669833943 test: dedup package limit checks via decorator in mempool_package_limits.py (Sebastian Falbesoner)
72f25e238c test: refactor: use Satoshis for fees in mempool_package_limits.py (Sebastian Falbesoner)

Pull request description:

  The subtests in the functional test mempool_package_limits.py all follow the same pattern:
  1. first, check that the mempool is currently empty
  2. create and submit certain single txs to the mempool, prepare list of hex transactions
  3. check that `testmempoolaccept` on the package hex fails with a "package-mempool-limits" error on each tx result
  4. after mining a block, check that submitting the package succeeds

  Note that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so this might be a nice opportunity to deduplicate code by using a newly introduced decorator which executes the necessary before and after the essential part of the subtest. This also makes it easier to add new subtests without having to copy-paste those parts once again.

  In addition, the first commit switches the fee unit from BTC to Satoshis, which allows to get rid of some imports (`COIN` and `Decimal`) and a comment for the `test_desc_size_limits` subtest is fixed (s/25KvB/21KvB/).

ACKs for top commit:
  ismaelsadeeq:
    ACK e669833943
  glozow:
    utACK e669833943

Tree-SHA512: 84a85e739de7387391c13bd46aeb015a74302ea7c6f0ca3d4e2b1b487d38df390dc118eb5b1c11d3e4206bff316a4dab60ef6b25d8feced672345d4e36ffd205
2023-03-30 18:47:17 +01:00
dergoegge
3a060ae7b6 scripted-diff: Rename nUnconnectingHeaders and fPreferHeaders
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); }

ren nUnconnectingHeaders     m_num_unconnecting_headers_msgs
ren fPreferHeaders           m_prefers_headers
ren MAX_UNCONNECTING_HEADERS MAX_NUM_UNCONNECTING_HEADERS_MSGS

-END VERIFY SCRIPT-
2023-03-30 14:56:34 +02:00
fanquake
c0311b1dda
Merge bitcoin/bitcoin#27349: test: use address_to_scriptpubkey instead of RPC call
e47ce42f67 refactor: use address_to_scriptpubkey to retrieve addresses scriptpubkey (ismaelsadeeq)
4142d19d74 refactor: move address_to_scriptpubkey to address.py (ismaelsadeeq)

Pull request description:

  PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).

  Instead of using RPC calls, this update replaces the process of obtaining an address scriptPubkey with the address_to_scriptpubkey method, resulting in improved performance for functional tests.

ACKs for top commit:
  josibake:
    re-ACK e47ce42f67
  theStack:
    ACK e47ce42f67 🌱

Tree-SHA512: 05285349a7d5ce7097b8f2582e573a5135c6deef85ea9936f68f6ce94e9ebb1d84d94f7fc7e5ed833a698e01585addd80deb52e6338f8aee985bf14db45417d2
2023-03-29 12:05:15 +01:00
Sebastian Falbesoner
e669833943 test: dedup package limit checks via decorator in mempool_package_limits.py 2023-03-28 22:03:31 +02:00
Sebastian Falbesoner
72f25e238c test: refactor: use Satoshis for fees in mempool_package_limits.py
This avoids having to convert from BTC to Sats and needs less imports.
Also specify the tx's target size in vsize rather than in weight, which
allows us to specify the fee-rate by a simple multiplication, rather
than having another magic number for it.
2023-03-28 22:03:27 +02:00
ismaelsadeeq
e47ce42f67 refactor: use address_to_scriptpubkey to retrieve addresses scriptpubkey
This commit updates the code by replacing the RPC call used to
decode an address and retrieve its corresponding scriptpubkey
with the address_to_scriptpubkey function. address_to_scriptpubkey
function can now decode all addresses formats, which makes
it more efficient to use.
2023-03-28 16:58:16 +01:00
ismaelsadeeq
4142d19d74 refactor: move address_to_scriptpubkey to address.py
The COINBASE_MATURITY constant in blocktools.py is imported in wallet.py.
However, importing address_to_scriptpubkey to blocktools.py will
generate a circular import error. Since the method is related to
addresses, it is best to move it to address.py, which will also
fix the circular import error.

Update imports of address_to_scriptpubkey accordingly.
2023-03-28 16:54:28 +01:00
fanquake
68828288e5
Merge bitcoin/bitcoin#27318: test: wallet_create_tx.py fix race
8aab5157c5 test: wallet_create_tx.py fix race (furszy)

Pull request description:

  Fixes #27316

  Because wallets are internally synchronized through the validation interface,
  and the interface dispatches events on a worker thread, it is possible for a
  transaction created by the first wallet to not arrive to the second wallet
  before the second wallet attempts to use one of its outputs. This is because
  we do not wait for the `BroadcastTransaction` callback during the wallet's
  "submit to mempool" process. To address this in the tests, we need to
  manually sync the validation queue.

ACKs for top commit:
  josibake:
    ACK 8aab5157c5
  theStack:
    ACK 8aab5157c5

Tree-SHA512: 76364370ab292a5c3ea1ed61cd353fc626a9e9cd6ce18464c24da1b3dcb34b65006e2bc42b84bbd25af03f9449231990bf789504728972db3217b569099eb309
2023-03-28 15:41:15 +01:00
fanquake
873a5062db
Merge bitcoin/bitcoin#27269: test: Support decoding segwit address in address_to_scriptpubkey()
d178082996 test: add bech32 decoding support to address_to_scriptpubkey() (ismaelsadeeq)
aac8793c7a test: test_bech32_decode in address.py (ismaelsadeeq)

Pull request description:

  [rpc_scantxoutset.py](e695d8536e/test/functional/rpc_scantxoutset.py (L26))  sendtodestination only sends to legacy addresses and scriptPubkeys because  [wallet.py](e695d8536e/test/functional/test_framework/wallet.py (L415)) address_to_scriptpubkey does not support conversion of segwit address.

  This update enables address_to_scriptpubkey to support the conversion of testnet segwit addresses to scriptPubkeys.

  This change will enable [rpc_scantxoutset.py](e695d8536e/test/functional/rpc_scantxoutset.py (L22)) ScantxoutsetTest to have more test coverage by adding more sendtodestination calls with bech32 and bech32m testnet addresses, then test the bech32 and bech32m  derivation subsets UTXO amount in [Test extended key derivation](e695d8536e/test/functional/rpc_scantxoutset.py (L84)).

  I will add the test coverage in a subsequent Pull request.

ACKs for top commit:
  josibake:
    ACK d178082996
  theStack:
    ACK d178082996 ✔️
  willcl-ark:
    ACK d17808299

Tree-SHA512: 312c20ce192c648faf7dd178622700c9b871d755db56c246250e25508c3c19e7b02c0ae901dda11a1794629b9a9429c877168c05e1c4c1dbf41493316e30e7e9
2023-03-24 12:17:38 +00:00
furszy
8aab5157c5
test: wallet_create_tx.py fix race
Because wallets are internally synchronized
through the validation interface, and the
interface dispatches events on a worker thread,
it is possible for a transaction created by the
first wallet to not arrive at the second wallet
before the second wallet attempts to use one of
its outputs. This is because we do not wait for
the BroadcastTransaction callback during the wallet's
"submit to mempool" process. To address this in the
tests, we need to sync the validation queue.
2023-03-23 17:06:54 -03:00
glozow
381593c906
Merge bitcoin/bitcoin#24845: wallet: return error msg for "too-long-mempool-chain"
f3221d373a test: add wallet too-long-mempool-chain error coverage (furszy)
acf0119d24 wallet: return error msg for too-long-mempool-chain failure (furszy)

Pull request description:

  Fixes #23144.

  We currently return a general "Insufficient funds" from Coin
  Selection when we actually skipped unconfirmed UTXOs that
  surpassed the mempool ancestors limit.

  This PR make the error clearer by returning:
  "Unconfirmed UTXOs are available, but spending them creates
  a chain of transactions that will be rejected by the mempool"

  Also, added an early return from Coin Selection if the sum of
  the discarded coins decreases the available balance below the
  target amount.

ACKs for top commit:
  achow101:
    ACK f3221d373a
  S3RK:
    Code review ACK f3221d373a
  Xekyo:
    ACK f3221d373a

Tree-SHA512: 13e5824b75ac302280ff894560a4ebf32a74f32fe49ef8281f2bc99c0104b92cef33d3b143c6e131f3a07eafe64533af7fc60abff585142c134b9d6e531a6a66
2023-03-23 15:53:56 +00:00
fanquake
483fb8d216
Merge bitcoin/bitcoin#27287: test: Replace threading with concurrent.futures
fa0696e786 test: Replace threading with concurrent.futures (MarcoFalke)

Pull request description:

  `threading` has no easy way to get the return value or exception once the target function stops. Not checking the return value or exception can make tests more fragile and failures harder to debug.

  Fix this by checking the return value (or exception) by wrapping the function execution into a future and calling `result()` on it.

  Can be reviewed with `--ignore-all-space`.

  (There are still some uses of `threading` around, because some tests do expect an exception to be thrown and caught in the target function)

ACKs for top commit:
  ishaanam:
    ACK fa0696e786
  stickies-v:
    ACK fa0696e786

Tree-SHA512: d9ddf6b3c530cd8c485a030a3c84d4e03d3e9f9ea8240b050afcd566a884f5cabe816ac56910cec9ea9fa299239e5abb99e672dda05a74974f61bb68dc3c1d65
2023-03-23 15:09:50 +00:00
fanquake
8acfb1f8e0
Merge bitcoin/bitcoin#18933: rpc: Add submit option to generateblock
fa18504d57 rpc: Add submit option to generateblock (MarcoFalke)
fab9a08e14 refactor: Replace block_hash with block_out (MarcoFalke)

Pull request description:

  When submit is turned off, a block can be generated and returned as hex, to be used for further tests. For example, it can be submitted on a different node, on a different interface (like p2p), or just never submitted and be used for other testing purposes.

ACKs for top commit:
  instagibbs:
    ACK fa18504d57
  TheCharlatan:
    tACK fa18504d57

Tree-SHA512: 1b2ab6b71bb7e155c6482d75f5373f4e77de6446cb16bc2dfd19e7a4075b3a6ad87d7ad7a049a9eed934cb71574acfd27202f54c8bb3b03fac869f2e95db7ee5
2023-03-23 13:40:30 +00:00
ismaelsadeeq
d178082996 test: add bech32 decoding support to address_to_scriptpubkey()
This permits functional tests to decode bech32 addresses to scriptpubkeys.
2023-03-23 12:00:54 +01:00
ismaelsadeeq
aac8793c7a test: test_bech32_decode in address.py
Adds bech32_to_bytes() which can decode a bech32 address and return the
version as an `int` and the payload in bytes.

bech32_to_bytes() is used by the test_bech32_decode unit test to test
decoding of segwit addresses.
2023-03-23 11:59:29 +01:00
Andrew Chow
fc7c21f664
Merge bitcoin/bitcoin#27271: RPC: Fix fund transaction crash when at 0-value, 0-fee
d7cc503843 Fix fund transaction case at 0-value, 0-fee (Greg Sanders)

Pull request description:

  and when no inputs are pre-selected.

  triggered via:

  walletcreatefundedpsbt '[]' '[{"data": "deadbeef"}]' 0 '{"fee_rate": "0"}'

ACKs for top commit:
  achow101:
    ACK d7cc503843
  josibake:
    ACK d7cc503843
  furszy:
    Crashes sucks code ACK d7cc5038

Tree-SHA512: 3f5e10875666aaf52c11d6a38b951aa75d0cbe684cc7f904e199f7a864923bf31d03a654687f8b746cae0eebb886a799bff2c6d200699438480d4c0ff8785f3a
2023-03-22 12:54:26 -04:00
fanquake
6e69fead2b
Merge bitcoin/bitcoin#27280: test: Fix TypeError (expected str instance, bytes found) in wait_for_debug_log
33337eb860 test: Fix TypeError in wait_for_debug_log (MarcoFalke)

Pull request description:

ACKs for top commit:
  davidgumberg:
    tACK 33337eb860

Tree-SHA512: e641f23f0adc074d12b0ee10cab5845c16f3ac2858e42f895c69857c375fcb15c31bc1c9476bf2b6e2b49d0d2db4944687733da16d4a464152ae3323cbc6ca68
2023-03-22 10:01:01 +00:00
MarcoFalke
fa0696e786
test: Replace threading with concurrent.futures 2023-03-21 09:49:39 +01:00
Andrew Chow
60f142e395
Merge bitcoin/bitcoin#26531: mempool: Add mempool tracepoints
4b7aec2951 Add mempool tracepoints (virtu)

Pull request description:

  This PR adds multiple mempool tracepoints.

  | tracepoint  | description |
  | ------------- | ------------- |
  | `mempool:added`  | Is called when a transaction enters the mempool  |
  | `mempool:removed`  | ... when a transaction is removed from the mempool |
  | `mempool:replaced`  | ... when a transaction is replaced in the mempool |
  | `mempool:rejected`  | ... when a transaction is rejected from entering the mempool |

  The tracepoints are further documented in `docs/tracing.md`. Usage is demonstrated in the example script `contrib/tracing/mempool_monitor.py`. Interface tests are provided in `test/functional/interface_usdt_mempool.py`.

  The rationale for passing the removal reason as a string instead of numerically is that the benefits of not having to maintain a redundant enum-string mapping seem to outweigh the small cost of string generation. The reject reason is passed as string as well, although in this instance the string does not have to be generated but is readily available.

ACKs for top commit:
  0xB10C:
    ACK 4b7aec2951
  achow101:
    ACK 4b7aec2951

Tree-SHA512: 6deb3ba2d1a061292fb9b0f885f7a5c4d11b109b838102d8a8f4828cd68f5cd03fa3fc64adc6fdf54a08a1eaccce261b0aa90c2b8c33cd5fd3828c8f74978958
2023-03-20 12:42:24 -04:00
virtu
4b7aec2951 Add mempool tracepoints
Tracepoints for added, removed, replaced, and rejected transactions.

The removal reason is passed as string instead of a numeric value, since
the benefits of not having to maintain a redundant enum-string mapping
seem to outweigh the small cost of string generation.  The reject reason
is passed as string as well, although here the string does not have to
be generated but is readily available.

So far, tracepoint PRs typically included two demo scripts: a naive
bpftrace script to show raw tracepoint data and a bcc script for a more
refined view. However, as some of the ongoing changes to bpftrace
introduce a certain degree of unreliability (running some of the
existing bpftrace scripts was not possible with standard kernels and
bpftrace packages on latest stable Ubuntu, Debian, and NixOS), this PR
includes only a single bcc script that fuses the functionality of former
bpftrace and bcc scripts.
2023-03-20 15:57:31 +01:00
MarcoFalke
33337eb860
test: Fix TypeError in wait_for_debug_log
Traceback:

print_log = " - " + "\n - ".join(log.splitlines())
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: sequence item 0: expected str instance, bytes found
2023-03-20 11:54:06 +01:00
fanquake
0973018067
Merge bitcoin/bitcoin#27265: test: check that sigop limit also affects ancestor/descendant size (27171 follow-up)
6d24d1ef2b test: check that sigop limit also affects ancestor/descendant size (Sebastian Falbesoner)

Pull request description:

  This is a follow-up to #27171, adding a check that the sigop-limit vsize logic is also respected for {ancestor,descendant}size calculation (as suggested in https://github.com/bitcoin/bitcoin/pull/27171#pullrequestreview-1331143909). For simplicity, we use a one-parent-one-child cluster here and only check for the case that the sigop-limit equivalent size is larger than the serialized vsize.

ACKs for top commit:
  glozow:
    code review ACK 6d24d1ef2b, thanks for taking!

Tree-SHA512: dc65e455d06cfef1f1d6a53b959f99ec1ca3fe51c98dc1ed5826614b5619773d34aff0171c43a0ede4fd45605b2eb7a9278e027196128bb7ad8586b859f1cf70
2023-03-19 12:11:47 +00:00
fanquake
50171df26c
Merge bitcoin/bitcoin#27212: test: Make the unlikely race in p2p_invalid_messages impossible
fa1eb0ecae test: Make the unlikely race in p2p_invalid_messages impossible (MarcoFalke)

Pull request description:

  After `add_p2p_connection` both sides have the verack processed.
  However the pong from conn in reply to the ping from the node has not
  been processed and recorded in totalbytesrecv.
  Flush the pong from conn by sending a ping from conn.

  This should make the unlikely race impossible.

ACKs for top commit:
  mzumsande:
    ACK fa1eb0ecae
  pinheadmz:
    ACK fa1eb0ecae

Tree-SHA512: 44166587572e8c0c758cac460fcfd5cf403b2883880128b13dc62e7f74ca5cb8f145bb68a903df177ff0e62faa360f913fd409b009d4cd1360f1f4403ade39ae
2023-03-17 14:55:48 +00:00
MarcoFalke
fa1eb0ecae
test: Make the unlikely race in p2p_invalid_messages impossible 2023-03-17 09:25:32 +01:00
Andrew Chow
db03248070
Merge bitcoin/bitcoin#27199: test: fix race condition in encrypted wallet rescan tests
dbeca792a9 test: fix race condition in encrypted wallet rescan tests (ishaanam)

Pull request description:

  This fixes https://github.com/bitcoin/bitcoin/pull/26347#discussion_r1123340738

ACKs for top commit:
  MarcoFalke:
    nice re-ACK dbeca792a9  🚜
  achow101:
    ACK dbeca792a9

Tree-SHA512: 7127254ac0274b5bc8ba0242736e77464acbf1f6e3f6af098b4e47742124c336cd67dffdb385e1e8dbd3a8ae74acd073c99e82fa35c44a615fd7d22b29a0daf7
2023-03-16 17:28:39 -04:00
Greg Sanders
d7cc503843 Fix fund transaction case at 0-value, 0-fee 2023-03-16 14:58:41 -04:00
Andrew Chow
09e86d7a1a
Merge bitcoin/bitcoin#27200: test: psbt: check non-witness UTXO removal for segwit v1 input
3dd2f6461b test: psbt: check non-witness UTXO removal for segwit v1 input (Sebastian Falbesoner)
dd78e3fa43 test: speedup rpc_psbt.py by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)
e194e3e93d test: PSBT: eliminate magic numbers for global unsigned tx key (0) (Sebastian Falbesoner)

Pull request description:

  This PR adds missing test coverage for dropping non-witness UTXOs from PSBTs for segwit v1+ inputs (see commit 103c6fd279). The formerly [disabled](4600479058) method `test_utxo_conversion` is re-enabled and adapted to spend a Taproot (`bech32m`) instead of a wrapped SegWit (`p2sh-segwit`) output. Note that in contrast to the original test, we have to add the non-witness UTXO manually here using the test framework's PSBT module, since the constructing node knows that the output is segwit v1 and hence doesn't add the non-witness UTXO in the first place (see also [BIP371]( https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki#user-content-UTXO_Types)).

  I strongly assume that most wallets would behave the same as Bitcoin Core here and wouldn't create PSBTs with non-witness UTXOs for Taproot inputs, but it's still good to test everything works as expected if it's still done and that the non-witness UTXO is simply dropped in that case.

  The first two commits contain a small refactor (magic number elimination in PSBT module) and test speedup of ~2-3x (using whitelisting peers / immediate tx relay).

ACKs for top commit:
  achow101:
    ACK 3dd2f6461b
  instagibbs:
    ACK 3dd2f6461b

Tree-SHA512: b8d7f7ea5d7d21def024b70dfca61991cc96a4193be8857018b4d7cf3ca1465d185619fd4a77623803d9da309aa489c53273e9b7683d970ce12e2399b5b50031
2023-03-16 14:48:10 -04:00
Andrew Chow
ebb15ea75a
Merge bitcoin/bitcoin#26207: rest: add verbose and mempool_sequence query params for mempool/contents
1ff5d61dfd doc: add mempool/contents rest verbose and mempool_sequence args (Andrew Toth)
52a31dccc9 tests: mempool/contents verbose and mempool_sequence query params tests (Andrew Toth)
a518fff0f2 rest: add verbose and mempool_sequence query params for mempool/contents (Andrew Toth)

Pull request description:

  The verbose mempool json response can get very large. This adds an option to return the non-verbose response of just the txids. It is identical to the rpc response so the diff here is minimal. This also adds the mempool_sequence parameter for rpc consistency. Verbose defaults to true to remain backwards compatible.

  It uses query parameters to be compatible with the efforts in https://github.com/bitcoin/bitcoin/issues/25752.

ACKs for top commit:
  achow101:
    ACK 1ff5d61dfd
  stickies-v:
    re-ACK [1ff5d61](1ff5d61dfd)
  pablomartin4btc:
    tested ACK 1ff5d61dfd.

Tree-SHA512: 1bf08a7ffde2e7db14dc746e421feedf17d84c4b3f1141e79e36feb6014811dfde80e1d8dbc476c15ff705de2d3c967b3081dcd80536d76b7edf888f1a92e9d1
2023-03-15 19:39:30 -04:00
ishaanam
dbeca792a9 test: fix race condition in encrypted wallet rescan tests 2023-03-15 17:27:57 -04:00
Sebastian Falbesoner
6d24d1ef2b test: check that sigop limit also affects ancestor/descendant size 2023-03-15 19:39:25 +01:00
glozow
f50fb178c3
Merge bitcoin/bitcoin#27235: Avoid integer overflow in CheckDiskSpace
05eeba2c5f [test] Add manual prune startup test case (dergoegge)
4517419628 [util] Avoid integer overflow in CheckDiskSpace (dergoegge)

Pull request description:

  Starting a fresh node with `-prune=1` causes an integer overflow to happen in `CheckDiskSpace` ([here](f7bdcfc83f/src/init.cpp (L1633-L1648))) because `nPruneTarget` is to the max `uint64_t` value.
  ```
   node1 stderr util/system.cpp:138:51: runtime error: unsigned integer overflow: 52428800 + 18446744073709551615 cannot be represented in type 'unsigned long'
      #0 0x564a482b5088 in CheckDiskSpace(fs::path const&, unsigned long) src/./src/util/system.cpp:138:51
      #1 0x564a4728dc59 in AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/./src/init.cpp:1639:14
      #2 0x564a47256e6a in AppInit(node::NodeContext&, int, char**) src/./src/bitcoind.cpp:221:43
      #3 0x564a47256087 in main src/./src/bitcoind.cpp:265:13
      #4 0x7fcb7cbffd8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
      #5 0x7fcb7cbffe3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
      #6 0x564a471957f4 in _start (/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/bitcoind+0xca07f4) (BuildId: 035cb22302d37317a630900a15a26ecb326d395c)
  SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow util/system.cpp:138:51 in
  ```

  I think side stepping the overflow for this specific case, is better than adding an exception to the UB suppresions file.

ACKs for top commit:
  MarcoFalke:
    ACK 05eeba2c5f 🥝
  john-moffett:
    ACK 05eeba2c5f

Tree-SHA512: 1d8e6bcb49818139f04b5ab2cbef7f9b422bf0c38a804cd532b6bd0ba4c4fd07f959ba977e59896343f213086c8ecc48180f50d006638dc84649c66ec379d58a
2023-03-13 17:01:48 +00:00
glozow
73a9892bce
Merge bitcoin/bitcoin#26514: Improve address decoding errors
962a0930e6 Improve address decoding errors (Aurèle Oulès)

Pull request description:

  Attempt to fix #21741.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 962a0930e6
  davidgumberg:
    ACK 962a0930e6
  1440000bytes:
    utACK 962a0930e6

Tree-SHA512: 6f216eeaeccf6bfdf0730d38835fdf26c935a5e1fc35006660393a9ad76bf38c85340f0f20e92f87840463d83d891d9714cfad313aab301a16bb8efa4490df06
2023-03-13 16:57:31 +00:00