27634 Commits

Author SHA1 Message Date
MarcoFalke
fa8ade300f
refactor: Avoid GCC false positive error
This avoids an overly agressive GCC false positive warning:

error: ‘tmp’ may be used uninitialized [-Werror=maybe-uninitialized]
2025-01-20 17:43:58 +01:00
Vasil Dimov
d3339a7cd5
util: fix compiler warning about deprecated space before _MiB
```
src/util/byte_units.h:13:29: error: identifier '_MiB' preceded by whitespace in a literal operator declaration is deprecated [-Werror,-Wdeprecated-literal-operator]
   13 | constexpr size_t operator"" _MiB(unsigned long long mebibytes)
      |                  ~~~~~~~~~~~^~~~
      |                  operator""_MiB
1 error generated.
```

Clang 20.0.0
2025-01-20 14:32:20 +01:00
merge-script
f9032a4abb
Merge bitcoin/bitcoin#31242: wallet, desc spkm: Return SigningProvider only if we have the privkey
f6a6d912059c66792f48689632d2a7f14f8bdad9 test: add check for getting SigningProvider for a CPubKey (Sebastian Falbesoner)
62a95f5af9b998e241eb72c16ba581e77c480126 test: refactor: move `CreateDescriptor` helper to wallet test util module (Sebastian Falbesoner)
493656763f73e5ef1cfb979a513c12983dca99dd desc spkm: Return SigningProvider only if we have the privkey (Ava Chow)

Pull request description:

  If we know about a pubkey that's in our descriptor, but we don't have the private key, don't return a SigningProvider for that pubkey.

  This is specifically an issue for Taproot outputs that use the H point as the resulting PSBTs may end up containing irrelevant information because the H point was detected as a pubkey each unrelated descriptor knew about.

  Split from #29675

ACKs for top commit:
  fjahr:
    ACK f6a6d912059c66792f48689632d2a7f14f8bdad9
  theStack:
    re-ACK f6a6d912059c66792f48689632d2a7f14f8bdad9
  furszy:
    utACK f6a6d912059. Only reviewed the actual change in detail, not the test commit.

Tree-SHA512: 30a196e611a0c5d9ebe5baf6d896caaa6af66f1615463dbb0c31e52604d53cf342922bb9967b3c697b47083d76b0485c77a5f545bd6381247c8bc44321c70f97
2025-01-16 17:30:36 +00:00
merge-script
9dc4eedb67
Merge bitcoin/bitcoin#31673: doc: fix minor typos in comments
b30cc71e853cbaaec66e7e23d7d1a32468079ee0 doc: fix typos (Adlai Chandrasekhar)

Pull request description:

  In the unrelated PR #31621 the linter reported a few typos, that are fixed in this commit. I used the "doc" prefix as it only modifies comments, so none of the more significant prefixes seem appropriate.

ACKs for top commit:
  maflcko:
    lgtm ACK b30cc71e853cbaaec66e7e23d7d1a32468079ee0

Tree-SHA512: 7bba2d928fc0b98f62f96d9abf6dba98f699b386b75730271fa3e7b57a8a220df2265b699007f066e585e1db2ee3cbe5a272b74a8c153f6f8814c01e6de7a3ee
2025-01-16 16:21:14 +00:00
Adlai Chandrasekhar
b30cc71e85 doc: fix typos 2025-01-16 17:36:16 +02:00
merge-script
df8bf65745
Merge bitcoin/bitcoin#31483: kernel: Move kernel-related cache constants to kernel cache
2a92702bafca5c78b270a9502a22cb9deac02cfc init: Use size_t consistently for cache sizes (TheCharlatan)
65cde3621dbb9ac7d210d4926e7601c4adf5f498 kernel: Move default cache constants to caches (TheCharlatan)
8826cae285490439dc1f19b25fa70b2b9e62dfe8 kernel: Move non-kernel db cache size constants (TheCharlatan)
e758b26b85da27ef44f3d2c924f3f08e8c1f4fdf kernel: Move kernel-specific cache size options to kernel (TheCharlatan)
d5e2c4a4097c799433cfc5367c61568fad2c784e fuzz: Add fuzz test for checked and saturating add and left shift (TheCharlatan)
c03a2795a8e044d17835bbf03de0c64dc7b41da8 util: Add integer left shift helpers (TheCharlatan)
8bd5f8a38ce903c05606841ebed1902398cb0b14 [refactor] init: Simplify coinsdb cache calculation (TheCharlatan)
5db7d4d3d28bd1269a09955b4695135c86c4827d doc: Correct docstring describing max block tree db cache (TheCharlatan)

Pull request description:

  Carrying non-kernel related fields in the cache sizes for the indexes is confusing for kernel library users. The cache sizes are set currently with magic numbers in bitcoin-chainstate. The comments for the cache size calculations are not completely clear. The constants for the cache sizes are also currently in `txdb.h`, which is not an ideal place for holding all cache size related constants.

  Solve these things by moving the kernel-specific cache size fields to their own struct and moving the constants to either the node or the kernel cache sizes.

  This slightly changes the way the cache is allocated if (and only if) the txindex and/or blockfilterindex is used. Since they are now given precedence over the block tree db cache, this results in a bit less cache being allocated to the block tree db, coinsdb and coins caches. The effect is negligible though, i.e. cache sizes with default dbcache reported through the logs are:

  master:
  ```
  Cache configuration:
  * Using 2.0 MiB for block index database
  * Using 56.0 MiB for transaction index database
  * Using 49.0 MiB for basic block filter index database
  * Using 8.0 MiB for chain state database
  * Using 335.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
  ```

  this PR:
  ```
  Cache configuration:
  * Using 2.0 MiB for block index database
  * Using 56.2 MiB for transaction index database
  * Using 49.2 MiB for basic block filter index database
  * Using 8.0 MiB for chain state database
  * Using 334.5 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
  ```

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

ACKs for top commit:
  stickies-v:
    re-ACK 2a92702bafca5c78b270a9502a22cb9deac02cfc
  ryanofsky:
    Code review ACK 2a92702bafca5c78b270a9502a22cb9deac02cfc. Changes since last review are fixing size options to use size_t instead of int64_t again, simplifying CheckedLeftShift more, and making other minor suggested cleanups
  hodlinator:
    re-ACK 2a92702bafca5c78b270a9502a22cb9deac02cfc

Tree-SHA512: 98376eaa0660b1b8c096a5ce1f3e7c8c30e7cd6644de36856c2d3e573108cfc9473c93ebb3952b7881047b5ae6c85c5b096e6726f30f35be58b98eca07c8c785
2025-01-16 15:04:58 +00:00
merge-script
335798c496
Merge bitcoin/bitcoin#31397: p2p: track and use all potential peers for orphan resolution
86d7135e36efd39781cf4c969011df99f0cbb69d [p2p] only attempt 1p1c when both txns provided by the same peer (glozow)
f7658d9b1475ecaa5cb8e543e5c66a3a3a2dc1fb [cleanup] remove p2p_inv from AddTxAnnouncement (glozow)
063c1324c143d98e6d5108bb51b3ca59b45f9b85 [functional test] getorphantxs reflects multiple announcers (glozow)
0da693f7e129fccaecf9a2c177083d2e80d37781 [functional test] orphan handling with multiple announcers (glozow)
b6ea4a9afe2d8bbf49b6b6c42f0a3ce4390c4535 [p2p] try multiple peers for orphan resolution (glozow)
1d2e1d709ce3d95d409254c860347bc3fedf30e1 [refactor] move creation of unique_parents to helper function (glozow)
c6893b0f0b7b205c8da4b9d281a55c9eb843b582 [txdownload] remove unique_parents that we already have (glozow)
163aaf285af91b49c2d788463dc1e1654c88ade6 [fuzz] orphanage multiple announcer functions (glozow)
22b023b09da3e2fe00467c77b105a61c1961081f [unit test] multiple orphan announcers (glozow)
96c1a822a274689611f409246ef1573906b0083e [unit test] TxOrphanage EraseForBlock (glozow)
04448ce32a3bc4b6d12293f71e40333abe35c224 [txorphanage] add GetTx so that orphan vin can be read (glozow)
e810842acda6fe56e0536ebecfbb9d17d26e1513 [txorphanage] support multiple announcers (glozow)
62a9ff187076686b39dca64ad4f2f439da0875d1 [refactor] change type of unique_parents to Txid (glozow)
6951ddcefd9e05f31ee7634bbfbf1d19e04ec00e [txrequest] GetCandidatePeers (glozow)

Pull request description:

  Part of #27463.

  (Transaction) **orphan resolution** is a process that kicks off when we are missing UTXOs to validate an unconfirmed transaction. We currently request missing parents by txid; BIP 331 also defines a way to [explicitly request ancestors](https://github.com/bitcoin/bips/blob/master/bip-0331.mediawiki#handle-orphans-better).

  Currently, when we find that a transaction is an orphan, we only try to resolve it with the peer who provided the `tx`. If this doesn't work out (e.g. they send a `notfound` or don't respond), we do not try again. We actually can't, because we've already forgotten who else could resolve this orphan (i.e. all the other peers who announced the transaction).

  What is wrong with this? It makes transaction download less reliable, particularly for 1p1c packages which must go through orphan resolution in order to be downloaded.

  Can we fix this with BIP 331 / is this "duct tape" before the real solution?
  BIP 331 (receiver-initiated ancestor package relay) is also based on the idea that there is an orphan that needs resolution, but it's just a new way of communicating information. It's not inherently more honest; you can request ancestor package information and get a `notfound`. So ancestor package relay still requires some kind of procedure for retrying when an orphan resolution attempt fails. See the #27742 implementation which builds on this orphan resolution tracker to keep track of what packages to download (it just isn't rebased on this exact branch). The difference when using BIP 331 is that we request `ancpkginfo` and then `pkgtxns` instead of the parent txids.

  Zooming out, we'd like orphan handling to be:
  - Bandwidth-efficient: don't have too many requests out at once. As already implemented today, transaction requests for orphan parents and regular download both go through the `TxRequestTracker` so that we don't have duplicate requests out.
  - Not vulnerable to censorship: don't give up too easily, use all candidate peers. See e.g. https://bitcoincore.org/en/2024/07/03/disclose_already_asked_for/
  - Load-balance between peers: don't overload peers; use all peers available. This is also useful for when we introduce per-peer orphan protection, since each peer will have limited slots.

  The approach taken in this PR is to think of each peer who announces an orphan as a potential "orphan resolution candidate." These candidates include:
  - the peer who sent us the orphan tx
  - any peers who announced the orphan prior to us downloading it
  - any peers who subsequently announce the orphan after we have started trying to resolve it
  For each orphan resolution candidate, we treat them as having "announced" all of the missing parents to us at the time of receipt of this orphan transaction (or at the time they announced the tx if they do so after we've already started tracking it as an orphan). We add the missing parents as entries to `m_txrequest`, incorporating the logic of typical txrequest processing, which means we prefer outbounds, try not to have duplicate requests in flight, don't overload peers, etc.

ACKs for top commit:
  marcofleon:
    Code review ACK 86d7135e36efd39781cf4c969011df99f0cbb69d
  instagibbs:
    reACK 86d7135e36efd39781cf4c969011df99f0cbb69d
  dergoegge:
    Code review ACK 86d7135e36efd39781cf4c969011df99f0cbb69d
  mzumsande:
    ACK 86d7135e36efd39781cf4c969011df99f0cbb69d

Tree-SHA512: 618d523b86e60c3ea039e88326d50db4e55e8e18309c6a20e8f2b10ed9e076f1de0315c335fd3b8abdabcc8b53cbceb66fb59147d05470ea25b83a2b4bd9c877
2025-01-16 13:42:26 +00:00
merge-script
98939ce7b7
Merge bitcoin/bitcoin#31655: refactor: Avoid UB in SHA3_256::Write
fabeca3458b38a3d8930cb0cbc866388c3f120f1 refactor: Avoid UB in SHA3_256::Write (MarcoFalke)
fad4032b219e58300f8d0a74bbcf38ef26fa89d0 refactor: Drop unused UCharCast (MarcoFalke)

Pull request description:

  It is UB to apply a distance to a pointer or iterator further than the
  end itself, even if the distance is (partially) revoked later on.

  Fix the issue by advancing the data pointer at most to the end.

  This fix is required to adopt C++ safe buffers https://github.com/bitcoin/bitcoin/issues/31272.

  Also included is a somewhat unrelated commit.

ACKs for top commit:
  sipa:
    utACK fabeca3458b38a3d8930cb0cbc866388c3f120f1
  theuni:
    utACK fabeca3458b38a3d8930cb0cbc866388c3f120f1
  hebasto:
    ACK fabeca3458b38a3d8930cb0cbc866388c3f120f1.

Tree-SHA512: 78c53691322b72c3ba9c25ec94eec275dbbbc3049b0ad45e7d9fb2df0afbbaa905b0d8fa7106a3582f937bb1dc15a7592c4ad2d80fe4cff1062a3acfd3638f08
2025-01-16 12:20:05 +00:00
TheCharlatan
2a92702baf
init: Use size_t consistently for cache sizes
This avoids having to rely on implicit casts when passing them to the
various functions allocating the caches.

This also ensures that if the requested amount of db_cache does not fit
in a size_t, it is clamped to the maximum value of a size_t.

Also take this opportunity to make the total amounts of cache in the
chainstate manager a size_t too.
2025-01-15 15:44:56 +01:00
TheCharlatan
65cde3621d
kernel: Move default cache constants to caches
They are not related to the txdb, so a better place for them is the
new kernel and node cache file. Re-use the default amount of kernel
cache for the default node cache.
2025-01-15 15:44:55 +01:00
TheCharlatan
8826cae285
kernel: Move non-kernel db cache size constants
These have nothing to do with the txdb, so move them out and into the
node caches.
2025-01-15 15:44:44 +01:00
TheCharlatan
e758b26b85
kernel: Move kernel-specific cache size options to kernel
Carrying non-kernel related fields in the cache sizes for the indexes is
confusing for kernel library users. The cache sizes also are set
currently with magic numbers in bitcoin-chainstate. The comments for the
cache size calculations are also not completely clear.

Solve these things by moving the kernel-specific cache size fields to
their own struct.

This slightly changes the way the cache is allocated if the txindex
and/or blockfilterindex is used. Since they are now given precedence
over the block tree db cache, this results in a bit less cache being
allocated to the block tree db, coinsdb and coins caches. The effect is
negligible though, i.e. cache sizes with default dbcache reported
through the logs are:

master:
Cache configuration:
* Using 2.0 MiB for block index database
* Using 56.0 MiB for transaction index database
* Using 49.0 MiB for basic block filter index database
* Using 8.0 MiB for chain state database
* Using 335.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)

this branch:
Cache configuration:
* Using 2.0 MiB for block index database
* Using 56.2 MiB for transaction index database
* Using 49.2 MiB for basic block filter index database
* Using 8.0 MiB for chain state database
* Using 334.5 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
2025-01-15 15:44:16 +01:00
TheCharlatan
d5e2c4a409
fuzz: Add fuzz test for checked and saturating add and left shift
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2025-01-15 15:44:03 +01:00
TheCharlatan
c03a2795a8
util: Add integer left shift helpers
The helpers are used in the following commits to increase the safety of
conversions during cache size calculations.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: stickies-v <stickies-v@protonmail.com>
2025-01-15 15:43:05 +01:00
MarcoFalke
fa3efb5729
refactor: Introduce struct to hold a runtime format string
This brings the format types closer to the standard library types:

* FormatStringCheck corresponds to std::basic_format_string, with
  compile-time checks done via ConstevalFormatString
* RuntimeFormat corresponds to std::runtime_format, with no compile-time
  checks done.

Also, it documents where no compile-time checks are done.
2025-01-15 12:16:08 +01:00
MarcoFalke
fadc6b9bac
refactor: Check translatable format strings at compile-time 2025-01-15 12:15:54 +01:00
MarcoFalke
fa1d5acb8d
refactor: Use TranslateFn type consistently
The type was introduced in the previous commit.
2025-01-15 12:15:40 +01:00
MarcoFalke
eeee6cf2ff
refactor: Delay translation of _() literals
This is required for a future commit that requires _() to be consteval
for format literals.

Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>
2025-01-14 19:21:37 +01:00
MarcoFalke
fabeca3458
refactor: Avoid UB in SHA3_256::Write
It is UB to apply a distance to a pointer or iterator further than the
end itself, even if the distance is (partially) revoked later on.

Fix the issue by advancing the data pointer at most to the end.
2025-01-14 19:09:30 +01:00
MarcoFalke
fad4032b21
refactor: Drop unused UCharCast
This is no longer needed after commit
6aa0e70ccbd5491ec9d7c81892820f3341ccd631
2025-01-14 19:01:53 +01:00
merge-script
35bf426e02
Merge bitcoin/bitcoin#28724: wallet: Cleanup accidental encryption keys in watchonly wallets
69e95c2b4f99eb4c2af6b2b6cc6a66abfea753df tests: Test cleanup of mkeys from wallets without privkeys (Andrew Chow)
2b9279b50a3682ab97308888b4f272d3ae379811 wallet: Remove unused encryption keys from watchonly wallets (Andrew Chow)
813a16a463326079472366aed42f5218a57db63b wallet: Add HasCryptedKeys (Andrew Chow)

Pull request description:

  An earlier version allowed users to create watchonly wallets (wallets without private keys) that were "encrypted". Such wallets would have a stored encryption keys, but nothing would actually be encrypted with them. This can cause unexpected behavior such as https://github.com/bitcoin-core/gui/issues/772.

  We can detect such wallets as they will have the disable private keys flag set, no encrypted keys, and encryption keys. For such wallets, we can remove those encryption keys thereby avoiding any issues that may result from this unexpected situation.

ACKs for top commit:
  sipa:
    utACK 69e95c2b4f99eb4c2af6b2b6cc6a66abfea753df.
  laanwj:
    Code review re-ACK 69e95c2b4f99eb4c2af6b2b6cc6a66abfea753df
  furszy:
    Code review ACK 69e95c2b4f99eb4

Tree-SHA512: 901932cd709c57e66c598f011f0105a243b5a8b539db2ef3fcf370dca4cf35ae09bc1110e8fca8353be470f159468855a4dd96b99bc9c1112adc86ccc50e1b9d
2025-01-10 15:29:47 +00:00
merge-script
528354e213
Merge bitcoin/bitcoin#31616: init,log: Unify block index log line
e04be3731f4921cd51d25b1d6210eace7600fea4 init,log: Unify block index and chainstate loading log line (Lőrinc)

Pull request description:

  The line has been present since the beginning.

  Removed redundant duration as well since it can be recovered from the timestamps.

  Example logs before the change:
  ```
  2025-01-07T11:58:33Z Verification progress: 99%
  2025-01-07T11:58:33Z Verification: No coin database inconsistencies in last 6 blocks (18905 transactions)
  2025-01-07T11:58:33Z  block index           31892ms
  2025-01-07T11:58:33Z Setting NODE_NETWORK on non-prune mode
  ```

ACKs for top commit:
  maflcko:
    lgtm ACK e04be3731f4921cd51d25b1d6210eace7600fea4
  TheCharlatan:
    ACK e04be3731f4921cd51d25b1d6210eace7600fea4
  danielabrozzoni:
    tACK e04be3731f4921cd51d25b1d6210eace7600fea4
  BrandonOdiwuor:
    Code Review ACK e04be3731f4921cd51d25b1d6210eace7600fea4

Tree-SHA512: cbe4569a17f56ff23e829b837a083c2f730cc490b47bee3bac12126e2257e0ba9ebe9b4384deb03203a0a60aac3b8d283c5d31a6d0481635ba011ac6e2c61ad1
2025-01-10 11:27:04 +00:00
Ava Chow
37af8bfb34
Merge bitcoin/bitcoin#31549: fuzz: Abort if system time is called without mock time being set
a96b84cb1b76e65a639e62f0224f534f89858c18 fuzz: Abort when calling system time without setting mock time (marcofleon)
ff21870e20b2391b684cc50fdd6879805055d6a1 fuzz: Add SetMockTime() to necessary targets (marcofleon)

Pull request description:

  This PR expands the `CheckGlobals` utility that was introduced in https://github.com/bitcoin/bitcoin/pull/31486 and should help with fuzz stability (https://github.com/bitcoin/bitcoin/issues/29018).

  System time shouldn't be used when running a fuzz test, as it is likely to introduce instability (non-determinism). This PR identifies and fixes the targets that were calling system time without setting mock time at the start of an iteration.

  Removing`SetMockTime()` from any one of these targets should result in a crash and a message describing the issue.

ACKs for top commit:
  achow101:
    ACK a96b84cb1b76e65a639e62f0224f534f89858c18
  dergoegge:
    Code review ACK a96b84cb1b76e65a639e62f0224f534f89858c18
  brunoerg:
    crACK a96b84cb1b76e65a639e62f0224f534f89858c18

Tree-SHA512: e093a9feb8a397954f7b1416dfa8790b2733f09d5ac51fda5a9d225a55ebd8f99135aa52bdf5ab531653ad1a3739c4ca2b5349c1d989bb4b009ec8eaad684f7d
2025-01-09 19:31:07 -05:00
Ava Chow
54115d8de5
Merge bitcoin/bitcoin#31617: build, test: Build db_tests.cpp regardless of USE_BDB
fd2d96d9087be234bdf4a6eb6d563e92b4fb4501 build, test: Build `db_tests.cpp` regardless of `USE_BDB` (Hennadii Stepanov)

Pull request description:

  When the building of `db_tests.cpp` was made conditional on `USE_BDB` in commit a58b719cf75e2d97205ec260bcff0d4780fe4fb8, all `db_tests` were indeed specific to BDB wallets.

  However, the tests have since been [extended](ba616b932c) to include SQLite wallets as well.

  On the master branch @ 433412fd8478923dfdb20044f74c5d1e19fa8dd8, tests specific to SQLite wallets are not built and run if configured with `WITH_BDB=OFF` (the default option).

  This PR resolves this issue by guarding BDB-specific code in `db_tests.cpp` and ensuring this source file is compiled regardless of the `WITH_BDB` option.

ACKs for top commit:
  achow101:
    ACK fd2d96d9087be234bdf4a6eb6d563e92b4fb4501
  maflcko:
    review ACK fd2d96d9087be234bdf4a6eb6d563e92b4fb4501 🔺
  theuni:
    utACK fd2d96d9087be234bdf4a6eb6d563e92b4fb4501

Tree-SHA512: bd9eddf16af60c568e931467d39e9e23a268e82e367ab630c23ac3cfd37e6007c6d78579b69ccbeebc1911c749cdbe75794fd56d7fbdb30c6fea6d2ab11017a3
2025-01-09 18:46:07 -05:00
Ava Chow
0a77441158
Merge bitcoin/bitcoin#31451: wallet: migration, avoid loading legacy wallet after failure when BDB isn't compiled
589ed1a8eafe1daed379ebb383602c8f220aef19 wallet: migration, avoid loading wallet after failure when it wasn't loaded before (furszy)

Pull request description:

  Fixes #31447.

  During migration failure, only load wallet back into memory when the wallet was
  loaded prior to migration. This fixes the case where BDB is not supported, which
  implies that no legacy wallet can be loaded into memory due to the lack of db
  writing functionality.

  Link to error description https://github.com/bitcoin/bitcoin/issues/31447#issuecomment-2528757140.

  This PR also improves migration backup related comments to better document the
  current workflow.

ACKs for top commit:
  achow101:
    ACK 589ed1a8eafe1daed379ebb383602c8f220aef19
  rkrux:
    ACK 589ed1a8eafe1daed379ebb383602c8f220aef19
  pablomartin4btc:
    tACK 589ed1a8eafe1daed379ebb383602c8f220aef19

Tree-SHA512: c7a489d2b253c574ee0287b691ebe29fe8d026f659f68a3f6108eca8b4e1e420c67ca7803c6bd70c1e1440791833fabca3afbcf8fe8524c6c9fc08de95b618d0
2025-01-09 18:33:23 -05:00
Lőrinc
e04be3731f init,log: Unify block index and chainstate loading log line
Example logs before the change:
```
2025-01-07T11:58:33Z Verification progress: 99%
2025-01-07T11:58:33Z Verification: No coin database inconsistencies in last 6 blocks (18905 transactions)
2025-01-07T11:58:33Z  block index           31892ms
2025-01-07T11:58:33Z Setting NODE_NETWORK on non-prune mode
2025-01-07T11:58:33Z block tree size = 878086
2025-01-07T11:58:33Z nBestHeight = 878085
```

Removed redundant duration as well since it can be recovered from the timestamps.

Co-authored-by: TheCharlatan <seb.kung@gmail.com>
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2025-01-09 14:14:43 +01:00
TheCharlatan
8bd5f8a38c
[refactor] init: Simplify coinsdb cache calculation
(total_cache / 4) + (1 << 23) is at least 8 MiB and nMaxCoinsDBCache is
also 8 MiB, so the minimum between the two will always be
nMaxCoinsDBCache. This is just a simplification and not changing the
result of the calculation.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2025-01-09 10:21:49 +01:00
ismaelsadeeq
7c123c08dd
miner: add package feerate vector to CBlockTemplate
- The package feerates are ordered by the sequence in which
  packages are selected for inclusion in the block template.

- The commit also tests this new behaviour.

Co-authored-by: willcl-ark <will@256k1.dev>
2025-01-07 15:29:17 -05:00
Hennadii Stepanov
fd2d96d908
build, test: Build db_tests.cpp regardless of USE_BDB
While some tests are specific to BDB, `db_tests` as a whole are not
limited to BDB.
2025-01-07 15:50:39 +00:00
Ava Chow
433412fd84
Merge bitcoin/bitcoin#31596: doc: Clarify comments about endianness after #30526
3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40 doc: Clarify comments about endianness after #30526 (Ryan Ofsky)

Pull request description:

  This is a documentation-only change following up on suggestions made in the #30526 review.

  Motivation for this change is that I was recently reviewing #31583, which reminded me how confusing the arithmetic blob code was and made me want to write better comments.

ACKs for top commit:
  achow101:
    ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40
  TheCharlatan:
    ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40
  Sjors:
    ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40
  BrandonOdiwuor:
    LGTM ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40

Tree-SHA512: 90d5582a25a51fc406d83ca6b8c4e5e4d3aee828a0497f4b226b2024ff89e29b9b50d0ae8ddeac6abf2757fe78548d58cf3dd54df4b6d623f634a2106048091d
2025-01-06 18:52:59 -05:00
Ava Chow
c506f2cee7
Merge bitcoin/bitcoin#31581: test: have miner_tests use Mining interface
04249682e381f976de6ba56bb4fb2996dfa194ab test: use Mining interface in miner_tests (Sjors Provoost)

Pull request description:

  Needed for both #31283 and #31564.

  By using the Mining interface in `miner_tests.cpp` we increase its coverage in unit tests.

ACKs for top commit:
  achow101:
    ACK 04249682e381f976de6ba56bb4fb2996dfa194ab
  ryanofsky:
    Code review ACK 04249682e381f976de6ba56bb4fb2996dfa194ab, just minor suggested changes (renames, comments, BOOST_REQUIREs) since last review and some more extra clarifications and checks added to the CreateNewBlock_validity test. The CreateNewBlock_validity changes seem clear and easy to understand now.
  vasild:
    ACK 04249682e381f976de6ba56bb4fb2996dfa194ab
  tdb3:
    ACK 04249682e381f976de6ba56bb4fb2996dfa194ab

Tree-SHA512: 2761cb7555d759670e40d8f37b96a079f8e12a588ac43313b9e63c69afd478321515873a8896ea56784f0100dac4476b0c0e0ef8b5418f8aea24d9965cace4d4
2025-01-06 18:28:33 -05:00
marcofleon
a96b84cb1b fuzz: Abort when calling system time without setting mock time 2025-01-06 15:43:13 +00:00
marcofleon
ff21870e20 fuzz: Add SetMockTime() to necessary targets 2025-01-06 15:43:04 +00:00
merge-script
41a2ce9b7d
Merge bitcoin/bitcoin#31464: util: Add missing types in make_secure_unique
fa397177acfa1006ea6feee0b215c53e51f284de util: Add missing types in make_secure_unique (MarcoFalke)

Pull request description:

  The return type of `std::forward` depends on the template type, and can not be recovered from the args. Attempting to do so will result in a compile failure. For example, `make_secure_unique<std::string>(std::string{});` does not compile on current master, but does with this pull.

  Another example would be `make_secure_unique<std::pair<std::string, std::unique_ptr<int>>>(std::string{}, std::make_unique<int>(21));`

ACKs for top commit:
  hodlinator:
    ACK fa397177acfa1006ea6feee0b215c53e51f284de
  hebasto:
    ACK fa397177acfa1006ea6feee0b215c53e51f284de.
  TheCharlatan:
    ACK fa397177acfa1006ea6feee0b215c53e51f284de

Tree-SHA512: cc902c1111c929a79a6f806b5097136a465e8c727474176bad30a5777ebbb30bedb0bd35273b43bf839d2c00492500ddec724bd17349250451f6b329cb71e6f2
2025-01-06 15:19:24 +00:00
glozow
86d7135e36 [p2p] only attempt 1p1c when both txns provided by the same peer
Now that we track all announcers of an orphan, it's not helpful to
consider an orphan provided by a peer that didn't send us this parent.
It can only hurt our chances of finding the right orphan when there are
multiple candidates.

Adapt the 2 tests in p2p_opportunistic_1p1c.py that looked at 1p1c
packages from different peers. Instead of checking that the right peer
is punished, we now check that the package is not submitted. We can't
use the functional test to see that the package was not considered
because the behavior is indistinguishable (except for the logs).
2025-01-06 09:02:05 -05:00
glozow
f7658d9b14 [cleanup] remove p2p_inv from AddTxAnnouncement
This param is no longer needed since orphan parent requests are added to
the TxRequestTracker directly.
2025-01-06 09:02:05 -05:00
glozow
b6ea4a9afe [p2p] try multiple peers for orphan resolution
Co-authored-by: dergoegge <n.goeggi@gmail.com>
2025-01-06 09:02:05 -05:00
glozow
1d2e1d709c [refactor] move creation of unique_parents to helper function
This function will be reused in a later commit.
2025-01-06 09:02:05 -05:00
glozow
c6893b0f0b [txdownload] remove unique_parents that we already have
This means we no longer return parents we already have in the
m_unique_parents result from MempoolRejectedTx.

We need to separate the loop that checks AlreadyHave parents from the
loop that adds parents as announcements, because we may do the latter
loop multiple times for different peers.
2025-01-06 09:02:05 -05:00
glozow
163aaf285a [fuzz] orphanage multiple announcer functions 2025-01-06 09:02:05 -05:00
glozow
22b023b09d [unit test] multiple orphan announcers 2025-01-06 09:02:05 -05:00
glozow
96c1a822a2 [unit test] TxOrphanage EraseForBlock 2025-01-06 09:02:05 -05:00
glozow
04448ce32a [txorphanage] add GetTx so that orphan vin can be read 2025-01-06 09:02:05 -05:00
glozow
e810842acd [txorphanage] support multiple announcers
Add ability to add and track multiple announcers per orphan transaction,
erasing announcers but not the entire orphan.

The tx creation code in orphanage_tests needs to be updated so that each
tx is unique, because the CountOrphans() check assumes that calling
EraseForPeer necessarily means its orphans are deleted.

Unused for now.
2025-01-06 09:02:05 -05:00
glozow
62a9ff1870 [refactor] change type of unique_parents to Txid 2025-01-06 09:02:05 -05:00
glozow
6951ddcefd [txrequest] GetCandidatePeers
Needed for a later commit adding logic to ask the TxRequestTracker for a
list of announcers.  These announcers should know the parents of the
transaction they announced.
2025-01-06 09:02:05 -05:00
merge-script
a0f0c48ae2
Merge bitcoin/bitcoin#31584: txmempool: fix typos in comments
34e8ee23b83eeecb1c4022b48e7b8db45a60ffdd txmempool: fix typos in comments (Boris Nagaev)

Pull request description:

  Fixed typos identified by codespell lint in CI.

ACKs for top commit:
  maflcko:
    lgtm ACK 34e8ee23b83eeecb1c4022b48e7b8db45a60ffdd
  hebasto:
    ACK 34e8ee23b83eeecb1c4022b48e7b8db45a60ffdd.

Tree-SHA512: b36cddb26e33f042c074185aa6ef1b0339ae0ff05643e1c30bac56b8c9620d89c834ea5befb25d2c602a3dd541209f5a7c04da6ef9f10724144c35a6548ea52c
2025-01-06 11:10:56 +00:00
Ava Chow
4036ee3f2b
Merge bitcoin/bitcoin#31542: test: Embed univalue json tests in binary
faf7eac364fb7f421a649b483286ac8681d92b31 test: clang-format -i src/univalue/test/unitester.cpp (MarcoFalke)
fafa9cc7a5994003a7dccb67f83ee409e4b8f544 test: Embed univalue json tests in binary (MarcoFalke)
fa044857caf70db5d9608e168e59f3169a66977a test: Re-enable univalue test fail18.json (MarcoFalke)
63b6b638aa5e1de1858294d0d14dce38f45a9cbb build: Use character literals for generated headers to avoid narrowing (Lőrinc)

Pull request description:

  All other benchmarks and tests have their data embedded, except for the univalue json tests.

  This is not only confusing, but also problematic, when the test binary is moved to a different system for testing, because one has to put the test files in the source dir that was used at compile-time.

  Fix all issues by embedding them. Also, re-enable a disabled test. Also, fix an issue in the GenerateHeaderFromJson.cmake.

  Requested in https://github.com/bitcoin/bitcoin/pull/31434/files#r1876000910

ACKs for top commit:
  l0rinc:
    ACK faf7eac364fb7f421a649b483286ac8681d92b31
  fjahr:
    tACK faf7eac364fb7f421a649b483286ac8681d92b31
  achow101:
    ACK faf7eac364fb7f421a649b483286ac8681d92b31
  TheCharlatan:
    Re-ACK faf7eac364fb7f421a649b483286ac8681d92b31
  hebasto:
    Re-ACK faf7eac364fb7f421a649b483286ac8681d92b31. The commit, which modifies CMake scripts, has been replaced with the one from https://github.com/bitcoin/bitcoin/pull/31547, and a formatting commit has been added since my recent [review](https://github.com/bitcoin/bitcoin/pull/31542#pullrequestreview-2517189261).

Tree-SHA512: 72ad202125746f32ccf07411ad3efd2771f27a40525c204cba3c9c83b3ca46d05dd18f6fa5985720c6684bdcbb4c4853fc609ced095ddd1a124832318dd8a55d
2025-01-03 13:58:20 -05:00
Sebastian Falbesoner
f6a6d91205 test: add check for getting SigningProvider for a CPubKey
Verify that the DescriptorSPKM method `GetSigningProvider` should
only return a signing provider for the passed public key if its
corresponding private key of the passed public key is available.
2025-01-03 13:12:32 -05:00
Sebastian Falbesoner
62a95f5af9 test: refactor: move CreateDescriptor helper to wallet test util module
Can be reviewed via `--color-moved=dimmed-zebra`.
2025-01-03 13:12:27 -05:00