0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-14 11:26:09 -05:00
Commit graph

42551 commits

Author SHA1 Message Date
Ava Chow
3f66642820
Merge bitcoin/bitcoin#30440: Have createNewBlock() return a BlockTemplate interface
a93c171faa Drop unneeded nullptr check from CreateNewBlock() (Sjors Provoost)
dd87b6dff3 Have createNewBlock return BlockTemplate interface (Sjors Provoost)

Pull request description:

  Suggested in https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2225337100

  An external program that uses the Mining interface may need quick access to some information in the block template, while it can wait a bit longer for the full raw transaction data.

  This would be the case for a Stratum v2 Template Provider which needs to send a [NewTemplate](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#72-newtemplate-server---client) message message (which doesn't include transactions) as quickly as possible. It does not include the serialized block transactions.

ACKs for top commit:
  achow101:
    ACK a93c171faa
  ryanofsky:
    Code review ACK a93c171faa. Since last review, just rebased with no changes or conflicts
  itornaza:
    Code review ACK a93c171faa
  TheCharlatan:
    Re-ACK a93c171faa

Tree-SHA512: 17cb61eb5548e9d4a69e34dd732f68a06cde2ad3d82c8339efee704c7860d5de144d93b23d6ecd6ee4ec205844e5560ad0f6d3917822fa75bb8e640c5f51af9a
2024-09-16 15:35:09 -04:00
glozow
2bf721e76a
Merge bitcoin/bitcoin#30661: fuzz: Test headers pre-sync through p2p
a97f43d63a fuzz: Add harness for p2p headers sync (marcofleon)
a0eaa4749f Add FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION in PoW check (marcofleon)
a3f6f5acd8 build: Automatically define FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION for fuzz builds (marcofleon)
0c02d4b2bd net_processing: Make MAX_HEADERS_RESULTS a PeerManager option (marcofleon)

Pull request description:

  This PR reopens https://github.com/bitcoin/bitcoin/pull/28043. It's a regression fuzz test for https://github.com/bitcoin/bitcoin/pull/26355 and [a couple bugs](ed6cddd98e) that were addressed in https://github.com/bitcoin/bitcoin/pull/25717. This should help us move forward with the [removal of mainnet checkpoints](https://github.com/bitcoin/bitcoin/pull/25725).

  It seems like the main concern in https://github.com/bitcoin/bitcoin/pull/28043 was the global mock function for proof of work. This PR aims to be an improvement by replacing the previous approach with a fuzz build configured using `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`. This ensures that the simplified test code will never be in a release binary. If we agree this is the way to go, there are some other places (for future targets) where this method could be used.

  In this target, PoW isn't being tested, so the goal is to bypass the check and let the fuzzer do its thing. In the other harnesses where PoW is actually being fuzzed, `CheckProofOfWork` is now `CheckProofOfWorkImpl`. So, the only change to that function is in the name.

  More about `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` can be found at https://llvm.org/docs/LibFuzzer.html#fuzzer-friendly-build-mode and https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/fuzzing_in_depth.md#d-modifying-the-target.

ACKs for top commit:
  naumenkogs:
    ACK a97f43d63a
  dergoegge:
    reACK a97f43d63a
  instagibbs:
    tested ACK a97f43d63a
  brunoerg:
    ACK a97f43d63a

Tree-SHA512: 60b0bc6aadd8ca4c39db9cbba2da2debaaf68afcb6a8dd75c1ce48ca9e3996948fda8020930b6771a424e0f7c41b0b1068db4aa7dbe517f8fc152f1f712058ad
2024-09-16 13:59:22 -04:00
glozow
c38e9993de
Merge bitcoin/bitcoin#30286: cluster mempool: optimized candidate search
9ad2fe7e69 clusterlin: only start/use search when enough iterations left (Pieter Wuille)
bd044356ed clusterlin: improve heuristic to decide split transaction (optimization) (Pieter Wuille)
71f2629398 clusterlin: include topological pot subsets automatically (optimization) (Pieter Wuille)
e20fda77a2 clusterlin: reduce computation of unnecessary pot sets (optimization) (Pieter Wuille)
6060a948ca clusterlin bench: add example hard cluster benchmarks (Pieter Wuille)
2965fbf203 clusterlin: track upper bound potential set for work items (optimization) (Pieter Wuille)
9e43e4ce10 clusterlin: use feerate-sorted depgraph in SearchCandidateFinder (Pieter Wuille)
b80e6dfe78 clusterlin: add reordering support for DepGraph (Pieter Wuille)
85a285a306 clusterlin: separate initial search entries per component (optimization) (Pieter Wuille)
e4faea9ca7 clusterlin bench: have low/high iter benchmarks instead of per-iter (Pieter Wuille)

Pull request description:

  Part of cluster mempool: #30289

  Depends on #30126, and was split off from it.

  This improves the candidate search algorithm introduced in the previous PR with a variety of optimizations.

  The resulting search algorithm largely follows Section 2 of [How to linearize your cluster](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303#h-2-finding-high-feerate-subsets-5), though with a few changes:
  * Connected component analysis is performed inside the search algorithm (creating initial work items per component for each candidate), rather than once at a higher level. This duplicates some work but is significantly simpler in implementation.
  * No ancestor-set based presplitting inside the search is performed; instead, the `best` value is initialized with the best topologically valid set known to the LIMO algorithm before search starts: the better one out of the highest-feerate remaining ancestor set, and the highest-feerate prefix of remaining transactions in `old_linearization`.
  * Work items are represented using an included set *inc* and an undefined set *und*, rather than included and excluded.
  * Potential sets *pot* are not computed for work items with empty *inc*.

  At a high level, the only missing optimization from that post is bottleneck analysis; my thinking is that it only really helps with clusters that are already relatively cheap to linearize (doing so would need to be done at a higher level, not inside the search algorithm).

  ---

  Overview of the impact of each commit here on linearize performance:
  * **[clusterlin bench: have low/high iter benchmarks instead of per-iter](21a184db63)**: no impact
  * **[separate initial search entries per component (optimization)](c84c5c86ba)**: reduce iterations, increase start-up cost
  * **[add reordering support for DepGraph](019ff29609)**: no impact
  * **[use feerate-sorted depgraph in SearchCandidateFinder](8e27dd5a22)**: typically reduce iterations, increase start-up cost
  * **[track upper bound potential set for work items](781e0fb3aa)**: reduce iterations, increase cost per iteration
  * **[reduce computation of unnecessary pot sets](9fe834fa97)**: reduce cost per iteration
  * **[include topological pot subsets automatically](30612710a4)**: reduce iterations, increase cost per iteration
  * **[improve heuristic to decide split transaction](1880c00ab1)**: typically reduce iterations, increase cost per iteration
  * **[only start/use search when enough iterations left](12760a57b3)**: just account for start-up cost as equivalent iterations

ACKs for top commit:
  sdaftuar:
    ACK 9ad2fe7e69
  instagibbs:
    reACK 9ad2fe7e69
  glozow:
    reACK 9ad2fe7e69, just have a question about the docs

Tree-SHA512: 108bcbb0676f36071eb83954059b5f3d6646c745015b644a2a5d7f5a8ac9424c2d01d339fa6318a3aff4cf313308e85bb80b0090899720a3fcba027b8025590a
2024-09-16 13:40:33 -04:00
merge-script
37679b856c
Merge bitcoin/bitcoin#30899: qt: Translations update
ae05295761 qt: Translations update (Hennadii Stepanov)

Pull request description:

  The recent translations from Transifex.com fetched with the bitcoin-maintainer-tools/update-translations.py tool.

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

  Translations are updated, while skipping removing translations for 2 languages.

  Related:
  - https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2349332544

ACKs for top commit:
  achow101:
    ACK ae05295761
  pablomartin4btc:
    ACK ae05295761

Tree-SHA512: 52107dfccaaaef187389ecb114eef4283ff40a3b855de811685fc2d6cfb1d869b2610edf86b25a235266fd7dcb36f693a6816a60639246b5d439c4f6482b2ebd
2024-09-16 09:59:32 +01:00
Ava Chow
0c4ff18ee9
Merge bitcoin/bitcoin#30896: kernel: Move background load thread to node context
bc7900f33d kernel: Move background load thread to node context (TheCharlatan)

Pull request description:

  The thread handle is never used by the ChainstateManager, so move it out and into the node context. Users of the kernel library now no longer have to manually join the thread when destructing the ChainstateManager.

ACKs for top commit:
  maflcko:
    ACK bc7900f33d 🔄
  achow101:
    ACK bc7900f33d
  ryanofsky:
    Code review ACK bc7900f33d. Nice cleanup
  jonatack:
    Light ACK bc7900f33d
  stickies-v:
    ACK bc7900f33d

Tree-SHA512: add9c4823731324e3db50f95e023e99d55db7cc75c69083ae7c9c2157e5540968caa6cf10674aa4901f91366b02ebb1ff18bb977fec0a46431e2196448958b9d
2024-09-13 17:58:16 -04:00
Ava Chow
87d54500bf
Merge bitcoin/bitcoin#30892: test: Check already deactivated network stays suspended after dumptxoutset
72c9a1fe94 test: Check that network stays suspended after dumptxoutset if it was off before (Fabian Jahr)

Pull request description:

  Follow-up to #30817 which covered the robustness of `dumptxoutset`: network is deactivated during the run but re-activated even when an issue was encountered. But it did not cover the case if the user had deactivated the network themselves before. In that case the user may want the network to stay off so the network is not reactivated after `dumptxoutset` finishes. A test for this behavior is added here.

ACKs for top commit:
  achow101:
    ACK 72c9a1fe94
  pablomartin4btc:
    ACK 72c9a1fe94
  theStack:
    utACK 72c9a1fe94
  tdb3:
    tested ACK 72c9a1fe94

Tree-SHA512: 18a57c5782e99a018414db0597e88debeb32666712c2a75dddbb55135d8f1ddd1eeacba8bbbd35fc03b6c4ab0522fe074ec08edea729560b018f51efabf00e89
2024-09-13 16:12:19 -04:00
Ava Chow
71af7435ef
Merge bitcoin/bitcoin#30233: refactor: move m_is_inbound out of CNodeState
07f4cebe57 refactor: move m_is_inbound out of CNodeState (Sergi Delgado Segura)

Pull request description:

  `m_is_inbound` cannot be changed throughout the life of a `Peer`. However, we are currently storing it in `CNodeState`, which requires locking `cs_main` in order to access it. This can be moved to the outside scope and only require `m_peer_mutex`.

  This is a refactor in preparation for Erlay reworks.

ACKs for top commit:
  maflcko:
    ACK 07f4cebe57 🗞
  achow101:
    ACK 07f4cebe57
  marcofleon:
    ACK 07f4cebe57
  naumenkogs:
    ACK 07f4cebe57

Tree-SHA512: bcc77135646c697204a4605971774cb3ccdf11b3e363a7339bfb4d1678de1681d6ca642dc467f7d71834a94dd56e05367e7fd32a3e6dc6be30c89342f39bf695
2024-09-13 15:40:43 -04:00
Hennadii Stepanov
ae05295761
qt: Translations update
The recent translations from Transifex.com 28.x fetched with the
bitcoin-maintainer-tools/update-translations.py tool.
2024-09-13 17:26:39 +01:00
merge-script
1d5b2406bb
Merge bitcoin/bitcoin#30877: code style: update .editorconfig file
95560616fb code style: update .editorconfig file (Sebastian Falbesoner)

Pull request description:

  Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d57) w.r.t. following changes:
  - consider Rust .rs files (relevant since #28076, commit bbbbdb0c)
  - reflect build system change to CMake (#30454, #30664)
  - add setting for bare Makefile still used for depends builds

  Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim). The PR is made under the assumption that the file is still considered useful, especially for new contributors. If people feel like that's not the case anymore, the alternative is to delete it, obviously.

Top commit has no ACKs.

Tree-SHA512: 8406b1caf31e310f7e17c607d97beac583481e71b4425e0be2bbd8207096aa374a70151b58aae5fdb648ef5ff5c7e1d0a2949e6de3355bdd2009d8353ee24af0
2024-09-13 17:00:10 +01:00
merge-script
fea550b480
Merge bitcoin/bitcoin#30890: doc: unit test runner help fixup and improvements
282f0e9255 Unit test runner documentation fix and improvements (Jon Atack)

Pull request description:

  Running `test_bitcoin --help` prints the list of arguments that may be passed, not the list of tests, so fix that.

  Improve the content and order of the unit test documentation.

ACKs for top commit:
  pablomartin4btc:
    re-ACK 282f0e9255
  tdb3:
    re ACK 282f0e9255

Tree-SHA512: 0d25108ab641bcd9b53f99d139afeec90a34f44d5b00c3c677f7539d87782440a28fadc348663b8c28ace43552834737b9c1e8f5517c68edc8547695a9cb5063
2024-09-13 16:58:38 +01:00
Sebastian Falbesoner
95560616fb code style: update .editorconfig file
Updates the .editorconfig file, first introduced in 2021
(see PR #21123, commit 7a135d57) w.r.t. following changes:
- consider Rust .rs files (relevant since #28076, commit bbbbdb0c)
- reflect build system change to CMake (#30454, #30664)
- add setting for the bare Makefile still used for depends builds

Can be tested e.g. by using the editorconfig-vim plugin
(https://github.com/editorconfig/editorconfig-vim).
2024-09-13 17:55:10 +02:00
Jon Atack
282f0e9255 Unit test runner documentation fix and improvements
- Running `test_bitcoin --help` prints the list of arguments that may be passed,
  not the list of tests, so fix that.

- Improve the content and order of the unit test documentation.
2024-09-13 08:54:51 -06:00
merge-script
06a9f7789e
Merge bitcoin/bitcoin#30433: build: add standard branch-protection to hardening flags for aarch64-linux
001b1cf010 build: use standard branch-protection for aarch64-linux (fanquake)

Pull request description:

  Use `-mbranch-protection=standard` when targetting `*aarch64-*-linux*`.
  Part of #24123, but this flag can already be used on a best effort basis.

  Note that this flag is also already used by default, in the toolchain, on various distros (i.e Fedora).

ACKs for top commit:
  hebasto:
    ACK 001b1cf010.
  TheCharlatan:
    ACK 001b1cf010

Tree-SHA512: 2d7ae60f59921a62d51139cb0fd5cecbed4f63266564b2623b7d160f5b0c2c42c78ef8aeff787f485eccc46a9ffd5da70023ec093df6add7c982e0d48a1601b5
2024-09-13 15:29:13 +01:00
TheCharlatan
bc7900f33d
kernel: Move background load thread to node context
The thread handle is never used by the ChainstateManager, so move it out
and into the node context. Users of the kernel library now no longer
have to manually join the thread when destructing the ChainstateManager.
2024-09-13 16:10:31 +02:00
Hennadii Stepanov
e43ce250c6
Merge bitcoin-core/gui#835: Fix crash when closing wallet
a965f2bc07 gui: fix crash when closing wallet (furszy)

Pull request description:

  The crash occurs because `WalletController::removeAndDeleteWallet` is called twice for the
  same wallet model: first in the GUI's button connected function `WalletController::closeWallet`,
  and then again when the backend emits the `WalletModel::unload` signal.

  This causes the issue because `removeAndDeleteWallet` inlines an `erase(std::remove())`.
  So, if `std::remove` returns an iterator to the end (indicating the element wasn't found
  because it was already erased), the subsequent call to `erase` leads to an undefined behavior.

  Test Notes:
  Try closing any wallet using the toolbar button in the GUI. It will crash in master, but not here.

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

ACKs for top commit:
  pablomartin4btc:
    tACK a965f2bc07
  jarolrod:
    ACK a965f2bc07
  hebasto:
    ACK a965f2bc07.

Tree-SHA512: c94681b95cb566f7aabd0d4fb10f797c2cea6ac569abc265e918f08e6abae3335432a0b0879372b54b2c109798ed0a4a249bf162c34add59cbd18d38a2d9660e
2024-09-13 11:28:23 +01:00
fanquake
001b1cf010
build: use standard branch-protection for aarch64-linux 2024-09-13 11:26:40 +01:00
Sjors Provoost
a93c171faa
Drop unneeded nullptr check from CreateNewBlock() 2024-09-13 10:14:53 +02:00
Sjors Provoost
dd87b6dff3
Have createNewBlock return BlockTemplate interface
An external program that uses the Mining interface may need quick access to some information in the block template, while it can wait a bit longer for the full raw transaction data.

This would be the case for a Stratum v2 Template Provider which needs to send a NewTemplate message (which doesn't include transactions) as quickly as possible.
2024-09-13 10:14:53 +02:00
furszy
a965f2bc07
gui: fix crash when closing wallet
The crash occurs because 'WalletController::removeAndDeleteWallet' is called
twice for the same wallet model: first in the GUI's button connected function
'WalletController::closeWallet', and then again when the backend emits the
'WalletModel::unload' signal.

This causes the issue because 'removeAndDeleteWallet' inlines an
erase(std::remove()). So, if 'std::remove' returns an iterator to the end
(indicating the element wasn't found because it was already erased), the
subsequent call to 'erase' leads to an undefined behavior.
2024-09-12 19:25:45 -03:00
Fabian Jahr
72c9a1fe94
test: Check that network stays suspended after dumptxoutset if it was off before 2024-09-12 23:21:58 +02:00
Pieter Wuille
9ad2fe7e69 clusterlin: only start/use search when enough iterations left 2024-09-12 15:15:36 -04:00
Pieter Wuille
bd044356ed clusterlin: improve heuristic to decide split transaction (optimization)
Empirically, this approach seems to be more efficient in common real-life
clusters, and does not change the worst case.

Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
2024-09-12 15:15:36 -04:00
Pieter Wuille
71f2629398 clusterlin: include topological pot subsets automatically (optimization)
Automatically add topologically-valid subsets of the potential set pot
to inc. It can be proven that these must be part of the best reachable
topologically-valid set from that work item.

This is a crucial optimization that (apparently) reduces the maximum
number of iterations from ~2^(N-1) to ~sqrt(2^N).

Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
2024-09-12 15:15:36 -04:00
Pieter Wuille
e20fda77a2 clusterlin: reduce computation of unnecessary pot sets (optimization)
Keep track of which transactions in the graph have an individual
feerate that is better than the best included set so far. Others do not
need to be added to the pot set, as they cannot possibly help beating
best.
2024-09-12 15:15:36 -04:00
Pieter Wuille
6060a948ca clusterlin bench: add example hard cluster benchmarks
Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
2024-09-12 15:15:36 -04:00
Pieter Wuille
2965fbf203 clusterlin: track upper bound potential set for work items (optimization)
In each work item, keep track of a conservative overestimate of the best
possible feerate that can be reached from it, and then use these to avoid
exploring hopeless work items.
2024-09-12 15:15:36 -04:00
Pieter Wuille
9e43e4ce10 clusterlin: use feerate-sorted depgraph in SearchCandidateFinder
This is a requirement for a future commit, which will rely on quickly iterating
over transaction sets in decreasing individual feerate order.
2024-09-12 15:15:36 -04:00
Pieter Wuille
b80e6dfe78 clusterlin: add reordering support for DepGraph
Add a DepGraph(depgraph, reordering) function that constructs a new DepGraph
corresponding to an old one, but with its transactions is a modified order
(given as a vector from old to new positions).

Also use this reordering feature inside DepGraphFormatter::Unser, which needs
a small modification so that its reordering mapping is old-to-new (rather than
the new-to-old it used before).
2024-09-12 15:15:36 -04:00
Pieter Wuille
85a285a306 clusterlin: separate initial search entries per component (optimization)
Before this commit, the worst case for linearization involves clusters which
break apart in several smaller components after the first candidate is
included in the output linearization.

Address this by never considering work items that span multiple components
of what remains of the cluster.
2024-09-12 15:15:36 -04:00
Pieter Wuille
e4faea9ca7 clusterlin bench: have low/high iter benchmarks instead of per-iter 2024-09-12 15:15:36 -04:00
Ava Chow
cf0120ff02
Merge bitcoin/bitcoin#30880: test: Wait for local services to update in feature_assumeutxo
19f4a7c95a test: Wait for local services to update in feature_assumeutxo (Fabian Jahr)

Pull request description:

  Closes #30878

  It seems like there is a race where the block is stored locally and `getblock` does not error anymore, but `ActivateBestChain` has not finished yet, so the local services are not updated yet either. Fix this by waiting for the local services to update.

  Can be reproduced locally by adding the sleep here:

  ```cpp
  ──────────────────────────────────────────────────────────────────────────────────────────────────────────┐
  src/validation.cpp:3567: bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< │
  ──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
          }

          if (WITH_LOCK(::cs_main, return m_disabled)) {
              std::this_thread::sleep_for(std::chrono::seconds(10));
              // Background chainstate has reached the snapshot base block, so exit.

              // Restart indexes to resume indexing for all blocks unique to the snapshot
  ```

ACKs for top commit:
  maflcko:
    review-only ACK 19f4a7c95a
  achow101:
    ACK 19f4a7c95a
  pablomartin4btc:
    tACK 19f4a7c95a
  furszy:
    Code review ACK [19f4a7c](19f4a7c95a).

Tree-SHA512: 70dad3795988956c5e20f2d2d895fb56c5e3ce257c7547d3fd729c88949f0e24cb34594da1537bce8794ad02b2db44e7e46e995aa32539cd4dd84c4f1d4bb1c4
2024-09-12 14:52:14 -04:00
Ryan Ofsky
e46bebb444
Merge bitcoin/bitcoin#30546: util: Use consteval checked format string in FatalErrorf, LogConnectFailure
fa5bc450d5 util: Use compile-time check for LogConnectFailure (MarcoFalke)
fa7087b896 util: Use compile-time check for FatalErrorf (MarcoFalke)
faa62c0112 util: Add ConstevalFormatString (MarcoFalke)
fae7b83eb5 lint: Remove forbidden functions from lint-format-strings.py (MarcoFalke)

Pull request description:

  The `test/lint/lint-format-strings.py` was designed to count the number of format specifiers and assert that they are equal to the number of parameters passed to the format function. The goal seems reasonable, but the implementation has many problems:

  * It is written in Python, meaning that C++ code can not be parsed correctly. Currently it relies on brittle regex and string parsing.
  * Apart from the parsing errors, there are also many logic errors. For example, `count_format_specifiers` allows a mix of positional specifiers and non-positional specifiers, which can lead to runtime format bugs. Also, `count_format_specifiers` silently skipped over "special" format specifiers, which are valid in tinyformat, which again can lead to runtime format bugs being undetected.
  * The brittle logic has a history of breaking in pull requests that are otherwise fine. This causes the CI to fail and the pull request being blocked from progress until the bug in the linter is fixed, or the code is rewritten to work around the bug.
  * It is only run in the CI, or when the developer invokes the script. It would be better if the developer got the error message at compile-time, directly when writing the code.

  Fix all issues by using a `consteval` checked format string in `FatalErrorf` and `LogConnectFailure`.

  This is the first step toward https://github.com/bitcoin/bitcoin/issues/30530 and a follow-up will apply the approach to the other places.

ACKs for top commit:
  stickies-v:
    re-ACK fa5bc450d5
  l0rinc:
    ACK fa5bc450d5
  hodlinator:
    ACK fa5bc450d5
  ryanofsky:
    Code review ACK fa5bc450d5

Tree-SHA512: d6189096b16083143687ed1b1559cf4f92f97dd87bc5d00673e44f4fb9fce7bb7b215cfdfc39b6e6a24f0b75a79a03ededce966639e554f7172e1fc22cf015ae
2024-09-12 13:21:53 -04:00
Ryan Ofsky
be768dbd18
Merge bitcoin/bitcoin#30618: test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage
1eac96a503 Compare FromUserHex result against other hex validators and parsers (Lőrinc)
19947863e1 Use BOOST_CHECK_EQUAL for optional, arith_uint256, uint256, uint160 (Lőrinc)
743ac30e34 Add std::optional support to Boost's equality check (Lőrinc)

Pull request description:

  Enhanced `FromUserHex` coverage by:

  * Added `std::optional` support to `BOOST_CHECK_EQUAL`, allowing direct comparisons of `std::optional<T>` with other `T` expected values.
  * Increased fuzz testing for hex parsing to validate against other hex validators and parsers.

  ----

  * Use BOOST_CHECK_EQUAL for https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706637780 arith_uint256, uint256, uint160

  Example error before:
  > unknown location:0: fatal error: in "validation_chainstatemanager_tests/chainstatemanager_args": std::bad_optional_access: bad_optional_access
  test/validation_chainstatemanager_tests.cpp:781: last checkpoint

  after:
  > test/validation_chainstatemanager_tests.cpp:801: error: in "validation_chainstatemanager_tests/chainstatemanager_args": check set_opts({"-assumevalid=0"}).assumed_valid_block == uint256::ZERO has failed [std::nullopt != 0000000000000000000000000000000000000000000000000000000000000000]

ACKs for top commit:
  stickies-v:
    re-ACK 1eac96a503
  ryanofsky:
    Code review ACK 1eac96a503. Only changes since last review were auto type and fuzz test tweaks.
  hodlinator:
    ACK 1eac96a503

Tree-SHA512: f1d2c65f0ee4e97830700be5b330189207b11ed0c89a8cebf0f97d43308402a6b3732e10130c79a0c044f7d2eeabfb5359990825aadf02c4ec19428dcd982b00
2024-09-12 12:36:37 -04:00
merge-script
07c7c96022
Merge bitcoin/bitcoin#30883: build: Revert "Minimize I/O operations in GenerateHeaderFrom{Json,Raw}.cmake"
fdeb717e78 Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`" (Hennadii Stepanov)

Pull request description:

  This reverts commit b07fe666f2 from https://github.com/bitcoin/bitcoin/pull/30842.

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

  Apparently, the `string(APPEND ...)` command isn't optimized for large strings.

ACKs for top commit:
  maflcko:
    review ACK fdeb717e78

Tree-SHA512: ad5c3d49d3395ab318edcd7c9a98090838bec0cd3c1f1cc6ebc6f4262df2494f605458b523251bf5e590bbcfda15ed963f0a814678135ce4cc2dca9a108d20c7
2024-09-12 16:53:31 +01:00
merge-script
24817e8b15
Merge bitcoin/bitcoin#30814: kernel: Create usable static kernel library
0dd16d7118 build: Add a pkg-config file for libbitcoinkernel (TheCharlatan)
45be32f838 build: Produce a usable static kernel library (TheCharlatan)

Pull request description:

  Since the move to cmake, the kernel static library that is installed after a cmake --install build is unusable. It lacks symbols for the internal libraries, besides those defined in the kernel library target.

  Fix this by explicitly installing all the required internal static libraries. To make usage of these installed libraries easy, add a pkg-config file that can be used during linking.

  This patch can be tested with:

  ```
  cmake -B build -DBUILD_SHARED_LIBS=OFF -DBUILD_KERNEL_LIB=ON
  cmake --build build
  cmake --install build
  g++ -std=c++20 -o test_chainstate src/bitcoin-chainstate.cpp -I/home/drgrid/bitcoin/src $(pkg-config --libs --static libbitcoinkernel)
  ```

  Attempts to solve #30801

ACKs for top commit:
  hebasto:
    ACK 0dd16d7118.
  fanquake:
    ACK 0dd16d7118 - this looks like a good place to start.
  ryanofsky:
    Code review ACK 0dd16d7118

Tree-SHA512: 92f7bc959584bdc595f4aa6d0ab133355481075fe8564224fd7ac122fd7bdd75f98cf26ef0a6a7d84fd552d2258ddca1b674eca91122469a58bacc5f0a0ec2ef
2024-09-12 16:39:34 +01:00
Hennadii Stepanov
fdeb717e78
Revert "build: Minimize I/O operations in GenerateHeaderFrom{Json,Raw}.cmake"
This reverts commit b07fe666f2.
2024-09-12 16:34:57 +01:00
Sergi Delgado Segura
07f4cebe57 refactor: move m_is_inbound out of CNodeState
`m_is_inbound` cannot be changed throughout the life of a `Peer`. However, we
are currently storing it in `CNodeState`, which requires locking `cs_main` in
order to access it. This can be moved to the outside scope and only require
`m_peer_mutex`.

This is a refactor in preparation for Erlay reworks.
2024-09-12 11:20:44 -04:00
Fabian Jahr
19f4a7c95a
test: Wait for local services to update in feature_assumeutxo 2024-09-12 16:30:50 +02:00
merge-script
7d43bca052
Merge bitcoin/bitcoin#30872: test: fix exclude parsing for functional runner
72b46f28bf test: fix exclude parsing for functional runner (Max Edwards)

Pull request description:

  This restores previous behaviour of being able to exclude a test by name without having to specify .py extension.

  It was noticed in https://github.com/bitcoin/bitcoin/issues/30851 that tests were no longer being excluded.

  PR https://github.com/bitcoin/bitcoin/pull/30244 introduced being able to exclude a specific tests based on args (such as `--exclude "rpc_bind.py --ipv6`) but it made the wrong assumption that test names intended to be excluded would include the .py extension.

  The following https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 shows that this is not how the `--exclude` flag was used in CI.

  https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 gave three examples of `--exclude` being used in CI so I compared the number of tests that the runner would run for these three examples in three situations, before #30244 was introduced, in master today and with this PR applied.

  Example:

  `--previous-releases --coverage --extended --exclude feature_dbcrash`

  Test count:
  Before #30244 introduced: 314
  Master: 315
  With this PR: 314

  Example:

  `--exclude feature_init,rpc_bind,feature_bind_extra`

  Test count:
  Before #30244 introduced: 306
  Master 311
  With this PR: 306

  Example:

  `--exclude rpc_bind,feature_bind_extra`

  Before #30244 introduced:  307
  Master 311
  With this PR: 307

  I've also tested that the functionality introduced with #30244 remains and we can still exclude specific tests by argument.

ACKs for top commit:
  maflcko:
    review ACK 72b46f28bf
  willcl-ark:
    ACK 72b46f28bf

Tree-SHA512: 37c0e3115f4e3efdf9705f4ff8cd86a5cc906aacc1ab26b0f767f5fb6a953034332b29b0667073f8382a48a2fe9d649b7e60493daf04061260adaa421419d8c8
2024-09-12 15:30:34 +01:00
merge-script
cf786eccd7
Merge bitcoin/bitcoin#30865: build: Skip secp256k1 ctime tests when tests are not being built
23eedc5d1e build: Skip secp256k1 ctime tests when tests are not being built (Hennadii Stepanov)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/pull/30791#issuecomment-2340860619:
  > Building with a fuzz engine fails, because the ctime tests are auto-detected in cmake, based on whether or not valgrind-devel is installed or not.

ACKs for top commit:
  maflcko:
    re-review ACK 23eedc5d1e
  fanquake:
    ACK 23eedc5d1e

Tree-SHA512: bfc0f2798acd36be9c52073d578b42c002606c60ef3fe8ef633eaea4f5382a3e9765d31637e4c25d8b71fd70473b29c24af4732e55e5183f27b48725b61fa15b
2024-09-12 15:29:33 +01:00
Hennadii Stepanov
23eedc5d1e
build: Skip secp256k1 ctime tests when tests are not being built
Co-authored-by: fanquake <fanquake@gmail.com>
2024-09-12 14:24:26 +01:00
MarcoFalke
fa5bc450d5
util: Use compile-time check for LogConnectFailure 2024-09-12 15:01:35 +02:00
MarcoFalke
fa7087b896
util: Use compile-time check for FatalErrorf 2024-09-12 15:01:20 +02:00
MarcoFalke
faa62c0112
util: Add ConstevalFormatString
The type is used to wrap a format string once it has been compile-time
checked to contain the right number of format specifiers.
2024-09-12 15:00:53 +02:00
Max Edwards
72b46f28bf test: fix exclude parsing for functional runner
This restores previous behaviour of being able to exclude a test by name without having to specify .py extension.
2024-09-12 13:42:34 +01:00
merge-script
a5e99669cc
Merge bitcoin/bitcoin#30733: test: remove unused src_dir param from run_tests after CMake migration
2ad560139b Remove unused src_dir param from run_tests (Lőrinc)

Pull request description:

  The `src_dir` usage was removed in  a8a2e364ac (diff-437d7f6e9f2229879b60aae574a8217f14c643bbf3cfa9225d8011d6d52df00cL598), making the parameter unused.

Top commit has no ACKs.

Tree-SHA512: 1fd8b93811b4ab467ba5a160a4fe204e9606e1bf237c7595ed6f8b7821cf59d2a776c0e1e154852a45b2a35e5bdbd8996314e4f63a9c750f21b9a17875cb636a
2024-09-12 12:17:06 +01:00
merge-script
0c1e507278
Merge bitcoin/bitcoin#30871: build: Add more cmake presets
f15e817811 build: add more CMake presets (dev-mode, libfuzzer, libfuzzer-nosan) (Pieter Wuille)

Pull request description:

  Add three more cmake presets to the project-wide `CMakePresets.json` file:
  * `dev-mode`: enables all features and dependencies
  * `libfuzzer`: builds for fuzzing with libfuzzer and the typical sanitizers (but not the optional ones that require suppressions) enabled.
  * `libfuzzer-nosan`: builds for fuzzing with libfuzzer and no (other) sanitizers

  ... and then uses these in some documentation.

ACKs for top commit:
  ryanofsky:
    Code review ACK f15e817811. This change is much needed to simplify my command line.
  TheCharlatan:
    ACK f15e817811

Tree-SHA512: a5f67bb7119fd36832ca5eb7189db04bfaf88f954aa461bfb2aeb866469057b0d0272835c418bc3a264c30dd8fba6d2e2cc8a6741a889f28f52c1c09b3ba9704
2024-09-12 11:33:15 +01:00
merge-script
fcb61bbc8d
Merge bitcoin/bitcoin#27038: security-check: test for _FORTIFY_SOURCE usage in release binaries
be4f78275f contrib: test for FORTIFY_SOURCE in security-check.py (fanquake)

Pull request description:

  Test for the existence of fortified functions in the ELF release binaries.
  Currently skips `bitcoin-util` and checks for RISC-V.

ACKs for top commit:
  TheCharlatan:
    ACK be4f78275f

Tree-SHA512: decea5f359f1e673aa0119916f674f409a13b69db7da366cd95c1540201e117ff5a979da67bc2517fe786c2ac23d1006a9aaf662d7eadeec35da6aae4998c065
2024-09-12 11:32:31 +01:00
merge-script
85833cf05f
Merge bitcoin/bitcoin#30847: test: Drop no longer needed workarounds
5c80192ff6 test: Drop no longer needed workarounds (Hennadii Stepanov)

Pull request description:

  This PR deletes the workarounds introduced in https://github.com/bitcoin/bitcoin/pull/16564 and https://github.com/bitcoin/bitcoin/pull/15382, as `ctest` skips these cases gracefully: 5c80192ff6/src/test/CMakeLists.txt (L201-L203)

ACKs for top commit:
  kevkevinpal:
    ACK [5c80192](5c80192ff6)
  fanquake:
    ACK 5c80192ff6. Looks correct:

Tree-SHA512: c47c606ecf7d64016b3c6353c3d4898350edc2caeac494dfd44484417f500a73f0c88c39f0f24651f3a02ef31ed9ca5c70d938bb9a8ca1eea54927e4d6a8fcd2
2024-09-12 11:28:27 +01:00
merge-script
11e2f9fff4
Merge bitcoin/bitcoin#30835: build: Introduce "Kernel" installation component
7b04fabe2d build: Introduce "Kernel" installation component (Hennadii Stepanov)

Pull request description:

  This PR enables building and installing only `libbitcoinkernel`, without the need to disable other targets during the project build system generation:

  ```
  $ rm -rf build && cmake -B build -DBUILD_KERNEL_LIB=ON
  $ cmake --build build --target bitcoinkernel
  $ cmake --install build --component Kernel --prefix /home/hebasto/INSTALL
  -- Install configuration: "RelWithDebInfo"
  -- Installing: /home/hebasto/INSTALL/lib/libbitcoinkernel.so
  ```

  Please note, that only the `bitcoinkernel` target is being built.

  Related to https://github.com/bitcoin/bitcoin/issues/30801 and https://github.com/bitcoin/bitcoin/pull/30814.

ACKs for top commit:
  TheCharlatan:
    ACK 7b04fabe2d
  ryanofsky:
    Code review ACK 7b04fabe2d

Tree-SHA512: eac114dde059e47c91938a4a9108fc0fc693b5342ed3b6ecb971615be8ad3225b9985aae12d6ad18e673edf1bd39a5ecf259c1b61734f221669091bf2ce93a67
2024-09-12 10:58:52 +01:00