aboutsummaryrefslogtreecommitdiff
path: root/src/validation.cpp
AgeCommit message (Collapse)Author
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-21Merge bitcoin/bitcoin#26265: POLICY: Relax MIN_STANDARD_TX_NONWITNESS_SIZE ↵Andrew Chow
to 65 non-witness bytes b2aa9e85289fc654106a890c35935e9c76c411fb Add release note for MIN_STANDARD_TX_NONWITNESS_SIZE relaxation (Greg Sanders) 8c5b3646b5afe8a61f5c66478d8e11f0d2ce5108 Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytes (Greg Sanders) Pull request description: Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled. There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical. Two changes could be accomplished: 1) Anything not 64 bytes could be allowed 2) Anything above 64 bytes could be allowed In the Great Consensus Cleanup, suggestion (2) was proposed as a consensus change, and is the simpler of the two suggestions. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5. The functional test is also modified to test the actual case we care about: 64 bytes Related mailing list discussions here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/020995.html And a couple years earlier: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-May/017883.html ACKs for top commit: achow101: reACK b2aa9e85289fc654106a890c35935e9c76c411fb glozow: reACK b2aa9e85289fc654106a890c35935e9c76c411fb pablomartin4btc: re-ACK https://github.com/bitcoin/bitcoin/commit/b2aa9e85289fc654106a890c35935e9c76c411fb jonatack: ACK b2aa9e85289fc654106a890c35935e9c76c411fb with some suggestions Tree-SHA512: c1ec1af9ddcf31b2272209a4f1ee0c5607399f8172e5a1dfd4604cf98bfb933810dd9369a5917ad122add003327c9fcf6ee26995de3aca41d5c42dba527991ad
2022-12-19Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytesGreg Sanders
Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled. There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical. Two changes could be accomplished: 1) Anything not 64 bytes could be allowed 2) Anything above 64 bytes could be allowed In the Great Consensus Cleanup, suggestion (2) was the route taken. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5. The functional test is also modified to test the actual case we care about: 64 bytes
2022-12-13Merge bitcoin/bitcoin#26477: validation: fix broken maxtipage comparisonfanquake
e4be0e9b0661a8af49c4e6d5472804913f04b8fc test: add -maxtipage test for the maximum allowable value (James O'Beirne) a451e832b46bcb984dfcd9478ea8ebb8b3de0c62 fix: validation: cast now() to seconds for maxtipage comparison (James O'Beirne) Pull request description: Since https://github.com/bitcoin/bitcoin/commit/faf44876db555f7488c8df96db9fa88b793f897c, the maxtipage comparison in IsInitialBlockDownload() has been broken, since the NodeClock::now() time_point is in the system's native denomination (nanoseconds). Without this patch, specifying the maximum allowable -maxtipage (9223372036854775807) results in a SIGABRT crash: ``` % gdb --args ./src/bitcoind -maxtipage=9223372036854775207 -minimumchainwork=0x00 -stopatheight=30000 ... 2022-11-09T15:55:17Z [dnsseed] dnsseed thread exit [Thread 0x7fff937fe640 (LWP 69883) exited] Thread 29 "b-msghand" received signal SIGABRT, Aborted. [Switching to Thread 0x7fff91ffb640 (LWP 69886)] __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44 44 ./nptl/pthread_kill.c: No such file or directory. (gdb) bt #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44 #1 0x00007ffff768989f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78 #2 0x00007ffff763da52 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 #3 0x00007ffff7628469 in __GI_abort () at ./stdlib/abort.c:79 #4 0x00007ffff7cf79a4 in __mulvdi3 () from /lib/x86_64-linux-gnu/libgcc_s.so.1 #5 0x00005555558d13ab in std::chrono::__duration_cast_impl<std::chrono::duration<long, std::ratio<1l, 1000000000l> >, std::ratio<1000000000l, 1l>, long, false, true>::__cast<long, std::ratio<1l, 1l> > (__d=...) at /usr/include/c++/12/bits/chrono.h:521 #6 std::chrono::duration_cast<std::chrono::duration<long, std::ratio<1l, 1000000000l> >, long, std::ratio<1l, 1l> > (__d=...) at /usr/include/c++/12/bits/chrono.h:260 #7 std::chrono::duration<long, std::ratio<1l, 1000000000l> >::duration<long, std::ratio<1l, 1l>, void> (__d=..., this=<optimized out>) at /usr/include/c++/12/bits/chrono.h:514 #8 std::chrono::operator-<long, std::ratio<1l, 1000000000l>, long, std::ratio<1l, 1l> > (__rhs=..., __lhs=...) at /usr/include/c++/12/bits/chrono.h:650 #9 std::chrono::operator-<NodeClock, std::chrono::duration<long, std::ratio<1l, 1000000000l> >, long, std::ratio<1l, 1l> > (__rhs=..., __lhs=...) at /usr/include/c++/12/bits/chrono.h:1020 #10 Chainstate::IsInitialBlockDownload (this=0x555556071940) at ./src/validation.cpp:1545 #11 0x00005555556efd1e in operator() (__closure=<optimized out>) at ./src/net_processing.cpp:3369 #12 (anonymous namespace)::PeerManagerImpl::ProcessMessage (this=0x555556219be0, pfrom=..., msg_type=..., vRecv=..., time_received=..., interruptMsgProc=...) at ./src/net_processing.cpp:3369 #13 0x00005555556f75cc in (anonymous namespace)::PeerManagerImpl::ProcessMessages (this=0x555556219be0, pfrom=<optimized out>, interruptMsgProc=std::atomic<bool> = { false }) at ./src/net_processing.cpp:4985 #14 0x00005555556a83c9 in CConnman::ThreadMessageHandler (this=0x5555560ebc70) at ./src/net.cpp:2014 #15 0x0000555555c4d5d6 in std::function<void ()>::operator()() const (this=0x7fff91ffadb0) at /usr/include/c++/12/bits/std_function.h:591 #16 util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) ( thread_name="0\255\377\221\377\177\000\000\v\000\000\000\000\000\000\000TraceThread\000\000\000\000\000P\255\377\221\377\177\000\000\017\000\000\000\000\000\000\000util/thread.cpp\000\000\000\000\000\000\000\000\000\000ihB鵿6\000\000\000\000\000\000\000\000\260\255\377\221\377\177\000\000\277\211\321UUU\000\000p\324\304UUU\000\000\002\000\000\000\000\000\000\000\240xh\367\377\177\000\000\000\000\000\000\000\000\000\000]\340iUUU\000\000p\274\016VUU\000\000\000\000\000\000\000\000\000\000\300\303iUUU\000\000p\206jUUU", '\000' <repeats 11 times>, "ihB鵿6\200\251!VUU\000\000"..., thread_func=...) at util/thread.cpp:21 #17 0x000055555569e05d in std::__invoke_impl<void, void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > (__f=<optimized out>) at /usr/include/c++/12/bits/invoke.h:61 #18 std::__invoke<void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > (__fn=<optimized out>) at /usr/include/c++/12/bits/invoke.h:96 #19 std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > >::_M_invoke<0, 1, 2> (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:252 #20 std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > >::operator() (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:259 #21 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > > >::_M_run(void) (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:210 #22 0x00007ffff7ad43d3 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #23 0x00007ffff7687b27 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:435 #24 0x00007ffff770a78c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 (gdb) ``` ACKs for top commit: MarcoFalke: review ACK e4be0e9b0661a8af49c4e6d5472804913f04b8fc 🏽 Tree-SHA512: d892d6264a284d952a68a8631a6301277373b8df939dafd9e2652f2f22ab60712cde63b90c27c67ea2d05f02443452e3e4e1b9f25479bfaca00d4c4de13b9fbd
2022-12-08Merge bitcoin/bitcoin#26513: Make static nLastFlush and nLastWrite ↵fanquake
Chainstate members 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 Make static nLastFlush and nLastWrite Chainstate members (Aurèle Oulès) Pull request description: Fixes #22189. The `static std::multimap<uint256, FlatFilePos> mapBlocksUnknownParent; ` referenced in the issue was already fixed by #25571. I don't believe Chainstate references any other static variables. ACKs for top commit: jamesob: ACK 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 ([`jamesob/ackr/26513.1.aureleoules.make_static_nlastflush_a`](https://github.com/jamesob/bitcoin/tree/ackr/26513.1.aureleoules.make_static_nlastflush_a)) theStack: Concept and code-review ACK 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 Tree-SHA512: 0f26463c079bbc5e0e62707d4ca4c8c9bbb99edfa3391d48d4915d24e2a1190873ecd4f9f11da25b44527671cdc82c41fd8234d56a4592a246989448d34406b0
2022-12-06Merge bitcoin/bitcoin#26609: refactor: Move `txmempool_entry.h` --> ↵MarcoFalke
`kernel/mempool_entry.h` 38941a703e079709bb465ad1fcde50e11350f8f1 refactor: Move `txmempool_entry.h` --> `kernel/mempool_entry.h` (Hennadii Stepanov) Pull request description: This PR addresses the https://github.com/bitcoin/bitcoin/pull/17786#discussion_r1027818360: > why not move it to the right place, that is to `kernel/txmempool_entry.h`? ACKs for top commit: MarcoFalke: review ACK 38941a703e079709bb465ad1fcde50e11350f8f1 📊 Tree-SHA512: 0145974b63b67ca1d9d89af2dd9d4438beca480c16a563f330da05fec49b8394d7ba20ed83cf7d50b2e19454e006978ebed42b0e07887b98d00210f3201ce9ba
2022-12-05Merge bitcoin/bitcoin#19888: rpc, test: Improve getblockstats for unspendablesAndrew Chow
d885bb2f6ea34cd21dacfebe763a07dbb389c1bd test: Test exclusion of OP_RETURN from getblockstats (Fabian Jahr) ba9d288b2468f047e5a8e4637dd8749e247ff547 test: Fix getblockstats test data generator (Fabian Jahr) 2ca5a496c2f3cbcc63ea15fa05c1658e7f527bbc rpc: Improve getblockstats (Fabian Jahr) cb94db119f4643f49da63520d64efc99fb0c0795 validation, index: Add unspendable coinbase helper functions (Fabian Jahr) Pull request description: Fixes #19885 The genesis block does not have undo data saved to disk so the RPC errored because of that. ACKs for top commit: achow101: ACK d885bb2f6ea34cd21dacfebe763a07dbb389c1bd aureleoules: ACK d885bb2f6ea34cd21dacfebe763a07dbb389c1bd stickies-v: ACK d885bb2f6 Tree-SHA512: f37bda736ed605b7a41a81eeb4bfbb5d2b8518f847819e5d6a090548a61caf1455623e15165d72589ab3f4478252b00e7b624f9313ad6708cac06dd5edb62e9a
2022-11-30refactor: Move `txmempool_entry.h` --> `kernel/mempool_entry.h`Hennadii Stepanov
2022-11-18Merge bitcoin/bitcoin#17786: refactor: Nuke policy/fees->mempool circular ↵glozow
dependencies c8dc0e3eaa9e0f956c5177bcb69632beb0d51770 refactor: Inline `CTxMemPoolEntry` class's functions (Hennadii Stepanov) 75bbe594e54402ed248ecf2bdfe54e8283d20fff refactor: Move `CTxMemPoolEntry` class to its own module (Hennadii Stepanov) Pull request description: This PR: - gets rid of the `policy/fees` -> `txmempool` -> `policy/fees` circular dependency - is an alternative to #13949, which nukes only one circular dependency ACKs for top commit: ryanofsky: Code review ACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770. Just include and whitespace changes since last review, and there's a moveonly commit now so it's very easy to review theStack: Code-review ACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770 glozow: utACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770, agree these changes are an improvement. Tree-SHA512: 36ece824e6ed3ab1a1e198b30a906c8ac12de24545f840eb046958a17315ac9260c7de26e11e2fbab7208adc3d74918db7a7e389444130f8810548ca2e81af41
2022-11-17log: improve some validation log messages to include hashPrevBlockSkuli Dulfari
When there is an issue with a previous block the current log messages do not indicate hashPrevBlock. Adding it makes debugging easier.
2022-11-16refactor: Move `CTxMemPoolEntry` class to its own moduleHennadii Stepanov
This change nukes the policy/fees->mempool circular dependency. Easy to review using `diff --color-moved=dimmed-zebra`.
2022-11-16Make static nLastFlush and nLastWrite Chainstate membersAurèle Oulès
2022-11-15Merge bitcoin/bitcoin#16981: Improve runtime performance of --reindexAndrew Chow
db929893ef0bc86ea2708cdbcf41152240cd7c73 Faster -reindex by initially deserializing only headers (Larry Ruane) c72de9990ae8f1744006d9c852023b882d5ed80c util: add CBufferedFile::SkipTo() to move ahead in the stream (Larry Ruane) 48a68908ba3d5e077cda7bd1e908b923fbead824 Add LoadExternalBlockFile() benchmark (Larry Ruane) Pull request description: ### Background During the first part of reindexing, `LoadExternalBlockFile()` sequentially reads raw blocks from the `blocks/blk00nnn.dat` files (rather than receiving them from peers, as with initial block download) and eventually adds all of them to the block index. When an individual block is initially read, it can't be immediately added unless all its ancestors have been added, which is rare (only about 8% of the time), because the blocks are not sorted by height. When the block can't be immediately added to the block index, its disk location is saved in a map so it can be added later. When its parent is later added to the block index, `LoadExternalBlockFile()` reads and deserializes the block from disk a second time and adds it to the block index. Most blocks (92%) get deserialized twice. ### This PR During the initial read, it's rarely useful to deserialize the entire block; only the header is needed to determine if the block can be added to the block index immediately. This change to `LoadExternalBlockFile()` initially deserializes only a block's header, then deserializes the entire block only if it can be added immediately. This reduces reindex time on mainnet by 7 hours on a Raspberry Pi, which translates to around a 25% reduction in the first part of reindexing (adding blocks to the index), and about a 6% reduction in overall reindex time. Summary: The performance gain is the result of deserializing each block only once, except its header which is deserialized twice, but the header is only 80 bytes. ACKs for top commit: andrewtoth: ACK db929893ef0bc86ea2708cdbcf41152240cd7c73 achow101: ACK db929893ef0bc86ea2708cdbcf41152240cd7c73 aureleoules: ACK db929893ef0bc86ea2708cdbcf41152240cd7c73 - minor changes and new benchmark since last review theStack: re-ACK db929893ef0bc86ea2708cdbcf41152240cd7c73 stickies-v: re-ACK db929893e Tree-SHA512: 5a5377192c11edb5b662e18f511c9beb8f250bc88aeadf2f404c92c3232a7617bade50477ebf16c0602b9bd3b68306d3ee7615de58acfd8cae664d28bb7b0136
2022-11-14fix: validation: cast now() to seconds for maxtipage comparisonJames O'Beirne
Since faf44876db555f7488c8df96db9fa88b793f897c, the maxtipage comparison in IsInitialBlockDownload() has been broken, since the NodeClock::now() time_point is in the system's native denomination (micrcoseconds). Without this patch, specifying the maximum allowable -maxtipage (9223372036854775807) results in a SIGABRT crash. Co-authored-by: MacroFake <falke.marco@gmail.com>
2022-10-26Merge bitcoin/bitcoin#25704: refactor: Remove almost all validation option ↵MacroFake
globals aaaa7bd0ba60aa7114810d4794940882d987c0aa iwyu: Add missing includes (MacroFake) fa9ebec096ae185576a54aa80bd2a9e57f867ed4 Remove g_parallel_script_checks (MacroFake) fa7c834b9f988fa7f2ace2d67b1628211b7819df Move ::fCheckBlockIndex into ChainstateManager (MacroFake) fa43188d86288fa6666307a77c106c8f069ebdbe Move ::fCheckpointsEnabled into ChainstateManager (MacroFake) cccca83099453bf0882bce4f897f77eee5836e8b Move ::nMinimumChainWork into ChainstateManager (MacroFake) fa29d0b57cdeb91c8798d5c90ba9cc18085e99fb Move ::hashAssumeValid into ChainstateManager (MacroFake) faf44876db555f7488c8df96db9fa88b793f897c Move ::nMaxTipAge into ChainstateManager (MacroFake) Pull request description: It seems preferable to assign globals to a class (in this case `ChainstateManager`), than to leave them dangling. This should clarify scope for code-readers, as well as clarifying unit test behaviour. ACKs for top commit: dergoegge: Code review ACK aaaa7bd0ba60aa7114810d4794940882d987c0aa ryanofsky: Code review ACK aaaa7bd0ba60aa7114810d4794940882d987c0aa. No changes since last review, other than rebase aureleoules: reACK aaaa7bd0ba60aa7114810d4794940882d987c0aa Tree-SHA512: 83ec3ba0fb4f1dad95810d4bd4e578454e0718dc1bdd3a794cc4e48aa819b6f5dad4ac4edab3719bdfd5f89cbe23c2740a50fd56c1ff81c99e521c5f6d4e898d
2022-10-24Faster -reindex by initially deserializing only headersLarry Ruane
When a block is initially read from a blk*.dat file during reindexing, it can be added to the block index only if all of its ancestor blocks have been added, which is rare. If the block's ancestors have not been added, the block must be re-read from disk later when it can be added. This commit: During the initial block read, deserialize only its header, rather than the entire block, since this is sufficient to determine if its parent (and thus all its ancestors) has been added. This is a performance improvement.
2022-10-23validation, index: Add unspendable coinbase helper functionsFabian Jahr
Making the checks to identify BIP30 available outside of validation.cpp is needed for reporting and tracking statistics on specific blocks and the UTXO set correctly.
2022-10-19Merge bitcoin/bitcoin#25830: refactor: Replace m_params with ↵MacroFake
chainman.GetParams() 5d3f98d27879cd6d84b8590e947336e8d09613ed refactor: Replace m_params with chainman.GetParams() (Aurèle Oulès) Pull request description: Fixes a TODO introduced in #24595. Removes `m_params` from `CChainState` class and replaces it with `m_chainman.GetParams()`. ACKs for top commit: MarcoFalke: review ACK 5d3f98d27879cd6d84b8590e947336e8d09613ed 🌎 Tree-SHA512: de0fe31450d281cc7307c0d820495e86c93c7998e77a148db2c703da66cff1059e6560c041f1864913c42075aa24d259c2623d45e929ca0a8056ed330a9f9978
2022-10-18Remove g_parallel_script_checksMacroFake
2022-10-18Move ::fCheckBlockIndex into ChainstateManagerMacroFake
This changes the flag for the bitcoin-chainstate executable. Previously it was false, now it is the chain's default value (still false for the main chain).
2022-10-18Move ::fCheckpointsEnabled into ChainstateManagerMacroFake
2022-10-18Move ::nMinimumChainWork into ChainstateManagerMacroFake
This changes the minimum chain work for the bitcoin-chainstate executable. Previously it was uint256{}, now it is the chain's default minimum chain work.
2022-10-18Move ::hashAssumeValid into ChainstateManagerMacroFake
This changes the assumed valid block for the bitcoin-chainstate executable. Previously it was uint256{}, now it is defaultAssumeValid.
2022-10-18Move ::nMaxTipAge into ChainstateManagerMacroFake
2022-10-13Merge bitcoin/bitcoin#24851: init: ignore BIP-30 verification in ↵Andrew Chow
DisconnectBlock for problematic blocks e899d4ca6f44785b6e5481e200a6a9a8d2448612 init: limit bip30 exceptions to coinbase txs (Chris Geihsler) 511eb7fdeac36da9d6576c878a1c8d390b38f1bd Ignore problematic blocks in DisconnectBlock (Chris Geihsler) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/22596 When using checklevel=4, block verification fails because of duplicate coinbase transactions involving blocks 91812 and 91722. There was already a check in place within `ConnectBlock` to ignore the problematic blocks, but `DisconnectBlock` did not contain a similar check to ignore these blocks when called from `VerifyDB`. By ignoring these two blocks in `DisconnectBlock`, the block verification process succeeds at checklevel=4. (Note to reviewers: this is my first contribution to Bitcoin Core, so any feedback is most welcome. Thanks in advance for reviewing!) ## Steps to reproduce: Use the following bitcoin.conf file and start bitcoind. I only used block data through block ~100000 so that the verification process was much faster. ``` assumevalid=0 checkblocks=0 checklevel=4 ``` Without this change, you will see the following error when the blocks are verified: ``` 2022-04-14T02:56:44Z init message: Verifying blocks… 2022-04-14T02:56:44Z Verifying last 101881 blocks at level 4 2022-04-14T02:56:44Z [0%]...[10%]...[20%]...[30%]...[40%]...ERROR: VerifyDB(): *** coin database inconsistencies found (last 10160 blocks, 142571 good transactions before that) 2022-04-14T02:57:01Z : Corrupted block database detected. Please restart with -reindex or -reindex-chainstate to recover. : Corrupted block database detected. Please restart with -reindex or -reindex-chainstate to recover. ``` With this change, you will see this instead: ``` 2022-04-14T02:32:29Z init message: Verifying blocks… 2022-04-14T02:32:29Z Verifying last 101746 blocks at level 4 2022-04-14T02:32:29Z [0%]...[10%]...[20%]...[30%]...[40%]...[50%]...[60%]...[70%]...[80%]...[90%]...[DONE]. 2022-04-14T02:32:48Z No coin database inconsistencies in last 101746 blocks (226126 transactions) ``` ACKs for top commit: laanwj: Code review ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 achow101: ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 jamesob: (Biased) ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 ([`jamesob/ackr/24851.2.seejee.init_ignore_bip_30_verif`](https://github.com/jamesob/bitcoin/tree/ackr/24851.2.seejee.init_ignore_bip_30_verif)) Tree-SHA512: d2f6d25e9619aee32c1a73fe846b1b587698eaa5a4994fa6424f1038f45654f9fd52b74a69843cc84d90168d74827130ccf8e9201502f5d52281acdb20429291
2022-10-13Merge bitcoin/bitcoin#25667: assumeutxo: snapshot initializationAndrew Chow
bf9597606166323158bbf631137b82d41f39334f doc: add note about snapshot chainstate init (James O'Beirne) e4d799528696c5ede38c257afaffd367917e0de8 test: add testcases for snapshot initialization (James O'Beirne) cced4e7336d93a2dc88e4a61c49941887766bd72 test: move-only-ish: factor out LoadVerifyActivateChainstate() (James O'Beirne) 51fc9241c08a00f1f407f1534853a5cddbbc0a23 test: allow on-disk coins and block tree dbs in tests (James O'Beirne) 3c361391b8f5971eb3c7b620aa7ad9b437cc515e test: add reset_chainstate parameter for snapshot unittests (James O'Beirne) 00b357c215ed900145bd770525a341ba0ed9c027 validation: add ResetChainstates() (James O'Beirne) 3a29dfbfb2c16a50d854f6f81428a68aa9180509 move-only: test: make snapshot chainstate setup reusable (James O'Beirne) 8153bd9247dad3982d54488bcdb3960470315290 blockmanager: avoid undefined behavior during FlushBlockFile (James O'Beirne) ad67ff377c2b271cb4683da2fb25fd295557f731 validation: remove snapshot datadirs upon validation failure (James O'Beirne) 34d159033106cc595cfa852695610bfe419c989c add utilities for deleting on-disk leveldb data (James O'Beirne) 252abd1e8bc5cdf4368ad55e827a873240535b28 init: add utxo snapshot detection (James O'Beirne) f9f1735f139b6a1f1c7fea50717ff90dc4ba2bce validation: rename snapshot chainstate dir (James O'Beirne) d14bebf100aaaa25c7558eeed8b5c536da99885f db: add StoragePath to CDBWrapper/CCoinsViewDB (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606) --- Half of the replacement for #24232. The original PR grew larger than expected throughout the review process. This change adds the ability to initialize a snapshot-based chainstate during init if one is detected on disk. This is of course unused as of now (aside from in unittests) given that we haven't yet enabled actually loading snapshots. Don't be scared! There are some big move-only commits in here. Accompanying changes include: - moving the snapshot coinsdb directory from being called `chainstate_[base blockhash]` to `chainstate_snapshot`, since we only support one snapshot in use at a time. This simplifies some logic, but it necessitates writing that base blockhash out to a file within the coinsdb dir. See [discussion here](https://github.com/bitcoin/bitcoin/pull/24232#discussion_r832762880). - adding a simple fix in `FlushBlockFile()` that avoids a crash when attemping to flush to disk before `LoadBlockIndexDB()` is called, which happens when calling `MaybeRebalanceCaches()` during multiple chainstate init. - improving the unittest to allow testing with on-disk chainstates - necessary to test a simulated restart and re-initialization. ACKs for top commit: naumenkogs: utACK bf9597606166323158bbf631137b82d41f39334f ariard: Code Review ACK bf9597606 ryanofsky: Code review ACK bf9597606166323158bbf631137b82d41f39334f. Changes since last review: rebasing, switching from CAutoFile to AutoFile, adding comments, switching from BOOST_CHECK to Assert in test util, using chainman.GetMutex() in tests, destroying one ChainstateManager before creating a new one in tests fjahr: utACK bf9597606166323158bbf631137b82d41f39334f aureleoules: ACK bf9597606166323158bbf631137b82d41f39334f Tree-SHA512: 15ae75caf19f8d12a12d2647c52897904d27b265a7af6b4ae7b858592eeadb8f9da6c2394b6baebec90adc28742c053e3eb506119577dae7c1e722ebb3b7bcc0
2022-10-12Merge bitcoin/bitcoin#24858: incorrect blk file size calculation during ↵glozow
reindex results in recoverable blk file corruption bcb0cacac28e98a39dc856c574a0872fe17059e9 reindex, log, test: fixes #21379 (mruddy) Pull request description: Fixes #21379. The blocks/blk?????.dat files are mutated and become increasingly malformed, or corrupt, as a result of running the re-indexing process. The mutations occur after the re-indexing process has finished, as new blocks are appended, but are a result of a re-indexing process miscalculation that lingers in the block manager's `m_blockfile_info` `nSize` data until node restart. These additions to the blk files are non-fatal, but also not desirable. That is, this is a form of data corruption that the reading code is lenient enough to process (it skips the extra bytes), but it adds some scary looking log messages as it encounters them. The summary of the problem is that the re-index process double counts the size of the serialization header (magic message start bytes [4 bytes] + length [4 bytes] = 8 bytes) while calculating the blk data file size (both values already account for the serialization header's size, hence why it is over accounted). This bug manifests itself in a few different ways, after re-indexing, when a new block from a peer is processed: 1. If the new block will not fit into the last blk file processed while re-indexing, while remaining under the 128MiB limit, then the blk file is flushed to disk and truncated to a size that is 8 greater than it should be. The truncation adds zero bytes (see `FlatFileSeq::Flush` and `TruncateFile`). 1. If the last blk file processed while re-indexing has logical space for the new block under the 128 MiB limit: 1. If the blk file was not already large enough to hold the new block, then the zeros are, in effect, added by `fseek` when the file is opened for writing. Eight zero bytes are added to the end of the last blk file just before the new block is written. This happens because the write offset is 8 too great due to the miscalculation. The result is 8 zero bytes between the end of the last block and the beginning of the next block's magic + length + block. 1. If the blk file was already large enough to hold the new block, then the current existing file contents remain in the 8 byte gap between the end of the last block and the beginning of the next block's magic + length + block. Commonly, when this occcurs, it is due to the blk file containing blocks that are not connected to the block tree during reindex and are thus left behind by the reindex process and later overwritten when new blocks are added. The orphaned blocks can be valid blocks, but due to the nature of concurrent block download, the parent may not have been retrieved and written by the time the node was previously shutdown. ACKs for top commit: LarryRuane: tested code-review ACK bcb0cacac28e98a39dc856c574a0872fe17059e9 ryanofsky: Code review ACK bcb0cacac28e98a39dc856c574a0872fe17059e9. This is a disturbing bug with an easy fix which seems well-worth merging. mzumsande: ACK bcb0cacac28e98a39dc856c574a0872fe17059e9 (reviewed code and did some testing, I agree that it fixes the bug). w0xlt: tACK https://github.com/bitcoin/bitcoin/pull/24858/commits/bcb0cacac28e98a39dc856c574a0872fe17059e9 Tree-SHA512: acc97927ea712916506772550451136b0f1e5404e92df24cc05e405bb09eb6fe7c3011af3dd34a7723c3db17fda657ae85fa314387e43833791e9169c0febe51
2022-10-10refactor: Replace m_params with chainman.GetParams()Aurèle Oulès
Fixes a TODO introduced in #24595.
2022-10-10Merge bitcoin/bitcoin#26118: log: Use steady clock for bench loggingMacroFake
fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 Use steady clock for bench logging (MacroFake) faed342a2338d6a1a26cf977671a736662debae4 scripted-diff: Rename time symbols (MacroFake) Pull request description: Instead of using `0.001` and similar constants to "convert" an int64_t to milliseconds, use the type-safe `Ticks<>` helper. Also, use steady clock instead of system clock, since the durations are used for benchmarking. ACKs for top commit: fanquake: ACK fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 - validation bench output still looks sane. Tree-SHA512: e6525b5fdad6045ca500c56014897d7428ad288aaf375933d3b5939feddf257f6910d562eb66ebcde9186bef9a604ee8d763a318253838318d59df2a285be7c2
2022-10-05refactor: mempool: use CTxMempool::Limitsstickies-v
Simplifies function signatures by removing repetition of all the ancestor/descendant limits, and increases readability by being more verbose by naming the limits, while still reducing the LoC.
2022-09-19Use steady clock for bench loggingMacroFake
2022-09-19scripted-diff: Rename time symbolsMacroFake
-BEGIN VERIFY SCRIPT- ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ':(exclude)src/versionbits.cpp') ; } ren nStart time_start ren nTimeStart time_start ren nTimeReadFromDiskTotal time_read_from_disk_total ren nTimeConnectTotal time_connect_total ren nTimeFlush time_flush ren nTimeChainState time_chainstate ren nTimePostConnect time_post_connect ren nTimeCheck time_check ren nTimeForks time_forks ren nTimeConnect time_connect ren nTimeVerify time_verify ren nTimeUndo time_undo ren nTimeIndex time_index ren nTimeTotal time_total ren nTime1 time_1 ren nTime2 time_2 ren nTime3 time_3 ren nTime4 time_4 ren nTime5 time_5 ren nTime6 time_6 ren nBlocksTotal num_blocks_total # Newline after semicolon perl -0777 -pi -e 's/; time_connect_total/;\n time_connect_total/g' src/validation.cpp perl -0777 -pi -e 's/; time_/;\n time_/g' src/validation.cpp -END VERIFY SCRIPT-
2022-09-16Merge bitcoin/bitcoin#25499: Use steady clock for all millis bench loggingfanquake
fa521c960337a65d4ce12cd1ef009c652ffe57e6 Use steady clock for all millis bench logging (MacroFake) Pull request description: Currently `GetTimeMillis` is used for bench logging in milliseconds integral precision. Replace it to use a steady clock that is type-safe and steady. Microsecond or float precision can be done in a follow-up. ACKs for top commit: fanquake: ACK fa521c960337a65d4ce12cd1ef009c652ffe57e6 - started making the same change. Tree-SHA512: 86a810e496fc663f815acb8771a6c770331593715cde85370226685bc50c13e8e987e3c5efd0b4e48b36ebd2372255357b709204bac750d41e94a9f7d9897fa6
2022-09-13validation: add ResetChainstates()James O'Beirne
Necessary for the following test commit.
2022-09-13validation: remove snapshot datadirs upon validation failureJames O'Beirne
If a UTXO snapshot fails to validate, don't leave the resulting datadir on disk as this will confuse initialization on next startup and we'll get an assertion error.
2022-09-13add utilities for deleting on-disk leveldb dataJames O'Beirne
Used in later commits to remove leveldb directories for - invalid snapshot chainstates, and - background-vaildation chainstates that have finished serving their purpose.
2022-09-13init: add utxo snapshot detectionJames O'Beirne
Add functionality for activating a snapshot-based chainstate if one is detected on-disk. Also cautiously initialize chainstate cache usages so that we don't somehow blow past our cache allowances during initialization, then rebalance at the end of init. Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
2022-09-13validation: rename snapshot chainstate dirJames O'Beirne
This changes the snapshot's leveldb chainstate dir name from `chainstate_[blockhash]` to `chainstate_snapshot`. This simplifies later logic that loads snapshot data, and enforces the limitation of a single snapshot at any given time. Since we still need to persis the blockhash of the base block, we write that out to a file (`chainstate_snapshot/base_blockhash`) for later use during initialization, so that we can reinitialize the snapshot chainstate. Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
2022-09-09scripted-diff: rename CChainState -> ChainstateJames O'Beirne
-BEGIN VERIFY SCRIPT- sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*') -END VERIFY SCRIPT- Co-authored-by: MacroFake <falke.marco@gmail.com>
2022-08-30Fix typo from PR25717Suhas Daftuar
2022-08-30Merge bitcoin/bitcoin#25717: p2p: Implement anti-DoS headers syncfanquake
3add23454624c4c79c9eebc060b6fbed4e3131a7 ui: show header pre-synchronization progress (Pieter Wuille) 738421c50f2dbd7395b50a5dbdf6168b07435e62 Emit NotifyHeaderTip signals for pre-synchronization progress (Pieter Wuille) 376086fc5a187f5b2ab3a0d1202ed4e6c22bdb50 Make validation interface capable of signalling header presync (Pieter Wuille) 93eae27031a65b4156df49015ae45b2b541b4e5a Test large reorgs with headerssync logic (Suhas Daftuar) 355547334f7d08640ee1fa291227356d61145d1a Track headers presync progress and log it (Pieter Wuille) 03712dddfbb9fe0dc7a2ead53c65106189f5c803 Expose HeadersSyncState::m_current_height in getpeerinfo() (Suhas Daftuar) 150a5486db50ff77c91765392149000029c8a309 Test headers sync using minchainwork threshold (Suhas Daftuar) 0b6aa826b53470c9cc8ef4a153fa710dce80882f Add unit test for HeadersSyncState (Suhas Daftuar) 83c6a0c5249c4ecbd11f7828c84a50fb473faba3 Reduce spurious messages during headers sync (Suhas Daftuar) ed6cddd98e32263fc116a4380af6d66da20da990 Require callers of AcceptBlockHeader() to perform anti-dos checks (Suhas Daftuar) 551a8d957c4c44afbd0d608fcdf7c6a4352babce Utilize anti-DoS headers download strategy (Suhas Daftuar) ed470940cddbeb40425960d51cefeec4948febe4 Add functions to construct locators without CChain (Pieter Wuille) 84852bb6bb3579e475ce78fe729fd125ddbc715f Add bitdeque, an std::deque<bool> analogue that does bit packing. (Pieter Wuille) 1d4cfa4272cf2c8b980cc8762c1ff2220d3e8d51 Add function to validate difficulty changes (Suhas Daftuar) Pull request description: New nodes starting up for the first time lack protection against DoS from low-difficulty headers. While checkpoints serve as our protection against headers that fork from the main chain below the known checkpointed values, this protection only applies to nodes that have been able to download the honest chain to the checkpointed heights. We can protect all nodes from DoS from low-difficulty headers by adopting a different strategy: before we commit to storing a header in permanent storage, first verify that the header is part of a chain that has sufficiently high work (either `nMinimumChainWork`, or something comparable to our tip). This means that we will download headers from a given peer twice: once to verify the work on the chain, and a second time when permanently storing the headers. The p2p protocol doesn't provide an easy way for us to ensure that we receive the same headers during the second download of peer's headers chain. To ensure that a peer doesn't (say) give us the main chain in phase 1 to trick us into permanently storing an alternate, low-work chain in phase 2, we store commitments to the headers during our first download, which we validate in the second download. Some parameters must be chosen for commitment size/frequency in phase 1, and validation of commitments in phase 2. In this PR, those parameters are chosen to both (a) minimize the per-peer memory usage that an attacker could utilize, and (b) bound the expected amount of permanent memory that an attacker could get us to use to be well-below the memory growth that we'd get from the honest chain (where we expect 1 new block header every 10 minutes). After this PR, we should be able to remove checkpoints from our code, which is a nice philosophical change for us to make as well, as there has been confusion over the years about the role checkpoints play in Bitcoin's consensus algorithm. Thanks to Pieter Wuille for collaborating on this design. ACKs for top commit: Sjors: re-tACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 mzumsande: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 sipa: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 glozow: ACK 3add234546 Tree-SHA512: e7789d65f62f72141b8899eb4a2fb3d0621278394d2d7adaa004675250118f89a4e4cb42777fe56649d744ec445ad95141e10f6def65f0a58b7b35b2e654a875
2022-08-29Emit NotifyHeaderTip signals for pre-synchronization progressPieter Wuille
2022-08-29Make validation interface capable of signalling header presyncPieter Wuille
This makes a number of changes: - Get rid of the verification_progress argument in the node interface NotifyHeaderTip (it was always 0.0). - Instead of passing a CBlockIndex* in the UI interface's NotifyHeaderTip, send separate height, timestamp fields. This is becuase in headers presync, no actual CBlockIndex object is available. - Add a bool presync argument to both of the above, to identify signals pertaining to the first headers sync phase.
2022-08-29Track headers presync progress and log itPieter Wuille
2022-08-29Require callers of AcceptBlockHeader() to perform anti-dos checksSuhas Daftuar
In order to prevent memory DoS, we must ensure that we don't accept a new header into memory until we've performed anti-DoS checks, such as verifying that the header is part of a sufficiently high work chain. This commit adds a new argument to AcceptBlockHeader() so that we can ensure that all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation. This patch also fixes two places where low-difficulty-headers could have been processed without such validation (processing an unrequested block from the network, and processing a compact block). Credit to Niklas Gögge for noticing this issue, and thanks to Sjors Provoost for test code.
2022-08-29Utilize anti-DoS headers download strategySuhas Daftuar
Avoid permanently storing headers from a peer, unless the headers are part of a chain with sufficiently high work. This prevents memory attacks using low-work headers. Designed and co-authored with Pieter Wuille.
2022-08-22refactor: Move ChainstateManager options into m_options structRyan Ofsky
Move ChainstateManager options into m_options struct to simplify class initialization, organize class members, and to name external option variables differently than internal state variables. This change was originally in #25862, but it was suggested to split off in https://github.com/bitcoin/bitcoin/pull/25862#discussion_r951459817 so it could be merged earlier and reduce conflicts with other PRs.
2022-08-22Merge bitcoin/bitcoin#25775: docs: remove non-signaling mentions of BIP125fanquake
1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e [doc] remove non-signaling mentions of BIP125 (glozow) 32024d40f03fbf47c64d814fa5f2c2a73ec14cb7 scripted-diff: remove mention of BIP125 from non-signaling var names (glozow) Pull request description: We have pretty thorough documentation of our RBF policy in doc/policy/mempool-replacements.md. It enumerates each rule with several sentences of rationale. Also, each rule pretty much has its own function (3 and 4 share one), with extensive comments. The doc states explicitly that our rules are similar but differ from BIP125, and contains a record of historical changes to RBF policy. We should not use "BIP125" as synonymous with our RBF policy because: - Our RBF policy is different from what is specified in BIP125, for example: - the BIP does not mention our rule about the replacement feerate being higher (our Rule 6) - the BIP uses minimum relay feerate for Rule 4, while we have used incremental relay feerate since #9380 - the "inherited signaling" question (CVE-2021-31876). Call it discrepancy, ambiguous wording, doc misinterpretation, or implementation details, I would recommend users refer to doc/policy/mempool-replacements.md - the signaling policy is configurable, see #25353 - Our RBF policy may change further - We have already marked BIP125 as only "partially implemented" in docs/bips.md since 1fd49eb498c75a1d14193bb736d195a3dc75ae12 - See comments from people who are not me recently: - https://github.com/bitcoin/bitcoin/pull/25038#discussion_r909507429 - https://github.com/bitcoin/bitcoin/pull/25575#issuecomment-1179519204 This PR removes all non-signaling mentions of BIP125 (if people feel strongly, we can remove all mentions of BIP125 period). It may be useful to refer to the concept of "tx opts in to RBF if it has at least one nSequence less than (0xffffffff - 1)" as "BIP125 signaling" because: - It is succint. - It has already been widely marketed as BIP125 opt-in signaling. - Our API uses it when referring to signaling (e.g. getmempoolentry["bip125-replaceable"] and wallet error message "not BIP 125 replaceable"). Changing those is more invasive. - If/when we have other ways to signal in the future, we can disambiguate them this way. See #25038 which proposes another way of signaling, and where I pulled these commits from. Alternatives: - Changing our policy to match BIP125. This doesn't make sense as, for example, we would have to remove the requirement that a replacement tx has a higher feerate (Rule 6). - Changing BIP125 to match what we have. This doesn't make sense as it would be a significant change to a BIP years after it was finalized and already used as a spec to implement RBF in other places. - Document our policy as a new BIP and give it a number. This might make sense if we don't expect things to change a lot, and can be done as a next step. ACKs for top commit: darosior: ACK 1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e ariard: ACK 1dc03dda t-bast: ACK https://github.com/bitcoin/bitcoin/commit/1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e Tree-SHA512: a3adc2039ec5785892d230ec442e50f47f7062717392728152bbbe27ce1c564141f85253143f53cb44e1331cf47476d74f5d2f4b3cd873fc3433d7a0aa783e02
2022-08-22Merge bitcoin/bitcoin#25786: refactor: Make adjusted time type safefanquake
eeee5ada23f2a71d245671556b6ecfdaabfeddf4 Make adjusted time type safe (MacroFake) fa3be799fe951a7ea9b4de78d5a907c6db71eeb8 Add time helpers (MacroFake) Pull request description: This makes follow-ups easier to review. Also, it makes sense by itself. ACKs for top commit: ryanofsky: Code review ACK eeee5ada23f2a71d245671556b6ecfdaabfeddf4. Confirmed type changes and equivalent code changes only. Tree-SHA512: 51bf1ae5428552177286113babdd49e82459d6c710a07b6e80a0a045d373cf51045ee010461aba98e0151d8d71b9b3b5f8f73e302d46ba4558e0b55201f99e9f
2022-08-12Merge bitcoin/bitcoin#25677: refactor: make active_chain_tip a referenceMacroFake
9376a6dae41022874df3b9302667796a9fb7b81d refactor: make active_chain_tip a reference (Aurèle Oulès) Pull request description: This PR fixes a TODO introduced in #21055. Makes `active_chain_tip` argument in `CheckFinalTxAtTip` function a reference instead of a pointer. ACKs for top commit: dongcarl: ACK 9376a6dae41022874df3b9302667796a9fb7b81d Tree-SHA512: c36d1769e0b9598b7f79334704b26b73e958d54caa3bd7e4eff954f3964fcf3f5e3a44a5a760497afad51b76e1614c86314fe035e4083c855e3574a620de7f4d
generated by cgit v1.2.3 (git 2.26.2) at 2024-08-26 13:19:03 +0000