Commit graph

43502 commits

Author SHA1 Message Date
MarcoFalke
fae76393bd
test: Avoid F541 (f-string without any placeholders) 2024-12-05 08:39:09 +01:00
Ava Chow
e8cc790fe2
Merge bitcoin/bitcoin#30445: test: addrman: tried 3 times and never a success so isTerrible=true
Some checks are pending
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
1807df3d9f test: addrman: tried 3 times and never a success so `isTerrible=true` (brunoerg)

Pull request description:

  This PR adds test coverage for the following verification:
  ```cpp
  if (TicksSinceEpoch<std::chrono::seconds>(m_last_success) == 0 && nAttempts >= ADDRMAN_RETRIES) { // tried N times and never a success
      return true;
  }
  ```

  If we've tried an address for 3 or more times and were unsuccessful, this address should be pointed out as "terrible".

  -------

  You can test this by applying:
  ```diff
  diff --git a/src/addrman.cpp b/src/addrman.cpp
  index 054a9bee32..93a9521b59 100644
  --- a/src/addrman.cpp
  +++ b/src/addrman.cpp
  @@ -81,7 +81,7 @@ bool AddrInfo::IsTerrible(NodeSeconds now) const
       }

       if (TicksSinceEpoch<std::chrono::seconds>(m_last_success) == 0 && nAttempts >= ADDRMAN_RETRIES) { // tried N times and never a success
  -        return true;
  +        return false;
       }
  ```

ACKs for top commit:
  jonatack:
    re-ACK 1807df3d9f
  naumenkogs:
    ACK 1807df3d9f
  achow101:
    ACK 1807df3d9f

Tree-SHA512: e3cc43c98bddfe90f585d5b4bd00543be443b77ecaf038615261aa8cc4d14fc2f1006d0b00c04188040eaace455c5c6dbb3bb200a2c0f29c3b4ef5128bf0973a
2024-12-04 15:34:59 -05:00
Matthew Zipkin
f9650e18ea
rbf: remove unecessary newline at end of error string 2024-12-04 14:37:48 -05:00
Matthew Zipkin
221c789e91
rpc: include verbose reject-details field in testmempoolaccept response 2024-12-04 14:37:37 -05:00
Ryan Ofsky
0184d33b3d scripted-diff: Replace strprintf(Untranslated) with Untranslated(strprintf)
This makes code more consistent and makes it easier to add compile-time checking to
enforce that format strings contain the right specifiers, because it stops
using Untranslated() to create the format string, so the Untranslated()
function will not need to get involved in formatting.

-BEGIN VERIFY SCRIPT-
quote='"[^"]+"'
quotes="(?:$quote|\\s)*"
nonparens="[^()]*"
single_level_paren="\($nonparens\)"
double_level_paren="\($nonparens\($nonparens\)$nonparens\)"
exprs="(?:$double_level_paren|$single_level_paren|$nonparens)*"
git grep -l 'Untranslated' | xargs perl -0777 -i -pe "s/strprintf\((\\W*)Untranslated\(($quotes)\)($exprs)(\))/Untranslated(\1strprintf(\2\3))/gs"
-END VERIFY SCRIPT-
2024-12-04 15:09:05 -04:00
Ryan Ofsky
17372d788e
Merge bitcoin/bitcoin#30906: refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests
Some checks are pending
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
50cce20013 test, refactor: Compact ccoins_access and ccoins_spend (Lőrinc)
0a159f0914 test, refactor: Remove remaining unbounded flags from coins_tests (Lőrinc)
c0b4b2c1ee test: Validate error messages on fail (Lőrinc)
d5f8d607ab test: Group values and states in tests into CoinEntry wrappers (Lőrinc)
ca74aa7490 test, refactor: Migrate GetCoinsMapEntry to return MaybeCoin (Lőrinc)
15aaa81c38 coins, refactor: Remove direct GetFlags access (Lőrinc)
6b733699cf coins, refactor: Assume state after SetClean in AddFlags to prevent dangling pointers (Lőrinc)
fc8c282022 coins, refactor: Make AddFlags, SetDirty, SetFresh static (Lőrinc)
cd0498eabc coins, refactor: Split up AddFlags to remove invalid states (Lőrinc)

Pull request description:

  Similarly to https://github.com/bitcoin/bitcoin/pull/30849, this cleanup is intended to de-risk https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1739909068 by simplifying the coin cache public interface.

  `CCoinsCacheEntry` provided general access to its internal flags state, even though, in reality, it could only be `clean`, `fresh`, `dirty`, or `fresh|dirty` (in the follow-up, we will remove `fresh` without `dirty`).

  Once it was marked as `dirty`, we couldn’t set the state back to clean with `AddFlags(0)`—tests explicitly checked against that.

  This PR refines the public interface to make this distinction clearer and to make invalid behavior impossible, rather than just checked by tests. We don't need extensive access to the internals of `CCoinsCacheEntry`, as many tests were simply validating invalid combinations in this way.

  The last few commits contain significant test refactorings to make `coins_tests` easier to change in follow-ups.

ACKs for top commit:
  andrewtoth:
    Code Review ACK 50cce20013
  laanwj:
    Code review ACK 50cce20013
  ryanofsky:
    Code review ACK 50cce20013. Looks good! Thanks for the followups.

Tree-SHA512: c0d65f1c7680b4bb9cd368422b218f2473c2ec75a32c7350a6e11e8a1601c81d3c0ae651b9f1dae08400fb4e5d43431d9e4ccca305a718183f9a936fe47c1a6c
2024-12-04 14:09:05 -05:00
Ryan Ofsky
006e4d1d59 refactor: Use + instead of strformat to concatenate translated & untranslated strings
This change manually removes two strprintf(Untranslated...) calls. All
remaining calls are removed in the next scripted-diff commit.

Removing these calls makes code more consistent and makes it easier to
implement compile-time checking enforcing that format strings contain valid
specifiers, by avoiding the need for the Untranslated() function to be involved
in formatting.

Additionally, using + and += instead of strprintf here makes code a little
shorter, and more type-safe because + unlike strprintf only works on strings of
the same type, making it less likely english strings and bilingual strings will
be unintentionally combined.
2024-12-04 15:09:05 -04:00
Ryan Ofsky
831d2bfcf9 refactor: Don't embed translated string in untranslated string.
This could produce an english error message containing non-english string
fragments if PopulateAndValidateSnapshot started returning any translated
strings in the future. This change is also needed to make the next
scripted-diff commit work.
2024-12-04 15:09:05 -04:00
Ryan Ofsky
058021969b refactor: Avoid concatenation of format strings
Instead just concatenate already formatted strings. This allows untranslated
format strings to be checked at compile time now, and translated format strings
to be checked at compile time in #31061.
2024-12-04 15:09:05 -04:00
Ava Chow
11f68cc810
Merge bitcoin/bitcoin#31212: util: Improve documentation and negation of args
95a0104f2e test: Add tests for directories in place of config files (Hodlinator)
e85abe92c7 args: Catch directories in place of config files (Hodlinator)
e4b6b1822c test: Add tests for -noconf (Hodlinator)
483f0dacc4 args: Properly support -noconf (Hodlinator)
312ec64cc0 test refactor: feature_config_args.py - Stop nodes at the end of tests, not at the beginning (Hodlinator)
7402658bc2 test: -norpccookiefile (Hodlinator)
39cbd4f37c args: Support -norpccookiefile for bitcoind and bitcoin-cli (Hodlinator)
e82ad88452 logs: Use correct path and more appropriate macros in cookie-related code (Hodlinator)
6e28c76907 test: Harden testing of cookie file existence (Hodlinator)
75bacabb55 test: combine_logs.py - Output debug.log paths on error (Hodlinator)
bffd92f00f args: Support -nopid (Hodlinator)
12f8d848fd args: Disallow -nodatadir (Hodlinator)
6ff9662760 scripted-diff: Avoid printing version information for -noversion (Hodlinator)
e8a2054edc doc args: Document narrow scope of -color (Hodlinator)

Pull request description:

  - Document `-color` as only applying to `-getinfo`, to be less confusing for bitcoin-cli users.
  - No longer print version information when getting passed `-noversion`.
  - Disallow `-nodatadir` as we cannot run without one. It was previously interpreted as a mix of unset and as a relative path of "0".
  - Support `-norpccookiefile`
  - Support `-nopid`
  - Properly support `-noconf` (instead of working by accident). Also detect when directories are specified instead of files.

  Prompted by investigation in https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2316714013.

ACKs for top commit:
  l0rinc:
    utACK 95a0104f2e
  achow101:
    ACK 95a0104f2e
  ryanofsky:
    Code review ACK 95a0104f2e. Looks good! Thanks for all your work on this breaking the changes down and making them simple.

Tree-SHA512: 5174251e6b9196a9c6d135eddcb94130295c551bcfccc78e633d9e118ff91523b1be0d72828fb49603ceae312e6e1f8ee2651c6a2b9e0f195603a73a9a622785
2024-12-04 13:20:46 -05:00
merge-script
893ccea7e4
Merge bitcoin/bitcoin#31419: test: fix MIN macro redefinition
00c1dbd26d test: fix MIN macro-redefinition (0xb10c)

Pull request description:

  Renames the `MIN` macro to `_TRACEPOINT_TEST_MIN`.

  From #31418:

  ```
  stderr:
  /virtual/main.c:70:9: warning: 'MIN' macro redefined [-Wmacro-redefined]
     70 | #define MIN(a,b) ({ __typeof__ (a) _a = (a); __typeof__ (b) _b = (b); _a < _b ? _a : _b; })
        |         ^
  include/linux/minmax.h:329:9: note: previous definition is here
    329 | #define MIN(a,b) __cmp(min,a,b)
        |         ^
  1 warning generated.
  ```

  fixes: https://github.com/bitcoin/bitcoin/issues/31418

ACKs for top commit:
  maflcko:
    review ACK 00c1dbd26d

Tree-SHA512: 1d91ed8b3c0b0410d42a11004286359546c17613c3ff03dd51c26896ee050e9280fff69fa2eaa6e6085f9b611663bcacedec80997a6c5e37874a79ba86bfa507
2024-12-04 17:13:00 +00:00
Ryan Ofsky
39950e148d
Merge bitcoin/bitcoin#31295: refactor: Prepare compile-time check of bilingual format strings
fa3e074304 refactor: Tidy fixups (MarcoFalke)
fa72646f2b move-only: Detail_CheckNumFormatSpecifiers and G_TRANSLATION_FUN (MarcoFalke)
faff8403f0 refactor: Pick translated string after format (MarcoFalke)

Pull request description:

  The changes are required for https://github.com/bitcoin/bitcoin/pull/31061, however they also make sense on their own. For example, they are fixing up an `inline namespace`, which lead to compile errors otherwise (can be tested by observing the compile error after reverting the changes to `src/util/strencodings.h`). Also, a unit test comment is fixed.

ACKs for top commit:
  ryanofsky:
    Code review ACK fa3e074304. Nice changes! These should allow related PRs to be simpler.
  l0rinc:
    ACK fa3e074304
  hodlinator:
    cr-ACK fa3e074304

Tree-SHA512: 37371181a348610442186b5fbb7a6032d0caf70aae566002ad60be329a3131a2b89f28f6c51e10872079f987986925dc8c0611bde639057bee4f572d2b9ba92a
2024-12-04 11:15:58 -05:00
0xb10c
00c1dbd26d
test: fix MIN macro-redefinition
Renames the `MIN` macro to `_TRACEPOINT_TEST_MIN`.

From #31418:

```
stderr:
/virtual/main.c:70:9: warning: 'MIN' macro redefined [-Wmacro-redefined]
   70 | #define MIN(a,b) ({ __typeof__ (a) _a = (a); __typeof__ (b) _b = (b); _a < _b ? _a : _b; })
      |         ^
include/linux/minmax.h:329:9: note: previous definition is here
  329 | #define MIN(a,b) __cmp(min,a,b)
      |         ^
1 warning generated.
```

fixes: https://github.com/bitcoin/bitcoin/issues/31418
2024-12-04 15:51:17 +01:00
merge-script
ae69fc37e4
Merge bitcoin/bitcoin#31391: util: Drop boost posix_time in ParseISO8601DateTime
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
faf70cc994 Remove wallet::ParseISO8601DateTime, use ParseISO8601DateTime instead (MarcoFalke)
2222aecd5f util: Implement ParseISO8601DateTime based on C++20 (MarcoFalke)

Pull request description:

  `boost::posix_time` in `ParseISO8601DateTime` has many issues:

  * It parses random strings that are clearly invalid and returns a time value for them, see [1] below.
  * None of the separators `-`, or `:`, or `T`, or `Z` are validated.
  * It may crash when running under a hardened C++ library, see https://github.com/bitcoin/bitcoin/issues/28917.
  * It has been unmaintained for years, so reporting or fixing any issues will most likely be useless.
  * It pulls in a third-party dependency, when the functionality is already included in vanilla C++20.

  Fix all issues by replacing it with a simple helper function written in C++20.

  Fixes https://github.com/bitcoin/bitcoin/issues/28917.

  [1] The following patch passes on current master:

  ```diff
  diff --git a/src/wallet/test/rpc_util_tests.cpp b/src/wallet/test/rpc_util_tests.cpp
  index 32f6f5ab46..c1c94c7116 100644
  --- a/src/wallet/test/rpc_util_tests.cpp
  +++ b/src/wallet/test/rpc_util_tests.cpp
  @@ -12,6 +12,14 @@ BOOST_AUTO_TEST_SUITE(wallet_util_tests)

   BOOST_AUTO_TEST_CASE(util_ParseISO8601DateTime)
   {
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("964296"), 242118028800);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("244622"), 15023836800);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("+INfINITy"), 9223372036854);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("7000802 01"), 158734166400);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("7469-2 +INfINITy"), 9223372036854);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("maXimum-datE-time"), 253402300799);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("577737     114maXimum-datE-time"), 253402300799);
  +
       BOOST_CHECK_EQUAL(ParseISO8601DateTime("1970-01-01T00:00:00Z"), 0);
       BOOST_CHECK_EQUAL(ParseISO8601DateTime("1960-01-01T00:00:00Z"), 0);
       BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T00:00:01Z"), 946684801);
  ```

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

Tree-SHA512: 9dd745a356d04acf6200e13a6af52c51a9e2a0eeccea110093ce5da147b3c669c0eda918e46db0164c081a78c8feae3fe557a4759bea18449a8ff2d090095931
2024-12-04 11:21:43 +00:00
Sjors Provoost
52fd1511a7
test: drop scriptPubKeyIn arg from CreateNewBlock
This removes the temporary overload added in the previous commit.

Also drop unneeded custom coinbase output scripts.
2024-12-04 12:46:33 +07:00
Sjors Provoost
ff41b9e296
Drop script_pub_key arg from createNewBlock
Providing a script for the coinbase transaction is only done in test code
and for CPU solo mining.

Production miners use the getblocktemplate RPC which omits the coinbase
transaction entirely from its block template, leaving it to external (pool)
software to construct it.

A coinbase script can still be passed via BlockCreateOptions instead.

A temporary overload is added so that the test can be modified in the
next commit.
2024-12-04 12:44:57 +07:00
Sjors Provoost
7ab733ede4
rpc: rename coinbase_script to coinbase_output_script 2024-12-04 12:44:57 +07:00
Ava Chow
ff873a20a7
Merge bitcoin/bitcoin#31313: refactor: Clamp worker threads in ChainstateManager constructor
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
8f85d36d68 refactor: Clamp worker threads in ChainstateManager constructor (TheCharlatan)

Pull request description:

  This ensures the options are applied consistently from contexts where they might not pass through the args manager, such as in some tests, or when used through the kernel library.

  This is similar to the patch applied in 09ef322acc, used to make applying the mempool options consistent.

  ---

  This is part of the libbitcoinkernel project https://github.com/bitcoin/bitcoin/issues/27587

ACKs for top commit:
  maflcko:
    ACK 8f85d36d68 🛳
  achow101:
    ACK 8f85d36d68
  furszy:
    Code ACK 8f85d36d68
  stickies-v:
    ACK 8f85d36d68

Tree-SHA512: 32d7cc177d6726ee9df62ac9eb43e49ba676f35bfcff47834bd97a1e33f2a9ea7be65d0a8a37be149de04e58c9c500ecef730e498f4e3909042324d3136160e9
2024-12-03 18:02:37 -05:00
Ava Chow
c9a7418a8d
Merge bitcoin/bitcoin#31096: Package validation: accept packages of size 1
32fc59796f rpc: Allow single transaction through submitpackage (glozow)

Pull request description:

  There's no particular reason to restrict single transaction submissions with submitpackage. This change relaxes the RPC checks as enables the `AcceptPackage` flow to accept packages of a single transaction.

  Resolves #31085

ACKs for top commit:
  naumenkogs:
    ACK 32fc59796f
  achow101:
    ACK 32fc59796f
  glozow:
    ACK 32fc59796f

Tree-SHA512: ffed353bfdca610ffcfd53b40b76da05ffc26df6bac4b0421492e067bede930380e03399d2e2d1d17f0e88fb91cd8eb376e3aabebbabcc724590bf068d09807c
2024-12-03 17:46:23 -05:00
Ava Chow
6f24662eb9
Merge bitcoin/bitcoin#31175: rpc: Remove submitblock pre-checks
73db95c65c kernel: Make bitcoin-chainstate's block validation mirror submitblock's (TheCharlatan)
bb53ce9bda tests: Add functional test for submitting a previously pruned block (Greg Sanders)
1f7fc73825 rpc: Remove submitblock duplicate pre-check (TheCharlatan)
e62a8abd7d rpc: Remove submitblock invalid-duplicate precheck (TheCharlatan)
36dbebafb9 rpc: Remove submitblock coinbase pre-check (TheCharlatan)

Pull request description:

  With the introduction of a mining ipc interface and the potential future introduction of a kernel library API it becomes increasingly important to offer common behaviour between them. An example of this is ProcessNewBlock, which is used by ipc, rpc, net_processing and (potentially) the kernel library. Having divergent behaviour on suggested pre-checks and checks for these functions is confusing to both developers and users and is a maintenance burden.

  The rpc interface for ProcessNewBlock (submitblock) currently pre-checks if the block has a coinbase transaction and whether it has been processed before. While the current example binary for how to use the kernel library, bitcoin-chainstate, imitates these checks, the other interfaces do not.

  The coinbase check is repeated again early during ProcessNewBlock. Pre-checking it may also shadow more fundamental problems with a block. In most cases the block header is checked first, before validating the transactions. Checking the coinbase first therefore masks potential issues with the header. Fix this by removing the pre-check.

  Similary the duplicate checks are repeated early in the contextual checks of ProcessNewBlock. If duplicate blocks are detected much of their validation is skipped. Depending on the constitution of the block, validating the merkle root of the block is part of the more intensive workload when validating a block. This could be an argument for moving the pre-checks into block processing. In net_processing this would have a smaller effect however, since the block mutation check, which also validates the merkle root, is done before.

  Testing spamming a node with valid, but duplicate unrequested blocks seems to exhaust a CPU thread, but does not seem to significantly impact keeping up with the tip. The benefits of adding these checks to net_processing are questionable, especially since there are other ways to trigger the more CPU-intensive checks without submitting a duplicate block. Since these DOS concerns apply even less to the RPC interface, which does not have banning mechanics built in, remove them too.

  Finally, also remove the pre-checks from `bitcoin-chainstate.cpp`.

  ---

  This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587).

ACKs for top commit:
  Sjors:
    re-utACK 73db95c65c
  achow101:
    ACK 73db95c65c
  instagibbs:
    ACK 73db95c65c
  mzumsande:
    ACK 73db95c65c

Tree-SHA512: 2d02e851cf402ecf6a1968c058df3576aac407e200cbf922a1a6391b7f97b4f42c6d9f6b0a78b9d1af0a6d40bdd529a7b11a1e6d88885bd7b8b090f6d1411861
2024-12-03 17:38:41 -05:00
Ava Chow
3867d2421a
Merge bitcoin/bitcoin#31112: Improve parallel script validation error debug logging
492e1f0994 [validation] merge all ConnectBlock debug logging code paths (Pieter Wuille)
b49df703f0 [validation] include all logged information in BlockValidationState (Pieter Wuille)
7b267c034f [validation] Add detailed txin/txout information for script error messages (Pieter Wuille)
146a3d5426 [validation] Make script error messages uniform for parallel/single validation (Pieter Wuille)
1ac1c33f3f [checkqueue] support user-defined return type through std::optional (Pieter Wuille)

Pull request description:

  ~~Builds on top of #31097~~ (now merged). Fixes #30960.

  So far, detailed information about script validation failures is only reported when running with `-par=1`, due to a lack of ability to transfer information from the script validation threads to the validation thread. Fix this by extending the `CCheckQueue` functionality to pass more results through than just success/failure, and use this to report the exact Script error, as well as the transaction input in which it occurred.

ACKs for top commit:
  achow101:
    ACK 492e1f0994
  furszy:
    Code review ACK 492e1f0
  maflcko:
    re-ACK 492e1f0994 🍈
  dergoegge:
    ACK 492e1f0994
  instagibbs:
    ACK 492e1f0994
  mzumsande:
    Code Review ACK 492e1f0994

Tree-SHA512: 234f2e7dfd03bdcd2a56200875fe370962f211ea7ed334038a6a9279a758030bf94bb6246f60d06dd0473dac4b9dbf050d9a32ecaa4176f7727eff63572bf4fd
2024-12-03 15:22:38 -05:00
Ryan Ofsky
8e02b48059
Merge bitcoin/bitcoin#31284: ci: Skip broken Wine64 tests by default
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
fa5e706459 ci: Skip broken Wine64 tests by default (MarcoFalke)

Pull request description:

  I don't think the unit tests run in Wine after the Windows cross-compilation have ever shown a true positive since the MSVC task was added. However, they are a source of frequent false-positives.

  Thus, disable them by default for now. Anyone can still enable them by setting `RUN_UNIT_TESTS=true`.

  A follow-up could run them on real Windows, see https://github.com/bitcoin/bitcoin/pull/31176.

  Conceptually there are many other nightly tasks, which rarely find issues and are not run by default, like the valgrind or s390x tasks. So putting the Wine unit tests in the same bucket should be fine.

ACKs for top commit:
  hebasto:
    ACK fa5e706459, to avoid false-positives.
  willcl-ark:
    ACK fa5e706459

Tree-SHA512: 6bd54470e4d5ce18923c5d724aba0dbf475d053d7097d3f87e822a455cc537b6ce5f0dfcc8ccd0719c12c5d0c8fc7355a6c84185a6c9b5d484d98aee763d0c49
2024-12-03 12:21:31 -05:00
Greg Sanders
0f84cdd266 func: test orphan parent is re-requested from 2nd peer 2024-12-03 11:12:49 -05:00
Hodlinator
95a0104f2e
test: Add tests for directories in place of config files 2024-12-03 11:04:10 +01:00
Hodlinator
e85abe92c7
args: Catch directories in place of config files
Previously passing a directory path as -conf would lead to an ifstream being opened for it, and would not trigger any errors.
2024-12-03 11:04:10 +01:00
Hodlinator
e4b6b1822c
test: Add tests for -noconf 2024-12-03 11:04:10 +01:00
Hodlinator
483f0dacc4
args: Properly support -noconf
-noconf would previously lead to an ifstream "successfully" being opened to the ".bitcoin"-directory (not a file). (Guards against the general case of directories as configs are added in grandchild commit to this one).

Other users of AbsPathForConfigVal() in combination with negated args have been updated earlier in this PR ("args: Support -nopid" and "args: Support -norpccookiefile...").
2024-12-03 11:04:10 +01:00
Hodlinator
312ec64cc0
test refactor: feature_config_args.py - Stop nodes at the end of tests, not at the beginning
This ensures we don't needlessly start the node, and reduces implicit dependencies between test functions.

test_seed_peers() - Move assert calling RPC to verify correct chain after our own function actually started the node.
2024-12-03 10:42:41 +01:00
Hodlinator
7402658bc2
test: -norpccookiefile
Both bitcoind and bitcoin-cli.
2024-12-03 10:38:21 +01:00
Hodlinator
39cbd4f37c
args: Support -norpccookiefile for bitcoind and bitcoin-cli
Replaces belt & suspenders check for initialization in RPCAuthorized() with not allowing empty passwords further down.
2024-12-03 10:38:21 +01:00
Hodlinator
e82ad88452
logs: Use correct path and more appropriate macros in cookie-related code
filepath_tmp -> filepath in last message.

More material changes to nearby code in next commit.
2024-12-03 10:38:21 +01:00
Hodlinator
6e28c76907
test: Harden testing of cookie file existence 2024-12-03 10:38:21 +01:00
Hodlinator
75bacabb55
test: combine_logs.py - Output debug.log paths on error 2024-12-03 10:38:20 +01:00
MarcoFalke
cccca8a77f
test: Avoid logging error when logging error 2024-12-03 09:40:18 +01:00
brunoerg
ee1b9bef00 test: replace is not to != when comparing block hash 2024-12-02 18:38:30 -03:00
Pieter Wuille
492e1f0994 [validation] merge all ConnectBlock debug logging code paths 2024-12-02 16:25:17 -05:00
Pieter Wuille
b49df703f0 [validation] include all logged information in BlockValidationState 2024-12-02 16:25:17 -05:00
Pieter Wuille
7b267c034f [validation] Add detailed txin/txout information for script error messages
Don't just report which script error occurred, but which in which input of which transaction,
and which UTXO was being spent.
2024-12-02 16:25:17 -05:00
Pieter Wuille
146a3d5426 [validation] Make script error messages uniform for parallel/single validation
This makes the debug output mostly the same for -par=1 and parallel validation runs. Of course,
parallel validation is non-deterministic in what error it may encounter first if there are
multiple issues. Also, the way certain script-related and non-script-related checks are
performed differs between the two modes still, which may result in discrepancies.
2024-12-02 16:25:17 -05:00
Pieter Wuille
1ac1c33f3f [checkqueue] support user-defined return type through std::optional
The check type function now needs to return a std::optional<R> for some type R,
and the check queue overall will return std::nullopt if all individual checks
return that, or one of the non-nullopt values if there is at least one.

For most tests, we use R=int, but for the actual validation code, we make it return
the ScriptError.
2024-12-02 16:25:13 -05:00
Ryan Ofsky
ebe4cac38b
Merge bitcoin/bitcoin#30991: test: enable running independent functional test sub-tests
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
409d0d6293 test: enable running individual independent functional test methods (ismaelsadeeq)

Pull request description:

  - Some test methods in the functional test framework are independent and do not require any prior context or setup in `run_test`.
  - This commit adds a new option for running these specific methods within a test file, allowing them to be executed individually without running the entire test suite.
  - Using this option reduces the time you need to wait before the test you are interested in starts executing.
  - The functionality added by this PR can be achieved manually by commenting out code, but having a pragmatic option to do this is more convenient.

  Note: Running test methods that require arguments or context will fail.

  **Example Usage**:
  ```zsh
  build/test/functional/feature_reindex.py --test_methods continue_reindex_after_shutdown
  ```

  ```zsh
  build/test/functional/feature_config_args.py --test_methods test_log_buffer test_args_log test_connect_with_seednode
  ```

ACKs for top commit:
  maflcko:
    review ACK 409d0d6293
  rkrux:
    reACK 409d0d6293
  ryanofsky:
    Code review ACK 409d0d6293. This seems like a good step towards making it easy to run independent tests quickly. I think ideally there would be some naming convention or @ annotation added to test methods that can run independently, so the test framework could provide more functionality like being able to list test methods, being able to show command lines to quickly reproduce problems when tests fails, and calling test methods automatically instead of requiring individual tests to call them. But these ideas are all compatible with the new `--test_methods` option

Tree-SHA512: b0daac7c3b322e6fd9b946962335d8279e8cb004ff76f502c8d597b9c4b0073840945be198a79d44c5aaa64bda421429829d5c84ceeb8c6139eb6ed079a35878
2024-12-02 11:45:32 -05:00
merge-script
1927674100
Merge bitcoin/bitcoin#31387: doc: Use more precise anchor link to codesigning docs
19f49c7489 doc: Use more precise anchor link to codesigning docs (Jeremy Rand)

Pull request description:

  The "Codesigning" section is what users presumably are looking for when they follow this link.

ACKs for top commit:
  fanquake:
    ACK 19f49c7489

Tree-SHA512: 0e25cf0d7160db7d564d67d3e3ac614f9bd209b2399414f1278fa01cfc1ff827aa8311f7c1c2666924d5ac2dc23fe9bc258b80ed8025d5b8d5b11bcf1d12b28c
2024-12-02 14:09:54 +00:00
MarcoFalke
faf70cc994
Remove wallet::ParseISO8601DateTime, use ParseISO8601DateTime instead 2024-12-02 15:09:31 +01:00
merge-script
e043618d44
Merge bitcoin/bitcoin#31396: test: simple reordering to reduce run time
62f6d9e1a4 test: simple ordering optimization to reduce runtime (tdb3)

Pull request description:

  Noticed in #31371 that the position of `mempool_ephemeral_dust` within `BASE_SCRIPTS` was lengthening total test runtime. Instead of moving only that test, looked for others to move to reduce runtime.

  This is a quick optimization that was found to reduce overall functional test runtime of up to around 20% (depending on jobs and machine characteristics). Since it seems like test ordering could be done in many different ways, with many variables, and bike shedding could creep in, a relatively straightforward approach was taken for now that minimized changes to test_runner.

ACKs for top commit:
  maflcko:
    lgtm ACK 62f6d9e1a4
  TheCharlatan:
    ACK 62f6d9e1a4

Tree-SHA512: 6f93fbe4de3fce202383d9f84aa0e96961af3de3c02b8cab73589339d701f32c5e1b57a191eeebf4b06b5cd7a82617f63f24110732940be1a5a4d9237813a570
2024-12-02 14:08:58 +00:00
Lőrinc
50cce20013 test, refactor: Compact ccoins_access and ccoins_spend
Also added an extra check for `AccessCoin` in `CheckAccessCoin` to make sure its result is also validated.
2024-12-02 14:49:45 +01:00
Lőrinc
0a159f0914 test, refactor: Remove remaining unbounded flags from coins_tests 2024-12-02 14:49:45 +01:00
Lőrinc
c0b4b2c1ee test: Validate error messages on fail
The `ccoins_add` and `ccoins_write` tests check the actual exception error messages now instead of just that they fail for the given parameters.
This enables us testing different exceptions in a more fine-grained way in later changes.
2024-12-02 14:49:43 +01:00
Lőrinc
d5f8d607ab test: Group values and states in tests into CoinEntry wrappers
Note: that this commit affects the test order, but doesn't change its behavior or coverage otherwise.
2024-12-02 14:49:36 +01:00
merge-script
a25b892ab1
Merge bitcoin/bitcoin#31386: doc: Use more precise anchor links to Xcode SDK extraction
8bf1b3039c doc: Use more precise anchor links to Xcode SDK extraction (Jeremy Rand)

Pull request description:

  The "SDK Extraction" section is what users presumably are looking for when they follow these links.

ACKs for top commit:
  fanquake:
    ACK 8bf1b3039c

Tree-SHA512: 38669a6b171aa102bb80f5b3a343bd6a067c6921c454f6d18087c5add8016eea2ba8196036f9968f0a9b7df1f642c96ff6c657338c32e775beb04038497cde1f
2024-12-02 13:41:22 +00:00
merge-script
eb646111cd
Merge bitcoin/bitcoin#31383: test: Add missing node.setmocktime(self.mocktime) to p2p_ibd_stalling.py
faa16ed4b9 test: Add missing node.setmocktime(self.mocktime) to p2p_ibd_stalling.py (MarcoFalke)

Pull request description:

  This was forgotten by myself in commit fa5b58ea01.

  This time, there is a diff to test, which fails on current master and passes with this pull request.

  ```diff
  diff --git a/src/net_processing.cpp b/src/net_processing.cpp
  index e503a68382..16438ebd08 100644
  --- a/src/net_processing.cpp
  +++ b/src/net_processing.cpp
  @@ -112,9 +112,9 @@ static_assert(MAX_BLOCKTXN_DEPTH <= MIN_BLOCKS_TO_KEEP, "MAX_BLOCKTXN_DEPTH too
    *  want to make this a per-peer adaptive value at some point. */
   static const unsigned int BLOCK_DOWNLOAD_WINDOW = 1024;
   /** Block download timeout base, expressed in multiples of the block interval (i.e. 10 min) */
  -static constexpr double BLOCK_DOWNLOAD_TIMEOUT_BASE = 1;
  +static constexpr double BLOCK_DOWNLOAD_TIMEOUT_BASE = .05; // 30 sec
   /** Additional block download timeout per parallel downloading peer (i.e. 5 min) */
  -static constexpr double BLOCK_DOWNLOAD_TIMEOUT_PER_PEER = 0.5;
  +static constexpr double BLOCK_DOWNLOAD_TIMEOUT_PER_PEER = 0.;
   /** Maximum number of headers to announce when relaying blocks with headers message.*/
   static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8;
   /** Minimum blocks required to signal NODE_NETWORK_LIMITED */
  diff --git a/test/functional/p2p_ibd_stalling.py b/test/functional/p2p_ibd_stalling.py
  index fa07873929..f8cdd8998c 100755
  --- a/test/functional/p2p_ibd_stalling.py
  +++ b/test/functional/p2p_ibd_stalling.py
  @@ -82,6 +82,7 @@ class P2PIBDStallingTest(BitcoinTestFramework):
           # Need to wait until 1023 blocks are received - the magic total bytes number is a workaround in lack of an rpc
           # returning the number of downloaded (but not connected) blocks.
           bytes_recv = 172761 if not self.options.v2transport else 169692
  +        time.sleep(31);
           self.wait_until(lambda: self.total_bytes_recv_for_blocks() == bytes_recv)

           self.all_sync_send_with_ping(peers)

ACKs for top commit:
  brunoerg:
    ACK faa16ed4b9

Tree-SHA512: 5a670e2dcf828ac83b721a3e20d897744cca50080b0583a8460a0d0c7bf2c2c988cf7e35f688dde6a3349f1c21cc83a16ea5242ed06a59d59a04130416690737
2024-12-02 13:34:18 +00:00