diff options
| author | Wladimir J. van der Laan <laanwj@protonmail.com> | 2019-10-25 13:46:41 +0200 |
|---|---|---|
| committer | Wladimir J. van der Laan <laanwj@protonmail.com> | 2019-10-25 13:47:28 +0200 |
| commit | 90ed98ae9a2ace4ea371d63ca7c3a83ea6224eab (patch) | |
| tree | 42bb308c66189fa5f33d479de7d4b9db4d01dcfb /src/validation.cpp | |
| parent | 366753e46e1fc17724c6464bf0213c5a602f90d0 (diff) | |
| parent | fa928134075220254a15107c1d9702f4e66271f8 (diff) | |
Merge #17080: consensus: Explain why fCheckDuplicateInputs can not be skipped and remove it
fa928134075220254a15107c1d9702f4e66271f8 consensus: Explain why fCheckDuplicateInputs can not be skipped and remove it (MarcoFalke)
Pull request description:
As a follow up to CVE-2018-17144, this removes the unused `fCheckDuplicateInputs` parameter and explains why the test can not be disabled. Apart from protecting against a dumb accident in the future, this should document the logic in the code. There is a technical write-up that explains how the underlying coins database behaves if this test is skipped: https://bitcoincore.org/en/2018/09/20/notice/#technical-details. However, it does not explicitly mention why the test can not be skipped. I hope my code comment does that.
ACKs for top commit:
jnewbery:
ACK fa928134075220254a15107c1d9702f4e66271f8
amitiuttarwar:
utACK fa928134075220254a15107c1d9702f4e66271f8
Empact:
Code review ACK https://github.com/bitcoin/bitcoin/commit/fa928134075220254a15107c1d9702f4e66271f8
promag:
ACK fa928134075220254a15107c1d9702f4e66271f8.
Tree-SHA512: fc1ef670f1a467c543b84f704b9bd8cc7a59a9f707be048bd9b4e85fe70830702aa560a880efa2c840bb43818ab44dfdc611104df04db2ddc14ff92f46bfb28e
Diffstat (limited to 'src/validation.cpp')
| -rw-r--r-- | src/validation.cpp | 3 |
1 files changed, 1 insertions, 2 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index 70b847d3b0..f1abcadefc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3301,9 +3301,8 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-multiple", "more than one coinbase"); // Check transactions - // Must check for duplicate inputs (see CVE-2018-17144) for (const auto& tx : block.vtx) - if (!CheckTransaction(*tx, state, true)) + if (!CheckTransaction(*tx, state)) return state.Invalid(state.GetReason(), false, state.GetRejectReason(), strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage())); |