aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2024-05-23Merge bitcoin/bitcoin#27064: system: use %LOCALAPPDATA% as default datadir ↵Ava Chow
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 https://github.com/bitcoin/bitcoin/commit/84900ac34f6888b7a851d0a6a5885192155f865c 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-23Add sanity checks for various ATMPArgs booleansGreg Sanders
2024-05-23[refactor] make some members MemPoolAccept-wideglozow
No change in behavior. For single transaction acceptance, this is a simple refactor: Workspace::m_all_conflicting Workspace::m_conflicting_fees Workspace::m_conflicting_size Workspace::m_replaced_transactions are now grouped under a new SubPackageState struct that is a member of MemPoolAccept. And local variables m_total_vsize and m_total_modified_fees are now SubpackageState members so they can be accessed from PackageMempoolChecks. We want these to be package-wide variables because - Transactions could conflict with the same tx (just not the same prevout), or their conflicts could share descendants. - We want to compare conflicts with the package fee rather than individual transaction fee. We reset these MemPoolAccept-wide fields for each subpackage evaluation to not cause state leaking, similar to temporary coins.
2024-05-23cpfp carveout is excluded in packagesglozow
The behavior is not new, but this rule exits earlier than before. Previously, a carve out could have been granted in PreChecks() but then nullified in PackageMempoolChecks() when CheckPackageLimits() is called with the default limits.
2024-05-23Add m_allow_sibling_eviction as separate ATMPArgs flagGreg Sanders
2024-05-23Add description for m_test_acceptGreg Sanders
2024-05-23Merge bitcoin/bitcoin#29873: policy: restrict all TRUC (v3) transactions to ↵Ava Chow
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 https://github.com/bitcoin/bitcoin/commit/3df62878c3cece15a8921fbbdee7859ee9368768 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 https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11 murchandamus: crACK 154b2b2296edccb5ed24e829798dacb6195edc11 Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
2024-05-23Merge bitcoin/bitcoin#30115: rpc: avoid copying into UniValueRyan Ofsky
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-23Merge bitcoin/bitcoin#30062: net: add ASMap info in `getrawaddrman` RPCglozow
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](https://github.com/bitcoin/bitcoin/commit/1e54d61c4698debf3329d1960e06078ccbf8063c) 0xB10C: ACK 1e54d61c4698debf3329d1960e06078ccbf8063c glozow: ACK 1e54d61c4698debf3329d1960e06078ccbf8063c Tree-SHA512: af86bcc7a2e69bebd3fa9eaa2e527e0758c44c0a958de7292514d5f99f8f01f5df3bae11400451268e0255f738ff3acccc77f48fe129937512f1e9d9963c4c5e
2024-05-22Merge bitcoin/bitcoin#30131: wallet, tests: Avoid stringop-overflow warning ↵merge-script
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-22net: rpc: return peer's mapped AS in getrawaddrmanbrunoerg
This information can be used to check bucketing logic.
2024-05-22Merge bitcoin/bitcoin#30120: Update libsecp256k1 subtree to current mastermerge-script
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-21Merge bitcoin/bitcoin#30137: build: Remove `--enable-threadlocal`merge-script
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-21Merge bitcoin/bitcoin#29421: net: make the list of known message types a ↵Ava Chow
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-21wallet, tests: Avoid stringop-overflow warning in PollutePubKeyAva Chow
2024-05-21[fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limitsglozow
2024-05-21[policy] restrict all v3 transactions to 10kvBglozow
2024-05-21[policy] explicitly require non-v3 for CPFP carve outglozow
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-21rpc: Enhance metadata of the dumptxoutset outputFabian Jahr
The following data is added: - A newly introduced utxo set magic - A version number - The network magic - The block height
2024-05-21assumeutxo: Add documentation on dumptxoutset serialization formatFabian Jahr
2024-05-21rpc: Optimize serialization disk space of dumptxoutsetFabian Jahr
Co-authored-by: Aurèle Oulès <aurele@oules.com> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2024-05-21build: remove --enable-threadlocalfanquake
2024-05-21doc: remove comment about using thread_localfanquake
Followup to https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1605655974.
2024-05-21Merge bitcoin/bitcoin#30095: util: avoid using thread_local variable that ↵merge-script
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-21Merge bitcoin/bitcoin#26606: wallet: Implement independent BDB parsermerge-script
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 https://github.com/bitcoin/bitcoin/commit/d51fbab4b32d56765e8faab6ad01245fb259b0ca TheCharlatan: Re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca furszy: reACK d51fbab4b32d56765e8faab6ad01245fb259b0ca laanwj: re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca theStack: ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca Tree-SHA512: 1e7b97edf223b2974eed2e9eac1179fc82bb6359e0a66b7d2a0c8b9fa515eae9ea036f1edf7c76cdab2e75ad994962b134b41056ccfbc33b8d54f0859e86657b
2024-05-21refactor: replace hardcoded numbersLőrinc
Replace ExpandHRP with a PreparePolynomialCoefficients function. Instead of using a hardcoded value for the size of the array (90 in this case) and a hardcoded value for the checksum, use the actual values vector and define checksum size as a constexpr. Use the new CHECKSUM_SIZE throughout instead 6. Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
2024-05-20rpc: avoid copying into UniValueCory Fields
These are simple (and hopefully obviously correct) copies that can be moves instead.
2024-05-17[[refactor]] Check CTxMemPool options in constructorTheCharlatan
This ensures that the tests run the same checks on the mempool options that the init code also applies.
2024-05-17Merge bitcoin/bitcoin#29817: kernel: De-globalize fReindexAva Chow
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-17Merge bitcoin/bitcoin#30048: crypto: add `NUMS_H` constAva Chow
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](https://github.com/ElementsProject/secp256k1-zkp/blob/11af7015de624b010424273be3d91f117f172c82/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-17bench: bugfix, properly release wallet before erasing directoryfurszy
Since the wallet is appended to the global WalletContext during creation, merely calling 'reset()' on the benchmark shared_pointer is insufficient to destruct the wallet. This no destruction of the wallet results in the db connection remaining open, which was the cause of the 'fs::remove_all()' error in Windows. Co-Authored-By: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2024-05-16wallet, test: Be able to always swap BDB endiannessAva Chow
2024-05-16bdb: Be able to make byteswapped databasesAva Chow
Byteswapped databases make it easier to test opening and deserializing other endian databases.
2024-05-16Error if LSNs are not resetAva Chow
2024-05-16Berkeley RO Database fuzz testTheCharlatan
2024-05-16tests: Add BerkeleyRO to db prefix testsAva Chow
2024-05-16wallettool: Optionally use BERKELEY_RO as format when dumping BDB walletsAva Chow
In order to ease the transition to not having BDB, make the dump tool use DatabaseFormmat::BERKELEY_RO when -withinternalbdb is set.
2024-05-16Add MakeBerkeleyRODatabaseAva Chow
Implements MakeBerkeleyRODatabase and adds DatabaseFormat::BERKELEY_RO so that MakeDatabase can use BerkeleyRO as the backend database.
2024-05-16Implement handling of other endianness in BerkeleyRODatabaseAva Chow
2024-05-16wallet: implement independent BDB deserializer in BerkeleyRODatabaseAva Chow
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-16fuzz: add more coverage for `ScriptPubKeyMan`brunoerg
2024-05-16util: avoid using thread_local variable that has a destructorVasil Dimov
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.
2024-05-16Merge bitcoin/bitcoin#29975: blockstorage: Separate reindexing from saving ↵Ryan Ofsky
new blocks e41667b720372dae8438ea86e9819027e62b54e0 blockstorage: Don't move cursor backwards in UpdateBlockInfo (Ryan Ofsky) 17103637c6fa2dfcf5374ebb0cd715e540dd4ce1 blockstorage: Rename FindBlockPos and have it return a FlatFilePos (Martin Zumsande) d9e477c4dc39d9623ed66c35c06e28f94ae62ad5 validation, blockstorage: Separate code paths for reindex and saving new blocks (Martin Zumsande) 064859bbad6984a6ec85c744064abdf757807c58 blockstorage: split up FindBlockPos function (Martin Zumsande) fdae638e83522c28a1222e65c43d1cbca3e34cba doc: Improve doc for functions involved in saving blocks to disk (Martin Zumsande) 0d114e3cb20cb9e03fc9ba8daf3d03436b491742 blockstorage: Add Assume for fKnown / snapshot chainstate (Martin Zumsande) Pull request description: `SaveBlockToDisk` / `FindBlockPos` are used for two purposes, depending on whether they are called during reindexing (`dbp` set,  `fKnown = true`) or in the "normal" case when adding new blocks (`dbp == nullptr`,  `fKnown = false`). The actual tasks are quite different - In normal mode, preparations for saving a new block are made, which is then saved: find the correct position on disk (maybe skipping to a new blk file), check for available disk space, update the blockfile info db, save the block. - during reindex, most of this is not necessary (the block is already on disk after all), only the blockfile info needs to rebuilt because reindex wiped the leveldb it's saved in. Using one function with many conditional statements for this leads to code that is hard to read / understand and bug-prone: - many code paths in `FindBlockPos` are conditional on `fKnown` or `!fKnown` - It's not really clear what actually needs to be done during reindex (we don't need to "save a block to disk" or "find a block pos" as the function names suggest) - logic that should be applied to only one of the two modes is sometimes applied to both (see first commit, or #27039) #24858 and #27039 were recent bugs directly related to the differences between reindexing and normal mode, and in both cases the simple fix took a long time to be reviewed and merged. This PR proposes to clean this code up by splitting out the reindex logic into a separate function (`UpdateBlockInfo`) which will be called directly from validation. As a result, `SaveBlockToDisk` and `FindBlockPos` only need to cover the non-reindex logic. ACKs for top commit: paplorinc: ACK e41667b720372dae8438ea86e9819027e62b54e0 TheCharlatan: Re-ACK e41667b720372dae8438ea86e9819027e62b54e0 ryanofsky: Code review ACK e41667b720372dae8438ea86e9819027e62b54e0. Just improvements to comments since last review. Tree-SHA512: a14ff9a0facf6b1e3c1cd724a2d19a79a25d4b48de64398fdd172671532a472bc10a20cbb64ac3a3e55814dcc877d0597a3e1699cabc4f9d9a86b439b6eaba20
2024-05-16util: Move util/string.h functions to util namespaceRyan Ofsky
There are no changes to behavior. Changes in this commit are all additions, and are easiest to review using "git diff -U0 --word-diff-regex=." options. Motivation for this change is to keep util functions with really generic names like "Split" and "Join" out of the global namespace so it is easier to see where these functions are defined, and so they don't interfere with function overloading, especially since the util library is a dependency of the kernel library and intended to be used with external code.
2024-05-16util: add TransactionError includes and namespace declarationsRyan Ofsky
Add TransactionError to node namespace and include it directly instead of relying on indirect include through common/messages.h This is a followup to a previous commit which moved the TransactionError enum. These changes were done in a separate followup just to keep the previous commit more minimal and easy to review.
2024-05-16util: move fees.h and error.h to common/messages.hRyan Ofsky
Move enum and message formatting functions to a common/messages header where they should be more discoverable, and also out of the util library, so they will not be a dependency of the kernel The are no changes in behavior and no changes to the moved code.
2024-05-16common: Add PSBTError enumRyan Ofsky
Add separate PSBTError enum instead of reusing TransactionError enum for PSBT operations, and drop unused error codes. The error codes returned by PSBT operations and transaction broadcast functions mostly do not overlap, so using an unified enum makes it harder to call any of these functions and know which errors actually need to be handled. Define PSBTError in the common library because PSBT functionality is implemented in the common library and used by both the node (for rawtransaction RPCs) and the wallet.
2024-05-16util: move error.h TransactionError enum to node/types.hRyan Ofsky
New node/types.h file is analagous to existing wallet/types.h and is a better place to define simple node types that are shared externally with wallet and gui code than the util library. Motivation for this change is to completely remove util/error.h file currently holding TransactionError in a followup commit.
2024-05-16util: move spanparsing.h to script/parsing.hRyan Ofsky
Move miniscript / descriptor script parsing functions out of util library so they are not a dependency of the kernel. There are no changes to code or behavior.
2024-05-16util: move spanparsing.h Split functions to string.hRyan Ofsky
This will help move the miniscript / descriptor parsing functions out of the util library in an upcoming commit, so they are not exposed to libbitcoinkernel applications. Moving the Split functions should also make them more discoverable since they now close to related functions like Join. The functions are moved verbatim without any changes.