aboutsummaryrefslogtreecommitdiff
path: root/src/validation.cpp
Commit message (Collapse)AuthorAgeFilesLines
...
* | | Merge #18410: Docs: Improve commenting for coins.cpp|hWladimir J. van der Laan2020-04-221-4/+5
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | [docs] use consistent naming for possible_overwriteJohn Newbery2020-04-211-4/+5
| | | | | | | | | | | | | | | | And other general comment improvements for adding coins.
* | | | Merge #18401: Refactor: Initialize PrecomputedTransactionData in ↵MarcoFalke2020-04-161-5/+12
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | | [REFACTOR] Initialize PrecomputedTransactionData in CheckInputScriptsPieter Wuille2020-04-111-5/+12
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | 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.
* | | | Merge #17737: Add ChainstateManager, remove BlockManager globalMarcoFalke2020-04-101-54/+144
|\ \ \ \ | |/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | protect g_chainman with cs_mainJames O'Beirne2020-03-171-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I'd previously attempted to create a specialized lock for ChainstateManager, but it turns out that because that lock would be required for functions like ChainActive() and ChainstateActive(), it created irreconcilable lock inversions since those functions are used so broadly throughout the codebase. Instead, I'm just using cs_main to protect the contents of g_chainman. Co-authored-by: Russell Yanofsky <[email protected]>
| * | | use ChainstateManager to initialize chainstateJames O'Beirne2020-03-171-36/+32
| | | | | | | | | | | | | | | | | | | | This allows us to easily initialize multiple chainstates on startup in future commits. It retires the g_chainstate global in lieu of g_chainman.
| * | | refactor: move RewindBlockIndex to CChainStateJames O'Beirne2020-03-171-19/+8
| | | | | | | | | | | | | | | | This is in preparation for multiple chainstate initialization in init.
| * | | validation: introduce unused ChainstateManagerJames O'Beirne2020-03-171-0/+96
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ChainstateManager is responsible for creating and managing multiple chainstates, and will provide a high-level interface for accessing the appropriate chainstate based upon a certain use. Incorporates feedback from Marco Falke. Additional documentation written by Russ Yanofsky. Co-authored-by: Russell Yanofsky <[email protected]>
| * | | validation: add CChainState.m_from_snapshot_blockhashJames O'Beirne2020-03-171-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | This parameter is unused, but in future commits will allow ChainstateManager to differentiate between chainstates created from a UTXO snapshot from those that weren't.
* | | | Merge #17477: Remove the mempool's NotifyEntryAdded and NotifyEntryRemoved ↵Wladimir J. van der Laan2020-03-191-26/+4
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | signals e57980b4738c10344baf136de3e050a3cb958ca5 [mempool] Remove NotifyEntryAdded and NotifyEntryRemoved callbacks (John Newbery) 2dd561f36124972d2364f941de9c3417c65f05b6 [validation] Remove pool member from ConnectTrace (John Newbery) 969b65f3f527631ede1a31c7855151e5c5d91f8f [validation] Remove NotifyEntryRemoved callback from ConnectTrace (John Newbery) 5613f9842b4000fed088b8cf7b99674c328d15e1 [validation] Remove conflictedTxs from PerBlockConnectTrace (John Newbery) cdb893443cc16edf974f099b8485e04b3db1b1d7 [validation interface] Remove vtxConflicted from BlockConnected (John Newbery) 1168394d759b13af68acec6d5bfa04aaa24561f8 [wallet] Notify conflicted transactions in TransactionRemovedFromMempool (John Newbery) Pull request description: These boost signals were added in #9371, before we had a `TransactionRemovedFromMempool` method in the validation interface. The `NotifyEntryAdded` callback was used by validation to build a vector of conflicted transactions when connecting a block, which the wallet was notified of in the `BlockConnected` CValidationInterface callback. Now that we have a `TransactionRemovedFromMempool` callback, we can fire that signal directly from the mempool for conflicted transactions. Note that #9371 was implemented to ensure `-walletnotify` events were fired for these conflicted transaction. We inadvertently stopped sending these notifications in #16624 (Sep 2019 commit 7e89994). We should probably fix that, but in a different PR. ACKs for top commit: jonatack: Re-ACK e57980b ryanofsky: Code review ACK e57980b4738c10344baf136de3e050a3cb958ca5, no code changes since previous review, but helpful new code comments have been added and the PR description is now more clear about where the old code came from Tree-SHA512: 3bdbaf1ef2731e788462d4756e69c42a1efdcf168691ce1bbfdaa4b7b55ac3c5b1fd4ab7b90bcdec653703600501b4224d252cfc086aef28f9ce0da3b0563a69
| * | | | [validation] Remove pool member from ConnectTraceJohn Newbery2020-03-111-3/+2
| | | | | | | | | | | | | | | | | | | | It's no longer used for anything.
| * | | | [validation] Remove NotifyEntryRemoved callback from ConnectTraceJohn Newbery2020-03-111-8/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ConnectTrace used to subscribe to the mempool's NotifyEntryRemoved callback to be notified of transactions removed for conflict. Since PerBlockConnectTrace no longer tracks conflicted transactions, ConnectTrace no longer requires these notifications.
| * | | | [validation] Remove conflictedTxs from PerBlockConnectTraceJohn Newbery2020-03-111-15/+1
| | | | | | | | | | | | | | | | | | | | | | | | | Since we don't add a vtxConflicted vector to BlockConnected the conflictedTxs member of PerBlockConnectTrace is no longer used.
| * | | | [validation interface] Remove vtxConflicted from BlockConnectedJohn Newbery2020-03-111-1/+1
| | |/ / | |/| | | | | | | | | | | | | | The wallet now uses TransactionRemovedFromMempool to be notified about conflicted wallet, and no other clients use vtxConflicted.
* | | | Merge #15283: log: Fix UB with bench on genesis blockMarcoFalke2020-03-171-2/+3
|\ \ \ \ | |_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ec30a79f1c430cc7fbda37e5d747b0b31b262fa5 Fix UB with bench on genesis block (Gregory Sanders) Pull request description: During the loading of the genesis block, the bench print lines in ConnectTip divide by zero due to early return in ConnectBlock. ACKs for top commit: practicalswift: ACK ec30a79f1c430cc7fbda37e5d747b0b31b262fa5 sipa: utACK ec30a79f1c430cc7fbda37e5d747b0b31b262fa5 promag: ACK ec30a79, `nBlocksTotal` is only used in logging. Tree-SHA512: b3bdbb58d10d002a2293d7f99196b227ed9f4ca8c6cd08981e95cc964be47efed98b91fad276ee6da5cf7e6684610998ace7ce9bace172dd6c51c386d985b83c
| * | | Fix UB with bench on genesis blockGregory Sanders2019-01-291-2/+3
| | | |
* | | | Replace remaining literals BTC with CURRENCY_UNITDaniel Kraft2020-03-141-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This replaces one remaining instance of the literal "BTC" string with the CURRENCY_UNIT constant, as is done in most of the codebase already. The other remaining instance (which is just part of a log message and thus not really user-visible) is just removed. After this change, no instance of literal "BTC" remains anywhere in the non-Qt and non-test codebase.
* | | | Refactor FormatStateMessage into ValidationStateJeffrey Czyz2020-02-271-23/+22
| |/ / |/| |
* | | Merge #16974: Walk pindexBestHeader back to ChainActive().Tip() if it is invalidWladimir J. van der Laan2020-02-031-0/+6
|\ \ \ | |_|/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 0a50019fde7781263e0c8f041d1d9dcb0dee77e8 Walk pindexBestHeader back to ChainActive().Tip() if it is invalid (Matt Corallo) Pull request description: Instead of keeping pindexBestHeader set to the best header we've ever seen, reset it back to our validated tip if we find an ancestor of it turns out to be invalid. While the name is now a bit confusing, this matches much better with how it is used in practice, see below. Further, this opens up more use-cases for it in the future, namely aggressively searching for new peers in case we have discovered (possibly via some covert channel) headers which we do not know to be invalid, but which we cannot find block data for. Places pindexBestHeader is used: * Various GUI displays of the best header and getblockchaininfo["headers"], I don't think changing this is bad, and if anything this is less confusing in the presence of an invalid block. * IsCurrentForFeeEstimation(): If anything I think ensuring pindexBestHeader isn't some crazy invalid chain is better than the alternative, even in the case where you are rejecting the current chain due to hardware error (since hopefully in that case you won't get any new blocks anyway). * ConnectBlock assumevalid checks: We use pindexBestHeader to check that the block we're connecting leads to something with nMinimumChainWork (preventing a user-set assumevalid from having bogus work) and that the block we're connecting leads to pindexBestHeader (I'm not too worried about this one - it's nice to "disable" assumevalid if we have a long invalid headers chain, but I don't see it as a critical protection). * BlockRequestAllowed() uses pindexBestHeader as its target to ensure the requested block is within a month of the "current chain". I don't think this is a meaningful difference, if we're rejecting the current tip we're trivially fingerprintable anyway, and if the chain really does have a bunch of invalid crap near the tip, using the best not-invalid header is likely a better criteria. * ProcessGetBlockData uses pindexBestHeader as the "current chain" definition of whether a block request is "historical" for the purpose of bandwidth limiting. Similarly, I don't see why this is a meaningful change. * We use pindexBestHeader for requesting missing headers on receipt of a headers/compact block message or block inv as well as for initial getheaders. I think this is definitely wrong, using the best not-invalid header for such requests is much better. * We use pindexBestHeader to define the "current chain" for deciding when we're close to done with initial headers sync. I don't think this is a meaningful change. * We use pindexBestHeader to decide if initial headers sync has timed out. If we're rejecting the chain due to hardware error this may result in additional cases where we ban a peer, but this is already true, so I think its fine. ACKs for top commit: fjahr: ACK 0a50019fde7781263e0c8f041d1d9dcb0dee77e8 kallewoof: ACK 0a50019fde7781263e0c8f041d1d9dcb0dee77e8 ariard: utACK 0a50019 Tree-SHA512: 2ecfa973a9878a00313ae7ede94a9bd7710e0caf55b544b10bbc46dc463a0478cbaf477e6cdd072356d5a0c5fb3848e9339284af785a2995c20bae8bd23f23e5
| * | Walk pindexBestHeader back to ChainActive().Tip() if it is invalidMatt Corallo2019-10-301-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of keeping pindexBestHeader set to the best header we've ever seen, reset it back to our validated tip if we find an ancestor of it turns out to be invalid. While the name is now a bit confusing, this matches much better with how it is used in practice, see below. Further, this opens up more use-cases for it in the future, namely aggressively searching for new peers in case we have discovered (possibly via some covert channel) headers which we do not know to be invalid, but which we cannot find block data for. Places pindexBestHeader is used: * Various GUI displays of the best header and getblockchaininfo["headers"], I don't think changing this is bad, and if anything this is less confusing in the presence of an invalid block. * IsCurrentForFeeEstimation(): If anything I think ensuring pindexBestHeader isn't some crazy invalid chain is better than the alternative, even in the case where you are rejecting the current chain due to hardware error (since hopefully in that case you won't get any new blocks anyway). * ConnectBlock assumevalid checks: We use pindexBestHeader to check that the block we're connecting leads to something with nMinimumChainWork (preventing a user-set assumevalid from having bogus work) and that the block we're connecting leads to pindexBestHeader (I'm not too worried about this one - it's nice to "disable" assumevalid if we have a long invalid headers chain, but I don't see it as a critical protection). * BlockRequestAllowed() uses pindexBestHeader as its target to ensure the requested block is within a month of the "current chain". I don't think this is a meaningful difference, if we're rejecting the current tip we're trivially fingerprintable anyway, and if the chain really does have a bunch of invalid crap near the tip, using the best not-invalid header is likely a better criteria. * ProcessGetBlockData uses pindexBestHeader as the "current chain" definition of whether a block request is "historical" for the purpose of bandwidth limiting. Similarly, I don't see why this is a meaningful change. * We use pindexBestHeader for requesting missing headers on receipt of a headers/compact block message or block inv as well as for initial getheaders. I think this is definitely wrong, using the best not-invalid header for such requests is much better. * We use pindexBestHeader to define the "current chain" for deciding when we're close to done with initial headers sync. I don't think this is a meaningful change. * We use pindexBestHeader to decide if initial headers sync has timed out. If we're rejecting the chain due to hardware error this may result in additional cases where we ban a peer, but this is already true, so I think its fine.
* | | scripted-diff: Bump copyright of files changed in 2020MarcoFalke2020-01-151-1/+1
| | | | | | | | | | | | | | | | | | -BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
* | | scripted-diff: Replace CCriticalSection with RecursiveMutexMarcoFalke2020-01-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | -BEGIN VERIFY SCRIPT- # Delete outdated alias for RecursiveMutex sed -i -e '/CCriticalSection/d' ./src/sync.h # Replace use of outdated alias with RecursiveMutex sed -i -e 's/CCriticalSection/RecursiveMutex/g' $(git grep -l CCriticalSection) -END VERIFY SCRIPT-
* | | Merge #16945: refactor: introduce CChainState::GetCoinsCacheSizeStateWladimir J. van der Laan2020-01-131-6/+35
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 02b9511d6bace5711e454d2b685b2fee0d65e341 tests: add tests for GetCoinsCacheSizeState (James O'Beirne) b17e91d842724d2888a179a73585cc4c2ef1dc21 refactoring: introduce CChainState::GetCoinsCacheSizeState (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 pulls out the routine for detection of how full the coins cache is from FlushStateToDisk. We use this logic independently when deciding when to flush the coins cache during UTXO snapshot activation ([see here](https://github.com/bitcoin/bitcoin/pull/15606/commits/231fb5f17e3bb8ce3323c559ef6f9b3aaa066543#diff-24efdb00bfbe56b140fb006b562cc70bR5275)). ACKs for top commit: ariard: Code review ACK 02b9511. ryanofsky: Code review ACK 02b9511d6bace5711e454d2b685b2fee0d65e341. Just rebase, new COIN_SIZE comment, and new test message since last review Tree-SHA512: 8bdd78bf68a4a5d33a776e73fcc2857f050d6d102caa4997ed19ca25468c1358e6e728199d61b423033c02e6bc8f00a1d9da52cf17a2d37d70860fca9237ea7c
| * | | refactoring: introduce CChainState::GetCoinsCacheSizeStateJames O'Beirne2019-12-121-6/+35
| | | | | | | | | | | | | | | | | | | | | | | | This separates out some logic for detecting how full the coins cache is from FlushStateToDisk. We'll want to reuse this logic when deciding when to flush the coins cache during UTXO snapshot activation.
* | | | Merge #16658: validation: Rename CheckInputs to CheckInputScriptsMarcoFalke2020-01-021-20/+22
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 3bd8db80d8d335ab63ece4f110b0fadd562e80b7 [validation] fix comments in CheckInputScripts() (John Newbery) 6f6465cefcd599c89c00f7b51f42a4b87a5ffb0b scripted-diff: [validation] Rename CheckInputs to CheckInputScripts (John Newbery) Pull request description: CheckInputs() used to check no double spends, scripts & sigs and amounts. Since 832e074, the double spend and amount checks have been moved to CheckTxInputs(), and CheckInputs() now just validates input scripts. Rename the function to CheckInputScripts(). Also fix incorrect comments. ACKs for top commit: MarcoFalke: re-ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7, did the rebase myself, checked the scripted diff 👡 promag: ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7 :trollface: Tree-SHA512: 7b3f8597d210492798fb784ee8ea47ea6377519111190161c7cc34a967509013f4337304f52e9bedc97b7710de7b0ff8880e08cd7f867754567f82e7b02c794c
| * | | | [validation] fix comments in CheckInputScripts()John Newbery2019-11-071-7/+9
| | | | |
| * | | | scripted-diff: [validation] Rename CheckInputs to CheckInputScriptsJohn Newbery2019-11-071-14/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | CheckInputs() used to check no double spends, scripts & sigs and amounts. Since 832e0744cb8b1e1625cdb19b257f97316ac16a90, the double spend and amount checks have been moved to CheckTxInputs(), and CheckInputs() now just validates input scripts. Rename the function to CheckInputScripts(). -BEGIN VERIFY SCRIPT- sed -i -E -e 's/CheckInputs\b/CheckInputScripts/g' $(git grep -l CheckInputs | grep -v doc/) -END VERIFY SCRIPT-
* | | | | scripted-diff: Bump copyright of files changed in 2019MarcoFalke2019-12-301-1/+1
| |/ / / |/| | | | | | | | | | | | | | | | | | | -BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
* | | | refactor: Remove unused COINBASE_FLAGSNeha Narula2019-11-201-3/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit d449772cf69c01932fc5d72c46054815d6300b3c stopped setting COINBASE_FLAGS, and it looks like it hasn't been used since P2SH. Update the help string to remove "flags", which is not specified in BIP 22.
* | | | Merge #17328: GuessVerificationProgress: cap the ratio to 1Wladimir J. van der Laan2019-11-181-1/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 2f5f7d6b135e4eab368bbafd9e6e979aa72398de GuessVerificationProgress: cap the ratio to 1 (darosior) Pull request description: Noticed `getblockchaininfo` would return a `verificationprogress` > 1, especially while generating. This caps the verification progress to `1`. Tried to append a check to functional tests but this would pass even without the patch, so it seems better to not add a superfluous check (but this can easily be reproduced by trying to generate blocks in the background and `watch`ing `getblockchainfo`). ACKs for top commit: laanwj: ACK 2f5f7d6b135e4eab368bbafd9e6e979aa72398de promag: ACK 2f5f7d6b135e4eab368bbafd9e6e979aa72398de. Tree-SHA512: fa3aca12acab9c14dab3b2cc94351082f548ea6e6c588987cd86e928a00feb023e8112433658a0e85084e294bfd940eaafa33fb46c4add94146a0901bc1c4f80
| * | | | GuessVerificationProgress: cap the ratio to 1darosior2019-10-311-1/+1
| | |/ / | |/| | | | | | | | | | | | | | The getblockchaininfo RPC call could sometime return a 'validationprogress' > 1, but this is absurd.
* | | | Merge #15931: Remove GetDepthInMainChain dependency on locked chain interfaceSamuel Dobson2019-11-081-1/+1
|\ \ \ \ | |_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 36b68de5b2938722911db900ca299f7008780d01 Remove getBlockDepth method from Chain::interface (Antoine Riard) b66c429c56c85fa16c309be0b2bca9c25fdd3e1a Remove locked_chain from GetDepthInMainChain and its callers (Antoine Riard) 0ff03871add000f8b4d8f82aeb168eed2fc9dc5f Use CWallet::m_last_block_processed_height in GetDepthInMainChain (Antoine Riard) f77b1de16feee097a88e99d2ecdd4d84beb4f915 Only return early from BlockUntilSyncedToCurrentChain if current tip is exact match (Antoine Riard) 769ff05e48fb53d4b62c59060424a0fea71d0aab Refactor some importprunedfunds checks with guard clause (Antoine Riard) 5971d3848e09abf571e5308185275296127efca4 Add block_height field in struct Confirmation (Antoine Riard) 9700fcb47feca9d78e005b8d18b41148c8f6b25f Replace CWalletTx::SetConf by Confirmation initialization list (Antoine Riard) 5aacc3eff15b9b5bdc951f1e274f00d581f63bce Add m_last_block_processed_height field in CWallet (Antoine Riard) 10b4729e33f76092bd8cfa06d1a5e0a066436f76 Pass block height in Chain::BlockConnected/Chain::BlockDisconnected (Antoine Riard) Pull request description: Work starter to remove Chain::Lock interface by adding m_last_block_processed_height in CWallet and m_block_height in CMerkleTx to avoid GetDepthInMainChain having to keep a lock . Once this one done, it should ease work to wipe out more cs_main locks from wallet code. I think it's ready for a first round of review before to get further. - `BlockUntilSyncedToCurrent` : restrain isPotentialTip to isTip because we want to be sure that wallet see BlockDisconnected callbacks if its height differs from the Chain one. It means during a reorg, an RPC could return before the BlockDisconnected callback had been triggered. This could cause a tx that had been included in the disconnected block to be displayed as confirmed, for example. ~~- `AbandonTransaction` : in case of conflicted tx (nIndex = -1), we set its m_block_height to the one of conflicting blocks, but if this height is superior to CWallet::m_last_block_processed_height, that means tx isn't conflicted anymore so we return 0 as tx is again unconfirmed~~ After #16624, we instead rely on Confirmation. ~~- `AddToWalletIfInvolvingMe`: in case of block disconnected, transactions are added to mempool again, so we need to replace old txn in `mapWallet` with a height set to zero so we remove check on block_hash.IsNull~~ Already done in #16624 ACKs for top commit: jnewbery: @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch (36b68de5b2938722911db900ca299f7008780d01). jkczyz: > @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch ([36b68de](https://github.com/bitcoin/bitcoin/commit/36b68de5b2938722911db900ca299f7008780d01)). meshcollider: utACK 36b68de5b2938722911db900ca299f7008780d01 ryanofsky: Code review ACK 36b68de5b2938722911db900ca299f7008780d01. Changes since last review: new jkczyz refactor importprunedfunds commit, changed BlockUntilSyncedToCurrentChainChanges commit title and description, changed Confirmation struct field order and line-wrapped comment jnewbery: utACK 36b68de5b2938722911db900ca299f7008780d01 promag: Code review ACK 36b68de5b2938722911db900ca299f7008780d01. Tree-SHA512: 08b89a0bcc39f67c82a6cb6aee195e6a11697770c788ba737b90986b4893f44e90d1ab9ef87239ea3766508b7e24ea882b7199df41173ab27a3d000328c14644
| * | | Pass block height in Chain::BlockConnected/Chain::BlockDisconnectedAntoine Riard2019-11-051-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To do so we update CValidationInterface::BlockDisconnect to take a CBlockIndex pointing to the block being disconnected. This new parameter will be use in the following commit to establish wallet height.
* | | | Merge #17342: refactor: Clean up nScriptCheckThreadsMarcoFalke2019-11-071-3/+3
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 5506ecfe7a65d5705616bc048f2f1735b89993fb [refactor] Replace global int nScriptCheckThreads with bool (John Newbery) d9957623b48a7c3eff0ac750d1245fabfb1843a2 [tests] Don't use TestingSetup in the checkqueue_tests (John Newbery) Pull request description: The meaning of this value is confusing. Refactor it and add comments. ACKs for top commit: sipa: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb promag: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb, only change was addressing my nits. laanwj: Code review ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb MarcoFalke: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb 🥐 Tree-SHA512: 78536727c98d2c23f3c0f3f169131474fef9a4486ae65029011caf06eab30f6f70ff73a65b2fb04a5d969fc1150858d1c6ea4767f04d48c1eea6b829316d0e63
| * | | | [refactor] Replace global int nScriptCheckThreads with boolJohn Newbery2019-11-061-3/+3
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The global nScriptCheckThreads int is confusing and is only needed for its int-ness in AppInitMain. Move all `-par` parsing logic there and replace the int nScriptCheckThreads with a bool g_parallel_script_checks. Also tidy up logic and improve comments.
* | | | Merge #16805: logs: add timing information to FlushStateToDisk()Wladimir J. van der Laan2019-11-051-3/+26
|\ \ \ \ | |_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | dcef9a2922317cb2849f397366b6c56d755db6c9 logs: add timing information to FlushStateToDisk() (James O'Beirne) 41edaf227a69bc4846d5996675e8763fdfe0f367 logs: add BCLog::Timer and related macros (James O'Beirne) Pull request description: It's currently annoying to detect FlushStateToDisk() calls when benchmarking since they have to be inferred from a drop in coins count from the `UpdateTip: ` log messages. This adds a new logging utility, `BCLog::Timer`, and some related macros that are generally useful for printing timing-related logging messages, and a message that is unconditionally written when the coins cache is flushed to disk. ``` 2019-09-04T20:17:51Z FlushStateToDisk: write block and undo data to disk completed (3ms) 2019-09-04T20:17:51Z FlushStateToDisk: write block index to disk completed (370ms) 2019-09-04T20:17:51Z FlushStateToDisk: write coins cache to disk (2068451 coins, 294967kB) completed (21481ms) ``` ACKs for top commit: laanwj: Thanks, ACK dcef9a2922317cb2849f397366b6c56d755db6c9 ryanofsky: Code review ACK dcef9a2922317cb2849f397366b6c56d755db6c9. No changes since last review other than moving code to new timer.h header Tree-SHA512: 6d61e48a062d3edb48d0e056a6f0b1f8031773cc99289ee4544f8349d24526b88519e1e304009d56e428f1eaf76c857bf8e7e1c0b6873a6f270306accb5edc3d
| * | | logs: add timing information to FlushStateToDisk()James O'Beirne2019-11-041-3/+26
| | | |
* | | | doc: Fix some misspellingsrandymcmillan2019-11-041-1/+1
| |/ / |/| |
* | | [validation] Remove fMissingInputs from AcceptToMemoryPool()John Newbery2019-10-291-16/+7
| | | | | | | | | | | | | | | Handle this failure in the same way as all other failures: call Invalid() with the reasons for the failure.
* | | [validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders()John Newbery2019-10-291-3/+1
| | | | | | | | | | | | | | | No callers use the returned value in first_invalid. Remove it from the function signature and don't set it in the function.
* | | [validation] Remove useless ret parameter from Invalid()John Newbery2019-10-291-56/+56
| | | | | | | | | | | | | | | | | | ValidationState::Invalid() takes a parameter `ret` which is returned to the caller. All call sites set this to false. Remove the `ret` parameter and just return false always.
* | | [validation] Remove error() calls from Invalid() callsJohn Newbery2019-10-291-26/+37
| | | | | | | | | | | | | | | | | | | | | | | | | | | This is in preparation for the next commit, which removes the useless `ret` parameter from ValidationState::Invalid(). error() is simply a convenience wrapper that calls LogPrintf and returns false. Call LogPrintf explicitly and substitute the error() call for a false bool literal.
* | | [validation] Add CValidationState subclassesJohn Newbery2019-10-291-139/+133
| | | | | | | | | | | | | | | Split CValidationState into TxValidationState and BlockValidationState to store validation results for transactions and blocks respectively.
* | | Merge #17242: refactor: Remove unused cacheSigStore from ↵fanquake2019-10-251-3/+4
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | CheckInputsFromMempooAndCache 0a433fc876d82df1005f175c1254fff62f0f36f8 [validation] Remove unused cacheSigStore from CheckInputsFromMempoolAndCache (John Newbery) Pull request description: CheckInputsFromMempoolAndCache() is only called in one place, and cacheSigStore is set to true in that call site. Remove the argument entirely. Also improve commenting. ACKs for top commit: MarcoFalke: unsigned ACK 0a433fc876d82df1005f175c1254fff62f0f36f8 Comment looks good jamesob: ACK https://github.com/bitcoin/bitcoin/pull/17242/commits/0a433fc876d82df1005f175c1254fff62f0f36f8 laanwj: ACK 0a433fc876d82df1005f175c1254fff62f0f36f8 fanquake: ACK 0a433fc876d82df1005f175c1254fff62f0f36f8. Checked that `CheckInputsFromMempoolAndCache` is only called once, in `MemPoolAccept::ConsensusScriptChecks`, and that `cacheSigStore` is true. Tree-SHA512: e4b4d2550e35df55c8f8fa4c539174cc2d3728112ddb937cb2ff759d8630a01566b5ec42a70a82e33994e6586f5a457a75a59f64b15d27c65331c723cbb097af
| * | | [validation] Remove unused cacheSigStore from CheckInputsFromMempoolAndCacheJohn Newbery2019-10-241-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | CheckInputsFromMempoolAndCache() is only called in one place, and cacheSigStore is set to true in that call site. Remove the argument entirely. Also improve commenting.
* | | | consensus: Explain why fCheckDuplicateInputs can not be skipped and remove itMarcoFalke2019-10-241-2/+1
|/ / /
* | | Merge #17004: validation: Remove REJECT code from CValidationStateWladimir J. van der Laan2019-10-241-57/+56
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 9075d13153ce06cd59a45644831ecc43126e1e82 [docs] Add release notes for removal of REJECT reasons (John Newbery) 04a2f326ec0f06fb4fce1c4f93500752f05dede8 [validation] Fix REJECT message comments (John Newbery) e9d5a59e34ff2d538d8f5315efd9908bf24d0fdc [validation] Remove REJECT code from CValidationState (John Newbery) 0053e16714323c1694c834fdca74f064a1a33529 [logging] Don't log REJECT code when transaction is rejected (John Newbery) a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab [validation] Fix peer punishment for bad blocks (John Newbery) Pull request description: We no longer send BIP 61 REJECT messages, so there's no need to set a REJECT code in the CValidationState object. Note that there is a minor bug fix in p2p behaviour here. Because the call to `MaybePunishNode()` in `PeerLogicValidation::BlockChecked()` only previously happened if the REJECT code was > 0 and < `REJECT_INTERNAL`, then there are cases were `MaybePunishNode()` can get called where it wasn't previously: - when `AcceptBlockHeader()` fails with `CACHED_INVALID`. - when `AcceptBlockHeader()` fails with `BLOCK_MISSING_PREV`. Note that `BlockChecked()` cannot fail with an 'internal' reject code. The only internal reject code was `REJECT_HIGHFEE`, which was only set in ATMP. This reverts a minor bug introduced in 5d08c9c579ba8cc7b684105c6a08263992b08d52. ACKs for top commit: ariard: ACK 9075d13, changes since last reviewed are splitting them in separate commits to ease understanding and fix nits fjahr: ACK 9075d13153ce06cd59a45644831ecc43126e1e82, confirmed diff to last review was fixing nits in docs/comments. ryanofsky: Code review ACK 9075d13153ce06cd59a45644831ecc43126e1e82. Only changes since last review are splitting the main commit and updating comments Tree-SHA512: 58e8a1a4d4e6f156da5d29fb6ad6a62fc9c594bbfc6432b3252e962d0e9e10149bf3035185dc5320c46c09f3e49662bc2973ec759679c0f3412232087cb8a3a7
| * | | [validation] Remove REJECT code from CValidationStateJohn Newbery2019-10-101-57/+56
| | | | | | | | | | | | | | | | | | | | We no longer send BIP 61 REJECT messages, so there's no need to set a REJECT code in the CValidationState object.
* | | | Remove unused includespracticalswift2019-10-151-3/+0
|/ / /
generated by cgit v1.2.3 (git 2.25.1) at 2026-03-15 00:16:00 +0000