aboutsummaryrefslogtreecommitdiff
path: root/src/validation.cpp
AgeCommit message (Collapse)Author
2019-07-26[mempool] log correct messages when CPFP failsJohn Newbery
2019-07-25Merge #15305: [validation] Crash if disconnecting a block failsfanquake
a47df13471e3168e2e02023fb20cdf2414141b36 [qa] Test disconnect block failure -> shutdown (Suhas Daftuar) 4433ed0f730cfd60eeba3694ff3c283ce2c0c8ee [validation] Crash if disconnecting a block fails (Suhas Daftuar) Pull request description: If we're unable to disconnect a block during normal operation, then that is a failure of our local system (such as disk failure) or the chain that we are on (eg CVE-2018-17144), but cannot be due to failure of the (more work) chain that we're trying to validate. We should abort rather than stay on a less work chain. Fixes #14341. ACKs for top commit: practicalswift: utACK a47df13471e3168e2e02023fb20cdf2414141b36 TheBlueMatt: utACK a47df13471e3168e2e02023fb20cdf2414141b36. Didn't bother to review the test in detail, it looked fine. Debated whether invalidateblock should ever crash the node, but *not* crashing in the case of hitting a pruned block (which is the only change here) is clearly better, even if there are other cases I'd argue we should crash in. ryanofsky: utACK a47df13471e3168e2e02023fb20cdf2414141b36. Only change since last review is new comment. promag: ACK a47df1347, it takes awhile to quit (RPC connection timeouts) but that's unrelated - hope to fix that soon. fanquake: ACK a47df13471e3168e2e02023fb20cdf2414141b36 Tree-SHA512: 4dec8cef6e7dbbe513c138fc5821a7ceab855e603ece3c16185b51a3830ab7ebbc844a28827bf64e75326f45325991dcb672f13bd7baede53304f27289c4af8d
2019-07-24scripted-diff: Make translation bilingualHennadii Stepanov
-BEGIN VERIFY SCRIPT- sed -i 's/inline std::string _(const char\* psz)/inline bilingual_str _(const char\* psz)/' src/util/translation.h sed -i 's/return G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz;/return bilingual_str{psz, G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz};/' src/util/translation.h sed -i 's/\b_("\([^"]\|\\"\)*")/&.translated/g' $(git grep --files-with-matches '\b_("' src) echo Hard cases - multiline strings. sed -i 's/"Visit %s for further information about the software.")/&.translated/g' src/init.cpp sed -i "s/\"Only rebuild the block database if you are sure that your computer's date and time are correct\")/&.translated/g" src/init.cpp sed -i 's/" restore from a backup.")/&.translated/g' src/wallet/db.cpp sed -i 's/" or address book entries might be missing or incorrect.")/&.translated/g' src/wallet/wallet.cpp echo Special case. sed -i 's/_(COPYRIGHT_HOLDERS)/&.translated/' src/util/system.cpp test/lint/lint-format-strings.py -END VERIFY SCRIPT-
2019-07-24Refactor out translation.hHennadii Stepanov
This is a prerequisite for introducing bilingual error messages. Note: #includes are arranged by clang-format-diff.py script.
2019-07-19Merge #15681: [mempool] Allow one extra single-ancestor transaction per packageWladimir J. van der Laan
50cede3f5a4d4fbfbb7c420b94e661a6a159bced [mempool] Allow one extra single-ancestor transaction per package (Matt Corallo) Pull request description: This implements the proposed policy change from [1], which allows certain classes of contract protocols involving revocation punishments to use CPFP. Note that some such use-cases may still want some form of one-deep package relay, though even this alone may greatly simplify some lightning fee negotiation. [1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html ACKs for top commit: ajtowns: ACK 50cede3f5a4d4fbfbb7c420b94e661a6a159bced -- looked over code again, compared with previous commit, compiles, etc. sdaftuar: ACK 50cede3f5a4d4fbfbb7c420b94e661a6a159bced ryanofsky: utACK 50cede3f5a4d4fbfbb7c420b94e661a6a159bced. Changes since last review: adding EXTRA_DESCENDANT_TX_SIZE_LIMIT constant, changing max ancestor size from 1,000,000 to nLimitAncestorSize constant (101,000), fixing test comment and getting rid of unused test node. Tree-SHA512: b052c2a0f384855572b4579310131897b612201214b5abbb225167224e4f550049e300b471dbf320928652571e92ca2d650050b7cf39ac92b3bc1d2bcd386c1c
2019-07-09[mempool] Allow one extra single-ancestor transaction per packageMatt Corallo
This implements the proposed policy change from [1], which allows certain classes of contract protocols involving revocation punishments to use CPFP. Note that some such use-cases may still want some form of one-deep package relay, though even this alone may greatly simplify some lightning fee negotiation. [1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
2019-07-08refactoring: remove mapBlockIndex globalJames O'Beirne
in lieu of ::BlockIndex().
2019-07-08refactoring: make pindexBestInvalid internal to validation.cppJames O'Beirne
There's no need to have this member live on CChainState since it's only used in validation.cpp.
2019-07-08refactoring: add block_index_candidates arg to LoadBlockIndexJames O'Beirne
Prevents BlockManager from having to reference ChainstateActive() within one of its methods which improves encapsulation and makes testing easier.
2019-07-08refactoring: move block metadata structures into BlockManagerJames O'Beirne
Separate out the management of chain-agnostic block metadata from any given CChainState instance. This allows us to avoid duplicating data like `mapBlockIndex` unnecessarily for multiple chainstates. This also adds a CChainState constructor that accepts and sets m_blockman. Ultimately this reference will point to a BlockMan instance that is shared across CChainStates. This commit can be decomposed into smaller commits if necessary.
2019-07-05[build]: use #if HAVE_SYSTEM instead of defined(HAVE_SYSTEM)Sjors Provoost
2019-07-05Merge #15457: Check std::system for -[alert|block|wallet]notifyWladimir J. van der Laan
f874e14cd3c84cd412bd3fb42b3ee1706ca6a267 [build]: check std::system for -[alert|block|wallet]notify (Sjors Provoost) cc3ad56ff2bc2583fe68c4a9e0b41072a47c0b07 [build] MSVC: set HAVE_SYSTEM for desktop apps (Sjors Provoost) c1c91bb78d7267f01ee3a3c156c218b46a92cd39 [build] detect std::system or ::wsystem (Sjors Provoost) Pull request description: Platforms such as iOs and Universal Windows Platform do not support launching a process through system(). ACKs for top commit: laanwj: code review ACK f874e14cd3c84cd412bd3fb42b3ee1706ca6a267 Tree-SHA512: 16bb4a8fa1896046ccb22a46c8985e1aa45f5b11ecf5539eb2299e9a58f1a5b085c0c12cb6939c7493d93abce7e84fadcbfc73374c887db63da6d00c08aa476d
2019-07-02Merge #14193: validation: Add missing mempool locksWladimir J. van der Laan
fa2b083c3feb0522baf652045efa6b73458761a3 [test] Add test to check mempool consistency in case of reorgs (MarcoFalke) fabeb1f613653a8c1560e4a093a9b6b7a069b60b validation: Add missing mempool locks (MarcoFalke) fa0c9dbf9156d64a4b9bff858da97825369a9134 txpool: Make nTransactionsUpdated atomic (MarcoFalke) Pull request description: Take the mempool read lock during reorgs, so that we don't accidentally read an inconsistent mempool. ACKs for top commit: laanwj: code review ACK fa2b083c3feb0522baf652045efa6b73458761a3 ryanofsky: utACK fa2b083c3feb0522baf652045efa6b73458761a3 [EDIT: was ~e284e422e75189794e24fe482819d8b1407857c3~, from bad copy and paste]. Changes since last review: rebase after #15976, adding vTxHashes lock annotation, adding new commit dropping mempool lock for nTransactionsUpdated and making it atomic to avoid deadlock between mempool lock and g_best_block_mutex Tree-SHA512: cfe7777993589087753e000e3736d79d320dca412383fb77b56bef8946a04049722bf888c11b6f722adf677165185c7e58b4a269f7c5fa25e84dda375f6c8a7d
2019-06-19Make AbortNode() aware of MSG_NOPREFIX flagHennadii Stepanov
2019-06-18Merge #16092: Don't use global (external) symbols for symbols that are used ↵MarcoFalke
in only one translation unit 0959d37e3e Don't use global (external) symbols for symbols that are used in only one translation unit (practicalswift) Pull request description: Don't use global (external) symbols for symbols that are used in only one translation unit. Before: ``` $ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h") N_REFERENCES=$(wc -l <<< "${REFERENCES}") if [[ ${N_REFERENCES} > 1 ]]; then continue fi echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}" done Global symbol g_chainstate is used in only one translation unit: src/validation.cpp Global symbol g_ui_signals is used in only one translation unit: src/ui_interface.cpp Global symbol instance_of_cmaincleanup is used in only one translation unit: src/validation.cpp Global symbol instance_of_cnetcleanup is used in only one translation unit: src/net.cpp Global symbol instance_of_cnetprocessingcleanup is used in only one translation unit: src/net_processing.cpp Global symbol pindexBestForkBase is used in only one translation unit: src/validation.cpp Global symbol pindexBestForkTip is used in only one translation unit: src/validation.cpp $ ``` After: ``` $ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h") N_REFERENCES=$(wc -l <<< "${REFERENCES}") if [[ ${N_REFERENCES} > 1 ]]; then continue fi echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}" done $ ``` ♻️ Think about future generations: save the global namespace from unnecessary pollution! ♻️ ACKs for commit 0959d3: Empact: ACK https://github.com/bitcoin/bitcoin/pull/16092/commits/0959d37e3e0f80010a78d175e3846dabf5d35919 MarcoFalke: ACK 0959d37e3e0f80010a78d175e3846dabf5d35919 hebasto: ACK 0959d37e3e0f80010a78d175e3846dabf5d35919 promag: ACK 0959d37. Tree-SHA512: 722f66bb50450f19b57e8a8fbe949f30cd651eb8564e5787cbb772a539bf3a288c048dc49e655fd73ece6a46f6dafade515ec4004729bf2b3ab83117b7c5d153
2019-06-08Remove -mempoolreplacement to prevent needless block prop slowness.Matt Corallo
At this point there is no reasonable excuse to disable opt-in RBF, and, unlike when this option was added, there are now significant issues created when disabling it (in the form of compact block reconstruction failures). Further, it breaks a lot of modern wallet behavior.
2019-06-07validation: Add missing mempool locksMarcoFalke
2019-06-06Merge #16129: refactor: Remove unused includesMarcoFalke
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift) eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift) Pull request description: Make reasoning about dependencies easier by not including unused dependencies. Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real. As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed: ``` $ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \ sed 's%^%src/%g' | xargs cat | wc -l 51393 ``` Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-) ACKs for commit 67f4e9: Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
2019-06-06[build]: check std::system for -[alert|block|wallet]notifySjors Provoost
Platforms such as iOs do not support launching a process through system().
2019-06-06Don't use global (external) symbols for symbols that are used in only one ↵practicalswift
translation unit
2019-06-05Merge #15976: refactor: move methods under CChainState (pt. 1)Wladimir J. van der Laan
403e677c9 refactoring: IsInitialBlockDownload -> CChainState (James O'Beirne) 3ccbc376d refactoring: FlushStateToDisk -> CChainState (James O'Beirne) 4d6688603 refactoring: introduce ChainstateActive() (James O'Beirne) d7c97edee move-only: make the CChainState interface public (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/2019-04-proposal/proposal --- This changeset starts moving functionality intimately related to CChainState into methods. Parameterizing these functions by a particular CChainState is necessary for the use of multiple chainstates simultaneously (e.g. for asynchronous background validation). In this change, we - make the CChainState interface public - since other units will start to invoke its methods directly, - introduce `::ChainstateActive()`, the CChainState equivalent for `::ChainActive()`, - and move `IsInitialBlockDownload()` and `FlushStateToDisk()` into methods on CChainState. Independent of assumeutxo, these changes better encapsulate chainstate behavior and allow easier use from a testing context. There are more methods that we'll move in the future, but they require other substantial changes (i.e. moving ownership of the `CCoinsView*` hierarchy into CChainState) so we'll save them for future PRs. --- The first move-only commit is most easily reviewed with `git diff ... --color-moved=dimmed_zebra`. ACKs for commit 403e67: Empact: utACK https://github.com/bitcoin/bitcoin/pull/15976/commits/403e677c9ebbf9744733010e6b0c2d1b182ee850 no need to address my nits herein Sjors: utACK 403e677 ryanofsky: utACK 403e677c9ebbf9744733010e6b0c2d1b182ee850. Only change since previous review is removing global state comment as suggested. MarcoFalke: utACK 403e677c9e, though the diff still seems a bit bloated with some unnecessary changes in the second commit. promag: utACK 403e677 and rebased with current [master](c7cfd20a7). Tree-SHA512: 6fcf260bb2dc201361170c0b4547405366f5f331fcc3a2bac29b24442814b7b244ca1b58aac5af716885f9a130c343b544590dff780da0bf835c7c5b3ccb2257
2019-06-05[validation] Crash if disconnecting a block failsSuhas Daftuar
If we're unable to disconnect a block during normal operation, then that is a failure of our local system (such as disk failure) or the chain that we are on (eg CVE-2018-17144), but cannot be due to failure of the (more work) chain that we're trying to validate. We should abort rather than stay on a less work chain.
2019-06-02Make reasoning about dependencies easier by not including unused dependenciespracticalswift
2019-05-22Merge #16015: validation: Hold cs_main when reading chainActive in ↵MarcoFalke
RewindBlockIndex 1609809fb2 validation: Hold cs_main when reading chainActive in RewindBlockIndex (practicalswift) Pull request description: Fixes #15980. Hold `cs_main` when reading `chainActive` (via `::ChainActive()`) in `RewindBlockIndex`. ACKs for commit 160980: MarcoFalke: utACK 1609809fb2a4c2ec15b7c26dc328e2e666bd5d57 Tree-SHA512: 54f180ab391f92f04950735c2bb337f0b7495826d2096f7a0f9a2da50bc29d08747f404a0495e33ca4edd4c842efbab4c4730d5e1a8b9da3e1249cf884268f4b
2019-05-19Merge #12980: Allow quicker shutdowns during LoadBlockIndex()Jonas Schnelli
af5fa82b6 Allow quicker shutdowns during LoadBlockIndex() (Jonas Schnelli) Pull request description: ACKs for commit af5fa8: promag: utACK af5fa82b676a36e60eda080ca0a946bdfffefd49. practicalswift: utACK af5fa82b676a36e60eda080ca0a946bdfffefd49 Tree-SHA512: 1c64dcc5d8a9d3411553257cd5a598dcd29be981660e5bca9283c1d957dc56798abcf41d9969cd573088137597a23e48e62a8c476c463d3f176b86a10048f47b
2019-05-17Merge #15999: init: Remove dead code in LoadChainTipMarcoFalke
fa86c8aec6 init: Remove dead code in LoadChainTip (MarcoFalke) Pull request description: `LoadChainTip` sets `::ChainActive()` based on `pcoinsTip`'s best block. `LoadChainTip` is never called when that block is null, so we can remove all code from within that method that is only executed when that block is null. Fixes #15967 Inconsistent locking behavior in LoadChainTip ACKs for commit fa86c8: promag: utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d. practicalswift: utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d Empact: utACK https://github.com/bitcoin/bitcoin/pull/15999/commits/fa86c8aec611a9b9d2f53960c92419cf2a8bb92d laanwj: utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d ryanofsky: utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d. LoadChainTip isn't called currently when pcoinsTip best block is null due to this line: jamesob: utACK https://github.com/bitcoin/bitcoin/pull/15999/commits/fa86c8aec611a9b9d2f53960c92419cf2a8bb92d Tree-SHA512: 8961c0e579800a52038ac5655478468852faac055299b64d6cfdf0c213d3bf09669c4889467d09d93457f6c8b073967bb0475a137f77ddd3a3a3c03ad90001c4
2019-05-16refactoring: IsInitialBlockDownload -> CChainStateJames O'Beirne
We introduce CChainState.m_cached_finished_ibd because the static state it replaces would've been shared across all CChainState instances.
2019-05-16refactoring: FlushStateToDisk -> CChainStateJames O'Beirne
Also renames global methods for clarity: - ::FlushStateToDisk() -> CChainState::ForceFlushStateToDisk() - This performs an unconditional flush. - ::PruneAndFlush() -> CChainState::PruneAndFlush()
2019-05-16refactoring: introduce ChainstateActive()James O'Beirne
To be used once we move global functions (e.g. FlushStateToDisk()) into CChainState methods. Thanks to Marco Falke for suggestions
2019-05-16move-only: make the CChainState interface publicJames O'Beirne
along with DisconnectResult, and CBlockIndexWorkComparator. The CChainState interface needs to be known to the rest of the system because many global functions will move to CChainState methods. This is to allow other parts of the system to be parameterized per chainstate instance instead of assuming a single global.
2019-05-15validation: Hold cs_main when reading chainActive in RewindBlockIndexpracticalswift
2019-05-13[refactor] interfaces: Add missing LockAnnotation for cs_mainMarcoFalke
2019-05-13init: Remove dead code in LoadChainTipMarcoFalke
2019-05-08Merge #15971: validation: Add compile-time checking for negative locking ↵MarcoFalke
requirement in LimitValidationInterfaceQueue 62d50ef308 Add LOCKS_EXCLUDED(cs_main) to LimitValidationInterfaceQueue(...) which does AssertLockNotHeld(cs_main) (practicalswift) Pull request description: This PR adds compile-time checking for negative locking requirements that follow from the run-time locking requirement `AssertLockNotHeld(cs_main)` in `LimitValidationInterfaceQueue(...)`. Changes: * Add `LOCKS_EXCLUDED(cs_main)` to `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)` * Add `LOCKS_EXCLUDED(cs_main)` to `CChainState::ActivateBestChain(…)`, `CChainState:: InvalidateBlock(…)` and `CChainState::RewindBlockIndex(…)` which all call `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)` * Add `LOCKS_EXCLUDED(cs_main)` to `InvalidateBlock(…)` which calls `CChainState::InvalidateBlock(...)` which in turn calls `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)` * Add `LOCKS_EXCLUDED(cs_main)` to `RewindBlockIndex(…)` which calls `CChainState::RewindBlockIndex(...)` which in turn calls `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)` ACKs for commit 62d50e: MarcoFalke: utACK 62d50ef308 Tree-SHA512: 73d092ccd08c851ae3c5d60370c369fc030c5793f5507e2faccb6f91c851ddc0ce059fbea3899f2856330d7a8c78f2ac6a2988e8268b03154f946be9e60e3be1
2019-05-08Add LOCKS_EXCLUDED(cs_main) to LimitValidationInterfaceQueue(...) which does ↵practicalswift
AssertLockNotHeld(cs_main) Add LOCKS_EXCLUDED(cs_main) to functions calling LimitValidationInterfaceQueue(...) which does AssertLockNotHeld(cs_main) Add LOCKS_EXCLUDED(cs_main) to functions calling CChainState::InvalidateBlock(...) which calls LimitValidationInterfaceQueue(...) which in turn does AssertLockNotHeld(cs_main) Add LOCKS_EXCLUDED(cs_main) to functions calling CChainState::RewindBlockIndex(...) which calls LimitValidationInterfaceQueue(...) which in turn does AssertLockNotHeld(cs_main)
2019-05-07Merge #15948: refactor: rename chainActiveMarcoFalke
486c1eea86 refactoring: remove unused chainActive (James O'Beirne) 631940aab2 scripted-diff: replace chainActive -> ::ChainActive() (James O'Beirne) a3a609079c refactoring: introduce unused ChainActive() (James O'Beirne) 1b6e6fcfd2 rename: CChainState.chainActive -> m_chain (James O'Beirne) Pull request description: This is part of the assumeutxo project: Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal --- This change refactors the `chainActive` reference into a `::ChainActive()` call. It also distinguishes `CChainState`'s `CChain` data member as `m_chain` instead of the current `chainActive`, which makes it easily confused with the global data. The active chain must be obtained via function because its reference will be swapped at some point during runtime after loading a UTXO snapshot. This change, though lengthy, should be pretty easy to review since most of it is contained within a scripted-diff. Once merged, the parent PR should be easier to review. ACKs for commit 486c1e: Sjors: utACK 486c1ee promag: utACK 486c1ee. practicalswift: utACK 486c1eea863a41e597ae4fddc392f446f2518b4b Tree-SHA512: 06ed8f9e77f2d25fc9bea0ba86436d80dbbce90a1e8be23e37ec4eeb26060483e60b4a5c4fba679cb1867f61e3921c24abeb9cabdfb4d0a9b1c4ddd77b17456a
2019-05-04Merge #15141: Rewrite DoS interface between validation and net_processingWladimir J. van der Laan
0ff1c2a838da9e8dc7f77609adc89124bbea3e2b Separate reason for premature spends (coinbase/locktime) (Suhas Daftuar) 54470e767bab37f9b7089782b1be73d5883bb244 Assert validation reasons are contextually correct (Suhas Daftuar) 2120c31521aa51aa1984ee33250b8320506d3a0f [refactor] Update some comments in validation.cpp as we arent doing DoS there (Matt Corallo) 12dbdd7a41bac73e51ed8f7b290b7671196bf9ea [refactor] Drop unused state.DoS(), state.GetDoS(), state.CorruptionPossible() (Matt Corallo) aa502b88d10c2c3ac56d9163555849b96dc4df1e scripted-diff: Remove DoS calls to CValidationState (Matt Corallo) 7721ad64f40a0c67edefaaf7353264d78df8803e [refactor] Prep for scripted-diff by removing some \ns which annoy sed. (Matt Corallo) 5e78c5734bb0c9aae7b0a7019a745b2d7059b3d9 Allow use of state.Invalid() for all reasons (Matt Corallo) 6b34bc6b6f54f85537494cbea3846d5d195a06d9 Fix handling of invalid headers (Suhas Daftuar) ef54b486d5333dfc85c56e6b933c81735196a25d [refactor] Use Reasons directly instead of DoS codes (Matt Corallo) 9ab2a0412e96e87956fe61257387683635213035 CorruptionPossible -> BLOCK_MUTATED (Matt Corallo) 6e55b292b0ea944897b6dc2f766446fd209af484 CorruptionPossible -> TX_WITNESS_MUTATED (Matt Corallo) 7df16e70e67c753c871797ce947ea09d7cb0e519 LookupBlockIndex -> CACHED_INVALID (Matt Corallo) c8b0d22698385f91215ce8145631e3d5826dc977 [refactor] Drop redundant nDoS, corruptionPossible, SetCorruptionPossible (Matt Corallo) 34477ccd39a8d4bfa8ad612f22d5a46291922185 [refactor] Add useful-for-dos "reason" field to CValidationState (Matt Corallo) 6a7f8777a0b193fae4f976196f3464ffac01bf1b Ban all peers for all block script failures (Suhas Daftuar) 7b999103e21509e1c2dec10f68e48744ffe90f55 Clean up banning levels (Matt Corallo) b8b4c80146780f9011abbd1be72343cc965c07b9 [refactor] drop IsInvalid(nDoSOut) (Matt Corallo) 8818729013e17c650a25f030b2b80e0997389155 [refactor] Refactor misbehavior ban decisions to MaybePunishNode() (Matt Corallo) 00e11e61c0211a62788611cd6a6714a393fdc26c [refactor] rename stateDummy -> orphan_state (Matt Corallo) f34fa719cf33a51d11f1d2219cbe73ccff6fd697 Drop obsolete sigops comment (Matt Corallo) Pull request description: This is a rebase of #11639 with some fixes for the last few comments which were not yet addressed. The original PR text, with some strikethroughs of text that is no longer correct: > This cleans up an old main-carryover - it made sense that main could decide what DoS scores to assign things because the DoS scores were handled in a different part of main, but now validation is telling net_processing what DoS scores to assign to different things, which is utter nonsense. Instead, we replace CValidationState's nDoS and CorruptionPossible with a general ValidationInvalidReason, which net_processing can handle as it sees fit. I keep the behavior changes here to a minimum, but in the future we can utilize these changes for other smarter behavior, such as disconnecting/preferring to rotate outbound peers based on them providing things which are invalid due to SOFT_FORK because we shouldn't ban for such cases. > > This is somewhat complementary with, though obviously conflicts heavily with #11523, which added enums in place of DoS scores, as well as a few other cleanups (which are still relevant). > > Compared with previous bans, the following changes are made: > > Txn with empty vin/vout or null prevouts move from 10 DoS > points to 100. > Loose transactions with a dependency loop now result in a ban > instead of 10 DoS points. > ~~BIP68-violation no longer results in a ban as it is SOFT_FORK.~~ > ~~Non-SegWit SigOp violation no longer results in a ban as it > considers P2SH sigops and is thus SOFT_FORK.~~ > ~~Any script violation in a block no longer results in a ban as > it may be the result of a SOFT_FORK. This should likely be > fixed in the future by differentiating between them.~~ > Proof of work failure moves from 50 DoS points to a ban. > Blocks with timestamps under MTP now result in a ban, blocks > too far in the future continue to not result in a ban. > Inclusion of non-final transactions in a block now results in a > ban instead of 10 DoS points. Note: The change to ban all peers for consensus violations is actually NOT the change I'd like to make -- I'd prefer to only ban outbound peers in those situations. The current behavior is a bit of a mess, however, and so in the interests of advancing this PR I tried to keep the changes to a minimum. I plan to revisit the behavior in a followup PR. EDIT: One reviewer suggested I add some additional context for this PR: > The goal of this work was to make net_processing aware of the actual reasons for validation failures, rather than just deal with opaque numbers instructing it to do something. > > In the future, I'd like to make it so that we use more context to decide how to punish a peer. One example is to differentiate inbound and outbound peer misbehaviors. Another potential example is if we'd treat RECENT_CONSENSUS_CHANGE failures differently (ie after the next consensus change is implemented), and perhaps again we'd want to treat some peers differently than others. ACKs for commit 0ff1c2: jnewbery: utACK 0ff1c2a838da9e8dc7f77609adc89124bbea3e2b ryanofsky: utACK 0ff1c2a838da9e8dc7f77609adc89124bbea3e2b. Only change is dropping the first commit (f3883a321bf4ab289edcd9754b12cae3a648b175), and dropping the temporary `assert(level == GetDoS())` that was in 35ee77f2832eaffce30042e00785c310c5540cdc (now c8b0d22698385f91215ce8145631e3d5826dc977) Tree-SHA512: e915a411100876398af5463d0a885920e44d473467bb6af991ef2e8f2681db6c1209bb60f848bd154be72d460f039b5653df20a6840352c5f7ea5486d9f777a3
2019-05-03refactoring: remove unused chainActiveJames O'Beirne
2019-05-03scripted-diff: replace chainActive -> ::ChainActive()James O'Beirne
Though at the moment ChainActive() simply references `g_chainstate.m_chain`, doing this change now clears the way for multiple chainstate usage and allows us to script the diff. -BEGIN VERIFY SCRIPT- git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/\(::\)\{0,1\}chainActive/::ChainActive()/g' -END VERIFY SCRIPT-
2019-05-03refactoring: introduce unused ChainActive()James O'Beirne
in preparation for the following scripted-diff commit.
2019-05-03rename: CChainState.chainActive -> m_chainJames O'Beirne
This can't be a scripted-diff due to the confusion of the global chainActive and the CChainState member of the same name. This specific rename makes the following chainActive -> ::ChainActive() diff scriptable.
2019-05-02Separate reason for premature spends (coinbase/locktime)Suhas Daftuar
2019-05-02Assert validation reasons are contextually correctSuhas Daftuar
2019-05-02[refactor] Update some comments in validation.cpp as we arent doing DoS thereMatt Corallo
2019-05-02scripted-diff: Remove DoS calls to CValidationStateMatt Corallo
-BEGIN VERIFY SCRIPT- sed -i 's/\.DoS(\(.*\), REJECT_\(.*\), \(true\|false\)/.DoS(\1, REJECT_\2/' src/validation.cpp src/consensus/tx_verify.cpp src/consensus/tx_check.cpp sed -i 's/state.GetRejectCode(), state.GetRejectReason(), [^,]\+, state.GetDebugMessage())/state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage())/' src/validation.cpp sed -i 's/\.DoS([^,]*, /.Invalid\(/' src/validation.cpp src/consensus/tx_verify.cpp src/consensus/tx_check.cpp -END VERIFY SCRIPT- Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
2019-05-02[refactor] Prep for scripted-diff by removing some \ns which annoy sed.Matt Corallo
2019-05-02CorruptionPossible -> BLOCK_MUTATEDMatt Corallo
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2019-05-02[refactor] Add useful-for-dos "reason" field to CValidationStateMatt Corallo
This is a first step towards cleaning up our DoS interface - make validation return *why* something is invalid, and let net_processing figure out what that implies in terms of banning/disconnection/etc. Behavior change: peers will now be banned for providing blocks with premature coinbase spends. Co-authored-by: Anthony Towns <aj@erisian.com.au> Suhas Daftuar <sdaftuar@gmail.com>
2019-05-02Ban all peers for all block script failuresSuhas Daftuar
This eliminates a discrepancy between block validation with multiple script check threads, versus a single script check thread.
2019-05-02Clean up banning levelsMatt Corallo
Compared with previous bans, the following changes are made: * Txn with empty vin/vout or null prevouts move from 10 DoS points to 100. * Loose transactions with a dependency loop now result in a ban instead of 10 DoS points. * Many pre-segwit soft-fork errors now result in a ban. Note: Transactions that violate soft-fork script flags since P2SH do not generally result in a ban. Also, banning behavior for invalid blocks is dependent on whether the node is validating with multiple script check threads, due to a long- standing bug. That inconsistency is still present after this commit. * Proof of work failure moves from 50 DoS points to a ban. * Blocks with timestamps under MTP now result in a ban, blocks too far in the future continue to *not* result in a ban. * Inclusion of non-final transactions in a block now results in a ban instead of 10 DoS points. Co-authored-by: Anthony Towns <aj@erisian.com.au>