Commit graph

5720 commits

Author SHA1 Message Date
Andrew Chow
459272d639
Merge bitcoin/bitcoin#26152: Bump unconfirmed ancestor transactions to target feerate
f18f9ef4d3 Amend bumpfee for inputs with overlapping ancestry (Murch)
2e35e944da Bump unconfirmed parent txs to target feerate (Murch)
3e3e052411 coinselection: Move GetSelectionWaste into SelectionResult (Andrew Chow)
c57889da66 [node] interface to get bump fees (glozow)
c24851be94 Make MiniMinerMempoolEntry fields private (Murch)
ac6030e4d8 Remove unused imports (Murch)
d2f90c31ef Fix calculation of ancestor set feerates in test (Murch)
a1f7d986e0 Match tx names to index in miniminer overlap test (Murch)

Pull request description:

  Includes some commits to address follow-ups from #27021: https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156

  Reduces the effective value of unconfirmed UTXOs by the fees necessary to bump their ancestor transactions to the same feerate.

  While the individual UTXOs always account for their full ancestry before coin-selection, we can correct potential overestimates with a second pass where we establish the ancestry and bump fee for the whole input set collectively.

  Fixes #9645
  Fixes #9864
  Fixes #15553

ACKs for top commit:
  S3RK:
    ACK f18f9ef4d3
  ismaelsadeeq:
    ACK f18f9ef4d3
  achow101:
    ACK f18f9ef4d3
  brunoerg:
    crACK f18f9ef4d3
  t-bast:
    ACK f18f9ef4d3, I reviewed the latest changes and run e2e tests against eclair, everything looks good 👍

Tree-SHA512: b65180c4243b1f9d13c311ada7a1c9f2f055d530d6c533b78c2068b50b8c29ac1321e89e85675b15515760d4f1b653ebd9da77b37c7be52d9bc565a3538f0aa6
2023-09-14 16:08:37 -04:00
Andrew Chow
541976b42e
Merge bitcoin/bitcoin#27850: test: Add unit & functional test coverage for blockstore
de8f9123af test: cover read-only blockstore (Matthew Zipkin)
5c2185b3b6 ci: enable chattr +i capability inside containers (Matthew Zipkin)
e573f24202 unit test: add coverage for BlockManager (Matthew Zipkin)

Pull request description:

  This PR adds unit and functional tests to cover the behavior described in #2039. In particular, that bitcoind will crash on startup if a reindex is requested but the `blk` files are read-only. Eventually this behavior can be updated with https://github.com/bitcoin/bitcoin/pull/27039. This PR just commits the test coverage from #27039 as suggested in https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1584915782

ACKs for top commit:
  jonatack:
    ACK de8f9123af modulo suggestions in https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1319010039, tested on macOS, but not on Linux for the Linux-related change in the last push
  achow101:
    ACK de8f9123af
  MarcoFalke:
    lgtm ACK de8f9123af 📶

Tree-SHA512: b9bd684035dcea11c901b649fc39f397a2155a9a8459f3348e67947e387e45312fddeccb52981aef486f8a31deebb5356a7901c1bb94b78f82c24192a369af73
2023-09-14 13:21:14 -04:00
Matthew Zipkin
de8f9123af
test: cover read-only blockstore
Co-authored-by: Andrew Chow <github@achow101.com>
2023-09-14 12:02:01 -04:00
Murch
f18f9ef4d3
Amend bumpfee for inputs with overlapping ancestry
At the end of coin selection reduce the fees by the difference between
the individual bump fee estimates and the collective bump fee estimate.
2023-09-13 15:46:59 -04:00
Murch
2e35e944da
Bump unconfirmed parent txs to target feerate
When a transaction uses an unconfirmed input, preceding this commit it
would not consider the feerate of the parent transaction. Given a parent
transaction with a lower ancestor feerate, this resulted in the new
transaction's ancestor feerate undershooting the target feerate.

This commit changes how we calculate the effective value of unconfirmed UTXOs.
The effective value of unconfirmed UTXOs is decreased by the fee
necessary to bump its ancestry to the target feerate. This also impacts
the calculation of the waste metric: since the estimate for the current
fee is increased by the bump fees, unconfirmed UTXOs current fees appear less
favorable compared to their unchanged long term fees.

This has one caveat: if multiple UTXOs have overlapping ancestries, each
of their individual estimates will account for bumping all ancestors.
2023-09-13 14:33:58 -04:00
fanquake
f1a9fd627b
Merge bitcoin/bitcoin#28251: validation: fix coins disappearing mid-package evaluation
32c1dd1ad6 [test] mempool coins disappearing mid-package evaluation (glozow)
a67f460c3f [refactor] split setup in mempool_limit test (glozow)
d08696120e [test framework] add ability to spend only confirmed utxos (glozow)
3ea71feb11 [validation] don't LimitMempoolSize in any subpackage submissions (glozow)
d227b7234c [validation] return correct result when already-in-mempool tx gets evicted (glozow)
9698b81828 [refactor] back-fill results in AcceptPackage (glozow)
8ad7ad3392 [validation] make PackageMempoolAcceptResult members mutable (glozow)
03b87c11ca [validation] add AcceptSubPackage to delegate Accept* calls and clean up m_view (glozow)
3f01a3dab1 [CCoinsViewMemPool] track non-base coins and allow Reset (glozow)
7d7f7a1189 [policy] check for duplicate txids in package (glozow)

Pull request description:

  While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mainly, since the coins created by the evicted transaction are cached in `m_view`, we don't realize the UTXO has disappeared until `CheckInputsFromMempoolAndCache` asserts that they exist. Also, the returned `PackageMempoolAcceptResult` reports that the transaction is in mempool even though it isn't anymore.

  Fix this by not calling `LimitMempoolSize()` until the very end, and editing the results map with "mempool full" if things fall out.

  Pointed out by instagibbs in faeed687e5 on top of the v3 PR.

ACKs for top commit:
  instagibbs:
    reACK 32c1dd1ad6

Tree-SHA512: 61e7f69db4712e5e5bfa27d037ab66bdd97f1bf60a8d9ffb96adb1f0609af012c810d681102ee5c7baec7b5fe8cb7c304a60c63ccc445d00d86a2b7f0e7ddb90
2023-09-13 17:51:00 +01:00
glozow
32c1dd1ad6 [test] mempool coins disappearing mid-package evaluation
Test for scenario(s) outlined in PR 28251.
Test what happens when a package transaction spends a mempool coin which
is fetched and then disappears mid-package evaluation due to eviction or
replacement.
2023-09-13 16:14:18 +01:00
glozow
a67f460c3f [refactor] split setup in mempool_limit test
We want to be able to re-use fill_mempool so that none of the tests
affect each other.

Change the logs from info to debug because they are otherwise repeated
many times in the test output.
2023-09-13 16:14:18 +01:00
glozow
d08696120e [test framework] add ability to spend only confirmed utxos
Useful to ensure that the topologies of packages/transactions are as
expected, preventing bugs caused by having unexpected mempool ancestors.
2023-09-13 16:14:18 +01:00
glozow
7d7f7a1189 [policy] check for duplicate txids in package
Duplicates of normal transactions would be found by looking for
conflicting inputs, but this doesn't catch identical empty transactions.
These wouldn't be valid but exiting early is good and AcceptPackage's
result sanity checks assume non-duplicate transactions.
2023-09-13 16:14:17 +01:00
Andrew Chow
8f9c74cb11
Merge bitcoin/bitcoin#28414: wallet rpc: return final tx hex from walletprocesspsbt if complete
2e249b9227 doc: add release note for PR #28414 (Matthew Zipkin)
4614332fc4 test: remove unnecessary finalizepsbt rpc calls (ismaelsadeeq)
e3d484b603 wallet rpc: return final tx hex from walletprocesspsbt if complete (Matthew Zipkin)

Pull request description:

  See https://github.com/bitcoin/bitcoin/pull/28363#discussion_r1315753887

  `walletprocesspsbt` currently returns a base64-encoded PSBT and a boolean indicating if the tx is "complete". If it is complete, the base64 PSBT can be finalized with `finalizepsbt` which returns the hex-encoded transaction suitable for `sendrawtransaction`.

  With this patch, `walletprocesspsbt` return object will ALSO include the broadcast-able hex string if the tx is already final. This saves users the extra step of calling `finalizepsbt` assuming they have already inspected and approve the transaction from earlier steps.

ACKs for top commit:
  ismaelsadeeq:
    re ACK 2e249b9227
  BrandonOdiwuor:
    re ACK 2e249b9
  Randy808:
    Tested ACK 2e249b9227
  achow101:
    ACK 2e249b9227
  ishaanam:
    ACK 2e249b9227

Tree-SHA512: 229c1103265a9b4248f080935a7ad5607c3be3f9a096a9ab6554093b2cd8aa8b4d1fa55b1b97d3925ba208dbc3ccba4e4d37c40e1491db0d27ba3d9fe98f931e
2023-09-12 12:28:13 -04:00
Anthony Towns
971bae9174 rpc: Deprecate rpcserialversion=0 2023-09-11 17:21:53 +10:00
fanquake
c5a63ea56f
Merge bitcoin/bitcoin#27944: test: various USDT functional test cleanups (27831 follow-ups)
9f55773a37 test: refactor: usdt_mempool: store all events (stickies-v)
bc43270450 test: refactor: remove unnecessary nonlocal (stickies-v)
326db63a68 test: log sanity check assertion failures (stickies-v)
f5525ad680 test: store utxocache events (stickies-v)
f1b99ac94f test: refactor: deduplicate handle_utxocache_* logic (stickies-v)
ad90ba36bd test: refactor:  rename inbound to is_inbound (stickies-v)
afc0224cdb test: refactor: remove unnecessary blocks_checked counter (stickies-v)

Pull request description:

  Various cleanups to the USDT functional tests, largely (but not exclusively) follow-ups to https://github.com/bitcoin/bitcoin/pull/27831#pullrequestreview-1491438045. Except for slightly different logging behaviour in "test: store utxocache events" and "test: log sanity check assertion failures", this is a refactor PR, removing unnecessary code and (imo) making it more readable and maintainable.

  The rationale for each change is in the corresponding commit message.

  Note: except for "test: store utxocache events" (which relies on its parent, and I separated into two commits because we may want the parent but not the child), all commits are stand-alone and I'm okay with dropping one/multiple commits if they turn out to be controversial or undesired.

ACKs for top commit:
  0xB10C:
    ACK 9f55773a37. Reviewed the code and ran the USDT interface tests. I stepped through the commits and think all changes are reasonable.

Tree-SHA512: 6c37a0265b6c26d4f9552a056a690b8f86f7304bd33b4419febd8b17369cf6af799cb87c16df35d0c2a1b839ad31de24661d4384eafa88816c2051c522fd3bf5
2023-09-10 14:10:16 +01:00
fanquake
0354206a30
Merge bitcoin/bitcoin#28412: test: remove unused variables in p2p_invalid_block
3eb03803c4 test: remove unused variables in `p2p_invalid_block` (brunoerg)

Pull request description:

ACKs for top commit:
  stickies-v:
    ACK 3eb03803c4

Tree-SHA512: eadae1eb323e5562d1ea0aed43ebf0145f0fdbb6cd6d4646bbf1ca89f384820e7b9cb69f0bb04a949e9f8983a879aee8387d6f7ac3d4e4ea027f8892e656fb98
2023-09-07 16:10:14 +01:00
fanquake
cf421820f5
Merge bitcoin/bitcoin#28409: test: Combine sync_send_with_ping and sync_with_ping
fae0b21e6c test: Combine sync_send_with_ping and sync_with_ping (MarcoFalke)

Pull request description:

  This reduces bloat, complexity, and makes tests less fragile to intermittent failures, see https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1315648343.

  This should not cause any noticeable slowdown, or may even be faster, because active polling will be done at most once.

ACKs for top commit:
  glozow:
    Concept ACK fae0b21e6c
  theStack:
    ACK fae0b21e6c 🏓

Tree-SHA512: 6c543241a7b85458dc7ff6a6203316b80a6227d83d38427e74f53f4c666a882647a8a221e5259071ee7bb5dfd63476fb03c9b558a1ea546734b14728c3c619ba
2023-09-06 17:08:33 +01:00
ismaelsadeeq
4614332fc4
test: remove unnecessary finalizepsbt rpc calls 2023-09-06 11:52:19 -04:00
Matthew Zipkin
e3d484b603
wallet rpc: return final tx hex from walletprocesspsbt if complete 2023-09-05 09:14:32 -04:00
brunoerg
3eb03803c4 test: remove unused variables in p2p_invalid_block 2023-09-05 09:09:45 -03:00
fanquake
ecab855838
Merge bitcoin/bitcoin#28195: blockstorage: Drop legacy -txindex check
fae405556d scripted-diff: Rename CBlockTreeDB -> BlockTreeDB (MarcoFalke)
faf63039cc Fixup style of moved code (MarcoFalke)
fa65111b99 move-only: Move CBlockTreeDB to node/blockstorage (MarcoFalke)
fa8685597e index: Drop legacy -txindex check (MarcoFalke)
fa69148a0a scripted-diff: Use blocks_path where possible (MarcoFalke)

Pull request description:

  The only reason for the check was to print a warning about an increase in storage use. Now that 22.x is EOL and everyone should have migrated (or decided to not care about storage use), remove the check.

  Also, a move-only commit is included. (Rebased from https://github.com/bitcoin/bitcoin/pull/22242)

ACKs for top commit:
  TheCharlatan:
    ACK fae405556d, though I lack historical context to really judge the second commit fa8685597e.
  stickies-v:
    ACK fae405556d

Tree-SHA512: 9da8f48767ae52d8e8e21c09a40c949cc0838794f1856cc5f58a91acd3f00a3bca818c8082242b3fdc9ca5badb09059570bb3870850d3807b75a8e23b5222da1
2023-09-05 11:37:35 +01:00
MarcoFalke
fae0b21e6c
test: Combine sync_send_with_ping and sync_with_ping 2023-09-05 12:11:10 +02:00
fanquake
c004ba48f4
Merge bitcoin/bitcoin#28386: test: remove fixed timeouts from feature_config_args
fbcacd4cf0 test: remove fixed timeouts from feature_config_args (Martin Zumsande)

Pull request description:

  Fixes #28290

  These fixed timeouts aren't affected by the `timeout_factor` option and can therefore cause timeouts in slow environments.
  They are also unnecessary for the test because they measure the wrong thing:
  While there is an internal waiting time of 60s within `ThreadOpenConnections` (beginning only when that thread is started) for fixed seeds querying, the timeouts here don't measure that but the time from startup until a debug log message is encountered, during which many other things happen in init, so they don't make much sense to me in the first place.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK fbcacd4cf0

Tree-SHA512: 7bb3b7db2f9666b1929ffb7773c838ee98b0845569428e5d00ecf5234973d534c4f474e213896c71baabd6096a79347bd21b41a17b130053049714eb8a447c79
2023-09-05 09:59:33 +01:00
fanquake
fb619e1bc3
Merge bitcoin/bitcoin#28396: test: p2p: check that getaddr msgs are only responded once per connection
668aa6af8d test: p2p: check that `getaddr` msgs are only responded once per connection (Sebastian Falbesoner)

Pull request description:

  This simple PR adds missing test coverage for ignoring repeated `getaddr` requests (introduced in #7856, commit 66b07247a7):
  6f03c45f6b/src/net_processing.cpp (L4642-L4648)

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 668aa6af8d
  brunoerg:
    crACK 668aa6af8d

Tree-SHA512: edcdc6501c684fb41911e393f55ded9b044cd2f92918877eca152edd5a4287d1a9d57ae999f1cb42185eae00c3a0af411fcb9bcd5b990ef48849c3834b141584
2023-09-05 09:58:50 +01:00
fanquake
337d6f35a2
Merge bitcoin/bitcoin#28291: rpc: removed StrFormatInternalBug quote delimitation
6e8f6468cb removed StrFormatInternalBug quote delimitation (Reese Russell)

Pull request description:

  This PR rectifies an unnecessary set of quotes delimiting the contents of  ```StrFormatInternalBug```. This is a follow up to MarcoFalke https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1297191493.  The method of action was to remove the escaped quotes that were a part of strprintf. A single functional test case was modified to reflect the new output format.

  ```STR_INTERNAL_BUG``` was applied to https://github.com/bitcoin/bitcoin/pull/28123 in ```std::string RPCArg::ToString(const bool oneline)``` in ```rpc/util.cpp```

  The results can be seen below.

  Previously
  ![image](https://github.com/bitcoin/bitcoin/assets/3104223/53f9ea59-317f-4c62-9fc1-04255eeb4641)

  This PR
  ![image](https://github.com/bitcoin/bitcoin/assets/3104223/5c6a3110-f1f3-4b3c-8e8a-9c8f1c3176e7)

  Additional context can be found here.
  https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271871716

  Thank you.

ACKs for top commit:
  MarcoFalke:
    review ACK 6e8f6468cb
  stickies-v:
    ACK 6e8f6468cb

Tree-SHA512: 35317e31a527630495b566407e37db9941dab7f81cfaeb1ea3309683c48e4273284645ad615f73e646a137b4f2ae35933603e9182a7dbdd22cac98d038c491dc
2023-09-05 11:41:02 +03:00
Sebastian Falbesoner
668aa6af8d test: p2p: check that getaddr msgs are only responded once per connection 2023-09-03 14:19:41 +02:00
Martin Zumsande
fbcacd4cf0 test: remove fixed timeouts from feature_config_args
They cannot be scaled by the timeout_factor option and can
therefore cause timeouts in slow environments.
They are also not necessary for the test, since they measure time
frome startup until a debug message is encountered, which
is not restricted to 1 minute by any internal logic within bitcoind.
2023-09-01 14:48:55 -04:00
Andrew Chow
5666966dff
Merge bitcoin/bitcoin#28364: log: log wtxids when possible, add TXPACKAGES category
a3b55c94b9 [doc] move comment about AlreadyHaveTx DoS score to the right place (glozow)
3b8c17838a [log] add more logs related to orphan handling (glozow)
51b3275cd1 [log] add category TXPACKAGES for orphanage and package relay (glozow)
a33dde1e41 [log] include wtxid in tx {relay,validation,orphanage} logging (glozow)

Pull request description:

  This was taken from #28031 (see #27463 for project tracking).

  - Log wtxids in addition to txids when possible. This allows us to track the fate of a transaction from inv to mempool accept/reject through logs.
  - Additional orphan-related logging to make testing and debugging easier. Suggested in https://github.com/bitcoin/bitcoin/pull/28031#pullrequestreview-1531022386 and https://github.com/bitcoin/bitcoin/pull/28031#discussion_r1269622220
  - Add `TXPACKAGES` category for logging.
  - Move a nearby comment block that was in the wrong place.

ACKs for top commit:
  instagibbs:
    reACK a3b55c94b9
  achow101:
    ACK a3b55c94b9
  brunoerg:
    crACK a3b55c94b9
  mzumsande:
    Code review ACK a3b55c94b9

Tree-SHA512: 21884ef7c2ea2fd006e715574a9dd3e6cbbe8f82d62c6187fe1d39aad5a834051203fda5f355a06ca40c3e2b9561aec50d7c922a662b1edc96f7b552c9f4b24d
2023-08-31 12:55:38 -04:00
fanquake
1348454d82
Merge bitcoin/bitcoin#28352: test: Support powerpc64le in get_previous_releases.py
faf7e69862 test: Support powerpc64le in get_previous_releases.py (MarcoFalke)

Pull request description:

  To test: `test/get_previous_releases.py -b -t /tmp/prev_releases v22.0`

  On master: `Not sure which binary to download for powerpc64le-unknown-linux-gnu`
  Here: (pass)

ACKs for top commit:
  fanquake:
    ACK faf7e69862

Tree-SHA512: 33d9348f99e0d3924a6a5cba8833ec9e413e80167012b557922f3628069dabd555b02f98a6bfd0eb80e2bbbcdb50865b7bca216e1d080b1546ee4812abda4bc2
2023-08-30 16:16:44 +01:00
glozow
a33dde1e41 [log] include wtxid in tx {relay,validation,orphanage} logging 2023-08-29 16:41:22 +01:00
MarcoFalke
faf7e69862
test: Support powerpc64le in get_previous_releases.py 2023-08-27 22:28:57 +02:00
Jon Atack
6c008a2006 script: replace deprecated pkg_resources with importlib.metadata
in our python linter:

```
./test/lint/lint-python.py:12: DeprecationWarning: pkg_resources is deprecated as an API.
  See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
```

The importlib.metadata library was added in Python 3.8, which is currently our
minimum-supported Python version.

For more details about importlib.metadata, see https://docs.python.org/3/library/importlib.metadata.html
2023-08-26 16:42:02 -06:00
Andrew Chow
c9273f68f6
Merge bitcoin/bitcoin#28287: rpc, test: add sendmsgtopeer rpc and a test for net-level deadlock situation
b3a93b409e test: add functional test for deadlock situation (Martin Zumsande)
3557aa4d0a test: add basic tests for sendmsgtopeer to rpc_net.py (Martin Zumsande)
a9a1d69391 rpc: add test-only sendmsgtopeer rpc (Martin Zumsande)

Pull request description:

  This adds a `sendmsgtopeer` rpc (for testing only) that allows a node to send a message (provided in hex) to a peer.
  While we would usually use a `p2p` object instead of a node for this in the test framework, that isn't possible in situations where this message needs to trigger an actual interaction of multiple nodes.

  Use this rpc to add test coverage for the bug fixed in #27981 (that just got merged):
  The test lets two nodes (almost) simultaneously send a single large (4MB) p2p message to each other, which would have caused a deadlock previously (making this test fail), but succeeds now.

  As can be seen from the discussion in #27981, it was not easy to reproduce this bug without `sendmsgtopeer`. I would imagine that `sendmsgtopeer` could also be helpful in various other test constellations.

ACKs for top commit:
  ajtowns:
    ACK b3a93b409e
  sipa:
    ACK b3a93b409e
  achow101:
    ACK b3a93b409e

Tree-SHA512: 6e22e72402f3c4dd70cddb9e96ea988444720f7a164031df159fbdd48056c8ac77ac53def045d9208a3ca07437c7c8e34f8b4ebc7066c0a84d81cd53f2f4fa5f
2023-08-24 17:34:57 -04:00
fanquake
7ee7c6d2c3
Merge bitcoin/bitcoin#28332: test: previous releases: speed up fetching sources with shallow clone
360ac64b90 test: previous releases: speed up fetching sources with shallow clone (Sebastian Falbesoner)

Pull request description:

  For the sake of building previous releases, fetching the whole history of the repository for each version seems to be overkill as it takes much more time, bandwidth and disk space than necessary. Create a shallow clone instead with history truncated to the one commit of the version tag, which is directly checked out in the same command. This has the nice side-effect that we can remove the extra `git checkout` step after as it's not needed anymore.

  Note that it might look confusing to pass a _tag_ to a parameter named `--branch`, but the git-clone manpage explicitly states that this is supported.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 360ac64b90

Tree-SHA512: c885a695c1ea90895cf7a785540c24e8ef8d1d9ea78db28143837240586beb6dfb985b8b0b542d2f64e2f0ffdca7c65fc3d55f44b5e1b22cc5535bc044566f86
2023-08-24 10:37:41 +01:00
fanquake
e8989f295d
Merge bitcoin/bitcoin#28325: test: wallet_backup.py, fix intermittent failure in "restore using dumped wallet"
c4929cfa50 test: wallet_backup.py, fix intermittent failure in "restore using dumped wallet" (furszy)

Pull request description:

  Aiming to fix #25652.

  The failure arises because the test expects `init_wallet()` (the test framework function) to create a wallet with no keys. However, the function also imports the deterministic private key used to receive the coinbase coins.

  This causes a race within the "restore using dumped wallet" case, where we intend to have a new wallet (with no existing keys) to test the 'importwallet()' RPC result.
  The reason why this failure is intermittent is that it depends on other peers delivering the chain right after node2 startup and prior to the test 'node2.getbalance()' call and also the synchronization of the validation queue.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK c4929cfa50

Tree-SHA512: 80faa590439305576086a7d6e328f2550c97b218771fc5eba0567feff78732a2605d028a30a368d50944ae3d25fdbd6d321fb97321791a356416f2b790999613
2023-08-24 10:30:58 +01:00
fanquake
e0ad847073
Merge bitcoin/bitcoin#28321: test: Fix intermittent issue in mempool_reorg
fa5cc3ccfb test: Fix intermittent issue in mempool_reorg (MarcoFalke)

Pull request description:

  Currently the test case may fail intermittently, see https://github.com/bitcoin/bitcoin/issues/28313

  Fix this by changing a number and reducing the failure rate a bit.

ACKs for top commit:
  glozow:
    ACK fa5cc3ccfb

Tree-SHA512: ff552111b434ca712c7dbdc5ba32a986a6fa4512cba5a756234eae69428063bf6ecfdc8f350ee84226ed4d3e4262b4639dbe49162a722e8da85f0d61e5690c51
2023-08-24 10:16:13 +01:00
Sebastian Falbesoner
360ac64b90 test: previous releases: speed up fetching sources with shallow clone
For the sake of building previous releases, fetching the whole history
of the repository for each version seems to be overkill as it takes much
more time, bandwidth and disk space than necessary. Create a shallow
clone instead with history truncated to the one commit of the version
tag, which is directly checked out in the same command. This has the
nice side-effect that we can remove the extra `git checkout` step after
as it's not needed anymore.

Note that it might look confusing to pass a _tag_ to a parameter named
`--branch`, but the git-clone manpage explicitly states that this is
supported.
2023-08-24 02:37:36 +02:00
Andrew Chow
8ff90d9dcf
Merge bitcoin/bitcoin#26291: Update MANDATORY_SCRIPT_VERIFY_FLAGS
1b09cc5959 Make post-p2sh consensus rules mandatory for tx relay (Anthony Towns)
69c31bc748 doc, policy: Clarify comment on STANDARD_SCRIPT_VERIFY_FLAGS (Anthony Towns)

Pull request description:

  The `MANDATORY_SCRIPT_VERIFY_FLAGS` constant was introduced in #3843 to distinguish between block consensus rules and relay standardness rules. However it was not actually used in the consensus code path: instead it only differentiates between the failure being reported as `TX_CONSENSUS` and `mandatory-script-verify-flag-failed` vs `TX_NOT_STANDARD` and `non-mandatory-script-verify-flag`.

  This updates the list of mandatory flags to include the post-p2sh soft forks that are enforced as consensus rules via `GetBlockScriptFlags()`. The effect of this change is that validation.cpp will report `TX_CONSENSUS` failures for txs that fail dersig/csv/cltv/nulldummy/witness/taproot checks, instead of `TX_NOT_STANDARD`, which in turn adds `Misbehaving(100)` via `MaybePunishNodeForTx` in `net_processing`.

ACKs for top commit:
  Sjors:
    Code review ACK 1b09cc5959
  darosior:
    ACK 1b09cc5959
  achow101:
    ACK 1b09cc5959
  theStack:
    Concept and code-review ACK 1b09cc5959

Tree-SHA512: d3e5868e8cece478f2e934956ba0c231d8bb9c2daefd0df1f817774e292049902cfc1d0cd76dbd2e7722627a93eab2d7046ff678199aac70a2b01642e69349f1
2023-08-23 16:19:39 -04:00
furszy
c4929cfa50
test: wallet_backup.py, fix intermittent failure in "restore using dumped wallet"
The failure arises because the test expects 'init_wallet()' (the test
framework function) creating a wallet with no keys. However, the function
also imports the deterministic private key used to receive the coinbase coins.

This causes a race within the "restore using dumped wallet" case, where we
intend to have a new wallet (with no existing keys) to test the
'importwallet()' RPC result.
The reason behind the intermittent failures might be other peers delivering
the chain right after node2 startup (sync of the validation queue included)
and prior to the 'node2.getbalance()' check.
2023-08-23 09:41:24 -03:00
Andrew Chow
5aa67eb365
Merge bitcoin/bitcoin#28199: test: tx orphan handling
9eac5a0529 [functional test] transaction orphan handling (glozow)
61e77bb901 [test framework] make it easier to fast-forward setmocktime (glozow)

Pull request description:

  I was doing some mutation testing (through reckless refactoring) locally and found some specific behaviors in orphan handling that weren't picked up by tests. Adding some of these test cases now can maybe help with reviewing refactors like #28031.

  - Parent requests aren't sent immediately. A delay is added and the requests are filtered by AlreadyHaveTx before they are sent, which means you can't use fake orphans to probe precise arrival timing of a tx.
  - Parent requests include all that are not AlreadyHaveTx. This means old confirmed parents may be requested.
  - The node does not give up on orphans if the peer responds to a parent request with notfound. This means that if a parent is an old confirmed transaction (in which notfound is expected), the orphan should still be resolved.
  - Rejected parents can cause an orphan to be dropped, but it depends on the reason and only based on txid.
  - Rejected parents can cause an orphan to be rejected too, by both wtxid and txid.
  - Requests for orphan parents should be de-duplicated with "regular" txrequest. If a missing parent has the same hash as an in-flight request, it shouldn't be requested.
  - Multiple orphans with overlapping parents should not cause duplicated parent requests.

ACKs for top commit:
  instagibbs:
    reACK 9eac5a0529
  dergoegge:
    reACK 9eac5a0529
  achow101:
    ACK 9eac5a0529
  fjahr:
    Code review ACK 9eac5a0529

Tree-SHA512: 85488dc6a3f62cf0c38e7dfe7839c01215b44b172d1755b18164d41d01038f3a749451241e4eba8b857fd344a445740b21d6382c45977234b21460e3f53b1b2a
2023-08-22 17:03:37 -04:00
Martin Zumsande
b3a93b409e test: add functional test for deadlock situation 2023-08-22 13:45:26 -04:00
Martin Zumsande
3557aa4d0a test: add basic tests for sendmsgtopeer to rpc_net.py 2023-08-22 13:28:15 -04:00
fanquake
38db2bd4e1
Merge bitcoin/bitcoin#28320: test: Support riscv64 in get_previous_releases.py
2222e15771 test: Support riscv64 in get_previous_releases.py (MarcoFalke)

Pull request description:

  To test: `test/get_previous_releases.py -b -t /tmp/prev_releases v0.18.1`

  On master: `Not sure which binary to download for riscv64-unknown-linux-gnu`
  Here: (pass)

ACKs for top commit:
  fanquake:
    ACK 2222e15771

Tree-SHA512: 18dc9a6c65f78adb5f7fc09e57db34c6b544071cb7bb3fa2846c86a23202e37d6ea1c5aca9acc1c2040b7d2b97bb93840a8a949f81f71fe6f01c395d2894739d
2023-08-22 10:35:13 +01:00
fanquake
00fc7cdc25
Merge bitcoin/bitcoin#28200: refactor: Remove unused includes from wallet.cpp
fa6286891f Remove unused includes from wallet.cpp (MarcoFalke)
fa8fdbe229 Remove unused includes from blockfilter.h (MarcoFalke)
fad8c36aa9 move-only: Create src/kernel/mempool_removal_reason.h (MarcoFalke)
fa57608800 Remove unused includes from txmempool.h (MarcoFalke)

Pull request description:

  This makes compilation of wallet.cpp use a few % less memory and time, locally.

  Created in the context of https://github.com/bitcoin/bitcoin/issues/28109, but I don't think it is enough to actually fix this problem.

ACKs for top commit:
  hebasto:
    ACK fa6286891f, I have reviewed the code and it looks OK.

Tree-SHA512: 06f1120af2a8ef3368dbd9ae747acda88ace2507bd261bcc10341d476a0b3d71c8485377ea6c108b47df3e4c13b7f75a15f486bafa6a8466303168dde16ebbc8
2023-08-22 10:34:10 +01:00
fanquake
c00bc63061
Merge bitcoin/bitcoin#28288: test: fix 'unknown named parameter' test in wallet_basic
452c094449 test: fix 'unknown named parameter' test in `wallet_basic` (brunoerg)

Pull request description:

  This PR removes loop when testing an unknown named parameter. They don't have any effect.

ACKs for top commit:
  jonatack:
    ACK 452c094449
  theStack:
    re-ACK 452c094449

Tree-SHA512: cf1a37d738bb6fdf9817e7b1d33bc69643dae61e3dbfae5c1e9f26220c55db6f134018dd9a1c65c13869ee58bcb6f3337c5999aabf2614d3126fbc01270705e8
2023-08-22 10:15:37 +01:00
MarcoFalke
fa5cc3ccfb
test: Fix intermittent issue in mempool_reorg 2023-08-22 11:08:46 +02:00
MarcoFalke
2222e15771
test: Support riscv64 in get_previous_releases.py 2023-08-22 10:32:33 +02:00
glozow
a84dade1f9
Merge bitcoin/bitcoin#28157: test doc: tests acceptstalefeeestimates option is only supported on regtest chain
ee5a0369cc test: ensure acceptstalefeeestimates is supported only on regtest chain (ismaelsadeeq)
22d5d4b2b2 tx fees, policy: doc: update and delete unnecessary comment (ismaelsadeeq)

Pull request description:

  This PR Follow up comments from [#27622](https://github.com/bitcoin/bitcoin/pull/27622)

  It test that the new `regtest-only` option `acceptstalefeeestimates` is not supported on [main, signet and test chains](https://github.com/bitcoin/bitcoin/pull/27622/files#r1235218268), removes an unnecessary [comment](https://github.com/bitcoin/bitcoin/pull/27622/files#r1235204323), and update fee estimator  `MAXFILEAGE` [description comment](https://github.com/bitcoin/bitcoin/pull/27622/files#r1233887314).

ACKs for top commit:
  jonatack:
    ACK ee5a0369cc
  glozow:
    utACK ee5a0369cc

Tree-SHA512: 4755f25b08db62f37614ea768272b12580ee0d481fb7fa339379901a6132c66828777c6747d3fe67490ceace3a6ff248bf13bdf65720f6e5ba8642eb762acd3c
2023-08-22 09:17:12 +01:00
brunoerg
452c094449 test: fix 'unknown named parameter' test in wallet_basic
Fixes loop when testing an unknown named parameter.
2023-08-21 20:59:15 -03:00
ismaelsadeeq
ee5a0369cc test: ensure acceptstalefeeestimates is supported only on regtest chain 2023-08-21 07:21:34 +01:00
fanquake
7bf078f2b7
Merge bitcoin/bitcoin#28237: refactor: Enforce C-str fmt strings in WalletLogPrintf()
fa60fa3b0c bitcoin-tidy: Apply bitcoin-unterminated-logprintf to spkm as well (MarcoFalke)
faa11434fe refactor: Enable all clang-tidy plugin bitcoin tests (MarcoFalke)
fa6dc57760 refactor: Enforce C-str fmt strings in WalletLogPrintf() (MarcoFalke)
fa244f3321 doc: Fix bitcoin-unterminated-logprintf tidy comments (MarcoFalke)

Pull request description:

  All fmt functions only accept a raw C-string as argument.

  There should never be a need to pass a format string that is not a compile-time string literal, so disallow it in `WalletLogPrintf()` to avoid accidentally introducing it.

  Apart from consistency, this also fixes the clang-tidy plugin bug https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1286821141.

ACKs for top commit:
  theuni:
    ACK fa60fa3b0c

Tree-SHA512: fa6f4984c50f9b34e850bdfee7236706af586e512d866cc869cf0cdfaf9aa707029c210ca72d91f85e75fcbd8efe0d77084701de8c3d2004abfd7e46b6fa9072
2023-08-18 11:38:38 +01:00
MarcoFalke
fa6286891f
Remove unused includes from wallet.cpp
This removes unused includes, such as undo.h or txmempool.h from
wallet.cpp.

Also, add missing ones, according to IWYU.
2023-08-18 08:20:43 +02:00