41102 Commits

Author SHA1 Message Date
Ava Chow
915d7276e4
Merge bitcoin/bitcoin#27064: system: use %LOCALAPPDATA% as default datadir on windows
84900ac34f6888b7a851d0a6a5885192155f865c doc: add release-notes-27064.md (Matthew Zipkin)
855dd8d592c951a2b3239867ffbf66bb8677d470 system: use %LOCALAPPDATA% as default datadir on windows (Matthew Zipkin)

Pull request description:

  Closes https://github.com/bitcoin/bitcoin/issues/2391

  This PR changes the default datadir location on Windows from `C:\Users\Username\AppData\Roaming\Bitcoin` to `C:\Users\Username\AppData\Local\Bitcoin`. This change only applies to fresh installs. To preserve backwards compatibility, on startup we check for the existence of `C:\Users\Username\AppData\Roaming\Bitcoin\chainstate` and if it is there, we continue using the "Roaming" directory as the default datadir location.

  [Note that in Windows 11 this change may be moot:](https://learn.microsoft.com/en-us/uwp/api/windows.storage.applicationdata.roamingfolder?view=winrt-22621)

  > Roaming data and settings is no longer supported as of Windows 11. The recommended replacement is [Azure App Service](https://learn.microsoft.com/en-us/azure/app-service/). Azure App Service is widely supported, well documented, reliable, and supports cross-platform/cross-ecosystem scenarios such as iOS, Android and web. Settings stored here no longer roam (as of Windows 11), but the settings store is still available.

ACKs for top commit:
  achow101:
    ACK 84900ac34f6888b7a851d0a6a5885192155f865c
  BenWestgate:
    crACK 84900ac34f
  hebasto:
    re-ACK 84900ac34f6888b7a851d0a6a5885192155f865c, only addressed feedback since my recent [review](https://github.com/bitcoin/bitcoin/pull/27064#pullrequestreview-2028718273).

Tree-SHA512: 807c6e89571287e2c8f4934229aec91ef28e7d0a675234acf1b7d085c24c7b73a08b6e345fbfc9038e6239187b6b69c08490ddaa1c057de5ea975c4a000bba42
2024-05-23 12:11:55 -04:00
Ava Chow
867f6af803
Merge bitcoin/bitcoin#29873: policy: restrict all TRUC (v3) transactions to 10kvB
154b2b2296edccb5ed24e829798dacb6195edc11 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits (glozow)
a29f1df289cf27c6cbd565448548b3dc1392a9b0 [policy] restrict all v3 transactions to 10kvB (glozow)
d578e2e3540e085942001350ff3aeb047bdac973 [policy] explicitly require non-v3 for CPFP carve out (glozow)

Pull request description:

  Opening for discussion / conceptual review.

  We like the idea of a smaller maximum transaction size because:
  - It lowers potential replacement cost (i.e. harder to do Rule 3 pinning via gigantic transaction)
  - They are easier to bin-pack in block template production
  - They equate to a tighter memory limit in data structures that are bounded by a number of transactions (e.g. orphanage and vExtraTxnForCompact). For example, the current memory bounds for orphanage is 100KvB * 100 = 40MB, and guaranteeing 1 tx per peer would require reserving a pretty large space.

  History for `MAX_STANDARD_TX_WEIGHT=100KvB` (copied from https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510):
  - 2010-09-13 In 3df62878c3 satoshi added a 100kB (MAX_BLOCK_SIZE_GEN/5 with MBS_GEN = MAX_BLOCK_SIZE/2) limit on new transactions in CreateTransaction()
  - 2013-02-04 https://github.com/bitcoin/bitcoin/pull/2273 In gavin gave that constant a name, and made it apply to transaction relay as well

  Lowering `MAX_STANDARD_TX_WEIGHT` for all txns is not being proposed, as there are existing apps/protocols that rely on large transactions. However, it's been brought up that we should consider this for TRUCs (which is especially designed to avoid Rule 3 pinning).

  This reduction should be ok because using nVersion=3 isn't standard yet, so this wouldn't break somebody's existing use case. If we find that this is too small, we can always increase it later. Decreasing would be much more difficult.
  ~[Expected size of a commitment transaction](https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction) is within (900 + 172 * 483 + 224) / 4 = 21050vB~ EDIT: this is incorrect, but perhaps not something that should affect how we choose this number.

ACKs for top commit:
  sdaftuar:
    ACK 154b2b2296edccb5ed24e829798dacb6195edc11
  achow101:
    ACK 154b2b2296edccb5ed24e829798dacb6195edc11
  instagibbs:
    ACK 154b2b2296edccb5ed24e829798dacb6195edc11
  t-bast:
    ACK 154b2b2296
  murchandamus:
    crACK 154b2b2296edccb5ed24e829798dacb6195edc11

Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
2024-05-23 11:54:18 -04:00
Ryan Ofsky
6300438a2e
Merge bitcoin/bitcoin#30115: rpc: avoid copying into UniValue
d7707d9843b03f20d2a8c5a45d7b3db58e169e6f rpc: avoid copying into UniValue (Cory Fields)

Pull request description:

  These are the simple (and hopefully obviously correct) copies that can be moves instead.

  This is a follow-up from https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2108751842

  As it turns out, there are hundreds of places where we copy UniValues needlessly. It should be the case that moves are always preferred over copies, so there should be no downside to these changes.

  willcl-ark, however, noticed that memory usage may increase in some cases. Logically this makes no sense to me. The only plausible explanation imo is that because the moves are faster, more ops/second occur in some cases.

  This list of moves was obtained by changing the function signatures of the UniValue functions to accept only rvalues, then compiling and fixing them up one by one. There still exist many places where copies are being made. These can/should be fixed up, but weren't done here for the sake of doing the easy ones first.

  I ran these changes through clang-tidy with `performance-move-const-arg` and `bugprone-use-after-move` and no bugs were detected (though that's obviously not to say it can be trusted 100%).

  As stated above, there are still lots of other less trivial fixups to do after these including:
  - Using non-const UniValues where possible so that moves can happen
  - Refactoring code in order to be able to move a UniValue without introducing a use-after-move
  - Refactoring functions to accept UniValues by value rather than by const reference

ACKs for top commit:
  achow101:
    ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f
  ryanofsky:
    Code review ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f. No changes since last review other than rebase. I agree benchmarks showing increased peak memory usage and RSS are surprising, but number of allocations is down as expected, and runtime is also decreased.
  willcl-ark:
    ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f

Tree-SHA512: 7f511be73984553c278186286a7d161a34b2574c7f5f1a0edc87c2913b4c025a0af5241ef9af2df17547f2e4ef79710aa5bbb762fc9472435781c0488dba3435
2024-05-23 10:53:37 -04:00
Ava Chow
e163d864d3
Merge bitcoin/bitcoin#30118: test: improve robustness of connect_nodes()
6629d1d0f8285d1bf2d87341a856abe903f26c13 test: improve robustness of connect_nodes() (furszy)

Pull request description:

  Decoupled from #27837 because this can help other too, found it investigating a CI failure https://cirrus-ci.com/task/5805115213348864?logs=ci#L3200.

  The `connect_nodes` function in the test framework relies on a stable number of peer
  connections to verify that the new connection between the nodes is successfully established.
  This approach is fragile, as any of the peers involved in the process can drop, lose, or
  create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely
  even when the peers in question were connected successfully.

  This commit improves the situation by using the nodes' subversion and the connection
  direction (inbound/outbound) to identify the exact peer connection and perform the
  checks exclusively on it.

ACKs for top commit:
  stratospher:
    reACK 6629d1d.
  achow101:
    ACK 6629d1d0f8285d1bf2d87341a856abe903f26c13
  maflcko:
    utACK 6629d1d0f8285d1bf2d87341a856abe903f26c13
  AngusP:
    re-ACK 6629d1d0f8285d1bf2d87341a856abe903f26c13

Tree-SHA512: 5f345c0ce49ea81b643e97c5cffd133e182838752c27592fcdeac14ad10919fb4b7ff38e289e42a7c3c638a170bd0d0b7a9cd493898997a2082a7b7ceef4aeeb
2024-05-23 10:00:00 -04:00
merge-script
f15778536a
Merge bitcoin/bitcoin#30149: contrib: Renew Windows code signing certificate
9f4ff1e9659597307f62510f1885ad8da3a1d9a3 windeploy: Renew certificate (Ava Chow)

Pull request description:

  Renewed the Windows code signing certificate for another 3 years.

ACKs for top commit:
  fanquake:
    ACK 9f4ff1e9659597307f62510f1885ad8da3a1d9a3
  glozow:
    tested ACK 9f4ff1e96595

Tree-SHA512: 6441aa55fa1aa4e532bbe1ce1ad8b9be8a197d8de0aa57a8b02d41681a433599f305eb0a519e90782cebe20f473322be519ff6398933f525c39cc3476e860283
2024-05-23 13:25:14 +01:00
glozow
83ae1bac9d
Merge bitcoin/bitcoin#30062: net: add ASMap info in getrawaddrman RPC
1e54d61c4698debf3329d1960e06078ccbf8063c test: add coverage for `mapped_as` from `getrawaddrman` (brunoerg)
8c2714907d1e1ffc58487b3b43e018c1ec10065b net: rpc: return peer's mapped AS in getrawaddrman (brunoerg)

Pull request description:

  This PR adds two new fields in `getrawaddrman` RPC: "mapped_as" and "source_mapped_as". These fields are used to return the ASN (Autonomous System Number) mapped to the peer and its source. With these informations we can have a better view of the bucketing logic with ASMap specially in projects like [addrman-observer](https://github.com/0xb10c/addrman-observer).

ACKs for top commit:
  fjahr:
    Code review ACK 1e54d61c4698debf3329d1960e06078ccbf8063c
  virtu:
    ACK [1e54d61](1e54d61c46)
  0xB10C:
    ACK 1e54d61c4698debf3329d1960e06078ccbf8063c
  glozow:
    ACK 1e54d61c4698debf3329d1960e06078ccbf8063c

Tree-SHA512: af86bcc7a2e69bebd3fa9eaa2e527e0758c44c0a958de7292514d5f99f8f01f5df3bae11400451268e0255f738ff3acccc77f48fe129937512f1e9d9963c4c5e
2024-05-23 12:08:36 +01:00
merge-script
ef44726918
Merge bitcoin/bitcoin#30151: depends: Fetch miniupnpc sources from an alternative website
21b8a14d37c19ce292d5529597e0d52338db48a9 depends: Fetch miniupnpc sources from an alternative website (Hennadii Stepanov)

Pull request description:

  The https://miniupnp.tuxfamily.org website is unavailable now.

ACKs for top commit:
  achow101:
    ACK 21b8a14d37c19ce292d5529597e0d52338db48a9
  edilmedeiros:
    utACK 21b8a14d37c19ce292d5529597e0d52338db48a9
  theuni:
    utACK 21b8a14d37c19ce292d5529597e0d52338db48a9

Tree-SHA512: 141427447d5f1902f17fe3774fadf0ac912f663d7fd6467dff42fd6131f6884906b77cf49bbe7387f118fd9e9d52af1d36778624f9f02237d2ba3387fc974be6
2024-05-23 09:39:47 +01:00
merge-script
6f36624147
Merge bitcoin/bitcoin#30150: doc: Correct pull request prefix for scripts and tools
fa3e1151a28345edff8f371283745bdd647f9a74 doc: Correct pull request prefix for scripts and tools (MarcoFalke)

Pull request description:

  `script` is confusing, because in the context of Bitcoin, it usually means Bitcoin script (c.f. `CScript` in `script.h`, or pull requests such as https://github.com/bitcoin/bitcoin/pull/27122 using the prefix).

  This could be fixed by renaming it to `scripts` (with a plural `s` at the end), however, looking at the current usage `contrib` and `cli` seem more common (https://github.com/bitcoin/bitcoin/pull/29687, https://github.com/bitcoin/bitcoin/pull/26953, https://github.com/bitcoin/bitcoin/pull/26584, https://github.com/bitcoin/bitcoin/pull/24864, https://github.com/bitcoin/bitcoin/pull/30074, https://github.com/bitcoin/bitcoin/pull/29433 ...)

ACKs for top commit:
  fanquake:
    ACK fa3e1151a28345edff8f371283745bdd647f9a74
  willcl-ark:
    ACK fa3e1151a28345edff8f371283745bdd647f9a74
  hebasto:
    ACK fa3e1151a28345edff8f371283745bdd647f9a74.
  theuni:
    ACK fa3e1151a28345edff8f371283745bdd647f9a74

Tree-SHA512: fb3a3892ca5f859e590c8a620350c397ef1f9eafd9e174c70ef50095d401a396758d6c93ad41888da8025c41e25e691f30c18f9e974af13597f2266bb2c53b6d
2024-05-22 14:43:32 +01:00
merge-script
93bec6e953
Merge bitcoin/bitcoin#30131: wallet, tests: Avoid stringop-overflow warning in PollutePubKey
2289d4524053ab71c0d9133987cb36412797c1a2 wallet, tests: Avoid stringop-overflow warning in PollutePubKey (Ava Chow)

Pull request description:

  Fixes  #30114

ACKs for top commit:
  maflcko:
    ACK 2289d4524053ab71c0d9133987cb36412797c1a2 with g++ 14.1.1 🦄
  theStack:
    utACK 2289d4524053ab71c0d9133987cb36412797c1a2
  laanwj:
    ACK 2289d4524053ab71c0d9133987cb36412797c1a2

Tree-SHA512: 173c3c299bdd890f73e8a67a37880dbf816265e8b3c8298557ef2fc4670f5447005c0d2d81726f9bc43f6a69d677365d90a604354b3cbab0e3c52c4526d0407e
2024-05-22 13:43:33 +01:00
Hennadii Stepanov
21b8a14d37
depends: Fetch miniupnpc sources from an alternative website
The https://miniupnp.tuxfamily.org website is unavailable now.
2024-05-22 12:05:26 +01:00
brunoerg
1e54d61c46 test: add coverage for mapped_as from getrawaddrman
Test addresses are being mapped according to the ASMap
file provided properly. Compare the result of the `getrawaddrman`
RPC with the result from the ASMap Health Check.
2024-05-22 07:58:08 -03:00
brunoerg
8c2714907d net: rpc: return peer's mapped AS in getrawaddrman
This information can be used to check bucketing
logic.
2024-05-22 07:57:04 -03:00
merge-script
fa8cb0516d
Merge bitcoin/bitcoin#30144: ci: Add mising -Wno-error=maybe-uninitialized to armhf task
fa73431dd4709754c34a4d5ad1c940ff9e628cf3 ci: Add mising -Wno-error=maybe-uninitialized to armhf task (MarcoFalke)

Pull request description:

  This happens after bd597c33e3e58cd3c6b22ed42f8f1fd7ff886bb2 in many pull requests as a silent merge conflict. For example:

  * https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2120847661
  * https://github.com/bitcoin/bitcoin/pull/29521#issuecomment-2106542236
  * (Probably many undetected, because the CI task was not yet re-run)
  * ...

ACKs for top commit:
  fjahr:
    utACK fa73431dd4709754c34a4d5ad1c940ff9e628cf3
  fanquake:
    ACK fa73431dd4709754c34a4d5ad1c940ff9e628cf3 - many fixed with 13.x

Tree-SHA512: 6e6ff8dc6f3c6a2abcd04c4203d3468f6e98c1ad3a4da4ad0037a9ee2cbec6bec079a5f778aba0273e38e173849927abcdfcfba7643d08ed66c1168cb89fab08
2024-05-22 09:20:45 +01:00
MarcoFalke
fa3e1151a2
doc: Correct pull request prefix for scripts and tools 2024-05-22 09:59:58 +02:00
merge-script
0388dd702b
Merge bitcoin/bitcoin#30120: Update libsecp256k1 subtree to current master
a057869aa3c42457570765966cb66accb2375b13 build: pass --with-ecmult-gen-kb=86 to secp256k1 (fanquake)
ca3d945dc66e177e8fa3e83c77236de89cc0072a Squashed 'src/secp256k1/' changes from d8311688bd..06bff6dec8 (fanquake)

Pull request description:

  This includes changes from the 0.5.0 release: https://github.com/bitcoin-core/secp256k1/releases/tag/v0.5.0

  > New function secp256k1_ec_pubkey_sort that sorts public keys using lexicographic (of compressed serialization) order.

  > The implementation of the point multiplication algorithm used for signing and public key generation was changed, resulting in improved performance for those operations.
  >    The related configure option --ecmult-gen-precision was replaced with --ecmult-gen-kb (ECMULT_GEN_KB for CMake).
  >    This changes the supported precomputed table sizes for these operations. The new supported sizes are 2 KiB, 22 KiB, or 86 KiB (while the old supported sizes were 32 KiB, 64 KiB, or 512 KiB).

ACKs for top commit:
  hebasto:
    ACK a057869aa3c42457570765966cb66accb2375b13, I've got a zero diff with my local branch, which reproduces the subtree update, and `ecmult gen table size   = 86 KiB` in the configure summary.
  jonasnick:
    utACK a057869aa3c42457570765966cb66accb2375b13

Tree-SHA512: 907012b0d7e0a6bd68b245c238e968f2318d8ac5de5ec9070245de8391c996eb5ec6428184d028f6f0f54d3b2f5a8292ad7081177e1c331397879505436dc38e
2024-05-22 08:50:42 +01:00
Ava Chow
9f4ff1e965 windeploy: Renew certificate 2024-05-21 23:19:51 -04:00
merge-script
2ec0a28a37
Merge bitcoin/bitcoin#30137: build: Remove --enable-threadlocal
17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af build: remove --enable-threadlocal (fanquake)
1e7c20bc19a216269c646177ab90cfa084c096a5 doc: remove comment about using thread_local (fanquake)
5bba43312c0ceccfe18bd4d086e12ec0497ed926 build: Enable `thread_local` for MinGW-w64 builds (Hennadii Stepanov)

Pull request description:

  Includes a commit from #30099.
  Closes #29952.

ACKs for top commit:
  laanwj:
    Code review ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af
  maflcko:
    utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af
  hebasto:
    ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af.
  theuni:
    utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af

Tree-SHA512: 2aad6d19e79c4d6d7aefd0f41b215ac8d9320f5908808221d78e6ee1c77503832a02759bee2ad397e235b6739e22aca8dcf5c5ef8854deb8c697b18ac56a06da
2024-05-21 21:14:30 +01:00
Ava Chow
6c13b1375f
Merge bitcoin/bitcoin#29421: net: make the list of known message types a compile time constant
b3efb486732f3caf8b8a8e9d744e6d20ae4255ef protocol: make message types constexpr (Vasil Dimov)
2fa9de06c2c8583ee8e2434dc97014b26e218ab5 net: make the list of known message types a compile time constant (Vasil Dimov)

Pull request description:

  Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size.

  ---

  This change is part of https://github.com/bitcoin/bitcoin/pull/29418 but it makes sense on its own and would be good to have it, regardless of the fate of https://github.com/bitcoin/bitcoin/pull/29418. Also, if this is merged, that would reduce the size of https://github.com/bitcoin/bitcoin/pull/29418, thus the current standalone PR.

ACKs for top commit:
  achow101:
    ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef
  jonatack:
    ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef
  maflcko:
    utACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef 🎊
  willcl-ark:
    ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef

Tree-SHA512: 6d3860c138c64514ebab13d97ea67893e2d346dfac30a48c3d9bc769a1970407375ea4170afdb522411ced306a14a9af4eede99e964d1fb1ea3efff5d5eb57af
2024-05-21 13:59:33 -04:00
Ava Chow
2289d45240 wallet, tests: Avoid stringop-overflow warning in PollutePubKey 2024-05-21 12:59:47 -04:00
glozow
154b2b2296 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits 2024-05-21 15:06:58 +01:00
glozow
a29f1df289 [policy] restrict all v3 transactions to 10kvB 2024-05-21 15:06:55 +01:00
glozow
d578e2e354 [policy] explicitly require non-v3 for CPFP carve out
This carve out is intended to allow a second child under restricted
circumstances, but this topology is not allowed for v3 transactions.

As CPFP carve out does not explicitly require a second child to actually
exist, it has the effect of granting a free +10KvB descendant size limit
when a single child is enough to bust the descendant limit.
2024-05-21 15:06:02 +01:00
furszy
6629d1d0f8
test: improve robustness of connect_nodes()
The 'connect_nodes' function in the test framework relies
on a stable number of peer connections to verify the new
connection between the nodes is successfully established.
This approach is fragile, as any of the peers involved in
the process can drop, lose, or create a connection at any
step, causing subsequent 'wait_until' checks to stall
indefinitely even when the peers in question are connected
successfully.

This commit improves the situation by using the nodes' subversion
and the connection direction (inbound/outbound) to identify the
exact peer connection and perform the checks exclusively on it.
2024-05-21 10:58:44 -03:00
fanquake
17fe948cce
build: remove --enable-threadlocal 2024-05-21 10:29:51 +01:00
fanquake
1e7c20bc19
doc: remove comment about using thread_local
Followup to https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1605655974.
2024-05-21 10:29:51 +01:00
Hennadii Stepanov
5bba43312c
build: Enable thread_local for MinGW-w64 builds
The assumption in the commit 188ca75e5fe4837d16241446558c7566912f67b2
about the broken `thread_local` implementation in GCC was misguided
because the initial failure was not due to GCC, but a bug in the Wine
runtime, as evidenced, for example, here:
 - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=917307
 - https://bugs.freedesktop.org/show_bug.cgi?id=108662

Consequently, it is safe to re-enable `thread_local` support for
MinGW-w64 builds.
2024-05-21 10:27:42 +01:00
merge-script
a786fd2041
Merge bitcoin/bitcoin#30143: doc: Update NetBSD Build Guide
85e480a41ad0901a8b46759e921c187ebbbcccdf doc: Update NetBSD Build Guide (Hennadii Stepanov)

Pull request description:

  This PR updates the NetBSD Build Guide to reflect:
  - the recent NetBSD Release
  - GCC minimum supported version update (https://github.com/bitcoin/bitcoin/pull/28348 and https://github.com/bitcoin/bitcoin/pull/29091)
  - Python minimum supported version update (https://github.com/bitcoin/bitcoin/pull/28211)

  Also a smaller package set has been suggested:
  - `boost-headers` instead of the full `boost`
  - `qt5-qtbase qt5-qttools` instead of the full `qt5` (similar to https://github.com/bitcoin/bitcoin/pull/29932 and https://github.com/bitcoin/bitcoin/pull/29947).

ACKs for top commit:
  maflcko:
    utACK 85e480a41ad0901a8b46759e921c187ebbbcccdf

Tree-SHA512: 0848b6f169a00f2da78eea452fd116193aece853680f4e615ba1df654246ec2a9d1600a4cfb238eb9305b72bbe4b2873519bf1e2987eaaf48aba900c64e85a48
2024-05-21 10:26:49 +01:00
merge-script
8804ec736a
Merge bitcoin/bitcoin#30095: util: avoid using thread_local variable that has a destructor
d35ba1b3f16071b8fe9b36398ba15352dbf2a54d util: avoid using thread_local variable that has a destructor (Vasil Dimov)

Pull request description:

  Store the thread name in a `thread_local` variable of type `char[]` instead of `std::string`. This avoids calling the destructor when the thread exits. This is a workaround for
  https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278701

  For type-safety, return `std::string` from
  `util::ThreadGetInternalName()` instead of `char[]`.

  As a side effect of this change, we no longer store a reference to a `thread_local` variable in `CLockLocation`. This was dangerous because if the thread quits while the reference still exists (in the global variable `lock_data`, see inside `GetLockData()`) then the reference will become dangling.

ACKs for top commit:
  laanwj:
    Code review ACK d35ba1b3f16071b8fe9b36398ba15352dbf2a54d
  hebasto:
    re-ACK d35ba1b3f16071b8fe9b36398ba15352dbf2a54d.
  theuni:
    utACK d35ba1b3f16071b8fe9b36398ba15352dbf2a54d

Tree-SHA512: a2a3bc4401654d6e99db5b9c46a7051855f5a26886142298662e681b78dd581ff4c6bebe42f649b8e1fb8a78d569c6117302db2cd6362e884a22f2a5839b7d43
2024-05-21 10:25:56 +01:00
merge-script
5acdc2b97d
Merge bitcoin/bitcoin#26606: wallet: Implement independent BDB parser
d51fbab4b32d56765e8faab6ad01245fb259b0ca wallet, test: Be able to always swap BDB endianness (Ava Chow)
0b753156ce60c29efb2386954ba7555ad8f642f5 test: Test bdb_ro dump of wallet without reset LSNs (Ava Chow)
c1984f128284589423b7e0cc06c9a3b23a242d95 test: Test dumping dbs with overflow pages (Ava Chow)
fd7b16e391ed320e35255157a28be14c947ef30a test: Test dumps of other endian BDB files (Ava Chow)
6ace3e953f0864bd7818f040c59a1bc70aa47512 bdb: Be able to make byteswapped databases (Ava Chow)
d9878903fb34939dee8e1462f079acc68110253d Error if LSNs are not reset (Ava Chow)
4d7a3ae78e55f25868979f1bd920857a4aecb825 Berkeley RO Database fuzz test (TheCharlatan)
3568dce9e93295674cdf5458c5bdf93ff01fd0a2 tests: Add BerkeleyRO to db prefix tests (Ava Chow)
70cfbfdadf16d3b115309c6938f07ef5b96c7cc1 wallettool: Optionally use BERKELEY_RO as format when dumping BDB wallets (Ava Chow)
dd57713f6ede3d46e97ee7df87c10001b0bf4c3d Add MakeBerkeleyRODatabase (Ava Chow)
6e50bee67d1d58aecd8a0ce8b7c3f5a7979365f5 Implement handling of other endianness in BerkeleyRODatabase (Ava Chow)
cdd61c9cc108df8e13f4e3891ff2c96355b3ee38 wallet: implement independent BDB deserializer in BerkeleyRODatabase (Ava Chow)
ecba23097955dad7208baa687fc405c846aee794 wallet: implement BerkeleyRODatabase::Backup (Ava Chow)
0c8e72847603540bb29b8b8aeb80fa3f2e3a2c9a wallet: implement BerkeleyROBatch (Ava Chow)
756ff9b478484b17c4a6e65c171c2e4fecb21ad4 wallet: add dummy BerkeleyRODatabase and BerkeleyROBatch classes (Ava Chow)
ca18aea5c4975ace4e307be96c74641d203fa389 Add AutoFile::seek and tell (Ava Chow)

Pull request description:

  Split from #26596

  This PR adds `BerkeleyRODatabase` which is an independent implementation of a BDB file parser. It provides read only access to a BDB file, and can therefore be used as a read only database backend for wallets. This will be used for dumping legacy wallet records and migrating legacy wallets without the need for BDB itself.

  Wallettool's `dump` command is changed to use `BerkeleyRODatabase` instead of `BerkeleyDatabase` (and `CWallet` itself) to demonstrate that this parser works and to test it against the existing wallettool functional tests.

ACKs for top commit:
  josibake:
    reACK d51fbab4b3
  TheCharlatan:
    Re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
  furszy:
    reACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
  laanwj:
    re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
  theStack:
    ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca

Tree-SHA512: 1e7b97edf223b2974eed2e9eac1179fc82bb6359e0a66b7d2a0c8b9fa515eae9ea036f1edf7c76cdab2e75ad994962b134b41056ccfbc33b8d54f0859e86657b
2024-05-21 10:05:09 +01:00
MarcoFalke
fa73431dd4
ci: Add mising -Wno-error=maybe-uninitialized to armhf task 2024-05-21 06:48:40 +02:00
Cory Fields
d7707d9843 rpc: avoid copying into UniValue
These are simple (and hopefully obviously correct) copies that can be moves
instead.
2024-05-20 16:48:19 +00:00
Hennadii Stepanov
85e480a41a
doc: Update NetBSD Build Guide 2024-05-20 12:59:15 +01:00
glozow
ecd23656db
Merge bitcoin/bitcoin#30133: test: remove unneeded -maxorphantx=1000 settings
8950053636cb38ed85fe2d58b53e5d0acb35c390 test: remove unneeded `-maxorphantx=1000` settings (Sebastian Falbesoner)

Pull request description:

  It's unclear what the motivation for increasing the orphan pool is here, and it seems that this not needed at all. None of these tests involve orphan transactions explicitly, and if they would occur occasionally, there is no good reason to prefer a value of 1000 over the default of 100 (see DEFAULT_MAX_ORPHAN_TRANSACTIONS).

ACKs for top commit:
  maflcko:
    utACK 8950053636cb38ed85fe2d58b53e5d0acb35c390
  edilmedeiros:
    Tested ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390
  AngusP:
    tACK 8950053636cb38ed85fe2d58b53e5d0acb35c390
  glozow:
    ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390 From skimming the tests, it appears that none of these need a larger `-maxorphantx`.

Tree-SHA512: 81d4a4fb2ea92b97119f21cbc6c4b1240d863269932e6adf4982aead9726f20652523a4707add3ad38eb332d4452de41de6735265f22e62298f3b4b45de75a57
2024-05-20 09:47:31 +01:00
glozow
063bb2fbb5
Merge bitcoin/bitcoin#30066: test: add conflicting topology test case
9365baa489e123d9bcaf986e4311d3fa3f1e3f88 test: add conflicting topology test case (Greg Sanders)

Pull request description:

  We want to ensure that even if topologies
  that are acceptable are relaxed, like
  removing package-not-child-with-unconfirmed-parents, that we don't end up accepting packages we shouldn't.

ACKs for top commit:
  glozow:
    reACK 9365baa489
  rkrux:
    reACK [9365baa](9365baa489)

Tree-SHA512: d58661064ca099ac0447c331a5020c74c0cdfe24259aa875592805bbd63de1bf23aa7ced9ff485fef90dc0602fcb997e631aaf1aa2e9805d2cf5f0e5c9b2f0e2
2024-05-20 09:27:37 +01:00
fanquake
a057869aa3
build: pass --with-ecmult-gen-kb=86 to secp256k1 2024-05-18 11:12:25 +08:00
Sebastian Falbesoner
8950053636 test: remove unneeded -maxorphantx=1000 settings
It's unclear what the motivation for increasing the orphan pool is, and
it seems that this not needed at all. None of these tests involve orphan
transactions explicitly, and if they would occur occasionally, there is
no good reason to prefer a value of 1000 over the default of 100 (see
DEFAULT_MAX_ORPHAN_TRANSACTIONS).
2024-05-17 22:38:59 +02:00
Ava Chow
058af75874
Merge bitcoin/bitcoin#29817: kernel: De-globalize fReindex
b47bd959207e82555f07e028cc2246943d32d4c3 kernel: De-globalize fReindex (TheCharlatan)

Pull request description:

  fReindex is one of the last remaining globals exposed by the kernel library, so move it into the blockstorage class to reduce the amount of global mutable state and make the kernel library a bit less awkward to use.

  ---

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

ACKs for top commit:
  achow101:
    ACK b47bd959207e82555f07e028cc2246943d32d4c3
  ryanofsky:
    Code review ACK b47bd959207e82555f07e028cc2246943d32d4c3. I rereviewed the whole PR, but the only change since last review was reverting the bugfix https://github.com/bitcoin/bitcoin/pull/29817#discussion_r1578327024 and make the change a pure refactoring.
  mzumsande:
    Code Review ACK b47bd959207e82555f07e028cc2246943d32d4c3
  stickies-v:
    ACK b47bd959207e82555f07e028cc2246943d32d4c3

Tree-SHA512: f7399d01f93bc0c0c7428fe95d19b9d29b4ed00a4f1deabca78fb0c4fecb434ec971e890feecb105938b5247c926850b1b7b4a4a9caa333a061e40777d0c8463
2024-05-17 15:50:56 -04:00
Ava Chow
4877fcdb42
Merge bitcoin/bitcoin#30048: crypto: add NUMS_H const
9408a04e424cee0d226bde79171bd4954f9caeb0 tests, fuzz: use new NUMS_H const (josibake)
b946f8a4c51be42e52d63a6d578158c0b2a6b7ed crypto: add NUMS_H const (josibake)

Pull request description:

  Broken out from #28122

  ---

  [BIP341](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs) defines a NUMS point `H` as *H = lift_x(0x50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0)* which is [constructed](11af7015de/src/modules/rangeproof/main_impl.h (L16)) by taking the hash of the standard uncompressed encoding of the [secp256k1](https://www.secg.org/sec2-v2.pdf) base point G as X coordinate."

  Add this as a constant so it can be used in our codebase. My primary motivation is BIP352 specifies a special case for when taproot spends use `H` as the internal key, but outside of BIP352 it seems generally useful to have `H` in the codebase, for testing or other use cases.

ACKs for top commit:
  paplorinc:
    re-ACK 9408a04e424cee0d226bde79171bd4954f9caeb0
  achow101:
    ACK 9408a04e424cee0d226bde79171bd4954f9caeb0
  theStack:
    Code-review ACK 9408a04e424cee0d226bde79171bd4954f9caeb0

Tree-SHA512: ad84492f5d635c0cb05bd82546079ded7e5138e95361f20d8285a9ad6e69c10ee2cc3fe46e16b46ef03c4253c8bee1051911c6b91264c90c3b1ad33a824bff4b
2024-05-17 14:10:51 -04:00
Ava Chow
d51fbab4b3 wallet, test: Be able to always swap BDB endianness 2024-05-16 15:03:13 -04:00
Ava Chow
0b753156ce test: Test bdb_ro dump of wallet without reset LSNs 2024-05-16 15:03:13 -04:00
Ava Chow
c1984f1282 test: Test dumping dbs with overflow pages 2024-05-16 15:03:13 -04:00
Ava Chow
fd7b16e391 test: Test dumps of other endian BDB files 2024-05-16 15:03:13 -04:00
Ava Chow
6ace3e953f bdb: Be able to make byteswapped databases
Byteswapped databases make it easier to test opening and deserializing
other endian databases.
2024-05-16 15:03:13 -04:00
Ava Chow
d9878903fb Error if LSNs are not reset 2024-05-16 15:03:13 -04:00
TheCharlatan
4d7a3ae78e Berkeley RO Database fuzz test 2024-05-16 15:03:13 -04:00
Ava Chow
3568dce9e9 tests: Add BerkeleyRO to db prefix tests 2024-05-16 15:03:13 -04:00
Ava Chow
70cfbfdadf wallettool: Optionally use BERKELEY_RO as format when dumping BDB wallets
In order to ease the transition to not having BDB, make the dump tool
use DatabaseFormmat::BERKELEY_RO when -withinternalbdb is set.
2024-05-16 15:03:13 -04:00
Ava Chow
dd57713f6e Add MakeBerkeleyRODatabase
Implements MakeBerkeleyRODatabase and adds DatabaseFormat::BERKELEY_RO
so that MakeDatabase can use BerkeleyRO as the backend database.
2024-05-16 15:03:13 -04:00
Ava Chow
6e50bee67d Implement handling of other endianness in BerkeleyRODatabase 2024-05-16 15:03:13 -04:00
Ava Chow
cdd61c9cc1 wallet: implement independent BDB deserializer in BerkeleyRODatabase
BerkeleyRODatabase is intended for use after BDB is removed, so it needs
to be able to read all of the records from a BDB file. Thus an
independent deserializer for BDB data files is implemented in it. This
deserializer is targeted towards the data files that Bitcoin Core
creates so it does not fully support all of BDB's features (e.g. other
database types, encryption, etc.).
2024-05-16 15:03:13 -04:00