diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index d0f5e8388c..e3207f1862 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -476,7 +476,6 @@ class CTransaction(object): def __init__(self, tx=None): if tx is None: self.nVersion = 1 - self.nTxFee = 0 self.vin = [] self.vout = [] self.wit = CTxWitness() @@ -485,7 +484,6 @@ class CTransaction(object): self.hash = None else: self.nVersion = tx.nVersion - self.nTxFee = tx.nTxFee self.vin = copy.deepcopy(tx.vin) self.vout = copy.deepcopy(tx.vout) self.nLockTime = tx.nLockTime @@ -495,7 +493,6 @@ class CTransaction(object): def deserialize(self, f): self.nVersion = struct.unpack("(pubkey.begin(), pubkey.end()); } tx.vout.push_back(txout); - tx.nTxFee -= value; } static void MutateTxAddOutPubKey(CMutableTransaction& tx, const std::string& strInput) @@ -414,7 +412,6 @@ static void MutateTxAddOutData(CMutableTransaction& tx, const std::string& strIn CTxOut txout(BITCOINID, value, CScript() << OP_RETURN << data); tx.vout.push_back(txout); - tx.nTxFee -= value; } static void MutateTxBlind(CMutableTransaction& tx, const std::string& strInput) @@ -517,13 +514,10 @@ static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& str // construct TxOut, append to transaction output list CTxOut txout(BITCOINID, value, scriptPubKey); tx.vout.push_back(txout); - tx.nTxFee -= value; } static void MutateTxDelInput(CMutableTransaction& tx, const std::string& strInIdx) { - // TODO: reduce nTxFee - // parse requested deletion index int inIdx = atoi(strInIdx); if (inIdx < 0 || inIdx >= (int)tx.vin.size()) { @@ -537,8 +531,6 @@ static void MutateTxDelInput(CMutableTransaction& tx, const std::string& strInId static void MutateTxDelOutput(CMutableTransaction& tx, const std::string& strOutIdx) { - // TODO: increase nTxFee - // parse requested deletion index int outIdx = atoi(strOutIdx); if (outIdx < 0 || outIdx >= (int)tx.vout.size()) { diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index ed21cfbd0b..07ecd8e13f 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -135,8 +135,8 @@ std::string CTxOut::ToString() const return strprintf("CTxOut(%snValue=%s, scriptPubKey=%s)", strAsset, (nValue.IsAmount() ? strprintf("%d.%08d", nValue.GetAmount() / COIN, nValue.GetAmount() % COIN) : std::string("UNKNOWN")), HexStr(scriptPubKey).substr(0, 30)); } -CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nTxFee(0), vin(), vout(), nLockTime(0) {} -CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), nTxFee(tx.nTxFee), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime) {} +CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), vin(), vout(), nLockTime(0) {} +CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime) {} uint256 CMutableTransaction::GetHash() const { @@ -156,10 +156,34 @@ uint256 CTransaction::GetWitnessHash() const return SerializeHash(*this, SER_GETHASH, 0); } +bool CTransaction::HasValidFee() const +{ + CAmount totalFee = 0; + for (unsigned int i = 0; i < vout.size(); i++) { + CAmount fee = 0; + if (vout[i].IsFee()) + fee = vout[i].nValue.GetAmount(); + if (!MoneyRange(fee)) { + return false; + } + totalFee += fee; + } + return MoneyRange(totalFee); +} + +CAmount CTransaction::GetFee() const +{ + CAmount fee = 0; + for (unsigned int i = 0; i < vout.size(); i++) + if (vout[i].IsFee()) + fee += vout[i].nValue.GetAmount(); + return fee; +} + /* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */ -CTransaction::CTransaction() : nVersion(CTransaction::CURRENT_VERSION), nTxFee(0), vin(), vout(), nLockTime(0), hash() {} -CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), nTxFee(tx.nTxFee), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), hash(ComputeHash()) {} -CTransaction::CTransaction(CMutableTransaction &&tx) : nVersion(tx.nVersion), nTxFee(tx.nTxFee), vin(std::move(tx.vin)), vout(std::move(tx.vout)), nLockTime(tx.nLockTime), hash(ComputeHash()) {} +CTransaction::CTransaction() : nVersion(CTransaction::CURRENT_VERSION), vin(), vout(), nLockTime(0), hash() {} +CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), hash(ComputeHash()) {} +CTransaction::CTransaction(CMutableTransaction &&tx) : nVersion(tx.nVersion), vin(std::move(tx.vin)), vout(std::move(tx.vout)), nLockTime(tx.nLockTime), hash(ComputeHash()) {} double CTransaction::ComputePriority(double dPriorityInputs, unsigned int nTxSize) const { @@ -194,11 +218,16 @@ unsigned int CTransaction::GetTotalSize() const std::string CTransaction::ToString() const { + CAmount fee = 0; + for (unsigned int i = 0; i < vout.size(); i++) + if (vout[i].IsFee()) + fee += vout[i].nValue.GetAmount(); + std::string str; str += strprintf("CTransaction(hash=%s, ver=%d, fee=%d.%08d, vin.size=%u, vout.size=%u, nLockTime=%u)\n", GetHash().ToString().substr(0,10), nVersion, - nTxFee / COIN, nTxFee % COIN, + fee / COIN, fee % COIN, vin.size(), vout.size(), nLockTime); diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index c3d40d8c9c..16deba00c1 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -367,6 +367,14 @@ public: return (nValue.GetAmount() < GetDustThreshold(minRelayTxFee)); } + bool IsFee() const + { + uint256 assetid; + if (scriptPubKey == CScript() && nValue.IsAmount() && nAsset.GetAssetID(assetid) && assetid == BITCOINID) + return true; + return false; + } + friend bool operator==(const CTxOut& a, const CTxOut& b) { return (a.nValue == b.nValue && @@ -466,14 +474,12 @@ struct CMutableTransaction; /** * Basic transaction serialization format: * - int32_t nVersion - * - int32_t nTxFee * - std::vector vin * - std::vector vout * - uint32_t nLockTime * * Extended transaction serialization format: * - int32_t nVersion - * - int32_t nTxFee * - unsigned char dummy = 0x00 * - unsigned char flags (!= 0) * - std::vector vin @@ -489,11 +495,6 @@ inline void UnserializeTransaction(TxType& tx, Stream& s) { const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS); const bool fIsBitcoinTx = (s.GetVersion() & SERIALIZE_BITCOIN_BLOCK_OR_TX); s >> tx.nVersion; - if (!fIsBitcoinTx) { - s >> tx.nTxFee; - } else { - tx.nTxFee = -42; - } unsigned char flags = 0; tx.vin.clear(); tx.vout.clear(); @@ -545,9 +546,6 @@ inline void SerializeTransaction(const TxType& tx, Stream& s) { const bool fIsBitcoinTx = (s.GetVersion() & SERIALIZE_BITCOIN_BLOCK_OR_TX); s << tx.nVersion; - if (!fIsBitcoinTx) { - s << tx.nTxFee; - } unsigned char flags = 0; // Consistency check @@ -609,7 +607,6 @@ public: // and bypass the constness. This is safe, as they update the entire // structure, including the hash. const int32_t nVersion; - const CAmount nTxFee; const std::vector vin; // The bitfield specifies which inputs of the transaction are used @@ -663,6 +660,12 @@ public: // Compute a hash that includes both transaction and witness data uint256 GetWitnessHash() const; + // Check if explicit TX fees overflow or are negative + bool HasValidFee() const; + + // Compute the fee from the explicit fee outputs. Must call HasValidFee first + CAmount GetFee() const; + // Compute priority, given priority of inputs and (optionally) tx size double ComputePriority(double dPriorityInputs, unsigned int nTxSize=0) const; @@ -708,7 +711,6 @@ public: struct CMutableTransaction { int32_t nVersion; - CAmount nTxFee; std::vector vin; std::vector vout; uint32_t nLockTime; diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 6af00410db..900ac39a66 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -211,7 +211,7 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, TransactionReco strHTML += "" + tr("Total credit") + ": " + BitcoinUnits::formatHtmlWithUnit(unit, nValue) + "
"; } - CAmount nTxFee = wtx.tx->nTxFee; + CAmount nTxFee = wtx.tx->GetFee(); if (nTxFee > 0) strHTML += "" + tr("Transaction fee") + ": " + BitcoinUnits::formatHtmlWithUnit(unit, -nTxFee) + "
"; } diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 30354948e5..010c84b890 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -113,7 +113,7 @@ QList TransactionRecord::decomposeTransaction(const CWallet * // // Debit // - CAmount nTxFee = wtx.tx->nTxFee; + CAmount nTxFee = wtx.tx->GetFee(); for (unsigned int nOut = 0; nOut < wtx.tx->vout.size(); nOut++) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 287ac2923f..54b4c91e9f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -94,7 +94,7 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) entry.push_back(Pair("vsize", (int)::GetVirtualTransactionSize(tx))); entry.push_back(Pair("version", tx.nVersion)); entry.push_back(Pair("locktime", (int64_t)tx.nLockTime)); - entry.push_back(Pair("fee", ValueFromAmount(tx.nTxFee))); + entry.push_back(Pair("fee", ValueFromAmount(tx.GetFee()))); UniValue vin(UniValue::VARR); for (unsigned int i = 0; i < tx.vin.size(); i++) { @@ -571,8 +571,6 @@ UniValue createrawtransaction(const JSONRPCRequest& request) } } - rawTx.nTxFee = inputValue[bitcoinid] - outputValue[bitcoinid]; - return EncodeHexTx(rawTx); } diff --git a/src/script/script.h b/src/script/script.h index 44d5e13d32..30cd835d01 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -655,11 +655,11 @@ public: /** * Returns whether the script is guaranteed to fail at execution, * regardless of the initial stack. This allows outputs to be pruned - * instantly when entering the UTXO set. + * instantly when entering the UTXO set. This includes fee outputs. */ bool IsUnspendable() const { - return (size() > 0 && *begin() == OP_RETURN) || (size() > MAX_SCRIPT_SIZE); + return (size() > 0 && *begin() == OP_RETURN) || (size() > MAX_SCRIPT_SIZE) || (size() == 0); } void clear() diff --git a/src/test/blind_tests.cpp b/src/test/blind_tests.cpp index 3df0c0a785..5eb6b585f8 100644 --- a/src/test/blind_tests.cpp +++ b/src/test/blind_tests.cpp @@ -70,8 +70,7 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) tx3.vout.resize(1); tx3.vout[0].nValue = 100; tx3.vout[0].nAsset = bitcoinID; - tx3.nTxFee = 22; - BOOST_CHECK(VerifyAmounts(cache, tx3, tx3.nTxFee, bitcoinID)); + BOOST_CHECK(VerifyAmounts(cache, tx3)); // Try to blind with a single output, which fails as its blinding factor ends up being zero. std::vector input_blinds; @@ -92,7 +91,7 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) // BOOST_CHECK(BlindOutputs(input_blinds, output_blinds, output_pubkeys, tx3)); BOOST_CHECK(!tx3.vout[0].nValue.IsAmount()); BOOST_CHECK(!tx3.vout[1].nValue.IsAmount()); - BOOST_CHECK(VerifyAmounts(cache, tx3, tx3.nTxFee, bitcoinID)); + BOOST_CHECK(VerifyAmounts(cache, tx3)); CAmount unblinded_amount; // BOOST_CHECK(UnblindOutput(key2, tx3.vout[0], unblinded_amount, blind3) == 0); @@ -106,8 +105,7 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) in3->vout[0] = tx3.vout[0]; in3->vout[1] = tx3.vout[1]; - tx3.nTxFee--; - BOOST_CHECK(!VerifyAmounts(cache, tx3, tx3.nTxFee, bitcoinID)); + BOOST_CHECK(!VerifyAmounts(cache, tx3)); } { @@ -123,8 +121,7 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) tx4.vout[1].nValue = 40; tx4.vout[0].nAsset = bitcoinID; tx4.vout[1].nAsset = bitcoinID; - tx4.nTxFee = 100 + 111 - 30 - 40; - BOOST_CHECK(!VerifyAmounts(cache, tx4, tx4.nTxFee, bitcoinID)); // Spends a blinded coin with no blinded outputs to compensate. + BOOST_CHECK(!VerifyAmounts(cache, tx4)); // Spends a blinded coin with no blinded outputs to compensate. std::vector input_blinds; std::vector output_blinds; @@ -153,8 +150,7 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) tx4.vout[0].nAsset = bitcoinID; tx4.vout[1].nAsset = bitcoinID; tx4.vout[2].nAsset = bitcoinID; - tx4.nTxFee = 100 + 111 - 30 - 40 - 50; - BOOST_CHECK(!VerifyAmounts(cache, tx4, tx4.nTxFee, bitcoinID)); // Spends a blinded coin with no blinded outputs to compensate. + BOOST_CHECK(!VerifyAmounts(cache, tx4)); // Spends a blinded coin with no blinded outputs to compensate. std::vector input_blinds; std::vector output_blinds; @@ -171,7 +167,7 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) BOOST_CHECK(!tx4.vout[0].nValue.IsAmount()); BOOST_CHECK(tx4.vout[1].nValue.IsAmount()); BOOST_CHECK(!tx4.vout[2].nValue.IsAmount()); - BOOST_CHECK(VerifyAmounts(cache, tx4, tx4.nTxFee, bitcoinID)); + BOOST_CHECK(VerifyAmounts(cache, tx4)); /* #ifdef ENABLE_WALLET //This tests the wallet blinding caching functionality @@ -225,8 +221,7 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) in4->vout[1] = tx4.vout[1]; in4->vout[2] = tx4.vout[2]; - tx4.nTxFee--; - BOOST_CHECK(!VerifyAmounts(cache, tx4, tx4.nTxFee, bitcoinID)); */ + BOOST_CHECK(!VerifyAmounts(cache, tx4)); */ } } diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index d2a31c7320..f30fe43c32 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -248,7 +248,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout.resize(1); tx.vout[0].scriptPubKey = CScript() << OP_TRUE; tx.vout[0].nValue = CTxOutValue(GENESISVALUE); - tx.nTxFee = 0; sighash = SignatureHash(genScriptPubKey, tx, 0, SIGHASH_ALL, 0, SIGVERSION_BASE); coinbaseKey.Sign(sighash, vchSig); @@ -281,7 +280,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) for (unsigned int i = 0; i < 1001; ++i) { tx.vout[0].nValue = CTxOutValue(tx.vout[0].nValue.GetAmount() - LOWFEE); - tx.nTxFee = LOWFEE; hash = tx.GetHash(); // If we don't set the # of sig ops in the CTxMemPoolEntry, template creation fails @@ -301,12 +299,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.hash = firstCoin; tx.vout[0].nValue = CTxOutValue(GENESISVALUE); tx.vout[0].scriptPubKey = CScript(); - tx.nTxFee = LOWFEE; for (unsigned int i = 0; i < 1001; ++i) { tx.vout[0].nValue = CTxOutValue(tx.vout[0].nValue.GetAmount() - LOWFEE); - tx.nTxFee = LOWFEE; hash = tx.GetHash(); // If we do set the # of sig ops in the CTxMemPoolEntry, template creation passes @@ -346,12 +342,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].scriptSig = CScript() << OP_1; tx.vin[0].prevout.hash = firstCoin; tx.vout[0].nValue = CTxOutValue(GENESISVALUE - HIGHFEE); - tx.nTxFee = HIGHFEE; hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); tx.vin[0].prevout.hash = hash; tx.vout[0].nValue = CTxOutValue(tx.vout[0].nValue.GetAmount()-HIGHERFEE); - tx.nTxFee = HIGHERFEE; tx.vout[0].nValue = CTxOutValue(tx.vout[0].nValue.GetAmount()+BLOCKSUBSIDY-HIGHERFEE); //First txn output + fresh coinbase - new txn fee hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(HIGHERFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); @@ -363,7 +357,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.SetNull(); tx.vin[0].scriptSig = CScript() << OP_0 << OP_1; tx.vout[0].nValue = CTxOutValue(0); - tx.nTxFee = LOWFEE; hash = tx.GetHash(); // give it a fee so it'll get mined mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); @@ -375,7 +368,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.n = 0; tx.vin[0].scriptSig = CScript() << OP_1; tx.vout[0].nValue = CTxOutValue(GENESISVALUE - HIGHFEE); - tx.nTxFee = HIGHFEE; script = CScript() << OP_0; tx.vout[0].scriptPubKey = GetScriptForDestination(CScriptID(script)); hash = tx.GetHash(); @@ -383,7 +375,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.hash = hash; tx.vin[0].scriptSig = CScript() << std::vector(script.begin(), script.end()); tx.vout[0].nValue = CTxOutValue(tx.vout[0].nValue.GetAmount() - LOWFEE); - tx.nTxFee = LOWFEE; hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); BOOST_CHECK_THROW(BlockAssembler(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error); @@ -393,7 +384,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.hash = firstCoin; tx.vin[0].scriptSig = CScript() << OP_1; tx.vout[0].nValue = CTxOutValue(GENESISVALUE - HIGHFEE); - tx.nTxFee = HIGHFEE; tx.vout[0].scriptPubKey = CScript() << OP_1; hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); @@ -454,7 +444,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].scriptSig = CScript() << OP_1; tx.vin[0].nSequence = chainActive.Tip()->nHeight + 1; // txFirst[0] is the 2nd block tx.vout[0].nValue = CTxOutValue(GENESISVALUE - HIGHFEE); - tx.nTxFee = HIGHFEE; prevheights[0] = baseheight + 1; tx.vout.resize(1); tx.vout[0].scriptPubKey = CScript() << OP_1; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 04e620c880..827334e90c 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -340,10 +340,10 @@ BOOST_AUTO_TEST_CASE(test_Get) t1.vout.resize(1); t1.vout[0].nValue = 90*CENT; t1.vout[0].scriptPubKey << OP_1; - t1.nTxFee = (50+21+22)*CENT - 90*CENT; + BOOST_CHECK(CTransaction(t1).GetFee() == (50+21+22)*CENT - 90*CENT); BOOST_CHECK(AreInputsStandard(t1, coins)); - BOOST_CHECK(VerifyAmounts(coins, t1, t1.nTxFee, BITCOINID)); + BOOST_CHECK(VerifyAmounts(coins, t1)); } void CreateCreditAndSpend(const CKeyStore& keystore, const CScript& outscript, CTransactionRef& output, CMutableTransaction& input, bool success = true) diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 0b69334489..c5367208ba 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -46,7 +46,6 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) spends[i].vout.resize(1); spends[i].vout[0].nValue = 11*CENT; spends[i].vout[0].scriptPubKey = scriptPubKey; - spends[i].nTxFee = coinbaseTxns[0].vout[0].nValue.GetAmount() - 11*CENT; // Sign: std::vector vchSig; diff --git a/src/validation.cpp b/src/validation.cpp index c7b157acda..db838346e7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -529,10 +529,10 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe if (tx.IsCoinBase()) { - // Coinbase transactions may not have eccessive scriptSigs or fees + // Coinbase transactions may not have eccessive scriptSigs if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100) return state.DoS(100, false, REJECT_INVALID, "bad-cb-length"); - if (tx.nTxFee != 0) + if (tx.GetFee() != 0) return state.DoS(100, false, REJECT_INVALID, "bad-cb-fee"); } else @@ -652,7 +652,7 @@ bool CSurjectionCheck::operator()() -bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, const CAmount& excess, const uint256& excessID, std::vector* pvChecks, const bool cacheStore) +bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, std::vector* pvChecks, const bool cacheStore) { assert(!tx.IsCoinBase()); @@ -762,20 +762,6 @@ bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, const C } - // Add fee to tally - if (excess != 0) { - assert(secp256k1_generator_generate(secp256k1_ctx_verify_amounts, &gen, excessID.begin())); - if (secp256k1_pedersen_commit(secp256k1_ctx_verify_amounts, &commit, explBlinds, excess > 0 ? excess : -excess, &gen) != 1) - return false; - - memcpy(p, &commit, sizeof(secp256k1_pedersen_commitment)); - if (excess > 0) - vpCommitsOut.push_back(p); - else - vpCommitsIn.push_back(p); - p++; - } - // Check balance if (!QueueCheck(pvChecks, new CBalanceCheck(vData, vpCommitsIn, vpCommitsOut))) { return false; @@ -1044,8 +1030,11 @@ 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(); + // nModifiedFees includes any fee deltas from PrioritiseTransaction - CAmount nFees = tx.nTxFee; CAmount nModifiedFees = nFees; double nPriorityDummy = 0; pool.ApplyDeltas(hash, nPriorityDummy, nModifiedFees); @@ -1849,7 +1838,6 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins return state.Invalid(false, 0, "", "Inputs unavailable"); CAmount nValueIn = 0; - CAmount nFees = 0; for (unsigned int i = 0; i < tx.vin.size(); i++) { const COutPoint &prevout = tx.vin[i].prevout; @@ -1886,14 +1874,9 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins } // Tally transaction fees - CAmount nTxFee = tx.nTxFee; - if (nTxFee < 0) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-negative"); - nFees += nTxFee; - if (!MoneyRange(nFees)) + if (!tx.HasValidFee()) return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange"); - - if (!VerifyAmounts(inputs, tx, nTxFee, BITCOINID, pvChecks, cacheStore)) + 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))); @@ -2548,8 +2531,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin txdata.emplace_back(tx); if (!tx.IsCoinBase()) { - nFees += tx.nTxFee; - std::vector vChecks; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, txdata[i], setWithdrawsSpent == NULL ? setWithdrawsSpentDummy : *setWithdrawsSpent, nScriptCheckThreads ? &vChecks : NULL)) @@ -2572,6 +2553,11 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin if (txout.scriptPubKey.IsWithdrawLock() && txout.nValue.IsAmount()) 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"); + nFees += tx.GetFee(); + if (!MoneyRange(nFees)) + return state.DoS(100, error("ConnectBlock(): total block reward overflowed"), REJECT_INVALID, "bad-blockreward-outofrange"); } int64_t nTime3 = GetTimeMicros(); nTimeConnect += nTime3 - nTime2; LogPrint("bench", " - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs]\n", (unsigned)block.vtx.size(), 0.001 * (nTime3 - nTime2), 0.001 * (nTime3 - nTime2) / block.vtx.size(), nInputs <= 1 ? 0 : 0.001 * (nTime3 - nTime2) / (nInputs-1), nTimeConnect * 0.000001); diff --git a/src/validation.h b/src/validation.h index 736e7b3b86..b5a1e28942 100644 --- a/src/validation.h +++ b/src/validation.h @@ -400,13 +400,11 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins * * @param[in] view CCoinsViewCache to find necessary outputs * @param[in] tx transaction for which we are checking totals - * @param[in] excess additional amount to consider as input value (eg fees), can be negative - * @param[in] excessID the asset id of the additional amount * @param[in] pvChecks multithreaded rangeproof and commitment checker * @param[in] cacheStore signal if rangeproof verification should be cached * @return True if totals are identical */ -bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, const CAmount& excess, const uint256& excessID, std::vector* pvChecks = NULL, const bool cacheStore = false); +bool VerifyAmounts(const CCoinsViewCache& cache, const CTransaction& tx, std::vector* pvChecks = NULL, const bool cacheStore = false); /** * Verify the amounts of coinbase transactions. It will fail for any blinded amount or type. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 720c76485b..c5bfdbbc63 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1960,7 +1960,8 @@ UniValue gettransaction(const JSONRPCRequest& request) CAmountMap nCredit = wtx.GetCredit(filter); CAmountMap nDebit = wtx.GetDebit(filter); - CAmount nFee = (wtx.IsFromMe(filter) ? -wtx.tx->nTxFee : 0); + assert(wtx.tx->HasValidFee()); + CAmount nFee = (wtx.IsFromMe(filter) ? -wtx.tx->GetFee() : 0); CAmountMap nNet = nCredit - nDebit; nNet[pwalletMain->GetAssetIDFromLabel("bitcoin")] -= nFee; @@ -3014,7 +3015,7 @@ UniValue bumpfee(const JSONRPCRequest& request) } // calculate the old fee and fee-rate - CAmount nOldFee = wtx.tx->nTxFee; + CAmount nOldFee = wtx.tx->GetFee(); CFeeRate nOldFeeRate(nOldFee, txSize); CAmount nNewFee; CFeeRate nNewFeeRate; @@ -3547,7 +3548,6 @@ UniValue claimpegin(const JSONRPCRequest& request) mtxn.vin.push_back(txin); mtxn.vout.push_back(txout); mtxn.vout.push_back(txrelock); - mtxn.nTxFee = 0; //No signing needed, just send CTransaction finalTxn(mtxn); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d217d066d4..f115a2ab28 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1466,7 +1466,7 @@ void CWalletTx::GetAmounts(list& listReceived, CAmountMap nDebit = GetDebit(filter); if (nDebit > CAmountMap()) // debit>0 means we signed/sent this transaction { - nFee = tx->nTxFee; + nFee = tx->GetFee(); } CTxDestination addressUnaccounted = CNoDestination(); @@ -2860,7 +2860,6 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second,CScript(), std::numeric_limits::max() - (fWalletRbf ? 2 : 1))); - txNew.nTxFee = nFeeRet; LogPrintf("Created transaction (before blinding): %s", CTransaction(txNew).ToString()); // Store amounts for storage in mapValue @@ -2914,6 +2913,13 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt // Fill in dummy signatures for fee calculation. if (!DummySignTx(txNew, setCoins)) { + } + + // Add fee + if (nFeeRet > 0) { + CTxOut fee(BITCOINID, nFeeRet, CScript()); + assert(fee.IsFee()); + txNew.vout.push_back(fee); strFailReason = _("Signing transaction failed"); return false; }