diff --git a/src/miner.h b/src/miner.h --- a/src/miner.h +++ b/src/miner.h @@ -56,7 +56,7 @@ uint64_t GetVirtualSizeWithAncestors() const; Amount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } size_t GetTxSize() const { return iter->GetTxSize(); } - size_t GetTxVirtualSize() const { return iter->GetTxVirtualSize(); } + size_t GetIncrementalVirtualSize() const; const CTransaction &GetTx() const { return iter->GetTx(); } CTxMemPool::txiter iter; diff --git a/src/miner.cpp b/src/miner.cpp --- a/src/miner.cpp +++ b/src/miner.cpp @@ -63,6 +63,14 @@ nSigOpCountWithAncestors); } +uint64_t CTxMemPoolModifiedEntry::GetIncrementalVirtualSize() const { + auto vsize_with_ancestors = GetVirtualSizeWithAncestors(); + auto vsize_only_ancestors = GetVirtualTransactionSize( + nSizeWithAncestors - iter->GetTxSize(), + nSigOpCountWithAncestors - iter->GetSigOpCount()); + return vsize_with_ancestors - vsize_only_ancestors; +} + BlockAssembler::Options::Options() : nExcessiveBlockSize(DEFAULT_MAX_BLOCK_SIZE), nMaxGeneratedBlockSize(DEFAULT_MAX_GENERATED_BLOCK_SIZE), diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -113,6 +113,9 @@ testPool.addUnchecked( entry.Fee(randFee).SigOpCount(randSigOpCount).FromTx(tx)); + uint64_t txVirtualSize = GetVirtualTransactionSize( + CTransaction(tx).GetTotalSize(), randSigOpCount); + // Add this transaction to the totals. minAncestors += 1; maxAncestors += 1; @@ -125,16 +128,14 @@ // is monotonically increasing in each argument, we can say the // following: minVirtualSize += 0; - maxVirtualSize += GetVirtualTransactionSize( - CTransaction(tx).GetTotalSize(), randSigOpCount); + maxVirtualSize += txVirtualSize; minSigOpCount += randSigOpCount; maxSigOpCount += randSigOpCount; // Calculate overall values totalFee += randFee; totalSize += CTransaction(tx).GetTotalSize(); - totalVirtualSize += GetVirtualTransactionSize( - CTransaction(tx).GetTotalSize(), randSigOpCount); + totalVirtualSize += txVirtualSize; totalSigOpCount += randSigOpCount; CTxMemPoolEntry parentEntry = *testPool.mapTx.find(parentOfAllId); CTxMemPoolEntry latestEntry = *testPool.mapTx.find(curId); @@ -154,6 +155,7 @@ BOOST_CHECK(totalVirtualSize_strict <= totalVirtualSize); // Ensure values are within the expected ranges + BOOST_CHECK(latestEntry.GetIncrementalVirtualSize() <= txVirtualSize); BOOST_CHECK(latestEntry.GetCountWithAncestors() >= minAncestors); BOOST_CHECK(latestEntry.GetCountWithAncestors() <= maxAncestors); @@ -174,8 +176,9 @@ BOOST_CHECK_EQUAL(parentEntry.GetCountWithDescendants(), testPool.mapTx.size()); BOOST_CHECK_EQUAL(parentEntry.GetSizeWithDescendants(), totalSize); - BOOST_CHECK_EQUAL(parentEntry.GetVirtualSizeWithDescendants(), - totalVirtualSize_strict); + BOOST_CHECK_EQUAL( + parentEntry.GetIncrementalVirtualSizeWithDescendants(), + totalVirtualSize_strict); BOOST_CHECK_EQUAL(parentEntry.GetModFeesWithDescendants(), totalFee); BOOST_CHECK_EQUAL(parentEntry.GetSigOpCountWithDescendants(), totalSigOpCount); diff --git a/src/txmempool.h b/src/txmempool.h --- a/src/txmempool.h +++ b/src/txmempool.h @@ -114,7 +114,6 @@ CTransactionRef GetSharedTx() const { return this->tx; } const Amount GetFee() const { return nFee; } size_t GetTxSize() const { return nTxSize; } - size_t GetTxVirtualSize() const; int64_t GetTime() const { return nTime; } unsigned int GetHeight() const { return entryHeight; } @@ -137,7 +136,6 @@ uint64_t GetCountWithDescendants() const { return nCountWithDescendants; } uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } - uint64_t GetVirtualSizeWithDescendants() const; Amount GetModFeesWithDescendants() const { return nModFeesWithDescendants; } int64_t GetSigOpCountWithDescendants() const { return nSigOpCountWithDescendants; @@ -153,6 +151,15 @@ return nSigOpCountWithAncestors; } + // These incremental virtual sizes take into account the baseline + // multidimensional resources consumed by ancestors, and may be smaller (but + // not larger) than the virtual sizes that would be calculated ignoring + // ancestors. + // Prior to filling in ancestor state, these act as if there are no + // ancestors. + uint64_t GetIncrementalVirtualSize() const; + uint64_t GetIncrementalVirtualSizeWithDescendants() const; + //!< Index in mempool's vTxHashes mutable size_t vTxHashesIdx; }; @@ -250,19 +257,22 @@ // Return the fee/size we're using for sorting this entry. void GetModFeeAndSize(const CTxMemPoolEntry &a, double &mod_fee, double &size) const { + // note: these could be both 0 + auto vsize_inc_with_descendants = + a.GetIncrementalVirtualSizeWithDescendants(); + auto vsize_inc_justme = a.GetIncrementalVirtualSize(); // Compare feerate with descendants to feerate of the transaction, and // return the fee/size for the max. - double f1 = - a.GetVirtualSizeWithDescendants() * (a.GetModifiedFee() / SATOSHI); + double f1 = vsize_inc_with_descendants * (a.GetModifiedFee() / SATOSHI); double f2 = - a.GetTxVirtualSize() * (a.GetModFeesWithDescendants() / SATOSHI); + vsize_inc_justme * (a.GetModFeesWithDescendants() / SATOSHI); if (f2 > f1) { mod_fee = a.GetModFeesWithDescendants() / SATOSHI; - size = a.GetVirtualSizeWithDescendants(); + size = vsize_inc_with_descendants; } else { mod_fee = a.GetModifiedFee() / SATOSHI; - size = a.GetTxVirtualSize(); + size = vsize_inc_justme; } } }; @@ -319,19 +329,20 @@ // Return the fee/size we're using for sorting this entry. template void GetModFeeAndSize(const T &a, double &mod_fee, double &size) const { + auto vsize_with_ancestors = a.GetVirtualSizeWithAncestors(); + // note: this could be 0 + auto vsize_inc_justme = a.GetIncrementalVirtualSize(); // Compare feerate with ancestors to feerate of the transaction, and // return the fee/size for the min. - double f1 = - a.GetVirtualSizeWithAncestors() * (a.GetModifiedFee() / SATOSHI); - double f2 = - a.GetTxVirtualSize() * (a.GetModFeesWithAncestors() / SATOSHI); + double f1 = vsize_with_ancestors * (a.GetModifiedFee() / SATOSHI); + double f2 = vsize_inc_justme * (a.GetModFeesWithAncestors() / SATOSHI); if (f1 > f2) { mod_fee = a.GetModFeesWithAncestors() / SATOSHI; - size = a.GetVirtualSizeWithAncestors(); + size = vsize_with_ancestors; } else { mod_fee = a.GetModifiedFee() / SATOSHI; - size = a.GetTxVirtualSize(); + size = vsize_inc_justme; } } }; diff --git a/src/txmempool.cpp b/src/txmempool.cpp --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -46,22 +46,26 @@ nSigOpCountWithAncestors = sigOpCount; } -size_t CTxMemPoolEntry::GetTxVirtualSize() const { - return GetVirtualTransactionSize(nTxSize, sigOpCount); +uint64_t CTxMemPoolEntry::GetVirtualSizeWithAncestors() const { + return GetVirtualTransactionSize(nSizeWithAncestors, + nSigOpCountWithAncestors); } -uint64_t CTxMemPoolEntry::GetVirtualSizeWithDescendants() const { - // note this is distinct from the sum of descendants' individual virtual - // sizes, and may be smaller. - return GetVirtualTransactionSize(nSizeWithDescendants, - nSigOpCountWithDescendants); +uint64_t CTxMemPoolEntry::GetIncrementalVirtualSize() const { + uint64_t vsize_with_me = GetVirtualSizeWithAncestors(); + uint64_t vsize_without_me = GetVirtualTransactionSize( + nSizeWithAncestors - nTxSize, nSigOpCountWithAncestors - sigOpCount); + return vsize_with_me - vsize_without_me; } -uint64_t CTxMemPoolEntry::GetVirtualSizeWithAncestors() const { - // note this is distinct from the sum of ancestors' individual virtual - // sizes, and may be smaller. - return GetVirtualTransactionSize(nSizeWithAncestors, - nSigOpCountWithAncestors); +uint64_t CTxMemPoolEntry::GetIncrementalVirtualSizeWithDescendants() const { + // "us" = me + descendants + uint64_t vsize_with_us = GetVirtualTransactionSize( + nSizeWithAncestors + nSizeWithDescendants - nTxSize, + nSigOpCountWithAncestors + nSigOpCountWithDescendants - sigOpCount); + uint64_t vsize_without_us = GetVirtualTransactionSize( + nSizeWithAncestors - nTxSize, nSigOpCountWithAncestors - sigOpCount); + return vsize_with_us - vsize_without_us; } void CTxMemPoolEntry::UpdateFeeDelta(Amount newFeeDelta) { @@ -1190,7 +1194,7 @@ // mempool with feerate equal to txn which were removed with no block in // between. CFeeRate removed(it->GetModFeesWithDescendants(), - it->GetVirtualSizeWithDescendants()); + it->GetIncrementalVirtualSizeWithDescendants()); removed += MEMPOOL_FULL_FEE_INCREMENT; trackPackageRemoved(removed); diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -701,7 +701,13 @@ CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, chainActive.Height(), fSpendsCoinbase, nSigOpsCount, lp); unsigned int nSize = entry.GetTxSize(); - unsigned int nVirtualSize = entry.GetTxVirtualSize(); + // Calculate virtual size for this tx in isolation. + // In principle we could update the ancestor info of the entry and + // use GetIncrementalVirtualSize which may be smaller, however this + // would create some odd 'parent pays for child' situations in times + // of mempool congestion. + unsigned int nVirtualSize = + GetVirtualTransactionSize(nSize, nSigOpsCount); // No transactions are allowed below minRelayTxFee except from // disconnected blocks. diff --git a/test/functional/abc-sigops-mempool-mining.py b/test/functional/abc-sigops-mempool-mining.py --- a/test/functional/abc-sigops-mempool-mining.py +++ b/test/functional/abc-sigops-mempool-mining.py @@ -33,6 +33,7 @@ P2PDataStore, ) from test_framework.script import ( + OP_CHECKSIG, OP_CHECKMULTISIG, OP_RETURN, OP_TRUE, @@ -185,6 +186,20 @@ self.spendable_outputs.popleft(), script_4000_sigops, 300, 80000) txid_highsigops = node.sendrawtransaction(ToHex(ctx)) + self.log.info( + "Create a mixed-sigops package where child would be low sat/vbyte in isolation.") + # Parent is 0-sigops at 1.5 sat/byte. + ctx = create_var_transaction( + self.spendable_outputs.popleft(), b'', 1000, 1500) + txid_parent2 = node.sendrawtransaction(ToHex(ctx)) + # Child is 0.32 sat/vbyte in isolation (400 sat / 1250 vbyte), but + # incrementally is 1.6 sat/vbyte (+400 sat / +250 vbyte). Whole package + # is 1.52 sat/vbyte (1900 sat / 1250 vbyte). + script_25_sigops = bytes([OP_CHECKSIG] * 25) + ctx_child = create_var_transaction(ctx, + script_25_sigops, 200, 400) + txid_child2 = node.sendrawtransaction(ToHex(ctx_child)) + self.log.info( "Flood the mempool with 10-kB transactions @ 9.9 sat/byte and 0.495 sat/vbyte, until it is full") # About 500 should fill up our 5 MB mempool. @@ -215,8 +230,10 @@ assert txid_highsigops not in mempool_txids # Other txes did get kept, including txid_parent which had ultralow - # virtual feerate (but its child supplied enough fee). - settxids = set([txid1, txid_parent, txid_child]) + # virtual feerate (but its child supplied enough fee), and txid_child2 + # where its high sigops density was cancelled by its low sigops parent. + settxids = set([txid1, txid_parent, txid_child, + txid_parent2, txid_child2]) assert_equal(settxids.difference(mempool_txids), set()) self.log.info('Mempool fee floor has jumped to 1.495 sat/vbyte') @@ -225,6 +242,22 @@ assert_equal(node.getmempoolinfo()[ 'mempoolminfee'], Decimal('0.00001495')) + self.log.info( + "Try a child with low sat/vbyte the same way now.") + # Parent is 0-sigops at 1.5 sat/byte. + ctx = create_var_transaction( + self.spendable_outputs.popleft(), b'', 1000, 1500) + txid_parent3 = node.sendrawtransaction(ToHex(ctx)) + # Child is 0.32 sat/vbyte in isolation (400 sat / 1250 vbyte), which + # causes it to be rejected, even though incrementally it would be 1.6 + # sat/vbyte (+400 sat / +250 vbyte) and it wouldn't be evicted if it had + # gotten in before the flooding (see txid_child2). + ctx_child = create_var_transaction(ctx, + script_25_sigops, 200, 400) + assert_raises_rpc_error(-26, "mempool min fee not met", node.sendrawtransaction, + ToHex(ctx_child)) + settxids.add(txid_parent3) + self.log.info( 'Broadcasting a regular tx still works, because wallet knows what fee to use.') txid2 = node.sendtoaddress(node.getnewaddress(), 0.001)