New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove duplicatable duplicate-input check from CheckTransaction #9049

Merged
merged 3 commits into from Nov 10, 2016
Jump to file or symbol
Failed to load files and symbols.
+12 −10
Diff settings

Always

Just for now

Viewing a subset of changes. View all

Remove redundant duplicate-input check from CheckTransaction

  • Loading branch information...
TheBlueMatt committed Nov 1, 2016
commit eecffe50efc3944d713c701fa375dacbf17fb7cf
Copy path View file
@@ -1104,7 +1104,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
bool CheckTransaction(const CTransaction& tx, CValidationState &state)
bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fCheckDuplicateInputs)
{
// Basic checks that don't depend on any context
if (tx.vin.empty())
@@ -1128,13 +1128,15 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state)
return state.DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge");
}
// Check for duplicate inputs
set<COutPoint> vInOutPoints;
for (const auto& txin : tx.vin)
{
if (vInOutPoints.count(txin.prevout))
return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
vInOutPoints.insert(txin.prevout);
// Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock

This comment has been minimized.

@gmaxwell

gmaxwell Nov 2, 2016

Member

point out that it's also redundant there please!

This comment has been minimized.

@dexX7

dexX7 Sep 19, 2018

Contributor

How is it redundant? Can you point to the case where it's checked a second time?

if (fCheckDuplicateInputs) {
set<COutPoint> vInOutPoints;
for (const auto& txin : tx.vin)
{
if (vInOutPoints.count(txin.prevout))

This comment has been minimized.

@theuni

theuni Nov 8, 2016

Member

Also worth noting (though not really related here), I measure a ~20%-25% time reduction by avoiding doing 2 lookups:

std::set<COutPoint> vInOutPoints;
for (const auto& txin : tx.vin)
{
    if (!vInOutPoints.insert(txin.prevout).second)
        return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
}
return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
vInOutPoints.insert(txin.prevout);
}
}
if (tx.IsCoinBase())
@@ -3461,7 +3463,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
// Check transactions
for (const auto& tx : block.vtx)
if (!CheckTransaction(tx, state))
if (!CheckTransaction(tx, state, false))
return state.Invalid(false, state.GetRejectCode(), state.GetRejectReason(),
strprintf("Transaction check failed (tx hash %s) %s", tx.GetHash().ToString(), state.GetDebugMessage()));
Copy path View file
@@ -343,7 +343,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight);
/** Transaction validation functions */
/** Context-independent validity checks */
bool CheckTransaction(const CTransaction& tx, CValidationState& state);
bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCheckDuplicateInputs=true);
namespace Consensus {
ProTip! Use n and p to navigate between commits in a pull request.