aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2021-05-04fuzz: Limit toxic test globals to their respective scopeMarcoFalke
2021-05-04Merge bitcoin/bitcoin#21840: test: Misc refactor to get rid of &foo[0] raw ↵MarcoFalke
pointers fa8a88849c08c810a82338bf0e70738eb6748906 bench: Remove duplicate constants (MarcoFalke) 000098f9647cd2e21660603b7d7a8f623f70f673 test: Use throwing variant accessor (MarcoFalke) fa2197c8b3178787d99e2acb5c3c717df14ddabf test: Use loop to register RPCs (MarcoFalke) Pull request description: Simplify test code ACKs for top commit: Empact: Code Review ACK fa8a88849c08c810a82338bf0e70738eb6748906 practicalswift: cr ACK fa8a88849c08c810a82338bf0e70738eb6748906 promag: Code review ACK fa8a88849c08c810a82338bf0e70738eb6748906. Tree-SHA512: 6a5bebaa9a3f43e9c332f4fbff606e9ece6dc8b95a769980082cc022f8e9bde6083c1e4a0145dcbf3741f514d6e97b4198f201a1bf1370ebf43bd3a5c0f85981
2021-05-03Merge bitcoin/bitcoin#21810: fuzz: Various RPC fuzzer follow-upsMarcoFalke
5252f86eb616a1112e427c268c8e8825f2a63d67 fuzz: Reduce maintenance requirements by allowing RPC annotations also for conditionally available RPC commands (such as wallet commands) without the fragility of #ifdef forests (practicalswift) 54549dda310e2becee9cb4997d1408a90e91934f fuzz: RPC fuzzer post-merge follow-ups. Remove unused includes. Update list of fuzzed RPC commands. (practicalswift) Pull request description: Various RPC fuzzer follow-ups: * Remove unused includes. * Update list of fuzzed RPC commands. * Reduce maintenance requirements by allowing RPC annotations also for conditionally available RPC commands (such as wallet commands) without the fragility of `#ifdef` forests. Context: https://github.com/bitcoin/bitcoin/pull/21169#pullrequestreview-646723483 ACKs for top commit: MarcoFalke: Concept ACK 5252f86eb616a1112e427c268c8e8825f2a63d67 Tree-SHA512: 286d70798131706ffb157758e1c73f7f00ed96ce120c7d9dc849e672b283f1362df47b206cfec9da44d5debb5869225e721761dcd5c38a7d5d1019dc6c912ab2
2021-05-03test: Use throwing variant accessorMarcoFalke
It does not matter if the tests fail due to a BOOST_CHECK failure or due to a thrown exception. Prefer the exception because it is less code. Example fail with the throwing accessor: unknown location(0): fatal error: in "script_standard_tests/script_standard_ExtractDestinations": std::bad_variant_access: std::get: wrong index for variant test/script_standard_tests.cpp(314): last checkpoint *** 1 failure is detected in the test module "Bitcoin Core Test Suite"
2021-05-03Merge bitcoin/bitcoin#20867: Support up to 20 keys for multisig under Segwit ↵fanquake
context ebd4be43cc945e643f91d3a91007b5a35bbbd5a1 doc: add release notes for 20867 (Antoine Poinsot) 5aa50ab9cc7994b16cf13e4c73af80f0098f1bea rpc/util: multisig: only check redeemScript size is <= 520 for P2SH (Antoine Poinsot) 063df9e89730fd2c92646577e2fab894e1692130 test/functional: standardness sanity checks for P2(W)SH multisig (Antoine Poinsot) ae0429d3af6de48f6191f144dff4ad4ab672dcd6 script: allow up to 20 keys in wsh() descriptors (Antoine Poinsot) 9fc68faf35c700ae955af194dd7f8c1aee85a05b script: match multisigs with up to MAX_PUBKEYS_PER_MULTISIG keys (Antoine Poinsot) Pull request description: As described in https://github.com/bitcoin/bitcoin/issues/20620 multisigs are currently limited to 16 keys in descriptors and RPC helpers, even for P2WSH and P2SH-P2WSH. This adds support for multisig with up to 20 keys (which are already standard) for Segwit v0 context for descriptors (`wsh()`, `sh(wsh())`) and RPC helpers. Fixes https://github.com/bitcoin/bitcoin/issues/20620 ACKs for top commit: meshcollider: re-utACK ebd4be43cc945e643f91d3a91007b5a35bbbd5a1 instagibbs: re-ACK https://github.com/bitcoin/bitcoin/pull/20867/commits/ebd4be43cc945e643f91d3a91007b5a35bbbd5a1 Tree-SHA512: 36141f10a8288010d17d5c4fe8d24878bcd4533b88a8aba3a44fa8f74ceb3182d70fee01427e0ab7f53ce7fab46c88c1cd3ac3b18ab8a10bd4a6b8b74ed79e46
2021-04-30Merge bitcoin/bitcoin#19521: Coinstats IndexW. J. van der Laan
5f96d7d22d8e05876c6fc014e70488699950fe38 rpc: gettxoutsetinfo rejects hash_serialized_2 for specific height (Fabian Jahr) 23fe50436be641d7417152adc683192649ba206a test: Add test for coinstatsindex behavior in reorgs (Fabian Jahr) 90c966b0f3cfbd6bce5883f46d8527c6853a86a2 rpc: Allow gettxoutsetinfo and getblockstats for stale blocks (Fabian Jahr) b9362392aef2689bc106c20925859ede555d082b index, rpc: Add use_index option for gettxoutsetinfo (Fabian Jahr) bb7788b121a30489bc81a1f46dde6a9b19ae4ec1 test: Test coinstatsindex robustness across restarts (Fabian Jahr) e0938c29099635150014ffc9bb0cafa8049ec55a test: Add tests for block_info in gettxoutsetinfo (Fabian Jahr) 2501576eccb08af80471c7b7b843b189ad6758c0 rpc, index: Add verbose amounts tracking to Coinstats index (Fabian Jahr) 655d929836a71af23d2035d2e2e99ad8b8c340c3 test: add coinstatsindex getindexinfo coverage, improve current tests (Jon Atack) ca01bb8d689f93e1c7669b0ba7a4994c0206dabd rpc: Add Coinstats index to getindexinfo (Fabian Jahr) 57a026c30fef3138bb8db46e6865acb9dc2674f8 test: Add unit test for Coinstats index (Fabian Jahr) 6a4c0c09ab4d073a26c3c4a02783d5dcd88f6eef test: Add functional test for Coinstats index (Fabian Jahr) 3f166ecc125fce6ccd995687fa16572090a5d099 rpc: gettxoutsetinfo can be requested for specific blockheights (Fabian Jahr) 3c914d58ff323255b32e717d0ce28209ec0abdaa index: Coinstats index can be activated with command line flag (Fabian Jahr) dd58a4de21469d6d848ae309edc47f558628221d index: Add Coinstats index (Fabian Jahr) a8a46c4b3cfda4b95c92a36f8cebd3606377e57d refactor: Simplify ApplyStats and ApplyHash (Fabian Jahr) 9c8a265fd21a87228c18a1661df99fedc1866baf refactor: Pass hash_type to CoinsStats in stats object (Fabian Jahr) 2e2648a9021dfbb6e17dfa81472f057dacbc34e0 crypto: Make MuHash Remove method efficient (Fabian Jahr) Pull request description: This is part of the coinstats index project tracked in #18000 While the review of the new UTXO set hash algorithm (MuHash) takes longer recently #19328 was merged which added the possibility to run `gettxoutsetinfo` with a specific hash type. As the first type it added `hash_type=none` which skips the hashing of the UTXO set altogether. This alone did not make `gettxoutsetinfo` much faster but it allows the use of an index for the remaining coin statistics even before a new hashing algorithm has been added. Credit to Sjors for the idea to take this intermediate step. Features summary: - Users can start their node with the option `-coinstatsindex` which syncs the index in the background - After the index is synced the user can use `gettxoutsetinfo` with `hash_type=none` or `hash_type=muhash` and will get the response instantly out of the index - The user can specify a height or block hash when calling `gettxoutsetinfo` to see coin statistics at a specific block height ACKs for top commit: Sjors: re-tACK 5f96d7d22d8e05876c6fc014e70488699950fe38 jonatack: Code review re-ACK 5f96d7d22d8e05876c6fc014e70488699950fe38 per `git range-diff 13d27b4 07201d3 5f96d7d` promag: Tested ACK 5f96d7d22d8e05876c6fc014e70488699950fe38. Light code review ACK 5f96d7d22d8e05876c6fc014e70488699950fe38. Tree-SHA512: cbca78bee8e9605c19da4fbcd184625fb280200718396c694a56c7daab6f44ad23ca9fb5456d09f245d8b8d9659fdc2b3f3ce5e953c1c6cf4003dbc74c0463c2
2021-04-29fuzz: Reduce maintenance requirements by allowing RPC annotations also for ↵practicalswift
conditionally available RPC commands (such as wallet commands) without the fragility of #ifdef forests
2021-04-28Merge bitcoin/bitcoin#18847: compressor: use a prevector in CompressScript ↵MarcoFalke
serialization [ZAP1] 83a425d25af033086744c1c8c892015014ed46bd compressor: use a prevector in compressed script serialization (William Casarin) Pull request description: This function was doing millions of unnecessary heap allocations during IBD. I'm start to catalog unnecessary heap allocations as a pet project of mine: as-zero-as-possible-alloc IBD. This is one small step. before: ![May01-174536](https://user-images.githubusercontent.com/45598/80850964-9a38de80-8bd3-11ea-8eec-08cd38ee1fa1.png) after: ![May01-174610](https://user-images.githubusercontent.com/45598/80850974-a91f9100-8bd3-11ea-94a1-e2077391f6f4.png) ~should I type alias this?~ *I type aliased it* This is a part of the Zero Allocations Project #18849 (ZAP1). This code came up as a place where many allocations occur. ACKs for top commit: Empact: ACK https://github.com/bitcoin/bitcoin/pull/18847/commits/83a425d25af033086744c1c8c892015014ed46bd elichai: tACK 83a425d25af033086744c1c8c892015014ed46bd sipa: utACK 83a425d25af033086744c1c8c892015014ed46bd Tree-SHA512: f0ffa6ab0ea1632715b0b76362753f9f6935f05cdcc80d85566774401155a3c57ad45a687942a1806d3503858f0bb698da9243746c8e2edb8fdf13611235b0e0
2021-04-28fuzz: RPC fuzzer post-merge follow-ups. Remove unused includes. Update list ↵practicalswift
of fuzzed RPC commands.
2021-04-28Merge bitcoin/bitcoin#20772: fuzz: bolster ExtractDestination(s) checksMarcoFalke
a29f522ba4aa71582b54025c5682b4c1687ae9f3 fuzz: bolster ExtractDestination(s) checks (Michael Dietz) Pull request description: ACKs for top commit: practicalswift: Tested ACK a29f522ba4aa71582b54025c5682b4c1687ae9f3 Tree-SHA512: 0fc194edb7b0fce77c7bb725fe65dec7976598edcd53882b5a0eb7cd83281a3ddcd2b3de00282468be659a7e5bc9991eb482816418f55b30e657cdc5a3bd7438
2021-04-28script: allow up to 20 keys in wsh() descriptorsAntoine Poinsot
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2021-04-28script: match multisigs with up to MAX_PUBKEYS_PER_MULTISIG keysAntoine Poinsot
We were previously ruling out 17-20 pubkeys multisig, while they are only invalid under P2SH context. This makes multisigs with up to 20 keys be detected as valid by the solver. This is however *not* a policy change as it would only apply to bare multisigs, which are already limited to 3 pubkeys. Note that this does not change the sigOpCount calculation (as it would break consensus). Therefore 1-16 keys multisigs are counted as 1-16 sigops and 17-20 keys multisigs are counted as 20 sigops. Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2021-04-28Merge bitcoin/bitcoin#21169: fuzz: Add RPC interface fuzzing. Increase ↵MarcoFalke
fuzzing coverage from 65% to 70%. 545404e7e1c72985557ccffe865cea269143e5dd fuzz: Add RPC interface fuzzing. Increase fuzzing coverage from 65% to 70%. (practicalswift) Pull request description: Add RPC interface fuzzing. This PR increases overall fuzzing line coverage from [~65%](https://marcofalke.github.io/btc_cov/fuzz.coverage/) to ~70% 🎉 To test this PR: ``` $ make distclean $ ./autogen.sh $ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined $ make -C src/ test/fuzz/fuzz $ FUZZ=rpc src/test/fuzz/fuzz ``` See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for more information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) ACKs for top commit: MarcoFalke: Concept ACK 545404e7e1c72985557ccffe865cea269143e5dd Tree-SHA512: 35fc1b508af42bf480ee3762326b09ff2eecdb7960a1917ad16345fadd5c0c21d666dafe736176e5a848ff6492483c782e4ea914cd9000faf50190df051950fd
2021-04-28fuzz: Add RPC interface fuzzing. Increase fuzzing coverage from 65% to 70%.practicalswift
2021-04-25fuzz: Ensure prevout is consensus-validMarcoFalke
2021-04-25Merge bitcoin/bitcoin#21563: net: Restrict period when cs_vNodes mutex is lockedMarcoFalke
8c8237a4a10feb2ac9ce46f67b5d14bf879b670f net, refactor: Fix style in CConnman::StopNodes (Hennadii Stepanov) 229ac1892d807a1eea5a7c24ae0fe27dc913b1bd net: Combine two loops into one, and update comments (Hennadii Stepanov) a3d090d1103cd6c25daf07afdf4e65febca6d3f7 net: Restrict period when cs_vNodes mutex is locked (Hennadii Stepanov) Pull request description: This PR restricts the period when the `cs_vNodes` mutex is locked, prevents the only case when `cs_vNodes` could be locked before the `::cs_main`. This change makes the explicit locking of recursive mutexes in the explicit order redundant. ACKs for top commit: jnewbery: utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f vasild: ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f ajtowns: utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f - logic seems sound MarcoFalke: review ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f 👢 Tree-SHA512: a8277924339622b188b12d260a100adf5d82781634cf974320cf6007341f946a7ff40351137c2f5369aed0d318f38aac2d32965c9b619432440d722a4e78bb73
2021-04-22net: Restrict period when cs_vNodes mutex is lockedHennadii Stepanov
2021-04-20Merge #21238: A few descriptor improvements to prepare for Taproot supportW. J. van der Laan
0b188b751f970027c52729e0c223cc9257669322 Clean up context dependent checks in descriptor parsing (Pieter Wuille) 33275a96490445e293c322a29a3b146ccb91083c refactor: move uncompressed-permitted logic into ParsePubkey* (Pieter Wuille) 17e006ff8d5e42f22474c5191d1b745bbc97571f refactor: split off subscript logic from ToStringHelper (Pieter Wuille) 6ba5dda0c9de75196c6a427d9e59d39e5a41bff7 Account for key cache indices in subexpressions (Pieter Wuille) 4441c6f3c046c0b28ce3f0ca6d938af71d797586 Make DescriptorImpl support multiple subscripts (Pieter Wuille) a917478db0788b244c0c799b98bf67a94df7035e refactor: move population of out.scripts from ExpandHelper to MakeScripts (Pieter Wuille) 84f3939ece9f4901141b28fd2dd6e3899d01d66e Remove support for subdescriptors expanding to multiple scripts (Pieter Wuille) Pull request description: These are a few refactors and non-invasive improvements to the descriptors code to prepare for adding Taproot descriptors. None of the commits change behavior in any way, except the last one which improves error reporting a bit. ACKs for top commit: S3RK: reACK 0b188b7 Sjors: re-ACK 0b188b7 achow101: re-ACK 0b188b751f970027c52729e0c223cc9257669322 Tree-SHA512: cb4e999134aa2bace0e13d4883454c65bcf1369e1c8585d93cc6444ddc245f3def5a628d58af7dab577e9d5a4a75d3bb46f766421fcc8cc5c85c01a11f148b3f
2021-04-20Merge bitcoin/bitcoin#21691: test: Check that no versionbits are re-usedMarcoFalke
fa8eaee6a8531db970cc84436bf2ae8150a58642 test: Run versionbits_sanity for all chains (MarcoFalke) faec1e9ee1f12612831ad5b0f0a767d87bd2d024 test: Address outstanding versionbits_test feedback (MarcoFalke) fad4167871c3c9fde462e64e3ef3be937e585084 test: Check that no versionbits are re-used (MarcoFalke) Pull request description: ACKs for top commit: jnewbery: Code review ACK fa8eaee6a8531db970cc84436bf2ae8150a58642 ajtowns: ACK fa8eaee6a8531db970cc84436bf2ae8150a58642 code review only Tree-SHA512: e99ffcca8970921fd07fa9e04cf1ea2515a317409865d34ddfd70be0f0b0616b29d1fad58262d96a3c3418c0cf7018a6a955802a178b8f78f6ecfaa30a37d91c
2021-04-20Merge #21244: Move GetDataDir to ArgsManagerfanquake
bb8d1c6e029a2fd01387599d2ff3bfc879a8ada5 Change ClearDataDirPathCache() to ArgsManager.ClearPathCache(). (Kiminuo) b4190eff72c00e384ad238f9c2f10c8b91be969b Change GetBlocksDir() to ArgsManager.GetBlocksDirPath(). (Kiminuo) 83292e2a700afbf39936bd67bb89fab5398d0066 scripted-diff: Modify unit tests to use the ArgsManager in the BasicTestingSetup class instead of implicitly relying on gArgs. (Kiminuo) 55c68e6f011ee604c8a65b9bca668eb4dec452aa scripted-diff: Replace m_args with m_local_args in getarg_tests.cpp (Kiminuo) 511ce3a26b3b78e14acd0d85496b5422a236cf63 BasicTestingSetup: Add ArgsManager. (Kiminuo) 1cb52ba0656e78ca6c2ef84b1558198ad113b76a Modify "util_datadir" unit test to not use gArgs. (Kiminuo) 1add318704108faa98f5b1b8e9c96d960e9d23a8 Move GetDataDir(fNetSpecific) implementation to ArgsManager. (Kiminuo) 70cdf679f8e665dbdc3301873a0267fe9faa72cd Move StripRedundantLastElementsOfPath before ArgsManager class. (Kiminuo) Pull request description: This PR attempts to contribute to "Remove gArgs" (#21005). Main changes: * `GetDataDir()` function is moved to `ArgsManager.GetDataDirPath()`. * `GetBlocksDir()` function is moved to `ArgsManager.GetBlocksDirPath()`. ACKs for top commit: ryanofsky: Code review ACK bb8d1c6e029a2fd01387599d2ff3bfc879a8ada5. Just minor const/naming changes and splitting/scripting commits since last review MarcoFalke: review ACK bb8d1c6e029a2fd01387599d2ff3bfc879a8ada5 📓 hebasto: re-ACK bb8d1c6e029a2fd01387599d2ff3bfc879a8ada5, addressed comments, and two commits made scripted-diffs since my [previous](https://github.com/bitcoin/bitcoin/pull/21244#pullrequestreview-638270583) review. Tree-SHA512: ba9408c22129d6572beaa103dca0324131766f06d562bb7d6b9e214a0a4d40b0216ce861384562bde24b744003b3fbe6fac239061c8fd798abd3981ebc1b9019
2021-04-19test: Add unit test for Coinstats indexFabian Jahr
2021-04-19refactor: Pass hash_type to CoinsStats in stats objectFabian Jahr
2021-04-19Merge bitcoin/bitcoin#21280: test: bug fix in transaction_testsMarcoFalke
b109bde46a8730afbc09c107b802a2c321293f4b [test] check that mapFlagNames is up to date (glozow) 5d3ced72f9b5f36db1a76bd8bc918d11b87dfd72 [test] remove unnecessary OP_1s from invalid tests (glozow) 5aee73d1759bcc0d1e951776942e616843934af1 [test] minor improvements / followups (glozow) 8a365df5586b36d1772c78069f9d93c56a81df6f [test] fix bug in ExcludeIndividualFlags (glozow) 8cac2923f57ac33848ff41b74c3be520b75936df [test] remove invalid test from tx_valid.json (glozow) Pull request description: This is a followup to #19698. - There was a bug in the `ExcludeIndividualFlags` function which is fixed here. - Fixing this bug also showed that there is a test that's supposed to fail (already existing in tx_invalid.json) in tx_valid.json, so I removed it. Other than that, the tests should all pass. - Also implements a few suggestions I received offline: removing the `OP_1`s from the invalid tests (similar to https://github.com/bitcoin/bitcoin/commit/19db590d044efe7d474a16720e5b56e7b55db54c), comments, and style. - A few other small fixes, like adding asserts, putting all the flags in `mapFlagNames`, better error messages ACKs for top commit: jnewbery: utACK b109bde46a8730afbc09c107b802a2c321293f4b Tree-SHA512: 7233a8c0f1ae1172fac8000ea6e05384ecf79074c39948d118464868505c7f02f17e96503c81bd05c07adb2087648a5d93d9899e16fdefa6b7efcb51319444a9
2021-04-18Change ClearDataDirPathCache() to ArgsManager.ClearPathCache().Kiminuo
2021-04-18scripted-diff: Modify unit tests to use the ArgsManager in the ↵Kiminuo
BasicTestingSetup class instead of implicitly relying on gArgs. -BEGIN VERIFY SCRIPT- git ls-files src/test/dbwrapper_tests.cpp src/test/denialofservice_tests.cpp src/test/flatfile_tests.cpp src/test/fs_tests.cpp src/test/settings_tests.cpp src/test/util_tests.cpp | xargs sed -i 's/GetDataDir()/m_args.GetDataDirPath()/g'; -END VERIFY SCRIPT-
2021-04-18scripted-diff: Replace m_args with m_local_args in getarg_tests.cppKiminuo
-BEGIN VERIFY SCRIPT- git ls-files src/test/getarg_tests.cpp | xargs sed -i "s/m_args/m_local_args/g"; -END VERIFY SCRIPT-
2021-04-18BasicTestingSetup: Add ArgsManager.Kiminuo
2021-04-18Modify "util_datadir" unit test to not use gArgs.Kiminuo
2021-04-17Move GetDataDir(fNetSpecific) implementation to ArgsManager.Kiminuo
2021-04-17Merge #21391: [Bundle 5/n] Prune g_chainman usage in RPC modulesMarcoFalke
586190f0b4740457cb86cba632e3d64e6dfe9b0c rpc/rest: Take and reuse local Chain/ChainState obj (Carl Dong) bc3bd369027273278a0541f3b991eb71de831aa2 rpc: style: Improve BuriedForkDescPushBack signature (Carl Dong) f99913969f92b8b9cef1b83f5ee8e6a9267b4af0 rpc: Remove unnecessary casting of block height (Carl Dong) 6a3d1920209cded0dae52fb9070a3530d9a4e5fd rpc: Tidy up local references (see commit message) (Carl Dong) 038854f31e3511e8bb6e163305cab0a96783d25b rest/rpc: Remove now-unused old Ensure functions (Carl Dong) 6fb65b49f4d393b091479be5a5df5a0a160cf986 scripted-diff: rest/rpc: Use renamed EnsureAny*() (Carl Dong) 1570c7ee98612366df031bebef9e0468fb57b8a2 rpc: Add renamed EnsureAny*() functions (Carl Dong) 306b1cd3eeb2502904ed4698646d2c86d028aad2 rpc: Add alt Ensure* functions acepting NodeContext (Carl Dong) d7824acdb9b18fe8f151771a83ccae1681f16c66 rest: Use existing NodeContext (Carl Dong) 3f0893479908ca28d6127c8d0ada30737cb830be rest: Pass in NodeContext to rest_block (Carl Dong) 7be0671b950842fc3a17641a4a21501de0a800b5 rpc/rawtx: Use existing NodeContext (Carl Dong) 60dc05afc6f6388c6f86729a0edd7cb69f1748e0 rpc/mining: Use existing NodeContext (Carl Dong) d485e815e2b62dc74a485569d08130dc3ef9ff63 rpc/blockchain: Use existing NodeContext (Carl Dong) d0abf0bf429586e3a5b4c3231fe430dc29695481 rpc/*,rest: Add review-only assertion to EnsureChainman (Carl Dong) cced0f46c9133e0fc6211e987421ad1d9be1a399 miner: Pass in previous CBlockIndex to RegenerateCommitments (Carl Dong) Pull request description: Overall PR: #20158 (tree-wide: De-globalize ChainstateManager) Based on: - [x] #21270 | [Bundle 4/n] Prune g_chainman usage in validation-adjacent modules - [x] #21525 | [Bundle 4.5/n] Followup fixups to bundle 4 Note to reviewers: 1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so: 1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only** 2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase** 3. Remove `old_function` ACKs for top commit: ryanofsky: Code review ACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c. Since last review, no changes to existing commits, just some simple new commits added: three new commits renaming std::any Ensure functions (scripted diff commit and manual pre/post commits), and one new commit factoring out a repeated `ActiveChain()` call made in a loop. Thanks for the updates! jnewbery: utACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c MarcoFalke: review ACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c 🍯 Tree-SHA512: 64b677fb50141805b55c3f1afe68fcd298f9a071a359bdcd63256d52e334f83e462f31fb3ebee9b630da8f1d912a03a128cfc38179e7aaec29a055744a98478c
2021-04-17test: Run versionbits_sanity for all chainsMarcoFalke
2021-04-17Merge #21689: test: Remove intermittently failing and not very meaningful ↵MarcoFalke
`BOOST_CHECK` in `cnetaddr_basic` 63631beef6a0046390469971adf4500718ab34ad test: Remove intermittently failing and not very meaningful `BOOST_CHECK` in `cnetaddr_basic` (practicalswift) Pull request description: Remove intermittently failing and not very meaningful `BOOST_CHECK` in `cnetaddr_basic`. Fixes #21682. Rationale from https://github.com/bitcoin/bitcoin/issues/21682#issuecomment-819897122: > I've looked at that test before and I don't think that specific `BOOST_CHECK` makes much sense TBH :) > > 1.) I don't understand why we test if `ToString()` output includes `%zone_index`: it clearly doesn't on some platforms, so we cannot rely on it anyways. Then why test it? > > 2.) And perhaps more fundamentally: why would we even _want_ to have `%zone_index` in our textual `ToString()` output? I think the expectation is to get say `fe80::1ff:fe23:4567:890a` (without zone index) and not say `fe80::1ff:fe23:4567:890a%eth2 ` or `fe80::1ff:fe23:4567:890a%3 `when doing `ipv6_addr.ToString()` :) ACKs for top commit: MarcoFalke: review ACK 63631beef6a0046390469971adf4500718ab34ad Tree-SHA512: 06863d1edfb9ad1ca9bcae09cf3f0f47b58bb29d222b70799c3dc059b96452889026e4b99b132782846d9896e3e798d17c7f9406e0e6a0bec1bffc6edb54e9df
2021-04-17test: Address outstanding versionbits_test feedbackMarcoFalke
* https://github.com/bitcoin/bitcoin/pull/21377#discussion_r609585080 * https://github.com/bitcoin/bitcoin/pull/21377#discussion_r613702341
2021-04-17test: Check that no versionbits are re-usedMarcoFalke
This allows to remove check that windows for the same bit are disjoint This addresses https://github.com/bitcoin/bitcoin/pull/21377#discussion_r611492633
2021-04-15Merge #21630: fuzz: split FuzzedSock interface and implementationMarcoFalke
549c82ad3a34a885ecca37a5f04c36dfbaa95d17 fuzz: use ConsumeBool() instead of !ConsumeBool() (Vasil Dimov) 29ae1c13a59187119f5b2a38b54dbbec936d8f87 fuzz: split FuzzedSock interface and implementation (Vasil Dimov) 9668e43d8e757c0185b900eb6ee6891a0ba41666 fuzz: make FuzzedSock::Wait() sometimes simulate an occurred event (Vasil Dimov) 0c90ff1429deaa556c0509c13cdd5aef5df9c0d4 fuzz: set errno from FuzzedSock::Wait() if it simulates a failure (Vasil Dimov) 5198a02de4e7a1b0efe28c6095745ce59f7f98c4 style: remove extra white space (Vasil Dimov) Pull request description: * split FuzzedSock interface and implementation * make FuzzedSock::Wait() sometimes simulate an occurred event * set errno from FuzzedSock::Wait() if it simulates a failure (this is a followup from https://github.com/bitcoin/bitcoin/pull/21617) ACKs for top commit: practicalswift: cr ACK 549c82ad3a34a885ecca37a5f04c36dfbaa95d17: patch looks correct and touches only `src/test/fuzz/` MarcoFalke: re-ACK 549c82ad3a34a885ecca37a5f04c36dfbaa95d17 only change is rebase 🎬 Tree-SHA512: 8ba965a8319074ad2ef840219c35c77e37cc79f00fb3926f20ccbf5f58e9616f5a3ac96434ad33996b47d292fa760d5d00a529001ac0d1d254262e5df93f616f
2021-04-15Merge #21676: test: Use mocktime to avoid intermittent failure in rpc_testsMarcoFalke
fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1 test: Reset mocktime in the common setup (MarcoFalke) fa78590a8fffdfc7e98ddb1f81218f05b1935a0a test: Use mocktime to avoid intermittent failure (MarcoFalke) Pull request description: See https://github.com/bitcoin/bitcoin/pull/21602#discussion_r611176103 ACKs for top commit: jonatack: Code review ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1 jarolrod: ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1 Tree-SHA512: 4967e006f3d2c4eb92f03c9086a6abe3190ad54755d251c30d20422c574bb1a154c06f3d5bcb0d4deaa3c4abfd3864d743b71d84897edd358e829bb42233ad12
2021-04-15fuzz: use ConsumeBool() instead of !ConsumeBool()Vasil Dimov
The former is shorter and ends up with a "random" bool anyway.
2021-04-15fuzz: split FuzzedSock interface and implementationVasil Dimov
Move the `FuzzedSock`'s implementation from `src/test/fuzz/util.h` to `src/test/fuzz/util.cpp`. A separate interface and implementation make the code more readable for consumers who don't need to (better not) know the implementation details.
2021-04-15fuzz: make FuzzedSock::Wait() sometimes simulate an occurred eventVasil Dimov
2021-04-15fuzz: set errno from FuzzedSock::Wait() if it simulates a failureVasil Dimov
2021-04-15style: remove extra white spaceVasil Dimov
2021-04-15test: Remove intermittently failing and not very meaningful `BOOST_CHECK` in ↵practicalswift
`cnetaddr_basic`
2021-04-15Merge #21677: fuzz: Avoid use of low file descriptor ids (which may be in ↵MarcoFalke
use) in FuzzedSock 6262182b3f1c9540291fb8de3bf7a785e7113c55 Avoid use of low file descriptor ids (which may be in use) in FuzzedSock and StaticContentsSock (practicalswift) Pull request description: Avoid use of low file descriptor ids (which may be in use) in `FuzzedSock`. Context: https://github.com/bitcoin/bitcoin/pull/21630/files#r610694541 ACKs for top commit: vasild: ACK 6262182b3f1c9540291fb8de3bf7a785e7113c55 Tree-SHA512: e622acb4d01446c3db01adbbbb779038be7247e13f3f4e72c614bc2880c3efd710fd3b189f87abb00f236fa5ddf91f4c215f420ca4eb08a97aaba31593254c3d
2021-04-15Merge #21377: Speedy trial support for versionbitsfanquake
ffe33dfbd4c3b11e3475b022b6c1dd077613de79 chainparams: drop versionbits threshold to 90% for mainnnet and signet (Anthony Towns) f054f6bcd2c2ce5fea84cf8681013f85a444e7ea versionbits: simplify state transitions (Anthony Towns) 55ac5f568a3b73d6f1ef4654617fb76e8bcbccdf versionbits: Add explicit NEVER_ACTIVE deployments (Anthony Towns) dd07e6da48040dc7eae46bc7941db48d98a669fd fuzz: test versionbits delayed activation (Anthony Towns) dd85d5411c1702c8ae259610fe55050ba212e21e tests: test versionbits delayed activation (Anthony Towns) 73d4a706393e6dbd6b6d6b6428f8d3233ac0a2d8 versionbits: Add support for delayed activation (Anthony Towns) 9e6b65f6fa205eee5c3b99343988adcb8d320460 tests: clean up versionbits test (Anthony Towns) 593274445004506c921d5d851361aefb3434d744 tests: test ComputeBlockVersion for all deployments (Anthony Towns) 63879f0a4760c0c0f784029849cb5d21ee088abb tests: pull ComputeBlockVersion test into its own function (Anthony Towns) Pull request description: BIP9-based implementation of "speedy trial" activation specification, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-March/018583.html Edge cases are tested by fuzzing added in #21380. ACKs for top commit: instagibbs: tACK https://github.com/bitcoin/bitcoin/pull/21377/commits/ffe33dfbd4c3b11e3475b022b6c1dd077613de79 jnewbery: utACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 MarcoFalke: review ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 💈 achow101: re-ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 gmaxwell: ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 benthecarman: ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 Sjors: ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 jonatack: Initial approach ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 after a first pass of review, building and testing each commit, mostly looking at the changes and diffs. Will do a more high-level review iteration. A few minor comments follow to pick/choose/ignore. ariard: Code Review ACK ffe33df Tree-SHA512: f79a7146b2450057ee92155cbbbcec12cd64334236d9239c6bd7d31b32eec145a9781c320f178da7b44ababdb8808b84d9d22a40e0851e229ba6d224e3be747c
2021-04-14Avoid use of low file descriptor ids (which may be in use) in FuzzedSock and ↵practicalswift
StaticContentsSock
2021-04-14test: Reset mocktime in the common setupMarcoFalke
Doing it there will reduce code bloat and also ensure no test can "forget" to reset it
2021-04-14test: Use mocktime to avoid intermittent failureMarcoFalke
2021-04-13Merge #21575: refactor: Create blockstorage modulefanquake
fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f doc: Remove irrelevant link to GitHub (MarcoFalke) fa121b628d51bb0e25eb3fbd716881fa55527dc7 blockstorage: [refactor] Use chainman reference where possible (MarcoFalke) fa0c7d9ad24d3c9515d3f9c136af4071cbd79055 move-only: Move *Disk functions to blockstorage (MarcoFalke) fa91b2b2b3447a3645e7958c7dc4e1946a69cb9c move-only: Move AbortNode to shutdown (MarcoFalke) fa413f07a14744e7d7f7746e861aabd9cf938f61 move-only: Move ThreadImport to blockstorage (MarcoFalke) faf843c07f99f91603e08ea858f972516f1d669a refactor: Move load block thread into ChainstateManager (MarcoFalke) Pull request description: This picks up the closed pull request #21030 and is the first step toward fixing #21220. The basic idea is to move all disk access into a separate module with benefits: * Breaking down the massive files init.cpp and validation.cpp into logical units * Creating a standalone-module to reduce the mental complexity * Pave the way to fix validation related circular dependencies * Pave the way to mock disk access for testing, especially where it is performance critical (like fuzzing) ACKs for top commit: promag: Code review ACK fadcd3f78e, checked (almost) moved only changes. This is a nice tidy up change and doesn't change behavior. Easily reviewed commit by commit. jamesob: ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f ([`jamesob/ackr/21575.1.MarcoFalke.refactor_create_blocksto`](https://github.com/jamesob/bitcoin/tree/ackr/21575.1.MarcoFalke.refactor_create_blocksto)) ryanofsky: Code review ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f. New organization makes sense, moves extraneous things outside of validation.cpp. PR is also easy to review with helpfully split up moveonly commits. Tree-SHA512: 917996592b6d8f9998289d8cb2b1b78b23d1fdb3b07216c9caec1380df33baa09dc2c1e706da669d440b497e79c9c62a01ca20dc202df5ad974a75f3ef7a143b
2021-04-13Merge #21633: refactor: add [[noreturn]] attribute where applicablefanquake
003929c0d55532038d5bf6fc0ff4a20628710fae refactor: add [[noreturn]] attribute where applicable (fanquake) Pull request description: Similar to #10843. We could build with `-Wmissing-noreturn`, however that would also mean modifying something like `--suppress-external-warnings` to suppress warnings for leveldb, which I don't think we want to do. In any case, the functions where this is applicable are only added/removed very rarely. ACKs for top commit: vasild: ACK 003929c0d55532038d5bf6fc0ff4a20628710fae Tree-SHA512: 33dfa6547d6b84f38a941f24d4c2effe8fde7b93dbc0b27a9309716420e4a879fdbe689d789fa5439d65f5f78292f89fd9dc1b61c97acf69316dfed954086705
2021-04-13Merge #21330: Deal with missing data in signature hashes more consistentlyfanquake
725d7ae0494d4a45f5a840bbbd19c008a7363965 Use PrecomputedTransactionData in signet check (Pieter Wuille) 497718b467330b2c6bb0d44786020c55f1aa75f9 Treat amount<0 also as missing data for P2WPKH/P2WSH (Pieter Wuille) 3820090bd619ac85ab35eff376c03136fe4a9f04 Make all SignatureChecker explicit about missing data (Pieter Wuille) b77b0cc507bdc716e5236b1d9880e648147e0af9 Add MissingDataBehavior and make TransactionSignatureChecker handle it (Pieter Wuille) Pull request description: Currently we have 2 levels of potentially-missing data in the transaction signature hashes: * P2WPKH/P2WSH hashes need the spent amount * P2TR hashes need all spent outputs (amount + scriptPubKey) Missing amounts are treated as -1 (thus leading to unexpected signature failures), while missing outputs in P2TR validation cause assertion failure. This is hard to extend for signing support, and also quite ugly in general. In this PR, an explicit configuration option to {Mutable,}TransactionSignatureChecker is added (MissingDataBehavior enum class) to either select ASSERT_FAIL or FAIL. Validation code passes ASSERT_FAIL (as at validation time all data should always be passed, and anything else is a serious bug in the code), while signing code uses FAIL. The existence of the ASSERT_FAIL option is really just an abundance of caution. Always using FAIL should be just fine, but if there were for some reason a code path in consensus code was introduced that misses certain data, I think we prefer as assertion failure over silently introducing a consensus change. Potentially useful follow-ups (not for this PR, in my preference): * Having an explicit script validation error code for missing data. * Having a MissingDataBehavior::SUCCEED option as well, for use in script/sign.cpp DataFromTransaction (if a signature is present in a witness, and we don't have enough data to fully validate it, we should probably treat it as valid and not touch it). ACKs for top commit: sanket1729: reACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 Sjors: ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 achow101: re-ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 benthecarman: ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 fjahr: Code review ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 Tree-SHA512: d67dc51bae9ca7ef6eb9acccefd682529f397830f77d74cd305500a081ef55aede0e9fa380648c3a8dd4857aa7eeb1ab54fe808979d79db0784ac94ceb31b657
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-19 22:56:59 +0000