From 4064bbe57be61bd090cc476acac15eda3a6a6465 Mon Sep 17 00:00:00 2001 From: Gregory Sanders Date: Thu, 13 Apr 2017 11:21:34 -0400 Subject: [PATCH 1/3] remove unused GetFee call --- src/main.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 4c9916a0d24..d3a879a9327 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2634,9 +2634,6 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi if (pvChecks) pvChecks->reserve(tx.vin.size()); - // Tally validity checked in CheckTxInputs - CAmountMap fee = tx.GetFee(); - // The first loop above does all the inexpensive checks. // Only if ALL inputs pass do we perform expensive ECDSA signature checks. // Helps prevent CPU exhaustion attacks. From 0c3bdeec1e0b65f79e18c3ae1c8ce51f7e34cd79 Mon Sep 17 00:00:00 2001 From: Gregory Sanders Date: Thu, 13 Apr 2017 11:24:03 -0400 Subject: [PATCH 2/3] remove redundant fee checking, allow 0-value fees --- src/main.cpp | 7 ------- src/primitives/transaction.cpp | 15 --------------- src/test/blind_tests.cpp | 11 +++++++++++ src/test/verify_amounts_tests.cpp | 6 ++++++ src/wallet/rpcwallet.cpp | 1 - 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index d3a879a9327..c976db3f7ff 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1765,8 +1765,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C int64_t nSigOpsCost = GetTransactionSigOpCost(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS); - if (!tx.HasValidFee()) - return state.DoS(0, false, REJECT_INVALID, "bad-fees"); CAmount nFees = tx.GetFee()[policyAsset]; // nModifiedFees includes any fee deltas from PrioritiseTransaction @@ -2613,9 +2611,6 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins } } - // Tally transaction fees - if (!tx.HasValidFee()) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange"); if (!VerifyAmounts(inputs, tx, pvChecks, cacheStore)) return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-belowout", false, strprintf("value in (%s) < value out", FormatMoney(nValueIn))); @@ -3289,8 +3284,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin if (txout.scriptPubKey.IsWithdrawLock() && txout.nValue.IsExplicit()) mLocksCreated.insert(std::make_pair(txout.scriptPubKey.GetWithdrawLockGenesisHash(), std::make_pair(COutPoint(tx.GetHash(), j), txout.nValue.GetAmount()))); } - if (!tx.HasValidFee()) - return state.DoS(100, error("ConnectBlock(): transaction fee overflowed"), REJECT_INVALID, "bad-fee-outofrange"); mapFees += tx.GetFee(); if (!MoneyRange(mapFees)) return state.DoS(100, error("ConnectBlock(): total block reward overflowed"), REJECT_INVALID, "bad-blockreward-outofrange"); diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index da2b51f68c3..a7d8e13fb85 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -176,21 +176,6 @@ uint256 CTransaction::ComputeWitnessHash() const return ComputeFastMerkleRoot(leaves); } -bool CTransaction::HasValidFee() const -{ - CAmountMap totalFee; - for (unsigned int i = 0; i < vout.size(); i++) { - CAmount fee = 0; - if (vout[i].IsFee()) { - fee = vout[i].nValue.GetAmount(); - if (fee == 0 || !MoneyRange(fee)) - return false; - totalFee[vout[i].nAsset.GetAsset()] += fee; - } - } - return MoneyRange(totalFee); -} - CAmountMap CTransaction::GetFee() const { CAmountMap fee; diff --git a/src/test/blind_tests.cpp b/src/test/blind_tests.cpp index cf6fb0955cb..a0cdd86cf23 100644 --- a/src/test/blind_tests.cpp +++ b/src/test/blind_tests.cpp @@ -85,6 +85,17 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) tx3.vout.push_back(CTxOut(bitcoinID, 22, CScript())); BOOST_CHECK(VerifyAmounts(cache, tx3)); + // Fees must have non-negative value + tx3.vout.push_back(CTxOut(bitcoinID, 0, CScript())); + BOOST_CHECK(VerifyAmounts(cache, tx3)); + + tx3.vout.push_back(CTxOut(bitcoinID, -1, CScript())); + BOOST_CHECK(!VerifyAmounts(cache, tx3)); + + tx3.vout.pop_back(); + tx3.vout.pop_back(); + BOOST_CHECK(VerifyAmounts(cache, tx3)); + // Try to blind with a single non-fee output, which fails as its blinding factor ends up being zero. std::vector input_blinds; std::vector input_asset_blinds; diff --git a/src/test/verify_amounts_tests.cpp b/src/test/verify_amounts_tests.cpp index d6f974f1278..36cc7b621ac 100644 --- a/src/test/verify_amounts_tests.cpp +++ b/src/test/verify_amounts_tests.cpp @@ -88,6 +88,12 @@ BOOST_AUTO_TEST_CASE(verify_coinbase_test) tx.vout.push_back(CTxOut(CAsset(), 1, CScript() << OP_RETURN)); BOOST_CHECK(!VerifyCoinbaseAmount(tx, mapFees)); tx.vout.pop_back(); + + // Push fee output, must fail since coinbase cannot have fee + tx.vout.push_back(CTxOut(CAsset(), 1, CScript())); + BOOST_CHECK(!VerifyCoinbaseAmount(tx, mapFees)); + tx.vout.pop_back(); + } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 07eea1b04af..8b696e5504f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2044,7 +2044,6 @@ UniValue gettransaction(const UniValue& params, bool fHelp) CAmountMap nCredit = wtx.GetCredit(filter); CAmountMap nDebit = wtx.GetDebit(filter); - assert(wtx.HasValidFee()); CAmount nFee = (wtx.IsFromMe(filter) ? -wtx.GetFee()[policyAsset] : 0); CAmountMap nNet = nCredit - nDebit; nNet[policyAsset] -= nFee; From 987e98c3f82141b06e75b114199a6b7d1d2a4555 Mon Sep 17 00:00:00 2001 From: Gregory Sanders Date: Thu, 13 Apr 2017 11:41:09 -0400 Subject: [PATCH 3/3] update error message on coinbase amount failure --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index c976db3f7ff..b617208b258 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3296,7 +3296,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return state.DoS(100, error("ConnectBlock(): total block reward overflowed"), REJECT_INVALID, "bad-blockreward-outofrange"); if (!VerifyCoinbaseAmount(block.vtx[0], blockReward)) return state.DoS(100, - error("ConnectBlock(): coinbase pays too much (limit=%d)", + error("ConnectBlock(): coinbase pays too much, has fee or blinded outputs (limit=%d)", blockReward[policyAsset]), REJECT_INVALID, "bad-cb-amount");