Merge pull request #160 from instagibbs/feecleanup

[HF] Fee output logic cleanup, allow 0-value fees

Author: instagibbs

src/main.cpp

@@ -1773,8 +1773,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
@@ -2621,9 +2619,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)));
@@ -2642,9 +2637,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.
@@ -3290,8 +3282,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
                 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");
@@ -3304,7 +3294,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");
 

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;

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<uint256> input_blinds;
         std::vector<uint256> input_asset_blinds;

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()

src/wallet/rpcwallet.cpp

@@ -2054,7 +2054,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;