aboutsummaryrefslogtreecommitdiff
path: root/src/validation.cpp
AgeCommit message (Collapse)Author
2020-07-01Add CChainState::ResizeCoinsCachesJames O'Beirne
Also adds CCoinsViewCache::ReallocateCache() to attempt to free memory that the cacheCoins's allocator may be hanging onto when downsizing the cache. Adds `CChainState::m_coins{tip,db}_cache_size_bytes` data members so that we can reference cache size on a per-chainstate basis for flushing.
2020-07-01Merge #19331: build: Do not include server symbols in walletWladimir J. van der Laan
faca73000fa8975c28f6be8be01957c1ae94ea62 ci: Install fixed version of clang-format for linters (MarcoFalke) fa4695da4c69646b58a8fa0b6b30146bb234deb8 build: Sort Makefile.am after renaming file (MarcoFalke) cccc2784a3bb10fa8e43be7e68207cafb12bd915 scripted-diff: Move ui_interface to the node lib (MarcoFalke) fa72ca6a9d90d66012765b0043fd819698b94ba8 qt: Remove unused includes (MarcoFalke) fac96e6450d595fe67168cb7afa7692da6cc9973 wallet: Do not include server symbols (MarcoFalke) fa0f6c58c1c6d10f04c4e65a424cc51ebca50a8c Revert "Fix link error with --enable-debug" (MarcoFalke) Pull request description: This reverts a hacky workaround from commit b83cc0f, which only happens to work due to compiler optimizations. Then, it actually fixes the linker error. The underlying problem is that the wallet includes symbols from the server (ui_interface), which usually results in linker failures. Though, in this specific case the linker failures have not been observed (unless `-O0`) because our compilers were smart enough to strip unused symbols. Fix the underlying problem by creating a new header-only with the needed symbol and move ui_interface to node to clarify that this is part of libbitcoin_server. ACKs for top commit: Sjors: ACK faca730 laanwj: ACK faca73000fa8975c28f6be8be01957c1ae94ea62 hebasto: re-ACK faca73000fa8975c28f6be8be01957c1ae94ea62, since the [previous](https://github.com/bitcoin/bitcoin/pull/19331#pullrequestreview-434420539) review: Tree-SHA512: e9731f249425aaea50b6db5fc7622e10078cf006721bb87989cac190a2ff224412f6f8a7dd83efd018835302337611f5839e29e15bef366047ed591cef58dfb4
2020-06-29refactor: Remove confusing BlockIndex globalMarcoFalke
2020-06-27scripted-diff: Move ui_interface to the node libMarcoFalke
-BEGIN VERIFY SCRIPT- # Move files git mv src/ui_interface.h src/node/ui_interface.h git mv src/ui_interface.cpp src/node/ui_interface.cpp sed -i -e 's/BITCOIN_UI_INTERFACE_H/BITCOIN_NODE_UI_INTERFACE_H/g' src/node/ui_interface.h # Adjust includes and makefile sed -i -e 's|ui_interface|node/ui_interface|g' $(git grep -l ui_interface) # Sort includes git diff -U0 | clang-format-diff -p1 -i -v -END VERIFY SCRIPT-
2020-06-21Change format of log2_work for uniform output (zero-padded)jmorgan
2020-06-19Add ChainstateManager::ActiveChainstateMarcoFalke
2020-06-16refactor: Use AbortError in FatalErrorMarcoFalke
This is needed for consistency with AbortNode
2020-06-16Merge #18927: Pass bilingual_str argument to AbortNode()MarcoFalke
5527be06277647dffe7cda587c4bbfbec2a5c8ca refactor: Add AbortError alias (Hennadii Stepanov) d924f2a596c8f37deb2dd94069c578244823c31f Drop MSG_NOPREFIX flag (Hennadii Stepanov) 083daf7fbaf02de61f8d197ef6a8df98c1a57f7b Pass bilingual_str argument to AbortNode() (Hennadii Stepanov) d1cca129b4b5b8e4830e442ebaee55dd0660b48a refactor: Use bilingual_str::empty() (Hennadii Stepanov) Pull request description: This PR is a [followup](https://github.com/bitcoin/bitcoin/issues/16218#issuecomment-625919724) of #16224, and it adds `bilingual_str` type argument support to the `AbortNode()` functions. ACKs for top commit: MarcoFalke: ACK 5527be06277647dffe7cda587c4bbfbec2a5c8ca 👟 Tree-SHA512: bf8b15b14912b1f672e6e588fffa1e6eb6f00b4b23d15d0ced7f18fbdf76919244427feb7217007fe29617049308e13def893a03a87358db819cca9692f59905
2020-06-14move-only: Move NDEBUG compile time check to util/checkMarcoFalke
2020-06-10Make SetMiscWarning() accept bilingual_str argumentHennadii Stepanov
2020-06-08refactor: Add AbortError aliasHennadii Stepanov
2020-06-08Merge #19069: refactor: replace pointers by references within tx_verify.{h,cpp}MarcoFalke
b00266fe0cf05fe6044f471105ce2bfed4349626 refactor: replace pointers by references within tx_verify.{h,cpp} (Sebastian Falbesoner) Pull request description: This PR gets rid of another unnecessary use of raw pointers, similar to PR #19053 (see also issue #19062 where useful commands for finding potential candidates are listed) but in the tx verification module. For the functions `CalculateSequenceLocks()` and `SequenceLocks()`, the `prevHeights` vector parameter type is changed to be passed as a reference. Note that there were no checks for null pointers -- if one would pass `nullptr` to one of the functions, the following line would immediately lead to a crash: https://github.com/bitcoin/bitcoin/blob/dcacea096e029a02a937bf96d002ca7e94c48c15/src/consensus/tx_verify.cpp#L32 ACKs for top commit: Empact: Code Review ACK https://github.com/bitcoin/bitcoin/pull/19069/commits/b00266fe0cf05fe6044f471105ce2bfed4349626 Tree-SHA512: 0eb71591467905434082029128bdca4df94988c372af40dca325654f6c002c72a00c73776cb5e72d6de2b2f218649211a5dbf19300a2e01f1841d6034e0f01e0
2020-06-07Merge #19005: doc: Add documentation for 'checklevel' argument in ↵MarcoFalke
'verifychain' RPC… 501e6ab4e778d8f4e95fdc807eeb8644df16203b doc: Add documentation for 'checklevel' argument in 'verifychain' RPC call (Calvin Kim) Pull request description: Rationale: When ```bitcoin-cli help verifychain``` is called, the user doesn't get any documentation about the ```checklevel``` argument, leading to issues like #18995. This PR addresses that issue and adds documentation for what each level does, and that each level includes the checks of the previous levels. ACKs for top commit: jonatack: ACK 501e6ab4e778d8f4e95fdc807eeb8644df16203b `git diff 292ed3c 501e6ab` shows only change since last review is the verifychain RPCHelpMan edit; rebuild and retested manually anyway MarcoFalke: ACK 501e6ab4e778d8f4e95fdc807eeb8644df16203b 🚝 Tree-SHA512: 09239f79c25b5c3022b8eb1f76198ba681305d7e8775038e46becffe5f6a14c572e0c5d06b0723fe9d4a015ec42c9f7ca7b80a2a93df0b1b66f5a84a80eeeeb1
2020-06-07doc: Add documentation for 'checklevel' argument in 'verifychain' RPC callCalvin Kim
2020-06-04Drop MSG_NOPREFIX flagHennadii Stepanov
Since bilingual_str type is fully supported, the MSG_NOPREFIX flag is no longer needed.
2020-06-04Pass bilingual_str argument to AbortNode()Hennadii Stepanov
2020-06-04Merge #17994: validation: flush undo files after last block writeWladimir J. van der Laan
ac94141af0c16161afa68de1c3720f254ae4e12c validation: delay flushing undo files in syncing node case (Karl-Johan Alm) Pull request description: Fixes #17890. Replaces #17892. Data files (`{blk|rev}<number>.dat`) pre-allocate space as they are written, and then trims down to the final size once they move on to the next sequence ("finalized flush"). The code currently assumes (incorrectly) that blk and rev files finish at the same time, but because blk files are written as blocks come in, and rev files are written in block height order, rev files end up being written to for awhile after moving on to the next block file, resulting in pre-allocation and waste of up to 1 MB of space per rev file. The exact point at which rev file writing finishes is the highest height block found inside the corresponding block file, which is already available in the CBlockFileInfo vector. This PR moves finalized flushing of undo files to to directly after the undo data for the previous block file has been written. There is a branch with annotation that demonstrates how this is handling flushing here: https://github.com/kallewoof/bitcoin/tree/200124-rev-files-annotated ACKs for top commit: vasild: ACK ac94141af (no changes in the code since ed34e00da). fjahr: Code review re-ACK ac94141af0c16161afa68de1c3720f254ae4e12c jonatack: Code review ACK ac94141af0c16 Tree-SHA512: 1d4e3b3d1d99bd7ebe7a2f632b1231146dd4f9f993c54db3a4090d9c086d95d2e4c327fd936066392b3afc6277b8f3a908d5c5993d4c8e49f72b92a417716dd2
2020-06-04Merge #19142: validation: Make VerifyDB level 4 interruptiblefanquake
fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 validation: Make VerifyDB level 4 interruptible (MarcoFalke) fa1d5800d9c5e33942b76f6667839a818723dee9 validation: Remove unused boost interruption_point (MarcoFalke) Pull request description: level 0,1,2, and 3 are already interruptible, so make level 4 also interruptible ACKs for top commit: laanwj: Code review ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 fanquake: ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 Tree-SHA512: d302c84a17add1b5993dd78339c88670d27eee45ce208c4d046ae188b50be9843ee5a9584739d5d25453b54ae08fd1cb6eeee8cb1307d84c05cde8a54a7c445b
2020-06-03Merge #19088: validation: use std::chrono throughout some validation functionsMarcoFalke
789e9dd3aa727176797529c35b2848f994630a82 validation: use std::chrono in IsCurrentForFeeEstimation() (fanquake) 47be28c8bc475eafeebd4fc58ea92f0d3df0d8c6 validation: use std::chrono in CChainState::FlushStateToDisk() (fanquake) Pull request description: Probably up for debate as to which type is used for the constants. Personally, swapping these to hours is more readable. ACKs for top commit: MarcoFalke: ACK 789e9dd3aa727176797529c35b2848f994630a82 jonatack: ACK 789e9dd3aa727176797529c35b2848f994630a82 Tree-SHA512: f4a25cbd00a49a54b7783a1f588be83706dd2a475cecb5c2e8b97b2d4b27c0955a7454d7486f2454e96351c44f233b300c4f4b9ca62fc7336277f10da34dd5c3
2020-06-03validation: Make VerifyDB level 4 interruptibleMarcoFalke
2020-06-03validation: Remove unused boost interruption_pointMarcoFalke
ActivateBestChain (ABC) is only called in the "msghand" or one of the RPC threads, neither of which is a boost::thread. However, ABC is also called in ThreadImport (which currently happens to be a boost::thread). In all cases, the interruption_point is redundant with the breakpoint in ABC that triggers when ShutdownRequested() VerifyDB is only called in the main thread ("init") or one of the RPC threads, neither of which is a boost::thread.
2020-06-02Merge #13204: Faster sigcache nonceMarcoFalke
152e8baf08c7379e5cc09f90863e6309bdd4866c Use salted hasher instead of nonce in sigcache (Jeremy Rubin) 5495fa585007b40b2e9285c23be275de71708af8 Add Hash Padding Microbenchmarks (Jeremy Rubin) Pull request description: This PR replaces nonces in two places with pre-salted hashers. The nonce is chosen to be 64 bytes long so that it forces the SHA256 hasher to process the chunk. This leaves the next 64 (or 56 depending if final chunk) open for data. In the case of the script execution cache, this does not make a big performance improvement because the nonce was already properly padded to fit into one buffer, but does make the code a little simpler. In the case of the sig cache, this should reduce the hashing overhead slightly because we are less likely to need an additional processing step. I haven't benchmarked this, but back of the envelope it should reduce the hashing by one buffer for all combinations except compressed public keys with compact signatures. ACKs for top commit: ryanofsky: Code review ACK 152e8baf08c7379e5cc09f90863e6309bdd4866c. No code changes, just rebase since last review and expanded commit message Tree-SHA512: b133e902fd595cfe3b54ad8814b823f4d132cb2c358c89158842ae27daee56ab5f70cde2585078deb46f77a6e7b35b4cc6bba47b65302b7befc2cff254bad93d
2020-05-28validation: use std::chrono in IsCurrentForFeeEstimation()fanquake
2020-05-28validation: use std::chrono in CChainState::FlushStateToDisk()fanquake
2020-05-26refactor: replace pointers by references within tx_verify.{h,cpp}Sebastian Falbesoner
affects "prevHeights" parameter of the functions - CalculateSequenceLocks() - SequenceLocks()
2020-05-25[mempool] Don't throw expected error message when upgradingAmiti Uttarwar
2020-05-21validation: Make ProcessNewBlock*() members of ChainstateManagerMarcoFalke
2020-05-21validation: Make PruneOneBlockFile() a member of ChainstateManagerMarcoFalke
2020-05-21validation: Make LoadBlockIndex() a member of ChainstateManagerMarcoFalke
2020-05-19refactor: Pass SynchronizationState enum to GUIHennadii Stepanov
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
2020-05-16log: Properly log txs rejected from mempoolMarcoFalke
2020-05-15Remove unnecessary blockfile SetPosTom Harding
Nothing could have changed the position since we retrieved it a few statements earlier. This dates from commit 16d5194165c8c83492b95f431a664d98c40ff254.
2020-05-08Merge #16224: gui: Bilingual GUI error messagesMarcoFalke
18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad util: Cleanup translation.h (Hennadii Stepanov) e95e658b8ec6e02229691a1941d688e96d4df6af doc: Do not translate technical or extremely rare errors (Hennadii Stepanov) 7e923d47ba9891856b86bc9f718cf2f1f773bdf6 Make InitError bilingual (Hennadii Stepanov) 917ca93553917251e0fd59717a347c63cdfd8a14 Make ThreadSafe{MessageBox|Question} bilingual (Hennadii Stepanov) 23b9fa2e5ec0425980301d2eebad81e660a5ea39 gui: Add detailed text to BitcoinGUI::message (Hennadii Stepanov) Pull request description: This is an alternative to #15340 (it works with the `Chain` interface; see: https://github.com/bitcoin/bitcoin/pull/15340#issuecomment-502674004). Refs: - #16218 (partial fix) - https://github.com/bitcoin/bitcoin/pull/15894#issuecomment-487947077 This PR: - makes GUI error messages bilingual: user's native language + untranslated (i.e. English) - insures that only untranslated messages are written to the debug log file and to `stderr` (that is not the case on master). If a translated string is unavailable only an English string appears to a user. Here are some **examples** (updated): ![Screenshot from 2020-04-24 17-08-37](https://user-images.githubusercontent.com/32963518/80222043-e2458780-864e-11ea-83fc-197b7121dba5.png) ![Screenshot from 2020-04-24 17-12-17](https://user-images.githubusercontent.com/32963518/80222051-e5407800-864e-11ea-92f7-dfef1144becd.png) * `qt5ct: using qt5ct plugin` message is my local environment specific; please ignore it. --- Note for reviewers: `InitWarning()` is out of this PR scope. ACKs for top commit: Sjors: re-tACK 18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad MarcoFalke: ACK 18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad 🐦 Tree-SHA512: 3cc8ec44f84403e54b57d11714c86b0855ed90eb794b5472e432005073354b9e3f7b4e8e7bf347a4c21be47299dbc7170f2d0c4b80e308205ff09596e55a4f96
2020-05-05Make ThreadSafe{MessageBox|Question} bilingualHennadii Stepanov
2020-05-05Merge #18088: build: ensure we aren't using GNU extensionsfanquake
0ae8f18dfe143051fec6ae10ea7df10142e3ff2f build: add -Wgnu to compile flags (fanquake) 3a0fd7726b8b916de6cce33bb67f48990575f923 Remove use of non-standard zero variadic macros (Ben Woosley) 49f6178c3e5e3ad54a419da9d8523207da17fc64 Drop unused LOG_TIME_MICROS helper (Ben Woosley) 5d4999951ee32e333b511245862628e80f83b703 prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes) Pull request description: Since we [started using](https://github.com/bitcoin/bitcoin/pull/7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile". However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former. #### anonymous structs ```bash ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct] struct { ``` This is fixed in https://github.com/bitcoin/bitcoin/commit/b849212c1ec01cc8633b8cdcd390da9b1051be0d. #### variadic macros ```bash ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments] ::Unserialize(s, VARINT(nVersionDummy)); ``` This is taken care of in #18087. The `LOG_TIME_*` macros introduced in #16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html). ```bash In file included from validation.cpp:22: ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ 6 warnings generated. ``` This is fixed in 081a0ab64eb442bc85c4d4a4d3bc2c8e97ac2a6d and 612e8e138b97fc5ad2f38847300132a8fc423c3f. #### prevention To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions. This would close #14130. Also related to #17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute. ACKs for top commit: practicalswift: ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f -- diff looks correct MarcoFalke: ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f vasild: utACK 0ae8f18df dongcarl: ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f
2020-05-04Merge #18786: init: Remove boost from ThreadImportWladimir J. van der Laan
faec3dc2adc487af97c22408f9f0bfe33f44a230 init: Remove boost from ThreadImport (MarcoFalke) Pull request description: Can be tested by calling `-reindex` or `-loadblock` and then pressing `CTRL`+`C`. Should print something like: ``` ... 2020-04-27T19:34:31Z [loadblk] Reindexing block file blk00005.dat... ^C2020-04-27T19:34:32Z [loadblk] Shutdown requested. Exit ThreadImport 2020-04-27T19:34:32Z [qt-init] Interrupting HTTP server ... ``` ACKs for top commit: laanwj: Code review ACK faec3dc2adc487af97c22408f9f0bfe33f44a230 hebasto: ACK faec3dc2adc487af97c22408f9f0bfe33f44a230, tested on Linux Mint 19.3 (x86_64) both `bitcoind` and `bitcoin-qt` binaries. Tree-SHA512: e105af18d98296d82ec99f48e478cf44577e3c32f7e4b47617a7bc7cbf71d6becb92722f229a1be38d58ad29712704509ad9740d8ab8cd3104cf90057664b437
2020-04-30Merge #18780: validation: add const for minimum witness commitment sizefanquake
692f8307fc1449299b90182e7d79efb81a55d7ab test: add test for witness commitment index (fanquake) 06442549f8b725f46c1c727e9eb6fde6b843503c validation: Add minimum witness commitment size constant (fanquake) Pull request description: https://github.com/bitcoin/bitcoin/commit/16101de5f33be494019a3f81755e204d00c22347: Per [BIP 141](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#Commitment_structure), the witness commitment structure is at least 38 bytes, OP_RETURN (0x6a) + 36 (0x24) + 4 byte header (0xaa21a9ed) + 32 byte SHA256 hash. It can be longer, however any additional data has no consensus meaning. https://github.com/bitcoin/bitcoin/commit/54f8c48d6ac973024df35c4db038791b7958a51d: As per BIP 141, if there is more than 1 pubkey that matches the witness commitment structure, the one with the highest output index should be chosen. This adds a sanity check that we are doing that, which will fail if anyone tries to "optimize" GetWitnessCommitmentIndex() by returning early. ACKs for top commit: MarcoFalke: ACK 692f8307fc1449299b90182e7d79efb81a55d7ab 🌵 jonatack: Code review ACK 692f830 ajtowns: ACK 692f8307fc1449299b90182e7d79efb81a55d7ab jnewbery: utACK 692f8307fc1449299b90182e7d79efb81a55d7ab laanwj: ACK 692f8307fc1449299b90182e7d79efb81a55d7ab Tree-SHA512: 7af3fe4b8a52fea2cdd0aec95f7bb935351a77b73d934bc88d6625a3503311b2a062cba5190b2228f97caa76840db3889032d910fc8e318ca8e7810a8afbafa0
2020-04-30Remove use of non-standard zero variadic macrosBen Woosley
These are a gnu extension warned against by: gnu-zero-variadic-macro-arguments
2020-04-29Use salted hasher instead of nonce in sigcacheJeremy Rubin
Use salted hasher instead of nonce in Script Execution Cache Don't read more than 32 bytes from GetRand Apply g_* naming convention to scriptExecutionCache in validation.cpp Fully apply g_* naming convention to scriptCacheHasher Write same uint256 nonce twice for cache hash rather than calling getrand twice Use salted hasher instead of nonce in sigcache Use salted hasher instead of nonce in Script Execution Cache Don't read more than 32 bytes from GetRand Apply g_* naming convention to scriptExecutionCache in validation.cpp Fully apply g_* naming convention to scriptCacheHasher Write same uint256 nonce twice for cache hash rather than calling getrand twice
2020-04-29Merge #18038: P2P: Mempool tracks locally submitted transactions to improve ↵fanquake
wallet privacy 50fc4df6c4e8a84bdda13ade7bed7a2131796f00 [mempool] Persist unbroadcast set to mempool.dat (Amiti Uttarwar) 297a1785360c4db662a7f3d3ade7b6b503258d39 [test] Integration tests for unbroadcast functionality (Amiti Uttarwar) 6851502472d3625416f0e7796e9f2a0379d14d49 [refactor/test] Extract P2PTxInvStore into test framework (Amiti Uttarwar) dc1da48dc5e5526215561311c184a8cbc345ecdc [wallet] Update the rebroadcast frequency to be ~1/day. (Amiti Uttarwar) e25e42f20a3aa39651fbc1f9fa3df1a49f1f5868 [p2p] Reattempt initial send of unbroadcast transactions (Amiti Uttarwar) 7e93eecce3bc5a1b7bb0284e06f9e2e69454f5ba [util] Add method that returns random time in milliseconds (Amiti Uttarwar) 89eeb4a3335f8e871cc3f5286af4546dff66172a [mempool] Track "unbroadcast" transactions (Amiti Uttarwar) Pull request description: This PR introduces mempool tracking of unbroadcast transactions and periodic reattempts at initial broadcast. This is a part of the rebroadcast project, and a standalone privacy win. The current rebroadcast logic is terrible for privacy because 1. only the source wallet rebroadcasts transactions and 2. it does so quite frequently. In the current system, if a user submits a transaction that does not immediately get broadcast to the network (eg. they are offline), this "rebroadcast" behavior is the safety net that can actually serve as the initial broadcast. So, keeping the attempts frequent is important for initial delivery within a reasonable timespan. This PR aims to improve # 2 by reducing the wallet rebroadcast frequency to ~1/day from ~1/15 min. It achieves this by separating the notion of initial broadcast from rebroadcasts. With these changes, the mempool tracks locally submitted transactions & periodically reattempts initial broadcast. Transactions submitted via the wallet or RPC are added to an "unbroadcast" set & are removed when a peer sends a `getdata` request, or the transaction is removed from the mempool. Every 10-15 minutes, the node reattempts an initial broadcast. This enables reducing the wallet rebroadcast frequency while ensuring the transactions will be propagated to the network. For privacy improvements around # 1, please see #16698. Thank you to gmaxwell for the idea of how to break out this subset of functionality (https://github.com/bitcoin/bitcoin/pull/16698#issuecomment-571399346) ACKs for top commit: fjahr: Code review ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00 MarcoFalke: ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00, I think this is ready for merge now 👻 amitiuttarwar: The current tip `50fc4df` currently has 6 ACKs on it, so I've opened #18807 to address the last bits. jnewbery: utACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00. ariard: Code Review ACK 50fc4df (minor points no need to invalid other ACKs) robot-visions: ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00 sipa: utACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00 naumenkogs: utACK 50fc4df Tree-SHA512: 2dd935d645d5e209f8abf87bfaa3ef0e4492705ce7e89ea64279cb27ffd37f4727fa94ad62d41be331177332f8edbebf3c7f4972f8cda10dd951b80a28ab3c0f
2020-04-29validation: Add minimum witness commitment size constantfanquake
Per BIP 141, the witness commitment structure is atleast 38 bytes, OP_RETURN (0x6a) + 36 (0x24) + 4 byte header (0xaa21a9ed) + 32 byte SHA256 hash. It can be longer, however any additional data has no consensus meaning.
2020-04-27init: Remove boost from ThreadImportMarcoFalke
2020-04-23[mempool] Persist unbroadcast set to mempool.datAmiti Uttarwar
Ensure that the unbroadcast set will still be meaningful if the node is restarted.
2020-04-23[validation] Move validation-only consts to validation.cppJohn Newbery
2020-04-22Merge #18410: Docs: Improve commenting for coins.cpp|hWladimir J. van der Laan
21fa0a44abe8c1b5c452e097eab20cf0ae988805 [docs] use consistent naming for possible_overwrite (John Newbery) 2685c214cce4b07695273503e60350e3f05fe3e2 [tests] small whitespace fixup (John Newbery) e9936966c08bd8a6ac02828131f619ddaa1ced13 scripted-diff: Rename PRUNED to SPENT in coins tests (John Newbery) c205979031ff4e8e32a5f05bae813405f233fccd [docs] Improve commenting in coins.cpp|h (John Newbery) Pull request description: - Add full commenting for spentness / DIRTYness / FRESHness and which combinations are valid - Remove the 'pruned' terminology, which doesn't make sense since per-txout chainstate db was merged (#10195). - Rename `potential_overwrite` to `possible_overwrite` to standardize terminology (there were previously examples of both, which made searching the codebase difficult). - Make other minor improvements to the comments ACKs for top commit: jonatack: Re-ACK 21fa0a4 per `git diff 98bee55 21fa0a4` the only change since my previous review is the following code commenting diff in `src/coins.cpp::L177-179`; rebuilt/ran unit tests anyway as a sanity check on the unit test changes. Tree-SHA512: 391e01588ef5edb417250080cec17361f982c4454bc5f8c6d78bbd528c68a2bb94373297760691295c24660ce1022ad3ef7599762f736c8eed772ce096d38c3d
2020-04-21[docs] use consistent naming for possible_overwriteJohn Newbery
And other general comment improvements for adding coins.
2020-04-16Merge #18401: Refactor: Initialize PrecomputedTransactionData in ↵MarcoFalke
CheckInputScripts f63dec189c3c8eee1ab2187681d5d0b2513b1b2e [REFACTOR] Initialize PrecomputedTransactionData in CheckInputScripts (Pieter Wuille) Pull request description: This is a single commit taken from the Schnorr/Taproot PR #17977. Add a default constructor to `PrecomputedTransactionData`, which doesn't initialize the struct's members. Instead they're initialized inside the `CheckInputScripts()` function. This allows a later commit to add the spent UTXOs to that structure. The spent UTXOs are required for the schnorr signature hash, since it commits to the scriptPubKeys. See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#common-signature-message for details. By itself, this isn't really an improvement to the code, but I think it makes sense to separate out the refactor/moveonly commits from PR #17977 so that PR is only the logical changes needed for Schnorr/Taproot. ACKs for top commit: jonatack: Re-ACK f63dec1 `git diff 851908d f63dec1` shows no change since last ACK. sipa: utACK f63dec189c3c8eee1ab2187681d5d0b2513b1b2e theStack: re-ACK f63dec189c3c8eee1ab2187681d5d0b2513b1b2e fjahr: Re-ACK f63dec189c3c8eee1ab2187681d5d0b2513b1b2e ariard: Code Review ACK f63dec1 Tree-SHA512: ecf9154077824ae4c274b4341e985797f3648c0cb0c31cb25ce382163b923a3acbc7048683720be4ae3663501801129cd0f48c441a36f049cc304ebe9f30994e
2020-04-11[REFACTOR] Initialize PrecomputedTransactionData in CheckInputScriptsPieter Wuille
Add a default constructor to `PrecomputedTransactionData`, which doesn't initialize the struct's members. Instead they're initialized inside the `CheckInputScripts()` function. This allows a later commit to add the spent UTXOs to that structure.
2020-04-10Merge #17737: Add ChainstateManager, remove BlockManager globalMarcoFalke
c9017ce3bc27665594c9d80f395780d40755bb22 protect g_chainman with cs_main (James O'Beirne) 2b081c4568e8019886fdb0f2a57babc73d7487f7 test: add basic tests for ChainstateManager (James O'Beirne) 4ae29f5f0c5117032debb722d7049664fdceeae8 use ChainstateManager to initialize chainstate (James O'Beirne) 5b690f0aae21e7d46cbefe3f5be645842ac4ae3b refactor: move RewindBlockIndex to CChainState (James O'Beirne) 89cdf4d5692d396b8c7177b3918aa9dab07f9624 validation: introduce unused ChainstateManager (James O'Beirne) 8e2ecfe2496d8a015f3ee8723025a438feffbd28 validation: add CChainState.m_from_snapshot_blockhash (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- This changeset introduces `ChainstateManager`, which is responsible for creating and managing access to multiple chainstates. Until we allow chainstate creation from UTXO snapshots (next assumeutxo PR?) it's basically unnecessary, but it is a prerequisite for background IBD support. Changes are also made to the initialization process to make use of `g_chainman` and thus clear the way for multiple chainstates being loaded on startup. One immediate benefit of this change is that we no longer have the `g_blockman` global, but instead have the ChainstateManager inject a reference of its shared BlockManager into any chainstate it creates. Another immediate benefit is that uses of `ChainActive()` and `ChainstateActive()` are now covered by lock annotations. Because use of `g_chainman` is annotated to require cs_main, these two functions subsequently follow. Because of whitespace changes, this diff looks bigger than it is. E.g., 4813167d98 is most easily reviewed with ```sh git show --color-moved=dimmed_zebra -w 4813167d98 ``` ACKs for top commit: MarcoFalke: re-ACK c9017ce3bc27665594c9d80f395780d40755bb22 📙 fjahr: Code Review Re-ACK c9017ce3bc27665594c9d80f395780d40755bb22 ariard: Code Review ACK c9017ce ryanofsky: Code review ACK c9017ce3bc27665594c9d80f395780d40755bb22. No changes since last review other than a straight rebase Tree-SHA512: 3f250d0dc95d4bfd70852ef1e39e081a4a9b71a4453f276e6d474c2ae06ad6ae6a32b4173084fe499e1e9af72dd9007f4a8a375c63ce9ac472ffeaada41ab508
2020-03-24validation: delay flushing undo files in syncing node caseKarl-Johan Alm
Data files are pre-allocated, and upon flush/finalization, they are trimmed down to their resulting size. Block (blk) files are written to disk as blocks come in, which is often out of order, whereas undo (rev) files are written sequentially, as each block is added to the top of the chain. When a block file hits the size limit, the system flushes and trims the file down to its final size, and moves on to the next block file. Case 1: blocks are added to the chain as they come in (synced up node case) -- in this case, we will flush and finalize the undo file together with the block file. Case 2: blocks are added to the chain after they have been downloaded (syncing node case) -- in this case, we postpone finalizing the undo file until we know the undo data for the last block in the file has been written to disk.