Separate Consensus::CheckTxInputs and GetSpendHeight in CheckInputs #6061
Conversation
|
Code change looks good to me, but why is CheckInputs not consensus, while CheckTxInputs is? |
|
CheckInputs uses the CScriptCheck class which consensus doesn't need. I later plan to create Consensus::CheckTxInputsScripts that does the remaining and it's used directly instead of CheckInputs in some places (in miner and AcceptToMemPool, but not in AcceptBlock). |
| @@ -507,4 +507,10 @@ extern CCoinsViewCache *pcoinsTip; | |||
| /** Global variable that points to the active block tree (protected by cs_main) */ | |||
| extern CBlockTreeDB *pblocktree; | |||
|
|
|||
| /** | |||
| * While checking, GetBestBlock() refers to the parent block. (protected by cs_main) | |||
There was a problem hiding this comment.
This comment does not actually describe what the function does :)
There was a problem hiding this comment.
Yeah, I just moved the comments from the cpp instead of moving the implementation of the new function. I've just added a line to the doc. Should I leave the rest here or move it back to the implementation?
…ight in CheckInputs
|
By the way, @laanwj (or anyone) feel free to propose additional edits to GetSpendHeight's documentation. |
|
ACK |
eb83719 Consensus: Refactor: Separate Consensus::CheckTxInputs and GetSpendHeight in CheckInputs (Jorge Timón)
Bitcoin 0.12 misc PRs 1 Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6198 - bitcoin/bitcoin#6206 - bitcoin/bitcoin#5927 - bitcoin/bitcoin#6213 - bitcoin/bitcoin#6061 - bitcoin/bitcoin#6283 (partial, remainder was pulled in #929) - bitcoin/bitcoin#6272 - bitcoin/bitcoin#6316 - bitcoin/bitcoin#6133 - bitcoin/bitcoin#6387 - bitcoin/bitcoin#6401 - bitcoin/bitcoin#6434 - bitcoin/bitcoin#6372 - bitcoin/bitcoin#6447 - bitcoin/bitcoin#6149 - bitcoin/bitcoin#6468 Part of #2074.
…ht in CheckInputs e27420e Separate Consensus::CheckTxInputs and GetSpendHeight in CheckInputs (furszy) Pull request description: Coming from bitcoin#6061 Refactor needed for an upcoming work, no functional changes. Had to do something little bit dirty to be able to get Consensus::Params from inside the Consensus namespace (struct `Params` name clashes with global method `Params()`) and not have any functional change there. Point of discussion for a later PR: could be moved to a function argument or check if there is another workaround to distinguish between the name clash. ACKs for top commit: random-zebra: utACK e27420e Fuzzbawls: utACK e27420e Tree-SHA512: 953921659a7ab41d954a8110d2c1b6911bc44b2b458ffbfd97f01262e4c946444bb6537081f34c4ba3ff6f5fca5e803b1d99a5fc84ff902f42a1c8f2956ff4d6
A simple refactor as preparation for moving consensus to code for transaction validation.
Consensus shouldn't depend on
std::vector<CScriptCheck> *pvChecks.This is part of #6051 but can be merged independently.