diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 0358aeae938..60783375ff8 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -303,7 +303,11 @@ static RPCMethod testmempoolaccept() {RPCResult::Type::STR, "package-error", /*optional=*/true, "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."}, {RPCResult::Type::BOOL, "allowed", /*optional=*/true, "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate. " "If not present, the tx was not fully validated due to a failure in another tx in the list."}, - {RPCResult::Type::NUM, "vsize", /*optional=*/true, "Virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted (only present when 'allowed' is true)"}, + {RPCResult::Type::NUM, "vsize_adjusted", /*optional=*/true, "Maximum of sigop-adjusted size (-bytespersigop) and virtual transaction size as defined in BIP 141 (only present when 'allowed' is true)."}, + {RPCResult::Type::NUM, "vsize", /*optional=*/true, "(DEPRECATED) Was previously erroneously described as the BIP 141 vsize, but is actually sigops-adjusted vsize.\n" + "Use vsize_bip141 to actually get that behavior or switch to the explicit vsize_adjusted for retained behavior."}, + {RPCResult::Type::NUM, "vsize_bip141", /*optional=*/true, "Virtual transaction size as defined in BIP 141.\n" + "This is different from actual serialized size for witness transactions as witness data is discounted (only present when 'allowed' is true)."}, {RPCResult::Type::OBJ, "fees", /*optional=*/true, "Transaction fees (only present if 'allowed' is true)", { {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, @@ -394,7 +398,9 @@ static RPCMethod testmempoolaccept() // Only return the fee and vsize if the transaction would pass ATMP. // These can be used to calculate the feerate. result_inner.pushKV("allowed", true); + result_inner.pushKV("vsize_adjusted", virtual_size); result_inner.pushKV("vsize", virtual_size); + result_inner.pushKV("vsize_bip141", GetVirtualTransactionSize(*tx)); UniValue fees(UniValue::VOBJ); fees.pushKV("base", ValueFromAmount(fee)); fees.pushKV("effective-feerate", ValueFromAmount(tx_result.m_effective_feerate.value().GetFeePerK())); @@ -443,7 +449,11 @@ static std::vector ClusterDescription() static std::vector MempoolEntryDescription() { std::vector list = { - RPCResult{RPCResult::Type::NUM, "vsize", "virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted."}, + {RPCResult::Type::NUM, "vsize", /*optional=*/true, "(DEPRECATED) Was previously erroneously described as the BIP 141 vsize, but is actually sigops-adjusted vsize.\n" + "Use vsize_bip141 to actually get that behavior or switch to the explicit vsize_adjusted for retained behavior."}, + {RPCResult::Type::NUM, "vsize_bip141", /*optional=*/true, "Virtual transaction size as defined in BIP 141.\n" + "This is different from actual serialized size for witness transactions as witness data is discounted (only present when 'allowed' is true)."}, + {RPCResult::Type::NUM, "vsize_adjusted", /*optional=*/true, "Maximum of sigop-adjusted size (-bytespersigop) and virtual transaction size as defined in BIP 141 (only present when 'allowed' is true)."}, RPCResult{RPCResult::Type::NUM, "weight", "transaction weight as defined in BIP 141."}, RPCResult{RPCResult::Type::NUM_TIME, "time", "local time transaction entered pool in seconds since 1 Jan 1970 GMT"}, RPCResult{RPCResult::Type::NUM, "height", "block height when transaction entered pool"}, @@ -527,7 +537,9 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool auto [ancestor_count, ancestor_size, ancestor_fees] = pool.CalculateAncestorData(e); auto [descendant_count, descendant_size, descendant_fees] = pool.CalculateDescendantData(e); + info.pushKV("vsize_adjusted", e.GetTxSize()); info.pushKV("vsize", e.GetTxSize()); + info.pushKV("vsize_bip141", GetVirtualTransactionSize(e.GetTx())); info.pushKV("weight", e.GetTxWeight()); info.pushKV("time", count_seconds(e.GetTime())); info.pushKV("height", e.GetHeight()); @@ -1229,7 +1241,8 @@ static std::vector OrphanDescription() RPCResult{RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, RPCResult{RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"}, RPCResult{RPCResult::Type::NUM, "bytes", "The serialized transaction size in bytes"}, - RPCResult{RPCResult::Type::NUM, "vsize", "The virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted."}, + RPCResult{RPCResult::Type::NUM, "vsize", "(DEPRECATED) use vsize_bip141 instead. The virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted."}, + RPCResult{RPCResult::Type::NUM, "vsize_bip141", "The virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted."}, RPCResult{RPCResult::Type::NUM, "weight", "The transaction weight as defined in BIP 141."}, RPCResult{RPCResult::Type::ARR, "from", "", { @@ -1245,6 +1258,7 @@ static UniValue OrphanToJSON(const node::TxOrphanage::OrphanInfo& orphan) o.pushKV("wtxid", orphan.tx->GetWitnessHash().ToString()); o.pushKV("bytes", orphan.tx->ComputeTotalSize()); o.pushKV("vsize", GetVirtualTransactionSize(*orphan.tx)); + o.pushKV("vsize_bip141", GetVirtualTransactionSize(*orphan.tx)); o.pushKV("weight", GetTransactionWeight(*orphan.tx)); UniValue from(UniValue::VARR); for (const auto fromPeer: orphan.announcers) { @@ -1358,7 +1372,10 @@ static RPCMethod submitpackage() {RPCResult::Type::OBJ, "wtxid", "transaction wtxid", { {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, {RPCResult::Type::STR_HEX, "other-wtxid", /*optional=*/true, "The wtxid of a different transaction with the same txid but different witness found in the mempool. This means the submitted transaction was ignored."}, - {RPCResult::Type::NUM, "vsize", /*optional=*/true, "Sigops-adjusted virtual transaction size."}, + {RPCResult::Type::NUM, "vsize_adjusted", /*optional=*/true, "Maximum of sigop-adjusted size (-bytespersigop) and virtual transaction size as defined in BIP 141."}, + {RPCResult::Type::NUM, "vsize", /*optional=*/true, "(DEPRECATED) Was previously erroneously described as the BIP 141 vsize, but is actually sigops-adjusted vsize.\n" + "Use vsize_bip141 to actually get that behavior or switch to the explicit vsize_adjusted for retained behavior."}, + {RPCResult::Type::NUM, "vsize_bip141", /*optional=*/true, "Virtual transaction size as defined in BIP 141."}, {RPCResult::Type::OBJ, "fees", /*optional=*/true, "Transaction fees", { {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, {RPCResult::Type::STR_AMOUNT, "effective-feerate", /*optional=*/true, "if the transaction was not already in the mempool, the effective feerate in " + CURRENCY_UNIT + " per KvB. For example, the package feerate and/or feerate with modified fees from prioritisetransaction."}, @@ -1506,7 +1523,9 @@ static RPCMethod submitpackage() break; case MempoolAcceptResult::ResultType::VALID: case MempoolAcceptResult::ResultType::MEMPOOL_ENTRY: + result_inner.pushKV("vsize_adjusted", it->second.m_vsize.value()); result_inner.pushKV("vsize", it->second.m_vsize.value()); + result_inner.pushKV("vsize_bip141", GetVirtualTransactionSize(*tx)); UniValue fees(UniValue::VOBJ); fees.pushKV("base", ValueFromAmount(it->second.m_base_fees.value())); if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) { diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 5893b1cc752..b3e3749ef75 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -136,7 +136,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): raw_tx_0 = tx.serialize().hex() txid_0 = tx.txid_hex self.check_mempool_result( - result_expected=[{'txid': txid_0, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': fee}}], + result_expected=[{'txid': txid_0, 'allowed': True, 'vsize_adjusted': tx.get_vsize(), 'vsize': tx.get_vsize(), 'vsize_bip141': tx.get_vsize(),'fees': {'base': fee}}], rawtxs=[raw_tx_0], ) @@ -151,7 +151,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): tx = tx_from_hex(raw_tx_final) fee_expected = Decimal('50.0') - output_amount self.check_mempool_result( - result_expected=[{'txid': tx.txid_hex, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': fee_expected}}], + result_expected=[{'txid': tx.txid_hex, 'allowed': True, 'vsize_adjusted': tx.get_vsize(), 'vsize': tx.get_vsize(), 'vsize_bip141': tx.get_vsize(), 'fees': {'base': fee_expected}}], rawtxs=[tx.serialize().hex()], maxfeerate=0, ) @@ -172,7 +172,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): raw_tx_0 = tx.serialize().hex() txid_0 = tx.txid_hex self.check_mempool_result( - result_expected=[{'txid': txid_0, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': (2 * fee)}}], + result_expected=[{'txid': txid_0, 'allowed': True, 'vsize_adjusted': tx.get_vsize(), 'vsize': tx.get_vsize(), 'vsize_bip141': tx.get_vsize(), 'fees': {'base': (2 * fee)}}], rawtxs=[raw_tx_0], ) node.sendrawtransaction(hexstring=tx.serialize().hex(), maxfeerate=0) @@ -218,7 +218,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): raw_tx_reference = tx.serialize().hex() # Reference tx should be valid on itself self.check_mempool_result( - result_expected=[{'txid': tx.txid_hex, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal('0.1') - Decimal('0.05')}}], + result_expected=[{'txid': tx.txid_hex, 'allowed': True, 'vsize_adjusted': tx.get_vsize(), 'vsize': tx.get_vsize(), 'vsize_bip141': tx.get_vsize(), 'fees': { 'base': Decimal('0.1') - Decimal('0.05')}}], rawtxs=[tx.serialize().hex()], maxfeerate=0, ) @@ -354,7 +354,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): tx.vout.append(CTxOut(0, CScript([OP_RETURN, b'\xff' * 50000]))) self.check_mempool_result( - result_expected=[{'txid': tx.txid_hex, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal('0.05')}}], + result_expected=[{'txid': tx.txid_hex, 'allowed': True, 'vsize_adjusted': tx.get_vsize(), 'vsize': tx.get_vsize(), 'vsize_bip141': tx.get_vsize(), 'fees': {'base': Decimal('0.05')}}], rawtxs=[tx.serialize().hex()], maxfeerate=0 ) @@ -366,7 +366,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): tx.vout[0].scriptPubKey = CScript([OP_RETURN, b'\xff']) tx.vout = [tx.vout[0]] * op_return_count self.check_mempool_result( - result_expected=[{"txid": tx.txid_hex, "allowed": True, "vsize": tx.get_vsize(), "fees": {"base": Decimal("0.05000026")}}], + result_expected=[{"txid": tx.txid_hex, "allowed": True, "vsize_adjusted": tx.get_vsize(), "vsize": tx.get_vsize(), 'vsize_bip141': tx.get_vsize(), "fees": {"base": Decimal("0.05000026")}}], rawtxs=[tx.serialize().hex()], ) @@ -377,7 +377,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): tx.vout[0].scriptPubKey = CScript([OP_RETURN, b"\xff" * (data_len)]) assert_equal(tx.get_vsize(), int(MAX_STANDARD_TX_WEIGHT / 4)) self.check_mempool_result( - result_expected=[{"txid": tx.txid_hex, "allowed": True, "vsize": tx.get_vsize(), "fees": {"base": Decimal("0.1") - Decimal("0.05")}}], + result_expected=[{"txid": tx.txid_hex, "allowed": True, "vsize_adjusted": tx.get_vsize(), "vsize": tx.get_vsize(), 'vsize_bip141': tx.get_vsize(), "fees": {"base": Decimal("0.1") - Decimal("0.05")}}], rawtxs=[tx.serialize().hex()], ) tx.vout[0].scriptPubKey = CScript([OP_RETURN, b"\xff" * (data_len + 1)]) @@ -430,7 +430,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): tx.vout[0] = CTxOut(COIN - 1000, DUMMY_MIN_OP_RETURN_SCRIPT) assert_equal(len(tx.serialize_without_witness()), MIN_STANDARD_TX_NONWITNESS_SIZE) self.check_mempool_result( - result_expected=[{'txid': tx.txid_hex, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal('0.00001000')}}], + result_expected=[{'txid': tx.txid_hex, 'allowed': True, 'vsize_adjusted': tx.get_vsize(), 'vsize': tx.get_vsize(), 'vsize_bip141': tx.get_vsize(), 'fees': { 'base': Decimal('0.00001000')}}], rawtxs=[tx.serialize().hex()], maxfeerate=0, ) @@ -468,7 +468,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): assert_equal(anchor_spend.txid_hex, anchor_spend.wtxid_hex) self.check_mempool_result( - result_expected=[{'txid': anchor_spend.txid_hex, 'allowed': True, 'vsize': anchor_spend.get_vsize(), 'fees': { 'base': Decimal('0.00000700')}}], + result_expected=[{'txid': anchor_spend.txid_hex, 'allowed': True, 'vsize_adjusted': anchor_spend.get_vsize(), 'vsize': anchor_spend.get_vsize(), 'vsize_bip141': anchor_spend.get_vsize(), 'fees': { 'base': Decimal('0.00000700')}}], rawtxs=[anchor_spend.serialize().hex()], maxfeerate=0, ) @@ -503,7 +503,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): sign_input_legacy(tx_spend, 0, tx.vout[0].scriptPubKey, privkey, sighash_type=SIGHASH_ALL) tx_spend.vin[0].scriptSig = bytes(CScript([OP_0])) + tx_spend.vin[0].scriptSig self.check_mempool_result( - result_expected=[{'txid': tx_spend.txid_hex, 'allowed': True, 'vsize': tx_spend.get_vsize(), 'fees': { 'base': Decimal('0.00000700')}}], + result_expected=[{'txid': tx_spend.txid_hex, 'allowed': True, 'vsize_adjusted': tx_spend.get_vsize(), 'vsize': tx_spend.get_vsize(), 'vsize_bip141': tx_spend.get_vsize(), 'fees': { 'base': Decimal('0.00000700')}}], rawtxs=[tx_spend.serialize().hex()], maxfeerate=0, ) diff --git a/test/functional/mempool_sigoplimit.py b/test/functional/mempool_sigoplimit.py index 823d70549ee..1ade84695bc 100755 --- a/test/functional/mempool_sigoplimit.py +++ b/test/functional/mempool_sigoplimit.py @@ -41,6 +41,7 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_greater_than_or_equal, + assert_not_equal, assert_raises_rpc_error, ) from test_framework.wallet import MiniWallet @@ -98,14 +99,20 @@ class BytesPerSigOpTest(BitcoinTestFramework): res = self.nodes[0].testmempoolaccept([tx.serialize().hex()])[0] assert_equal(res['allowed'], True) + assert_equal(sigop_equivalent_vsize, tx.get_vsize()) assert_equal(res['vsize'], sigop_equivalent_vsize) + assert_equal(res['vsize_adjusted'], sigop_equivalent_vsize) + assert_equal(res['vsize_bip141'], tx.get_vsize()) # increase the tx's vsize to be right above the sigop-limit equivalent size # => tx's vsize in mempool should also grow accordingly tx.vout[0].scriptPubKey = CScript([OP_RETURN, b'X'*(256+vsize_to_pad+1)]) res = self.nodes[0].testmempoolaccept([tx.serialize().hex()])[0] assert_equal(res['allowed'], True) + assert_equal(sigop_equivalent_vsize + 1, tx.get_vsize()) assert_equal(res['vsize'], sigop_equivalent_vsize+1) + assert_equal(res['vsize_adjusted'], sigop_equivalent_vsize + 1) + assert_equal(res['vsize_bip141'], tx.get_vsize()) # decrease the tx's vsize to be right below the sigop-limit equivalent size # => tx's vsize in mempool should stick at the sigop-limit equivalent @@ -114,7 +121,10 @@ class BytesPerSigOpTest(BitcoinTestFramework): tx.vout[0].scriptPubKey = CScript([OP_RETURN, b'X'*(256+vsize_to_pad-1)]) res = self.nodes[0].testmempoolaccept([tx.serialize().hex()])[0] assert_equal(res['allowed'], True) + assert_not_equal(sigop_equivalent_vsize, tx.get_vsize()) assert_equal(res['vsize'], sigop_equivalent_vsize) + assert_equal(res['vsize_adjusted'], sigop_equivalent_vsize) + assert_equal(res['vsize_bip141'], tx.get_vsize()) # check that the ancestor and descendant size calculations in the mempool # also use the same max(sigop_equivalent_vsize, serialized_vsize) logic @@ -166,7 +176,7 @@ class BytesPerSigOpTest(BitcoinTestFramework): parent_individual_testres = self.nodes[0].testmempoolaccept([tx_parent.serialize().hex()])[0] assert parent_individual_testres["allowed"] max_multisig_vsize = MAX_PUBKEYS_PER_MULTISIG * 5000 - assert_equal(parent_individual_testres["vsize"], max_multisig_vsize) + assert_equal(parent_individual_testres["vsize_adjusted"], max_multisig_vsize) # But together, it's exceeding limits in the *package* context. If sigops adjusted vsize wasn't being checked # here, it would get further in validation and give too-large-cluster error instead. diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 92ebe21ad7d..6224e9c5ddc 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -615,7 +615,9 @@ class SegWitTest(BitcoinTestFramework): 'txid': tx3.txid_hex, 'wtxid': tx3.wtxid_hex, 'allowed': True, + 'vsize_adjusted': tx3.get_vsize(), 'vsize': tx3.get_vsize(), + 'vsize_bip141': tx3.get_vsize(), 'fees': { 'base': Decimal('0.00001000'), }, @@ -633,7 +635,9 @@ class SegWitTest(BitcoinTestFramework): 'txid': tx3.txid_hex, 'wtxid': tx3.wtxid_hex, 'allowed': True, + 'vsize_adjusted': tx3.get_vsize(), 'vsize': tx3.get_vsize(), + 'vsize_bip141': tx3.get_vsize(), 'fees': { 'base': Decimal('0.00011000'), }, diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 12e086212a2..647d16df2c0 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -302,6 +302,7 @@ class RPCPackagesTest(BitcoinTestFramework): assert_equal(testres_replaceable["wtxid"], replaceable_tx["wtxid"]) assert testres_replaceable["allowed"] assert_equal(testres_replaceable["vsize"], replaceable_tx["tx"].get_vsize()) + assert_equal(testres_replaceable["vsize_bip141"], replaceable_tx["tx"].get_vsize()) assert_equal(testres_replaceable["fees"]["base"], fee) assert_fee_amount(fee, replaceable_tx["tx"].get_vsize(), testres_replaceable["fees"]["effective-feerate"]) assert_equal(testres_replaceable["fees"]["effective-includes"], [replaceable_tx["wtxid"]]) @@ -339,9 +340,11 @@ class RPCPackagesTest(BitcoinTestFramework): # No "allowed" if the tx was already in the mempool if "allowed" in testres_tx and testres_tx["allowed"]: assert_equal(submitres_tx["vsize"], testres_tx["vsize"]) + assert_equal(submitres_tx["vsize_bip141"], testres_tx["vsize"]) assert_equal(submitres_tx["fees"]["base"], testres_tx["fees"]["base"]) entry_info = node.getmempoolentry(submitres_tx["txid"]) assert_equal(submitres_tx["vsize"], entry_info["vsize"]) + assert_equal(submitres_tx["vsize_bip141"], entry_info["vsize"]) assert_equal(submitres_tx["fees"]["base"], entry_info["fees"]["base"]) def test_submit_child_with_parents(self, num_parents, partial_submit): @@ -371,7 +374,9 @@ class RPCPackagesTest(BitcoinTestFramework): assert wtxid in submitpackage_result["tx-results"] tx_result = submitpackage_result["tx-results"][wtxid] assert_equal(tx_result["txid"], tx.txid_hex) + assert_equal(tx_result["vsize_adjusted"], tx.get_vsize()) assert_equal(tx_result["vsize"], tx.get_vsize()) + assert_equal(tx_result["vsize_bip141"], tx.get_vsize()) assert_equal(tx_result["fees"]["base"], DEFAULT_FEE) if wtxid not in presubmitted_wtxids: assert_fee_amount(DEFAULT_FEE, tx.get_vsize(), tx_result["fees"]["effective-feerate"])