aboutsummaryrefslogtreecommitdiff
path: root/src/node
AgeCommit message (Collapse)Author
2024-08-27Merge bitcoin/bitcoin#30697: Bugfix: Ensure Atomicity in Wallet Settings ↵Ava Chow
Updates from Chain Interface 1b41d45d462d856a9d0b44ae0039bbb2cd78407c wallet: bugfix: ensure atomicity in settings updates (ismaelsadeeq) Pull request description: This PR fixes #30620. As outlined in the issue, creating two wallets with `load_on_startup=true` simultaneously results in only one wallet being added to the startup file. The current issue arises because the wallet settings update process involves: 1. Obtaining the settings value while acquiring the settings lock. 2. Modifying the settings value. 3. Overwriting the settings value while acquiring the settings lock again. This sequence is not thread-safe. Different threads could modify the same base value simultaneously, overwriting data from other workers without realizing it. The PR attempts to fix this by modifying the chain interface's `updateRwSetting` method to accept a function that will be called with the settings reference. This function will either update or delete the setting and return an enum indicating whether the settings need to be overwritten in this or not. Additionally, this PR introduces two new methods to the chain interface: - `overwriteRwSetting`: This method replaces the setting with a new value. Used in `VerifyWallets` - `deleteRwSettings`: This method completely erases a specified setting. This method is currently used only in `overwriteRwSetting`. These changes ensure that updates are race-free across all clients. ACKs for top commit: achow101: ACK 1b41d45d462d856a9d0b44ae0039bbb2cd78407c furszy: self-code-ACK https://github.com/bitcoin/bitcoin/commit/1b41d45d462d856a9d0b44ae0039bbb2cd78407c Tree-SHA512: 50cda612b782aeb5e03e2cf63cc44779a013de1c535b883b57af4de22f24b0de80b4edecbcda235413baec0a12bdf0e5750fb6731c9e67d32e742d8c63f08c13
2024-08-26wallet: bugfix: ensure atomicity in settings updatesismaelsadeeq
- Settings updates were not thread-safe, as they were executed in three separate steps: 1) Obtain settings value while acquiring the settings lock. 2) Modify settings value. 3) Overwrite settings value while acquiring the settings lock. This approach allowed concurrent threads to modify the same base value simultaneously, leading to data loss. When this occurred, the final settings state would only reflect the changes from the last thread that completed the operation, overwriting updates from other threads. Fix this by making the settings update operation atomic. - Add test coverage for this behavior. Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2024-08-20miner: adjust clock to timewarp ruleSjors Provoost
2024-08-12add missing #include <cstdint> for GCC 15Matt Whitlock
2024-08-09Merge bitcoin/bitcoin#30598: assumeutxo: Drop block height from metadataAva Chow
00618e8745192d209c23e3ae873c077e58168957 assumeutxo: Drop block height from metadata (Fabian Jahr) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/30514 which has more context and shows how the issue can be reproduced. Since the value in question is removed, there is no test to add to reproduce anything. This is an alternative approach to #30516 with much of the [code being suggested there](https://github.com/bitcoin/bitcoin/pull/30516#discussion_r1689146902). ACKs for top commit: maflcko: re-ACK 00618e8745192d209c23e3ae873c077e58168957 🎌 achow101: ACK 00618e8745192d209c23e3ae873c077e58168957 theStack: Code-review ACK 00618e8745192d209c23e3ae873c077e58168957 ismaelsadeeq: Re-ACK 00618e8745192d209c23e3ae873c077e58168957 mzumsande: ACK 00618e8745192d209c23e3ae873c077e58168957 Tree-SHA512: db9575247bae838ad7742a27a216faaf55bb11e022f9afdd05752bb09bbf9614717d0ad64304ff5722a16bf41d8dea888af544e4ae26dcaa528c1add0269a4a8
2024-08-09Merge bitcoin/bitcoin#28687: C++20 std::views::reversemerge-script
2925bd537cbd8c70594e23f6c4298b7101f7f73d refactor: use c++20 std::views::reverse instead of reverse_iterator.h (stickies-v) Pull request description: C++20 introduces [`std::ranges::views::reverse`](https://en.cppreference.com/w/cpp/ranges/reverse_view), which allows us to drop our own `reverse_iterator.h` implementation and also makes it easier to chain views (even though I think we currently don't use this). ACKs for top commit: achow101: ACK 2925bd537cbd8c70594e23f6c4298b7101f7f73d maflcko: ACK 2925bd537cbd8c70594e23f6c4298b7101f7f73d 🎷 Tree-SHA512: 567666ec44af5d1beb7a271836bcc89c4c577abc77f522fcc18bc6d4de516ae9b0df766d0bfa6dd217569e6878331c2aee1d9815620860375e3510dad7fed476
2024-08-08assumeutxo: Drop block height from metadataFabian Jahr
The Snapshot format version is updated to 2 to indicate this change. Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2024-08-07Merge bitcoin/bitcoin#30194: refactor: use recommended type hiding on ↵glozow
multi_index types a3cb309e7c31853f272bffaa65fb6ab0a7cc4083 refactor: use recommended type hiding on multi_index types (Cory Fields) Pull request description: Recommended by boost docs: https://www.boost.org/doc/libs/1_85_0/libs/multi_index/doc/compiler_specifics.html#type_hiding This significantly reduces the size of the symbol name lengths that end up in the binaries as well as in compiler warnings/errors. Otherwise there should be no functional change. Example before: > 0000000000000000 W unsigned long boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to<uint256>, boost::multi_index::detail::nth_layer<1, CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher, mpl_::na, mpl_::na>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, mempoolentry_wtxid, SaltedTxidHasher, mpl_::na>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByAncestorFee>, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, std::allocator<CTxMemPoolEntry> >, boost::mpl::vector0<mpl_::na>, boost::multi_index::detail::hashed_unique_tag>::count<uint256, SaltedTxidHasher, std::equal_to<uint256> >(uint256 const&, SaltedTxidHasher const&, std::equal_to<uint256> const&, mpl_::bool_<false>) const After: > 0000000000000000 W unsigned long boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to<uint256>, boost::multi_index::detail::nth_layer<1, CTxMemPoolEntry, CTxMemPool::CTxMemPoolEntry_Indicies, std::allocator<CTxMemPoolEntry> >, boost::mpl::vector0<mpl_::na>, boost::multi_index::detail::hashed_unique_tag>::count<uint256, SaltedTxidHasher, std::equal_to<uint256> >(uint256 const&, SaltedTxidHasher const&, std::equal_to<uint256> const&, mpl_::bool_<false>) const ACKs for top commit: glozow: ACK a3cb309e7c31853f272bffaa65fb6ab0a7cc4083, TIL, makes sense to me TheCharlatan: ACK a3cb309e7c31853f272bffaa65fb6ab0a7cc4083 fanquake: ACK a3cb309e7c31853f272bffaa65fb6ab0a7cc4083 Tree-SHA512: f6bb3d133daec126cf064ed6fe4457f457c0cfdbea28778c8ff426be7b41b271ada2d790c6b4129ca22156182c99aaf287e3aa9fb6b076ee55946da40e06e5d8
2024-08-07Merge bitcoin/bitcoin#30594: docs: doc update for mempoolfullrbf default + ↵glozow
log deprecation 1f93e3c360f3da38a02fe6c01551f424a2d63dc9 add deprecation warning for mempoolfullrbf (glozow) 4400c979a309ed03c7ed55644122622eb7def089 [doc] update documentation for new mempoolfullrbf default (glozow) Pull request description: Followup to #30493. Update bips.md and policy/*.md to reflect new default rules around signaling requirements in RBF. Also, log a warning when `-mempoolfullrbf=0` that this config option is deprecated and will be removed in a future release. ACKs for top commit: petertodd: ACK 1f93e3c360f3da38a02fe6c01551f424a2d63dc9 instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/30594/commits/1f93e3c360f3da38a02fe6c01551f424a2d63dc9 tdb3: ACK 1f93e3c360f3da38a02fe6c01551f424a2d63dc9 Tree-SHA512: f60a9524f15cfaa4c10c40b6f62b787d3f9865aac48ca883def30efac4f8a118f1359532f1b209ea34e201f0b1c92398abc8bc1e439e6b60910cc7f75c51e9ae
2024-08-07add deprecation warning for mempoolfullrbfglozow
2024-08-06Merge bitcoin/bitcoin#30212: rename TransactionError:ALREADY_IN_CHAINRyan Ofsky
e9de0a76b99fd4708295e1178f6c079db92e7f36 doc: release note for 30212 (willcl-ark) 87b188052528e97729a85d9701864eaff1ea5ec6 rpc: clarify ALREADY_IN_CHAIN rpc errors (willcl-ark) Pull request description: Closes: #19363 Renaming this error improves clarity around the returned error both internally and externally when a transactions' outputs are already found in the utxo set (`TransactionError::ALREADY_IN_CHAIN -> TransactionError::ALREADY_IN_UTXO_SET`) ACKs for top commit: tdb3: ACK e9de0a76b99fd4708295e1178f6c079db92e7f36 ismaelsadeeq: ACK e9de0a76b99fd4708295e1178f6c079db92e7f36 ryanofsky: Code review ACK e9de0a76b99fd4708295e1178f6c079db92e7f36. Tree-SHA512: 7d2617200909790340951fe56a241448f9ce511900777cb2a712e8b9c0778a27d1f912b460f82335844224f1abb4322bc898ca076440959edade55c082a09237
2024-08-06refactor: use c++20 std::views::reverse instead of reverse_iterator.hstickies-v
Use std::ranges::views::reverse instead of the implementation in reverse_iterator.h, and remove it as it is no longer used.
2024-08-05Merge bitcoin/bitcoin#28052: blockstorage: XOR blocksdir *.dat filesAva Chow
fa895c72832f9555b52d5bb1dba1093f73de3136 mingw: Document mode wbx workaround (MarcoFalke) fa359255fe6b4de5f26784bfc147dbfb58bef116 Add -blocksxor boolean option (MarcoFalke) fa7f7ac040a9467c307b20e77dc47c87d7377ded Return XOR AutoFile from BlockManager::Open*File() (MarcoFalke) Pull request description: Currently the *.dat files in the blocksdir store the data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan them and move them into quarantine, or delete them, or corrupt them. This may cause Bitcoin Core to fail a reorg, or fail to reply to block requests (via P2P, RPC, REST, ...). Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a random XOR pattern over the dat files when writing or reading them. Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat files. Any program that intentionally wants to mess with the dat files can still trivially do so. The XOR pattern is only applied when the blocksdir is freshly created, and there is an option to disable it (on creation), so that people can disable it, if needed. ACKs for top commit: achow101: ACK fa895c72832f9555b52d5bb1dba1093f73de3136 TheCharlatan: Re-ACK fa895c72832f9555b52d5bb1dba1093f73de3136 hodlinator: ACK fa895c72832f9555b52d5bb1dba1093f73de3136 Tree-SHA512: c92a6a717da83bc33a9b8671a779eeefde2c63b192362ba1d71e6535ee31d08e2802b74acc908345197de9daac6930e4771595ee25b09acd5a67f7ea34854720
2024-08-05rpc: clarify ALREADY_IN_CHAIN rpc errorswillcl-ark
When using `sendrawtransaction` the ALREADY_IN_CHAIN error help string may be confusing. Rename TransactionError::ALREADY_IN_CHAIN to TransactionError::ALREADY_IN_UTXO_SET and update the rpc help string. Remove backwards compatibility alias as no longer required.
2024-08-04scripted-diff: Modernize naming of nChainTx and nTxCountFabian Jahr
-BEGIN VERIFY SCRIPT- sed -i 's/nChainTx/m_chain_tx_count/g' $(git grep -l 'nChainTx' ./src) sed -i 's/nTxCount/tx_count/g' $(git grep -l 'nTxCount' ./src) -END VERIFY SCRIPT-
2024-08-02Merge bitcoin-core/gui#626: Showing Local Addresses in Node WindowHennadii Stepanov
189c987386a0da132d7ef076cdf539f9eb75fc3c Showing local addresses on the Node Window (Jadi) a5d7aff867a3df9ac77664deed03e930e2636db0 net: Providing an interface for mapLocalHost (Jadi) Pull request description: This change adds a new row to the Node Window (debugwindow.ui) under the Network section which shows the LocalAddresses. fixes #564 <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md --> <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. --> <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. --> ACKs for top commit: pablomartin4btc: re-ACK 189c987386a0da132d7ef076cdf539f9eb75fc3c furszy: utACK 189c987 Tree-SHA512: 93f201bc6d21d81b27b87be050a447b841f01e3efb69b9eca2cc7af103023d7cd69eb5e16e2875855573ef51a5bf74a6ee6028636c1b6798cb4bb11567cb4996
2024-08-02net: Providing an interface for mapLocalHostJadi
Contributes to #564 by providing an interface for mapLocalHost through net -> node interface -> clientModel. Later this value can be read by GUI to show the local addresses.
2024-07-26mingw: Document mode wbx workaroundMarcoFalke
2024-07-26Add -blocksxor boolean optionMarcoFalke
2024-07-26Return XOR AutoFile from BlockManager::Open*File()MarcoFalke
This is a refactor, because the XOR key is empty.
2024-07-24refactor: Add FlatFileSeq member variables in BlockManagerTheCharlatan
Instead of constructing a new class every time a file operation is done, construct them once for each of the undo and block file when a new BlockManager is created. In future, this might make it easier to introduce an abstract block store.
2024-07-17refactor: add coinbase constraints to BlockCreateOptionsSjors Provoost
When generating a block template through e.g. getblocktemplate RPC, we reserve 4000 weight units and 400 sigops. Pools use this space for their coinbase outputs. At least one pool patched their Bitcoin Core node to adjust these hardcoded values. They eventually produced an invalid block which exceeded the sigops limit. https://bitcoin.stackexchange.com/questions/117837/how-many-sigops-are-in-the-invalid-block-783426 The existince of such patches suggests it may be useful to make this value configurable. This commit would make such a change easier. The main motivation however is that the Stratum v2 spec requires the pool to communicate the maximum bytes they intend to add to the coinbase outputs. A proposed change to the spec would also require them to communicate the maximum number of sigops. This commit also documents what happens when -blockmaxweight is lower than the coinbase reserved value. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-07-16refactor: pass BlockCreateOptions to createNewBlockSjors Provoost
Rather than pass options individually to createNewBlock and then combining them into BlockAssembler::Options, this commit introduces BlockCreateOptions and passes that instead. Currently there's only one option (use_mempool) but the next commit adds more. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-07-15Merge bitcoin/bitcoin#30428: log: LogError with FlatFilePos in UndoReadFromDiskRyan Ofsky
fa14e1d9d5c5dc44396a01583ae94480b7bc29ee log: Fix __func__ in LogError in blockstorage module (MarcoFalke) fad59a2f0f37f5b7f6076fd91be43448e35f4b7e log: LogError with FlatFilePos in UndoReadFromDisk (MarcoFalke) aaaa3323f37526862ebf2a2a4bf522c661e6976e refactor: Mark IsBlockPruned const (MarcoFalke) Pull request description: These errors should never happen in normal operation. If they do, knowing the `FlatFilePos` may be useful to determine if data corruption happened. Also, handle the error `pos.IsNull()` as part of `OpenUndoFile`, because it may as well have happened due to data corruption. This mirrors the `LogError` behavior from `ReadBlockFromDisk`. Also, two other fixup commits in this module. ACKs for top commit: kevkevinpal: ACK [fa14e1d](https://github.com/bitcoin/bitcoin/pull/30428/commits/fa14e1d9d5c5dc44396a01583ae94480b7bc29ee) tdb3: cr and light test ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee ryanofsky: Code review ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee. This should make logging clearer and more consistent Tree-SHA512: abb492a919b4796698d1de0a7874c8eae355422b992aa80dcd6b59c2de1ee0d2949f62b3cf649cd62892976fee640358f7522867ed9d48a595d6f8f4e619df50
2024-07-15Merge bitcoin/bitcoin#30412: MiniMiner: use FeeFrac in AncestorFeerateComparatormerge-script
09370529fb9f6d06f6d16bdb1fb336f7a265d8ba fuzz: mini_miner_selection fixups. (glozow) de273d53004f48e4c8c965f7ce0bd375fd8d0d69 MiniMiner: use FeeFrac in AncestorFeerateComparator (glozow) Pull request description: Closes #30284. Closes #30367, see https://github.com/bitcoin/bitcoin/issues/30367#issuecomment-2217459257 Previously, we were only comparing feerates up to 1/1000 precision, since CFeeRate comparison just looks at their respective nSatoshisPerK. This could lead to MiniMiner selecting packages in the wrong order (i.e. by txid) if their feerates were less than 0.001sat/vB different. Fix this by creating + comparing `FeeFrac`s instead. Also, `FeeFrac::Mul` doesn't have the overflow problem. Also added a few minor fuzzer fixups that caught my eye while I was debugging this. ACKs for top commit: ismaelsadeeq: Tested ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba murchandamus: ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba with nits dergoegge: tACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba Tree-SHA512: e5b6d6c3f7289f30cd8280d0a47cd852d0180b83d1b27ff9514f50c97103b0f069484e48cba2ca3a57419beadc1996c1b9dd8d0a0f34bc4f4223d8adaf414ce5
2024-07-14Merge bitcoin-core/gui#825: Show maximum mempool size in information windowHennadii Stepanov
4a028cf54c0502bc9ba0804bf1ae413b20a007cb gui: show maximum mempool size in information window (Sebastian Falbesoner) bbde6ffefea9587b07a9f8d4043b2dd23ef8c3c5 add node interface method for getting maximum mempool size (Sebastian Falbesoner) Pull request description: This PR adds the maximum mempool size to the information window (Menu "Window" -> "Information" -> section "Memory Pool" -> line "Memory usage"). master: ![image](https://github.com/bitcoin-core/gui/assets/91535/157e92f5-7d06-4303-b4ef-bcdfac5527e3) PR: ![image](https://github.com/bitcoin-core/gui/assets/91535/796322aa-9f16-4b09-9893-bf52a3898a5c) ACKs for top commit: MarnixCroes: tested ACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb pablomartin4btc: tACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb luke-jr: tACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb & in Knots hebasto: ACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb, tested on Ubuntu 24.04. Tree-SHA512: c10fb23605d060cea19a86d11822fc4d12496b19547870052aace503670e62e4c4e19ae4c2c4fbf7420a472adb071c9ddebe82447e0cfbce5a6fb9fcd7b9eda3
2024-07-11log: Fix __func__ in LogError in blockstorage moduleMarcoFalke
These errors should never happen. However, when they do happen, it is useful to log the correct error location (function name). For example, this fixes an incorrect "ConnectBlock()" in "WriteUndoDataForBlock".
2024-07-11log: LogError with FlatFilePos in UndoReadFromDiskMarcoFalke
These errors should never happen in normal operation. If they do, knowing the FlatFilePos may be useful to determine if data corruption happened. Also, handle the error pos.IsNull() as part of OpenUndoFile, because it may as well have happened due to data corruption. This mirrors the LogError behavior from ReadBlockFromDisk.
2024-07-11refactor: Mark IsBlockPruned constMarcoFalke
Member fields are used read-only in this method.
2024-07-10Merge bitcoin/bitcoin#29668: prune, rpc: Check undo data when finding ↵Ava Chow
pruneheight 8789dc8f315a9d9ad7142d831bc9412f780248e7 doc: Add note to getblockfrompeer on missing undo data (Fabian Jahr) 4a1975008b602aeacdad9a74d1837a7455148074 rpc: Make pruneheight also reflect undo data presence (Fabian Jahr) 96b4facc912927305b06a233cb8b36e7e5964c08 refactor, blockstorage: Generalize GetFirstStoredBlock (Fabian Jahr) Pull request description: The function `GetFirstStoredBlock()` helps us find the first block for which we have data. So far this function only looked for a block with `BLOCK_HAVE_DATA`. However, this doesn't mean that we also have the undo data of that block, and undo data might be required for what a user would like to do with those blocks. One example of how this might happen is if some blocks were fetched using the `getblockfrompeer` RPC. Blocks fetched from a peer will have data but no undo data. The first commit here allows `GetFirstStoredBlock()` to check for undo data as well by passing a parameter. This alone is useful for #29553 and I would use it there. In the second commit I am applying the undo check to the RPCs that report `pruneheight` to the user. I find this much more intuitive because I think the user expects to be able to do all operations on blocks up until the `pruneheight` but that is not the case if undo data is missing. I personally ran into this once before and now again when testing for assumeutxo when I had used `getblockfrompeer`. The following commit adds test coverage for this change of behavior. The last commit adds a note in the docs of `getblockfrompeer` that undo data will not be available. ACKs for top commit: achow101: ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7 furszy: Code review ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7. stickies-v: ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7 Tree-SHA512: 90ae8bdd07a496ade579aa25240609c61c9ed173ad38d30533f6c631fe674e5a41727478ade69ca4b71a571ad94c9da4b33ebba6b5d8821109313c2de3bdfb3d
2024-07-09MiniMiner: use FeeFrac in AncestorFeerateComparatorglozow
Comparing using FeeFracs is more precise, allows us to simply the code since FeeFrac comparison internally does cross-multiplication, and avoids potential overflow in the multiplication. Previously, we were only comparing feerates up to 0.001sat/vB precision, since CFeeRate comparison just looks at their respective nSatoshisPerK. This could lead to MiniMiner selecting packages in the wrong order (i.e. by txid) if their feerates were less than 0.001sat/vB different.
2024-07-08Merge bitcoin/bitcoin#30141: kernel: De-globalize validation cachesRyan Ofsky
606a7ab862470413ced400aa68a94fd37c8ad3d3 kernel: De-globalize signature cache (TheCharlatan) 66d74bfc45ae0f743084475ac3bbfb4355bb6ec2 Expose CSignatureCache class in header (TheCharlatan) 021d38822c0e6a1b9497bcb20401c5c37e1bb84d kernel: De-globalize script execution cache hasher (TheCharlatan) 13a3661aba95b54b822c99ecbb695b14a22536d2 kernel: De-globalize script execution cache (TheCharlatan) ab14d1d6a4a8ef5fe5013150e6c5ebcb5f5e4ea9 validation: Don't error if maxsigcachesize exceeds uint32::max (TheCharlatan) Pull request description: The validation caches are currently setup independently from where the rest of the validation code is initialized. This makes their ownership semantics unclear. There is also no clear enforcement on when and in what order they need to be initialized. The caches are always initialized in the `BasicTestingSetup` although a number of tests don't actually need them. Solve this by moving the caches from global scope into the `ChainstateManager` class. This simplifies the usage of the kernel library by no longer requiring manual setup of the caches prior to using the `ChainstateManager`. Tests that need to access the caches can instantiate them independently. --- This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). ACKs for top commit: stickies-v: re-ACK 606a7ab862470413ced400aa68a94fd37c8ad3d3 glozow: reACK 606a7ab ryanofsky: Code review ACK 606a7ab862470413ced400aa68a94fd37c8ad3d3. Just small formatting, include, and static_assert changes since last review. Tree-SHA512: e7f3ee41406e3b233832bb67dc3a63c4203b5367e5daeed383df9cb590f227fcc62eae31311029c077d5e81b273a37a88a364db3dee2efe91bb3b9c9ddc8a42e
2024-07-06Use WITH_LOCK in Warnings::SetAva Chow
The scope of the lock should be limited to just guarding m_warnings as anything listening on `NotifyAlertChanged` may execute code that requires the lock as well.
2024-07-05kernel: De-globalize signature cacheTheCharlatan
Move its ownership to the ChainstateManager class. Next to simplifying usage of the kernel library by no longer requiring manual setup of the cache prior to using validation code, it also slims down the amount of memory allocated by BasicTestingSetup. Use this opportunity to make SignatureCache RAII styled Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-07-04kernel: De-globalize script execution cacheTheCharlatan
Move its ownership to the ChainstateManager class. Next to simplifying usage of the kernel library by no longer requiring manual setup of the cache prior to using validation code, it also slims down the amount of memory allocated by BasicTestingSetup.
2024-07-04Merge bitcoin/bitcoin#29625: Several randomness improvementsmerge-script
ce8094246ee95232e9d84f7e37f3c0a43ef587ce random: replace construct/assign with explicit Reseed() (Pieter Wuille) 2ae392d561ecfdf81855e6df6b9ad3d8843cdfa2 random: use LogError for init failure (Pieter Wuille) 97e16f57042cab07e5e73f6bed19feec2006e4f7 tests: make fuzz tests (mostly) deterministic with fixed seed (Pieter Wuille) 2c91330dd68064e402e8eceea3df9474bb7afd48 random: cleanup order, comments, static (Pieter Wuille) 8e31cf9c9b5e9fdd01e8b220c08a3ccde5cf584c net, net_processing: use existing RNG objects more (Pieter Wuille) d5fcbe966bc501db8bf6a3809633f0b82e6ae547 random: improve precision of MakeExponentiallyDistributed (Pieter Wuille) cfb0dfe2cf0b46f3ea9e62992ade989860f086c8 random: convert GetExponentialRand into rand_exp_duration (Pieter Wuille) 4eaa239dc3e189369d59144b524cb2808cbef8c3 random: convert GetRand{Micros,Millis} into randrange (Pieter Wuille) 82de1b80d95fc9447e64c098dcadb6b8a2f1f2ee net: use GetRandMicros for cache expiration (Pieter Wuille) ddc184d999d7e1a87efaf6bcb222186f0dcd87ec random: get rid of GetRand by inlining (Pieter Wuille) e2d1f84858485650ff743753ffa5c679f210a992 random: make GetRand() support entire range (incl. max) (Pieter Wuille) 810cdf6b4e12a1fdace7998d75b4daf8b67d7028 tests: overhaul deterministic test randomness (Pieter Wuille) 6cfdc5b104caf9952393f9dac2a36539d964077f random: convert XoRoShiRo128PlusPlus into full RNG (Pieter Wuille) 8cc2f45065fc1864f879248d1e1444588e27076b random: move XoRoShiRo128PlusPlus into random module (Pieter Wuille) 8f5ac0d0b608bdf396d8f2d758a792f869c2cd2a xoroshiro128plusplus: drop comment about nonexisting copy() (Pieter Wuille) 8924f5120f66269c04633167def01f82c74ea730 random: modernize XoRoShiRo128PlusPlus a bit (Pieter Wuille) ddb7d26cfd96c1f626def4755e0e1b5aaac94d3e random: add RandomMixin::randbits with compile-known bits (Pieter Wuille) 21ce9d8658fed0d3e4552e8b02a6902cb31c572e random: Improve RandomMixin::randbits (Pieter Wuille) 9b14d3d2da05f74ffb6a2ac20b7d9efefbe29634 random: refactor: move rand* utilities to RandomMixin (Pieter Wuille) 40dd86fc3b60d7a67a9720a84a685f16e3f05b06 random: use BasicByte concept in randbytes (Pieter Wuille) 27cefc7fd6a6a159779f572f4c3a06170f955ed8 random: add a few noexcepts to FastRandomContext (Pieter Wuille) b3b382dde202ad508baf553817c5b38fdd2d4a0c random: move rand256() and randbytes() to .h file (Pieter Wuille) 493a2e024e845e623e202e3eefe1cc2010e9b514 random: write rand256() in function of fillrand() (Pieter Wuille) Pull request description: This PR contains a number of vaguely-related improvements to the random module. The specific changes and more detailed rationale is in the commit messages, but the highlights are: * `XoRoShiRo128PlusPlus` (previously a test-only RNG) moves to random.h and becomes `InsecureRandomContext`, which is even faster than `FastRandomContext` but non-cryptographic. It also gets all helper randomness functions (`randrange`, `fillrand`, ...), making it a lot more succinct to use. * During tests, **all** randomness is made deterministic (except for `GetStrongRandBytes`) but non-repeating (like `GetRand()` used to be when `g_mock_deterministic_tests` was used), either fixed, or from a random seed (overridden by env var). * Several infrequently used top-level functions (`GetRandMillis`, `GetRandMicros`, `GetExponentialRand`) are converted into member functions of `FastRandomContext` (and `InsecureRandomContext`). * `GetRand<T>()` (without argument) can now return the maximum value of the type (previously e.g. `GetRand<uint32_t>()` would never return 0xffffffff). ACKs for top commit: achow101: ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce maflcko: re-ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce 🐈 hodlinator: ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce dergoegge: utACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce Tree-SHA512: 79bc0cbafaf27e95012c1ce2947a8ca6f9a3c78af5f1f16e69354b6fc9b987a28858adf4cd356dc5baf21163e9af8dcc24e70f8d7173be870e8a3ddcdd47c02c
2024-07-02Merge bitcoin/bitcoin#30324: optimization: Moved repeated `-printpriority` ↵Ava Chow
fetching out of AddToBlock 323ce303086d42292ed9fe7c98e8b459bdf6d1a6 Moved the repeated -printpriority fetching out of AddToBlock (Lőrinc) Pull request description: `AddToBlock` was called repeatedly from `addPackageTxs` where the constant value of `printpriority` is recalculated every time. <img src="https://github.com/bitcoin/bitcoin/assets/1841944/6fd89647-7b6c-4f44-bd04-98d16cd2a938"> This showed up during profiling of AssembleBlock, fetching it once in the constructor results in a small speed increase for many iterations. > ./src/bench/bench_bitcoin --filter='AssembleBlock' --min-time=10000 before: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 156,460.15 | 6,391.40 | 0.1% | 11.03 | `AssembleBlock` after: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 149,289.55 | 6,698.39 | 0.3% | 10.97 | `AssembleBlock` --- The slight speedup shows up in CI as well: <img src="https://github.com/bitcoin/bitcoin/assets/1841944/3be779c9-2dce-4a96-ae5f-cab5435bd72f"> ACKs for top commit: maflcko: ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6 achow101: ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6 tdb3: re ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6 furszy: utACK 323ce303086 Tree-SHA512: c2a0aab429646453ad0470956529f1cac8c38778c4c53f82c92c6cbaaaeb69f3d3603c0014ff097844b151e9da7caa2371a4676244caea96527cb540e66825a3
2024-07-02Merge bitcoin/bitcoin#30344: kernel: remove mempool_persistglozow
f1478c05458562a9bef5c2ba43959d758e7b4745 mempool: move LoadMempool/DumpMempool to node (Cory Fields) 6d242ff1e9ca02fd8f5cb3ffe82dfb48a52366cc kernel: remove mempool_persist.cpp (Cory Fields) Pull request description: DumpMempool/LoadMempool are not necessary for the kernel. Noticed while working on instantiated logging. I suppose these could have been left in on purpose, but I'm assuming it was probably just an oversight. ACKs for top commit: TheCharlatan: Re-ACK f1478c05458562a9bef5c2ba43959d758e7b4745 glozow: ACK f1478c0545 stickies-v: ACK f1478c05458562a9bef5c2ba43959d758e7b4745 Tree-SHA512: 5825da0cf2e67470524eb6ebe397eb90755a368469a25f184df99ab935b3eb6d89eb802b41a6c3661e869bba3bbfa8ba9d95281bc75ebbf790ec5d9d1f79c66f
2024-07-01random: get rid of GetRand by inliningPieter Wuille
2024-07-01Merge bitcoin/bitcoin#30306: fuzz: Improve stability for txorphan and ↵merge-script
mini_miner harnesses e009bf681c0e38a6451afa594ba3c7c8861f61c3 Don't use iterator addresses in IteratorComparator (dergoegge) Pull request description: See #29018. Stability for `txorphan` is now >90%. `mini_miner` needs further investigation, stability still low (although slightly improved by this PR) at ~62%. ACKs for top commit: marcofleon: Tested ACK e009bf681c0e38a6451afa594ba3c7c8861f61c3. Using afl++, stability for `txorphan` went from 82% to ~94% and for `mini_miner` it went from 84% to 97%. I ran them both using the corpora in qa-assets. glozow: utACK e009bf681c0e38a6451afa594ba3c7c8861f61c3 Tree-SHA512: 6d0a20fd7ceedca8e702d8adde5fca500d8b0187147aee8d43b4e9eb5176dcacf60180f42a7158f037d18dbb27e479b6c069a0f3c912226505cbff5aa073a415
2024-06-30Moved the repeated -printpriority fetching out of AddToBlockLőrinc
AddToBlock was called repeatedly from `addPackageTxs` where the constant value of `printpriority` is recalculated every time. Since its behavior was changed in 400b151, I've named the variable accordingly. This showed up during profiling of AssembleBlock, fetching it once in the constructor results in a measurable speed increase for many iterations. > ./src/bench/bench_bitcoin --filter='AssembleBlock' --min-time=1000 before: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 155,558.97 | 6,428.43 | 0.1% | 1.10 | `AssembleBlock` after: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 148,083.68 | 6,752.94 | 0.1% | 1.10 | `AssembleBlock` Co-authored-by: furszy <mfurszy@protonmail.com>
2024-06-27Have testBlockValidity hold cs_main instead of callerSjors Provoost
The goal of interfaces is to eventually run in their own process, so we can't use EXCLUSIVE_LOCKS_REQUIRED in their declaration. However TestBlockValidaty will crash (in its call to ConnectBlock) if the tip changes from under the proposed block. Have the testBlockValidity implementation hold the lock instead, and non-fatally check for this condition.
2024-06-27Drop unneeded lock from createNewBlockSjors Provoost
This was added in 4bf2e361da1964f7c278b4939967a0e5afde20b0, but BlockAssembler::CreateNewBlock already locks cs_main internally.
2024-06-26mempool: move LoadMempool/DumpMempool to nodeCory Fields
2024-06-26refactor: testBlockValidity make out argument lastSjors Provoost
2024-06-21refactor, blockstorage: Generalize GetFirstStoredBlockFabian Jahr
GetFirstStoredBlock is generalized to check for any data status with a status mask that needs to be passed as a parameter. To reflect this the function is also renamed to GetFirstBlock. Co-authored-by: stickies-v <stickies-v@protonmail.com>
2024-06-20add node interface method for getting maximum mempool sizeSebastian Falbesoner
2024-06-19Don't use iterator addresses in IteratorComparatordergoegge
The addresses of the iterator values are non-deterministic (i.e. they depend on where the values were allocated). This causes stability issues when fuzzing (e.g. in the `txorphan` and `mini_miner` harnesses), due the orders (derived from IteratorComparator) not being deterministic. Improve stability by comparing the first element in the iterator value pair instead of using the the value addresses.
2024-06-18rpc: call IsInitialBlockDownload via miner interfaceSjors Provoost
2024-06-18rpc: minize getTipHash() calls in gbtSjors Provoost
Set tip at the start of the function and only update it for a long poll. Additionally have getTipHash return an optional, so the caller can explicitly check that a tip exists.
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-04 22:52:20 +0000