aboutsummaryrefslogtreecommitdiff
path: root/src/validation.cpp
AgeCommit message (Collapse)Author
2024-09-25validation: Disable CheckForkWarningConditions for background chainstateMartin Zumsande
The comparison of m_best_invalid with the tip of the respective chainstate makes no sense for the background chainstate, and can lead to incorrect error messages. Github-Pull: bitcoin/bitcoin#30962 Rebased-From: c0a0c72b4d68a4f0c53c2c4b95f4d6e399f8e4ee
2024-09-11assumeUTXO: fix peers disconnection during syncfurszy
Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (NODE_NETWORK), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness (lack of response) from the AssumeUTXO node. This lack of response occurs because nodes ignore getdata requests when they do not have the block data available (further discussion can be found in PR 30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal 'NODE_NETWORK_LIMITED' support. Then, full-node ('NODE_NETWORK') support will be re-enabled once the background chain sync is completed. Github-Pull: bitcoin/bitcoin#30807 Rebased-From: 6d5812e5c852c233bd7ead2ceef051f8567619ed
2024-08-20miner: adjust clock to timewarp ruleSjors Provoost
2024-08-20consensus: enable BIP94 on regtestSjors Provoost
2024-08-13Move maximum timewarp attack threshold back to 600s from 7200sMatt Corallo
In 6bfa26048dbafb91e9ca63ea8d3960271e798098 the testnet4 timewarp attack fix block time variation was increased from the Great Consensus Cleanup value of 600s to 7200s on the thesis that this allows miners to always create blocks with the current time. Sadly, doing so does allow for some nonzero inflation, even if not a huge amount. While it could be that some hardware ignores the timestamp provided to it over Stratum and forces the block header timestamp to the current time, I'm not aware of any such hardware, and it would also likely suffer from random invalid blocks due to relying on NTP anyway, making its existence highly unlikely. This leaves the only concern being pools, but most of those rely on work generated by Bitcoin Core (in one way or another, though when spy mining possibly not), and it seems likely that they will also not suffer any lost work. While its possible that a pool does generate invalid work due to spy mining or otherwise custom logic, it seems unlikely that a substantial portion of hashrate would do so, making the difference somewhat academic (any pool that screws this up will only do so once and the network would come out just fine). Further, while we may end up deciding these assumptions were invalid and we should instead use 7200s, it seems prudent to try with the value we "want" on testnet4, giving us the ability to learn if the compatibility concerns are an issue before we go to mainnet.
2024-08-10doc: Remove outdated nTx faking commentMarcoFalke
This was fixed in commit b50554babdddf452acaa51bac757736766c70e81. Also, address the typo nits from: * https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1531789314 * https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1711982543
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#30604: doc, chainparams: 29775 release notes and ↡Ava Chow
follow-ups 92c1d7d1f8664fe2d259cc609c87f603609b2b60 validation: Use MAX_TIMEWARP constant as testnet4 timewarp defense delta (Fabian Jahr) 4b2fad502e5c264012e94d2915476f4dfcbd192d doc: Add release notes for 29775 (Fabian Jahr) f7cc97313b91483ed4fed298919ecb16054931ee doc: Align deprecation warnings (Fabian Jahr) 1163b08378a50a9be00ced434d55f1b04bc9dea6 chainparams: Add initial minimum chain work for Testnet4 (Fabian Jahr) Pull request description: This completes follow-ups left open in #29775. - Adds release notes - Addresses the [misalignment](https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1706982102) in deprecation warnings and hints at the intention to remove support for Testnet3. - Adds initial minimum chainwork for Testnet4. - Use the `MAX_TIMEWARP` constant as the timewarp defense delta, equal to `MAX_FUTURE_BLOCK_TIME`. ACKs for top commit: Sjors: ACK 92c1d7d1f8664fe2d259cc609c87f603609b2b60 achow101: ACK 92c1d7d1f8664fe2d259cc609c87f603609b2b60 tdb3: re ACK 92c1d7d1f8664fe2d259cc609c87f603609b2b60 Tree-SHA512: 7ebdac7809f96231f75ca62706af59cd1ed27f713a4c7be5e2ad69fae95832b146b3ea23c712fb03b412da1deda7e8a5dae55bb2bbd2dcfd9f926e85c2a72666
2024-08-09validation: Use MAX_TIMEWARP constant as testnet4 timewarp defense deltaFabian Jahr
The value is equal to MAX_FUTURE_BLOCK_TIME.
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#28280: Don't empty dbcache on prune flushes: >30% ↡Ava Chow
faster IBD 589db872e116779ab9cae693171ac8a8c02d9923 validation: don't erase coins cache on prune flushes (Andrew Toth) 0e8918755f725b6269ed2be5a0b46f1611233515 Add linked-list test to CCoinsViewCache::SanityCheck (Pieter Wuille) 05cf4e18758618bb493d26014d3a9c89bf28d898 coins: move Sync logic to CoinsViewCacheCursor (Andrew Toth) 7825b8b9aeceb4ff607650cdc9c49e5de9c7719f coins: pass linked list of flagged entries to BatchWrite (Andrew Toth) a14edada8a051e280af6fedd5130be40247e2d7a test: add cache entry linked list tests (Andrew Toth) 24ce37cb867b95e86d9fd4e50858d64ee8a59abf coins: track flagged cache entries in linked list (Andrew Toth) 58b7ed156d5993b69375bb455b03bd8b17f64fa4 coins: call ClearFlags in CCoinsCacheEntry destructor (Andrew Toth) 8bd3959feaa0e71585bc5e0976f584fb06ee6d14 refactor: require self and sentinel parameters for AddFlags (Andrew Toth) 75f36d241d2a344c5c4ce2c80250bdde91b3295e refactor: add CoinsCachePair alias (Andrew Toth) f08faeade2f99ae1de6f3c481697541cc16186c7 refactor: move flags to private uint8_t and rename to m_flags (Andrew Toth) 4e4fb4cbabcc10e723534f5f80f3e3cf09f6ca50 refactor: disallow setting flags in CCoinsCacheEntry constructors (Andrew Toth) 8737c0cefa6ec49a4d17d9bef9e5e1a7990af1ac refactor: encapsulate flags setting with AddFlags and ClearFlags (Andrew Toth) 9715d3bf1e4013476539f1523a6b54d2055c32c6 refactor: encapsulate flags get access for all other checks (Andrew Toth) df34a94e57659c31873c26c86a8de5a032400061 refactor: encapsulate flags access for dirty and fresh checks (Andrew Toth) Pull request description: Since https://github.com/bitcoin/bitcoin/pull/17487 we no longer need to clear the coins cache when syncing to disk. A warm coins cache significantly speeds up block connection, and only needs to be fully flushed when nearing the `dbcache` limit. For frequent pruning flushes there's no need to empty the cache and kill connect block speed. However, simply using `Sync` in place of `Flush` actually slows down a pruned full IBD with a high `dbcache` value. This is because as the cache grows, sync takes longer since every coin in the cache is scanned to check if it's dirty. For frequent prune flushes and a large cache this constant scanning starts to really slow IBD down, and just emptying the cache on every prune becomes faster. To fix this, we can add two pointers to each cache entry and construct a doubly linked list of dirty entries. We can then only iterate through all dirty entries on each `Sync`, and simply clear the pointers after. With this approach a full IBD with `dbcache=16384` and `prune=550` was 32% faster than master. For default `dbcache=450` speedup was ~9%. All benchmarks were run with `stopatheight=800000`. | | prune | dbcache | time | max RSS | speedup | |-----------:|----------:|------------:|--------:|-------------:|--------------:| | master | 550 | 16384 | 8:52:57 | 2,417,464k | - | | branch | 550 | 16384 | 6:01:00 | 16,216,736k | 32% | | branch | 550 | 450 | 8:05:08 | 2,818,072k | 8.8% | | master | 10000 | 5000 | 8:19:59 | 2,962,752k | - | | branch | 10000 | 5000| 5:56:39 | 6,179,764k | 28.8% | | master | 0 | 16384 | 4:51:53 | 14,726,408k | - | | branch | 0 | 16384 | 4:43:11 | 16,526,348k | 2.7% | | master | 0 | 450 | 7:08:07 | 3,005,892k | - | | branch | 0 | 450 | 6:57:24 | 3,013,556k |2.6%| While the 2 pointers add memory to each cache entry, it did not slow down IBD. For non-pruned IBD results were similar for this branch and master. When I performed the initial IBD, the full UTXO set could be held in memory when using the max `dbcache` value. For non-pruned IBD with max `dbcache` to tip ended up using 12% more memory, but it was also 2.7% faster somehow. For smaller `dbcache` values the `dbcache` limit is respected so does not consume more memory, and the potentially more frequent flushes were not significant enough to cause any slowdown. For reviewers, the commits in order do the following: First 4 commits encapsulate all accesses to `flags` on cache entries, and then the 5th makes `flags` private. Commits `refactor: add CoinsCachePair alias` to `coins: call ClearFlags in CCoinsCacheEntry destructor` create the linked list head nodes and cache entry self references and pass them into `AddFlags`. Commit `coins: track flagged cache entries in linked list` actually adds the entries into a linked list when they are flagged DIRTY or FRESH and removes them from the linked list when they are destroyed or the flags are cleared manually. However, the linked list is not yet used anywhere. Commit `test: add cache entry linked list tests` adds unit tests for the linked list. Commit `coins: pass linked list of flagged entries to BatchWrite` uses the linked list to iterate through DIRTY entries instead of using the entire coins cache. Commit `validation: don't erase coins cache on prune flushes` uses `Sync` instead of `Flush` for pruning flushes, so the cache is no longer cleared. Inspired by [this comment](https://github.com/bitcoin/bitcoin/pull/15265#issuecomment-457720636). Fixes https://github.com/bitcoin/bitcoin/issues/11315. ACKs for top commit: paplorinc: ACK 589db872e116779ab9cae693171ac8a8c02d9923 sipa: reACK 589db872e116779ab9cae693171ac8a8c02d9923 achow101: ACK 589db872e116779ab9cae693171ac8a8c02d9923 mzumsande: re-ACK 589db872e116779ab9cae693171ac8a8c02d9923 Tree-SHA512: 23b2bc01c83edacb5b39aa60bb0b766de9a74ce17f0c59bf13b97b4328a7b758ad9aff6581c3ca88e2973f7658380651530d497444f48d6e22ea0bfc51cc921d
2024-08-05validation: don't erase coins cache on prune flushesAndrew Toth
2024-08-06testnet: Add timewarp attack prevention for Testnet4Fabian Jahr
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#30497: rpc: Return errors in loadtxoutset that ↡Ryan Ofsky
currently go to logs fa530ec54386a3fd56b51e50699b424cc8647821 rpc: Return precise loadtxoutset error messages (MarcoFalke) faa5c86dbfec1ed7bdcd6a07316315147a7e87a2 refactor: Use untranslated error message in ActivateSnapshot (MarcoFalke) Pull request description: The error messages should never happen in normal operation. However, if they do, they are helpful to return to the user to debug the issue. For example, to notice a truncated file. This fixes https://github.com/bitcoin/bitcoin/issues/28621 Also includes a minor refactor commit. ACKs for top commit: fjahr: Code review ACK fa530ec54386a3fd56b51e50699b424cc8647821 ryanofsky: Code review ACK fa530ec54386a3fd56b51e50699b424cc8647821, just adjusting error messages a little since last review. (Thanks!) Tree-SHA512: 224968c9b13d082ca2ed1f6a8fcc5f51ff16d6c96bd38c3679699505b54337b99cccaf7a8474391f6b11f9ccb101977b4e626898c1217eae95802e290cf105f1
2024-08-05scripted-diff: Replace uint256S("str") -> uint256{"str"}Hodlinator
-BEGIN VERIFY SCRIPT- sed -i --regexp-extended -e 's/\buint256S\("(0x)?([^"]{64})"\)/uint256{"\2"}/g' $(git grep -l uint256S) -END VERIFY SCRIPT-
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-04chainparams: Change nChainTx to uint64_tFabian Jahr
Also update types of assumeutxo chainparams and some related local variables for consistency. Co-authored-by: russeree <reese.russell@ymail.com>
2024-07-26rpc: Return precise loadtxoutset error messagesMarcoFalke
The error messages should never happen in normal operation. However, if they do, they are helpful to return to the user to debug the issue. For example, to notice a truncated file.
2024-07-25refactor: Use untranslated error message in ActivateSnapshotMarcoFalke
The message is not exposed in the GUI or another translated context, so translating it is useless for now. Also, fix a nit from https://github.com/bitcoin/bitcoin/pull/30395#discussion_r1670972864
2024-07-25[refactor] change ActiveTipChange to use CBlockIndex ref instead of ptrglozow
2024-07-24Merge bitcoin/bitcoin#30111: locks: introduce mutex for tx download, flush ↡merge-script
rejection filters once per tip change c85accecafc20f6a6ae94bdf6cdd3ba9747218fd [refactor] delete EraseTxNoLock, just use EraseTx (glozow) 6ff84069a5dd92303ed2ec28f0ec7c96bbda3938 remove obsoleted TxOrphanage::m_mutex (glozow) 61745c7451ec64b26c74f672c688e82efb3b96aa lock m_recent_confirmed_transactions using m_tx_download_mutex (glozow) 723ea0f9a5b5e3f3f58ea049a98299ff0ebde468 remove obsoleted hashRecentRejectsChainTip (glozow) 18a43552509603ddf83b752fd7b4b973ba1dcf82 update recent_rejects filters on ActiveTipChange (glozow) 36f170d87924e50d0ff9be2a1b0f2a8f13950a9b add ValidationInterface::ActiveTipChange (glozow) 3eb1307df0a38ac4ea52995fbb03ead37387b41e guard TxRequest and rejection caches with new mutex (glozow) Pull request description: See #27463 for full project tracking. This contains the first few commits of #30110, which require some thinking about thread safety in review. - Introduce a new `m_tx_download_mutex` which guards the transaction download data structures including `m_txrequest`, the rolling bloom filters, and `m_orphanage`. Later this should become the mutex guarding `TxDownloadManager`. - `m_txrequest` doesn't need to be guarded using `cs_main` anymore - `m_recent_confirmed_transactions` doesn't need its own lock anymore - `m_orphanage` doesn't need its own lock anymore - Adds a new `ValidationInterface` event, `ActiveTipChanged`, which is a synchronous callback whenever the tip of the active chainstate changes. - Flush `m_recent_rejects` and `m_recent_rejects_reconsiderable` on `ActiveTipChanged` just once instead of checking the tip every time `AlreadyHaveTx` is called. This should speed up calls to that function (no longer comparing a block hash each time) and removes the need to lock `cs_main` every time it is called. Motivation: - These data structures need synchronization. While we are holding `m_tx_download_mutex`, these should hold: - a tx hash in `m_txrequest` is not also in `m_orphanage` - a tx hash in `m_txrequest` is not also in `m_recent_rejects` or `m_recent_confirmed_transactions` - In the future, orphan resolution tracking should also be synchronized. If a tx has an entry in the orphan resolution tracker, it is also in `m_orphanage`, and not in `m_txrequest`, etc. - Currently, `cs_main` is used to e.g. sync accesses to `m_txrequest`. We should not broaden the scope of things it locks. - Currently, we need to know the current chainstate every time we call `AlreadyHaveTx` so we can decide whether we should update it. Every call compares the current tip hash with `hashRecentRejectsChainTip`. It is more efficient to have a validation interface callback that updates the rejection filters whenever the chain tip changes. ACKs for top commit: instagibbs: reACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd dergoegge: Code review ACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd theStack: Light code-review ACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd hebasto: ACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd, I have reviewed the code and it looks OK. Tree-SHA512: c3bd524b5de1cafc9a10770dadb484cc479d6d4c687d80dd0f176d339fd95f73b85cb44cb3b6b464d38a52e20feda00aa2a1da5a73339e31831687e4bd0aa0c5
2024-07-22Fix lint-spelling warningsLΕ‘rinc
These warnings were often polluting the CI output, e.g. https://github.com/bitcoin/bitcoin/pull/30499/checks?check_run_id=27745036545 > ./test/lint/lint-spelling.py before the change: ``` doc/design/libraries.md:100: targetted ==> targeted doc/developer-notes.md:495: dependant ==> dependent src/bench/sign_transaction.cpp:49: hashIn ==> hashing, hash in src/bitcoin-chainstate.cpp:213: hashIn ==> hashing, hash in src/bitcoin-chainstate.cpp:213: hashIn ==> hashing, hash in src/coins.cpp:24: viewIn ==> viewing, view in src/coins.cpp:24: viewIn ==> viewing, view in src/coins.cpp:29: viewIn ==> viewing, view in src/coins.cpp:29: viewIn ==> viewing, view in src/coins.h:44: outIn ==> outing, out in src/coins.h:44: outIn ==> outing, out in src/coins.h:45: outIn ==> outing, out in src/coins.h:45: outIn ==> outing, out in src/coins.h:215: viewIn ==> viewing, view in src/coins.h:220: viewIn ==> viewing, view in src/primitives/transaction.h:37: hashIn ==> hashing, hash in src/primitives/transaction.h:37: hashIn ==> hashing, hash in src/protocol.cpp:51: hashIn ==> hashing, hash in src/protocol.cpp:51: hashIn ==> hashing, hash in src/protocol.h:497: hashIn ==> hashing, hash in src/qt/forms/optionsdialog.ui:344: incomin ==> incoming src/qt/optionsdialog.cpp:445: proxys ==> proxies src/rpc/mining.cpp:987: hashIn ==> hashing, hash in src/rpc/mining.cpp:987: hashIn ==> hashing, hash in src/script/interpreter.h:298: amountIn ==> amounting, amount in src/script/interpreter.h:298: amountIn ==> amounting, amount in src/script/interpreter.h:299: amountIn ==> amounting, amount in src/script/interpreter.h:299: amountIn ==> amounting, amount in src/script/sigcache.h:70: amountIn ==> amounting, amount in src/script/sigcache.h:70: amountIn ==> amounting, amount in src/signet.cpp:144: amountIn ==> amounting, amount in src/test/fuzz/util/net.cpp:386: occured ==> occurred src/test/fuzz/util/net.cpp:398: occured ==> occurred src/util/vecdeque.h:79: deques ==> dequeues src/util/vecdeque.h:160: deques ==> dequeues src/util/vecdeque.h:184: deques ==> dequeues src/util/vecdeque.h:194: deques ==> dequeues src/validation.cpp:2130: re-declared ==> redeclared src/validation.h:348: outIn ==> outing, out in src/validation.h:349: outIn ==> outing, out in test/functional/wallet_bumpfee.py:851: atleast ==> at least ```
2024-07-18Merge bitcoin/bitcoin#30320: assumeutxo: Don't load a snapshot if it's not ↡Ava Chow
in the best header chain 55b6d7be68a6f6c3882588ffd5b9349d885ed953 validation: Don't load a snapshot if it's not in the best header chain. (Martin Zumsande) Pull request description: This was suggested by me in the discussion of #30288, which has more context. If the snapshot is not an ancestor of the most-work header (`m_best_header`), syncing from that alternative chain leading to `m_best_header` should be prioritised. Therefore it's not useful loading the snapshot in this situation. If the other chain turns out to be invalid or the chain with the snapshot retrieves additional headers so that it's the most-work one again (see functional test), `m_best_header` will change and loading the snapshot will be possible again. Because of the work required to generate a conflicting headers chain, a situation with two conflicting chains should only be possible under extreme circumstances, such as major forks. ACKs for top commit: fjahr: re-ACK 55b6d7be68a6f6c3882588ffd5b9349d885ed953 achow101: ACK 55b6d7be68a6f6c3882588ffd5b9349d885ed953 alfonsoromanz: Re ACK 55b6d7be68a6f6c3882588ffd5b9349d885ed953 Tree-SHA512: 4fbea5ab1038ae353fc949a186041cf9b397e7ce4ac59ff36f881c9437b4f22ada922490ead5b2661389eb1ca0f3d1e7e7e6a4261057678643e71594a691ac36
2024-07-17refactor: Make m_last_notified_header privateMarcoFalke
2024-07-16Merge bitcoin/bitcoin#30425: kernel: De-globalize static validation variablesRyan Ofsky
51fa26239af9bbfd44029aaf595cb4c6a8d4a75d refactor: Mark some static global vars as const (TheCharlatan) 39f9b80fba85d9818222c4d76e99ea1a804f5dda refactor: De-globalize last notified header index (TheCharlatan) 3443943f86678eb27d9895f3871fcf3945eb1c4f refactor: De-globalize validation benchmark timekeeping (TheCharlatan) Pull request description: In future, users of the kernel library might run multiple chainstates in parallel, or create and destroy multiple chainstates over the lifetime of a process. Having static, mutable variables could lead to state inconsistencies in these scenarios. --- This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). ACKs for top commit: dergoegge: Code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d maflcko: ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d 🍚 tdb3: code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d Tree-SHA512: da91aa7ffa343325cabb8764ef03c8358845662cf0ba8a6cc1dd38e40e5462d88734f2b459c2de8e7a041551eda9143d92487842609f7f30636f61a0cd3c57ee
2024-07-16add ValidationInterface::ActiveTipChangeglozow
This is a synchronous callback notifying clients of all tip changes. It allows clients to respond to a new block immediately after it is connected. The synchronicity is important for things like m_recent_rejects, in which a transaction's validity can change (rejected vs accepted) when this event is processed. For example, the transaction might have a timelock condition that has just been met. This is distinct from something like m_recent_confirmed_transactions, in which the validation outcome is the same (valid vs already-have), so it does not need to be reset immediately.
2024-07-12Merge bitcoin/bitcoin#30295: #28984 package rbf followupsmerge-script
3f00aae14092ca076cff7f9ddf6155c601979fcd package rbf: cpfp structure requires package > parent feerate (Greg Sanders) ad7f1f697f01845470f8df0944a94012fabcba09 test package rbf boundary conditions more closely (Greg Sanders) ff4558d441f377a372647972d35b5476b94579c9 doc: reword package RBF documentation (Greg Sanders) de669a883bf65c576cc596b20a576d7bb1618182 doc: replace mention of V3 with TRUC (Greg Sanders) Pull request description: Some suggested nits/changes from #28984 ACKs for top commit: glozow: ACK 3f00aae1409 murchandamus: ACK 3f00aae14092ca076cff7f9ddf6155c601979fcd Tree-SHA512: 79434cc8aba25a43e99793298cdc99cad807db2c3a2e780a31953f244b95eecd97b90559abd67fbf30996c00966675fa257253a7812ec4727420226162c629ae
2024-07-11validation: Don't load a snapshot if it's not in the best header chain.Martin Zumsande
If the snapshot is not an ancestor of the most-work header (m_best_header), syncing from that alternative chain should be prioritised. Therefore don't accept loading a snapshot in this situation. If that other chain turns out to be invalid, m_best_header would be reset and loading the snapshot should be possible again. Because of the work required to generate a conflicting headers chain, this should only be possible under extreme circumstances, such as major forks.
2024-07-09Merge bitcoin/bitcoin#30395: rpc: Use untranslated error strings in loadtxoutsetRyan Ofsky
fa5b8920be041380fbfa4c7b443918637423d7a0 rpc: Use untranslated error strings in loadtxoutset (MarcoFalke) fa458657788cc142f14551d86604e3f434d56c0a refactor: Use named arguments to get path arg in loadtxoutset (MarcoFalke) Pull request description: Motivation: * Some are not translated at all, anyway. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663631973 * For others translation is not yet needed, because they are not called by the GUI (yet) * For others translations will never be needed, because they are RPC code. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663611194 Also, while touching this: * Remove the trailing `\n`. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663647981 * Add back the path. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663666751 * Use named args to get the path. ACKs for top commit: fjahr: re-ACK fa5b8920be041380fbfa4c7b443918637423d7a0 tdb3: ACK fa5b8920be041380fbfa4c7b443918637423d7a0 ryanofsky: Code review ACK fa5b8920be041380fbfa4c7b443918637423d7a0 Tree-SHA512: 46504dc5fd55a6274ef885dbe071aa9efb25bca247cd68cd86fb2ff066d70d295e0522e1fe42e63f1fdf7e4c89bd696220edaf06e33b804aba746492eafd852e
2024-07-09package rbf: cpfp structure requires package > parent feerateGreg Sanders
2024-07-08refactor: De-globalize last notified header indexTheCharlatan
In future, users of the kernel library might run multiple chainstates in parallel, or create and destroy multiple chainstates over the lifetime of a process. Having static, mutable variables could lead to state inconsistencies in these scenarios.
2024-07-08refactor: De-globalize validation benchmark timekeepingTheCharlatan
In future, users of the kernel library might run multiple chainstates in parallel, or create and destroy multiple chainstates over the lifetime of a process. Having static, mutable variables could lead to state inconsistencies in these scenarios.
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-05rpc: Use untranslated error strings in loadtxoutsetMarcoFalke
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 cache hasherTheCharlatan
Move it to the ChainstateManager class.
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-04validation: Don't error if maxsigcachesize exceeds uint32::maxTheCharlatan
Instead clamp it to uint32::max if it exceeds it. Co-authored-by: Anthony Towns <aj@erisian.com.au>
2024-07-04Merge bitcoin/bitcoin#30388: validation: Check if mempool exists before size ↡glozow
check in ActivateSnapshot 33c48c106cf03ff62994ff5777a775982d90eab9 validation: Check if mempool exists before asserting in ActivateSnapshot (TheCharlatan) Pull request description: The mempool is an optional component of the chainstate manager, so don't assume its presence and instead check if it is there first. ACKs for top commit: maflcko: re-ACK 33c48c106cf03ff62994ff5777a775982d90eab9 fjahr: ACK 33c48c106cf03ff62994ff5777a775982d90eab9 Tree-SHA512: 7a3568d5b7af45efa7bf54bae7bac1f00dc99bc9d47a744d73594f283c952be9500168f680d72f4aee09761da4e878ddca83ba675cdea8ee9e44eeff00ac09da
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-04validation: Check if mempool exists before asserting in ActivateSnapshotTheCharlatan
2024-07-02Merge bitcoin/bitcoin#30272: doc: use TRUC instead of v3 and add release noteAva Chow
926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746 [doc] add release note for TRUC (glozow) 19a9b90617419f68d0f1c90ee115b5220be99a16 use version=3 instead of v3 in debug strings (glozow) 881fac8e609be17eb71bd9a54c0284b304e2e2e2 scripted-diff: change names from V3 to TRUC (glozow) a573dd261748d2a80560f73db08f7dca788c7fcf [doc] replace mentions of v3 with TRUC (glozow) 089b5757dff39a9a06cdb625aaced9beeb72958d rename mempool_accept_v3.py to mempool_truc.py (glozow) f543852a89d93441645250c40c3980aeb0c3b664 rename policy/v3_policy.* to policy/truc_policy.* (glozow) Pull request description: Adds a release note for TRUC policy which will be live in v28.0. For clarity, replaces mentions of "v3" with "TRUC" in most places. Suggested in - https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1629749583 - https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1624500904 I changed error strings from "v3-violation" to "TRUC-violation" but left v3 in the debug strings because I think it might be clearer for somebody who is debugging. Similarly, I left some variables unchanged because I think they're more descriptive this way, e.g. `tx_v3_from_v2_and_v3`. I'm happy to debate places that should or shouldn't be documented differently in this PR, whatever is clearest to everyone. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30272/commits/926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746 achow101: ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746 ismaelsadeeq: Code review ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746 Tree-SHA512: 16c88add0a29dc6d1236c4d45f34a17b850f6727b231953cbd52eb9f7268d1d802563eadfc8b7928c94ed3d7a615275dd103e57e81439ebf3ba2b12efa1e42af
2024-07-02Merge bitcoin/bitcoin#30267: assumeutxo: Check snapshot base block is not in ↡Ava Chow
invalid chain 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28 test: Remove unnecessary restart in assumeutxo test (Fabian Jahr) 19ce3d407ef546fa50d18b2ffbd67b7417797064 assumeutxo: Check snapshot base block is not marked invalid (Fabian Jahr) 80315c011863d69e7785673283e4c9033fbcd5ac refactor: Move early loadtxoutset checks into ActiveSnapshot (Fabian Jahr) Pull request description: This was discovered in a discussion in #29996 If the base block of the snapshot is marked invalid or part of an invalid chain, we currently still load the snapshot and get stuck in a weird state where we have the snapshot chainstate but it will never connect to our valid chain. While this scenario is highly unlikely to occur on mainnet, it still seems good to prevent this inconsistent state. The behavior change described above is in the second commit. The first commit refactors the early checks in the `loadtxoutset` RPC by moving them into `ActivateSnapshot()` in order to have the chance to cover them by unit tests in the future and have a more consistent interface. Previously checks were spread out between `rpc/blockchain.cpp` and `validation.cpp`. In order to be able to return the error message to users of the RPC, the return type of `ActivateSnapshot()` is changed from `bool` to `util::Result`. The third commit removes an unnecessary restart introduced in #29428. ACKs for top commit: mzumsande: re-ACK 2f9bde6 alfonsoromanz: Re-ACK 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28. The RPC code looks much cleaner after the refactor. Also, it seems very useful to get the error message in the RPC response rather than having to rely on the logs in some scenarios if you are an RPC user. achow101: ACK 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28 Tree-SHA512: 5328dd88c3c7be3f1be97c9eef52ac3666c27188c30a798b3e949f3ffcb83be075127c107e4046f7f39f961a79911ea3d61b61f3c11e451b3e4c541c264eeed4
2024-07-02scripted-diff: change names from V3 to TRUCglozow
-BEGIN VERIFY SCRIPT- sed -i 's/SingleV3Checks/SingleTRUCChecks/g' $(git grep -l 'SingleV3Checks') sed -i 's/PackageV3Checks/PackageTRUCChecks/g' $(git grep -l 'PackageV3Checks') sed -i 's/PV3C/PTRUCC/g' src/policy/truc_policy.h sed -i 's/V3_MAX_VSIZE/TRUC_MAX_VSIZE/g' $(git grep -l 'V3_MAX_VSIZE') sed -i 's/V3_CHILD_MAX_VSIZE/TRUC_CHILD_MAX_VSIZE/g' $(git grep -l 'V3_CHILD_MAX_VSIZE') sed -i 's/V3_DESCENDANT_LIMIT/TRUC_DESCENDANT_LIMIT/g' $(git grep -l 'V3_DESCENDANT_LIMIT') sed -i 's/V3_ANCESTOR_LIMIT/TRUC_ANCESTOR_LIMIT/g' $(git grep -l 'V3_ANCESTOR_LIMIT') sed -i 's/CheckMempoolV3Invariants/CheckMempoolTRUCInvariants/g' $(git grep -l 'CheckMempoolV3Invariants') -END VERIFY SCRIPT-
2024-07-02[doc] replace mentions of v3 with TRUCglozow
Keep mentions of v3 in debug strings to help people who might not know that TRUC is applied when version=3. Also keep variable names in tests, as it is less verbose to keep v3 and v2.
2024-07-01random: get rid of GetRand by inliningPieter Wuille
2024-06-21assumeutxo: Check snapshot base block is not marked invalidFabian Jahr
Co-authored-by: Alfonso Roman Zubeldia <alfonsoromanz24@gmail.com>
2024-06-20refactor: remove extraneous lock annotations from function definitionsCory Fields
These annotations belong in the declarations rather than the definitions. While harmless now, future versions of clang may warn about these.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 08:04:22 +0000