aboutsummaryrefslogtreecommitdiff
path: root/src/validation.cpp
AgeCommit message (Collapse)Author
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-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-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-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>
2019-05-02Drop obsolete sigops commentMatt Corallo
This comment was confusing and incorrect when first added ("invalid rather than merely non-standard" has the opposite meaning of what is actually the case), and was also not updated after segwit with the correct variable names. Delete it since the code reads just fine on its own. Co-authored by: Anthony Towns <aj@erisian.com.au> Suhas Daftuar <sdaftuar@gmail.com>
2019-05-01Merge #15323: rpc: Expose g_is_mempool_loaded via getmempoolinfoMarcoFalke
effe81f750 Move g_is_mempool_loaded into CTxMemPool::m_is_loaded (Ben Woosley) bb8ae2c419 rpc: Expose g_is_mempool_loaded via getmempoolinfo and /rest/mempool/info.json (Ben Woosley) Pull request description: And use it to fix a race condition in mempool_persist.py: https://travis-ci.org/Empact/bitcoin/jobs/487577243 Since e.g. getrawmempool returns errors based on this status, this enables users to test it for readiness. Fixes #12863 ACKs for commit effe81: MarcoFalke: utACK effe81f750 jnewbery: utACK effe81f7503d2ca3c88cfdea687f9f997f353e0d Tree-SHA512: 74328b0c17a97efb8a000d4ee49b9a673c2b6dde7ea30c43a6a2eff961a233351c9471f9a42344412135786c02bdf2ee1b2526651bb8fed68bd94d2120c4ef86
2019-04-30Merge #15849: Thread names in logs and deadlock debug toolsMarcoFalke
8722e54e56 threads: add thread names to deadlock debugging message (James O'Beirne) 383b186c28 threads: prefix log messages with thread names (James O'Beirne) ddd95ccb80 tests: add threadutil tests (James O'Beirne) ae5f2b6a6c threads: introduce util/threadnames, refactor thread naming (James O'Beirne) 188ca75e5f disable HAVE_THREAD_LOCAL on unreliable platforms (James O'Beirne) Pull request description: I'm resurrecting this one (from #13168) because I need it to make progress on #15735. It's now off by default and can be turned on with `-logthreadnames=1`. Ran some benchmarks (IBD from local peer from 500_000 -> 504_000) and it's within spitting distance either on or off: ### threadnames off (default) #### 2018-05-threadnames.3 vs. master (absolute) | name | iterations | 2018-05-threadnames.3 | master | |------------------------------------------------|-----------:|----------------------------|----------------------------| | ibd.local.500000.504000.dbcache=2048 | 3 | 376.1584 (± 9.2944) | 392.3414 (± 13.4238) | | ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 2236117.3333 (± 1845.9623) | 2238690.6667 (± 2669.3487) | #### 2018-05-threadnames.3 vs. master (relative) | name | iterations | 2018-05-threadnames.3 | master | |------------------------------------------------|-----------:|----------------------:|-------:| | ibd.local.500000.504000.dbcache=2048 | 3 | 1 | 1.043 | | ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 1 | 1.001 | ### threadnames on #### 2018-05-threadnames-take-2 vs. master (absolute) | name | iterations | 2018-05-threadnames-take-2 | master | |------------------------------------------------|-----------:|----------------------------|----------------------------| | ibd.local.500000.504000.dbcache=2048 | 3 | 367.6861 (± 0.3941) | 364.1667 (± 0.9776) | | ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 2238461.3333 (± 3697.8730) | 2237014.6667 (± 3307.6966) | #### 2018-05-threadnames-take-2 vs. master (relative) | name | iterations | 2018-05-threadnames-take-2 | master | |------------------------------------------------|-----------:|---------------------------:|-------:| | ibd.local.500000.504000.dbcache=2048 | 3 | 1.010 | 1.00 | | ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 1.001 | 1.00 | ``` ACKs for commit 8722e5: Empact: utACK https://github.com/bitcoin/bitcoin/pull/15849/commits/8722e54e56fd959fd4ff2321b36a7640dee440c5 jnewbery: utACK 8722e54e56fd959fd4ff2321b36a7640dee440c5 MarcoFalke: re-utACK 8722e54e56fd959fd4ff2321b36a7640dee440c5 (Only change since my previous review is DEFAULT_LOGTHREADNAMES=false and stylistic updates Tree-SHA512: 50af992708295b8d680cf10025262dd964e599a356bdfc1dfc84fb18c00afabcb34d3d12d551b0677ff81f8fccad0e17c1d5b24dfecb953a913bc77fdd1a4577
2019-04-29threads: introduce util/threadnames, refactor thread namingJames O'Beirne
This work is prerequisite to attaching thread names to log lines and deadlock debug utilities. This code allows setting of an "internal" threadname per thread on platforms where thread_local is available. This commit also moves RenameThread() out of a more general module and adds a numeric suffix to disambiguate between threads with the same name. It explicitly names a few main threads using the new util::ThreadRename().
2019-04-27Merge #15778: [wallet] Move maxtxfee from node to walletMarcoFalke
5c759c73b2 [wallet] Move maxTxFee to wallet (John Newbery) Pull request description: Closes #15355 Moves the `-maxtxfee` from the node to the wallet. See discussion in issue for details. This is a cleanup. There is no change in behaviour. Completes #15620 ACKs for commit 5c759c: MarcoFalke: utACK 5c759c73b2602c7fde1c50dbafe5525904c1b64c ryanofsky: utACK 5c759c73b2602c7fde1c50dbafe5525904c1b64c. Changes since last review: updated commit message and an error message and method name. meshcollider: utACK https://github.com/bitcoin/bitcoin/pull/15778/commits/5c759c73b2602c7fde1c50dbafe5525904c1b64c Tree-SHA512: 2f9b2729da3940a5cda994d3f3bc11ee1a52fcc1c5e9842ea0ea63e4eb0300e8416853046776311298bc449ba07554aa46f0f245ce28598a5b0bd7347c12e752
2019-04-26Merge #15267: doc: explain AcceptToMemoryPoolWorker's coins_to_uncacheMarcoFalke
5d26205272 doc: explain AcceptToMemoryPoolWorker's coins_to_uncache (James O'Beirne) Pull request description: I found ATMPW's `coins_to_uncache` a little hard to understand (see #15264). This adds some doc for posterity. ACKs for commit 5d2620: jnewbery: ACK 5d262052728acdaa2d108a35ba9921a23b3d761a Tree-SHA512: 088508fa78012fab8680663c4e30f5cee29768416c2ca8b8b2abc29b6ac7067c5a589674f0254474a7ccc95477889d41719760f5796792bf492f51b3dd499c6c
2019-04-26doc: explain AcceptToMemoryPoolWorker's coins_to_uncacheJames O'Beirne
2019-04-19Merge #15655: Resolve the checkpoints <-> validation circular dependencyMarcoFalke
418d3230f8 Resolve the checkpoints <-> validation CD. (251) Pull request description: This pull request attempts to resolve the `checkpoints -> validation -> checkpoints` circular dependency. The circular dependency is resolved by moving the `CheckPoints::GetLastCheckpoint(const CCheckpointData& data)` function to `validation.cpp` where it used exclusively by the private function `ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime)`. ACKs for commit 418d32: promag: utACK 418d323, only `GetLastCheckpoint` usage is in `validation.cpp` and so makes sense to move it there. practicalswift: utACK 418d3230f86f77dde6e817f502baff8a54b707fa MarcoFalke: utACK 418d3230f86f77dde6e817f502baff8a54b707fa sipa: utACK 418d3230f86f77dde6e817f502baff8a54b707fa Tree-SHA512: 03c3556bc192e65f5e3fa76fd545d4ee7d63d3fb06b132f7a1fa6131aa21ddd2e5b2d19e2222dfe524f422daaca30efde219bed188db8c74ff4b088876b5bc16
2019-04-18[wallet] Move maxTxFee to walletJohn Newbery
This commit moves the maxtxfee setting to the wallet. There is only one minor behavior change: - an error message in feebumper now refers to -maxtxfee instead of maxTxFee.
2019-04-10Merge #15638: Move-only: Pull wallet code out of libbitcoin_serverWladimir J. van der Laan
4d074e84a2cf419510e2920417799f62747f4b07 [build] Move AnalyzePSBT from psbt.cpp to node/psbt.cpp (Russell Yanofsky) fd509bd1f71df628b933ea7a135a9a957a5e0136 [docs] Document src subdirectories and different libraries (John Newbery) 9eaeb7fb8d4ab0d4493849e6c17e314fd75fea9c [build] Move wallet load functions to wallet/load unit (John Newbery) 91a25d1e711bfc0617027eee18b9777ff368d6b9 [build] Add several util units (John Newbery) 99517866b62c261f990e1f897502855afc12f2a7 [build] Move several units into common libraries (John Newbery) 0509465542d63a5bbe7296f283f44dd491e74f78 [build] Move rpc rawtransaction util functions to rpc/rawtransaction_util.cpp (John Newbery) 1acc61f8746bc6efb905e121a9f607c4f5982b35 [build] Move rpc utility methods to rpc/util (John Newbery) 4a75c9d6512a5580e60104103ea11d2cd9586354 [build] Move policy settings to new src/policy/settings unit (John Newbery) fdf8888b6f0c63e8a4cb1459752625e642d6a4dd [build] Move CheckTransaction from lib_server to lib_consensus (John Newbery) Pull request description: This is a move-only commit. No code is changing and the moves can be easily verified with: ```sh git log -p -n1 --color-moved=dimmed_zebra ``` This commit moves functions and variables that wallet code depends on out of libbitcoin_server.a, so the bitcoin-wallet tool can be built without libbitcoin_server.a in #15639, and attempting to access server state from wallet code will result in link errors instead of silently broken code. List of moves: - `CheckTransaction` moves from `consensus/tx_verify.cpp` to `consensus/tx_check.cpp` - `urlDecode` moves from `httpserver.cpp` to `util/url.cpp` - `TransactionErrorString` moves from `node/transaction.cpp` to `util/error.cpp` - `StringForFeeReason` and `FeeModeFromString` move from `policy/fees.cpp` to `util/fees.cpp` - `incrementalRelayFee` `dustRelayFee` and `nBytesPerSigOp` move from `policy/policy.cpp` to `policy/settings.cpp` - `SignalsOptInRBF` moves from `policy/rbf.cpp` to `util/rbf.cpp` - `fIsBareMultisigStd` moves from `validation.cpp` to `policy/settings.cpp` - `ConstructTransaction` `TxInErrorToJSON` and `SignTransaction` move from `rpc/rawtransaction.cpp` to `rpc/rawtransaction_util.cpp` - `RPCTypeCheck` `RPCTypeCheckArgument` `RPCTypeCheckObj` `AmountFromValue` `ParseHashV``ParseHashO` `ParseHexV` `ParseHexO` `HelpExampleCli` and `HelpExampleRpc` move from `rpc/server.cpp` to `rpc/util.cpp` - `AmountHighWarn` and `AmountErrMsg` move from `ui_interface.cpp` to `util/error.cpp` - `FormatStateMessage` and `strMessageMagic` move from `validation.cpp` to `util/validation.cpp` - `VerifyWallets` `LoadWallets` `StartWallets` `FlushWallets` `StopWallets` and `UnloadWallets` move from `wallet/init.cpp` to `wallet/node.cpp` ACKs for commit 4d074e: jnewbery: utACK 4d074e84a2cf419510e2920417799f62747f4b07 (checked by doing the rebase myself and verifying no difference between my branch and 4d074e84a2cf419510e2920417799f62747f4b07) Tree-SHA512: 5e1604a9fb06475f2b96da0de0baa8330f4dda834dc20a0183ef11e1e4c27631d1d1bbb9abf0054efc03d56945fdf9920f63366b6a4f200f665b742a479ff75c
2019-04-09Merge #15629: init: Throw error when network specific config is ignoredMarcoFalke
fae38c3dc6 doc: Fix all typos reported by codespell (MarcoFalke) fa9058f0ed doc: Add release notes for 15629 (MarcoFalke) fa4a922d78 qa: Add test for missing testnet section in conf file (MarcoFalke) dddd6f0f58 init: Throw error when network specific config is ignored (MarcoFalke) Pull request description: This should have no effect on mainnet users, but simplifies testing, where config settings are currently ignored with only a warning. Fix this by making it an error. Issues: * bitcoin client 0.17.0 ignores wallet's name (file) #14523 * Can't set custom rpcport on testnet #13777 * ... ACKs for commit fae38c: Tree-SHA512: 2e209526898eea6e444c803ec2666989cee4ca137492d32984998733c50a70056cb54657df8dc3027a6a0612738a8afce0bc35824b868c5f22281e00e0188530
2019-04-09[build] Add several util unitsJohn Newbery
Adds the following util units and adds them to libbitcoin_util: - `util/url.cpp` takes `urlDecode` from `httpserver.cpp` - `util/error.cpp` takes `TransactionErrorString` from `node/transaction.cpp` and `AmountHighWarn` and `AmountErrMsg` from `ui_interface.cpp` - `util/fees.cpp` takes `StringForFeeReason` and `FeeModeFromString` from `policy/fees.cpp` - `util/rbf.cpp` takes `SignalsOptInRBF` from `policy/rbf.cpp` - 'util/validation.cpp` takes `FormatStateMessage` and `strMessageMagic` from 'validation.cpp`
2019-04-09[build] Move policy settings to new src/policy/settings unitJohn Newbery
This moves the following policy settings functions and globals to a new src/policy/settings unit in lib_server: - `incrementalRelayFee` - `dustRelayFee` - `nBytesPerSigOp` - `fIsBareMultisigStd` These settings are only required by the node and should not be accessed by other libraries.
2019-04-09[build] Move CheckTransaction from lib_server to lib_consensusJohn Newbery
CheckTransaction is a context-free function that does not require access to the blockchain or mempool. Move it from src/consensus/tx_verify in lib_server to a new unit src/consensus/tx_check in lib_consensus so that it can be called by non-server libraries.
2019-03-23Resolve the checkpoints <-> validation CD.251
This commit resolves the checkpoints -> validation -> checkpoints cirular dependency by moving `CheckPoints::GetLastCheckpoint(const CCheckpointData& data)` from `checkpoints.cpp` to `validation.cpp`.
2019-03-22doc: Fix all typos reported by codespellMarcoFalke
2019-03-22Move g_is_mempool_loaded into CTxMemPool::m_is_loadedBen Woosley
So the loaded state is explicitly mempool-specific.