diff options
author | fanquake <fanquake@gmail.com> | 2020-09-19 14:34:52 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2020-09-19 15:04:03 +0800 |
commit | c30f79d418e2e001f1f804a2370db47e5d9ea587 (patch) | |
tree | 5073f9436079ebba8b60f01c3891a6c18cacb368 | |
parent | 967be53aeec9f02b90f132e0482d27af4c85dd95 (diff) | |
parent | 23c35bf0059bd6270218e0b732959e9c754f9812 (diff) |
Merge #19940: rpc: Return fee and vsize from testmempoolaccept
23c35bf0059bd6270218e0b732959e9c754f9812 [test] add get_vsize util for more programmatic testing (gzhao408)
2233a93a109b10b6fe0f5f26c2bb529c8de3dde7 [rpc] Return fee and vsize from testmempoolaccept (codeShark149)
Pull request description:
From #19093 and resolves #19057.
Difference from #19093: return `vsize` and `fees` object (similar to `getmempoolentry`) when the test accept is successful. Updates release-notes.md.
ACKs for top commit:
jnewbery:
utACK 23c35bf0059bd6270218e0b732959e9c754f9812
fjahr:
utACK 23c35bf
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/19940/commits/23c35bf0059bd6270218e0b732959e9c754f9812
Tree-SHA512: dcb81b7b817a4684e9076bc5d427a6f2d549d2edc66544e718260c4b5f8f1d5ae1d47b754175e9f0c8a3bd8371ce116c2dca0583588d513a7d733d5d614f2b04
-rw-r--r-- | doc/release-notes.md | 3 | ||||
-rw-r--r-- | src/rpc/rawtransaction.cpp | 18 | ||||
-rw-r--r-- | src/validation.cpp | 14 | ||||
-rw-r--r-- | src/validation.h | 5 | ||||
-rwxr-xr-x | test/functional/mempool_accept.py | 13 | ||||
-rwxr-xr-x | test/functional/p2p_segwit.py | 5 | ||||
-rwxr-xr-x | test/functional/test_framework/messages.py | 10 |
7 files changed, 53 insertions, 15 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index a8bd68370d..0ce2b32c61 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -102,6 +102,9 @@ will trigger BIP 125 (replace-by-fee) opt-in. (#11413) option `-deprecatedrpc=banscore` is used. The `banscore` field will be fully removed in the next major release. (#19469) +- The `testmempoolaccept` RPC returns `vsize` and a `fee` object with the `base` fee + if the transaction passes validation. (#19940) + - The `walletcreatefundedpsbt` RPC call will now fail with `Insufficient funds` when inputs are manually selected but are not enough to cover the outputs and fee. Additional inputs can automatically be added through the diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 93e8357e86..1a43ffcc53 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -878,6 +878,11 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) { {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, {RPCResult::Type::BOOL, "allowed", "If the mempool allows this tx to be inserted"}, + {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 (only present when 'allowed' is true)"}, + {RPCResult::Type::OBJ, "fees", "Transaction fees (only present if 'allowed' is true)", + { + {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, + }}, {RPCResult::Type::STR, "reject-reason", "Rejection string (only present when 'allowed' is false)"}, }}, } @@ -924,13 +929,22 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) TxValidationState state; bool test_accept_res; + CAmount fee; { LOCK(cs_main); test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx), - nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true); + nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true, &fee); } result_0.pushKV("allowed", test_accept_res); - if (!test_accept_res) { + + // Only return the fee and vsize if the transaction would pass ATMP. + // These can be used to calculate the feerate. + if (test_accept_res) { + result_0.pushKV("vsize", virtual_size); + UniValue fees(UniValue::VOBJ); + fees.pushKV("base", ValueFromAmount(fee)); + result_0.pushKV("fees", fees); + } else { if (state.IsInvalid()) { if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { result_0.pushKV("reject-reason", "missing-inputs"); diff --git a/src/validation.cpp b/src/validation.cpp index 6dd0e0ae45..0823c6b124 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -475,6 +475,7 @@ public: */ std::vector<COutPoint>& m_coins_to_uncache; const bool m_test_accept; + CAmount* m_fee_out; }; // Single transaction acceptance @@ -687,6 +688,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return false; // state filled in by CheckTxInputs } + // If fee_out is passed, return the fee to the caller + if (args.m_fee_out) { + *args.m_fee_out = nFees; + } + // Check for non-standard pay-to-script-hash in inputs if (fRequireStandard && !AreInputsStandard(tx, m_view)) { return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs"); @@ -1061,10 +1067,10 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs /** (try to) add transaction to memory pool with a specified acceptance time **/ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced, - bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) + bool bypass_limits, const CAmount nAbsurdFee, bool test_accept, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::vector<COutPoint> coins_to_uncache; - MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept }; + MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept, fee_out }; bool res = MemPoolAccept(pool).AcceptSingleTransaction(tx, args); if (!res) { // Remove coins that were not present in the coins cache before calling ATMPW; @@ -1083,10 +1089,10 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, std::list<CTransactionRef>* plTxnReplaced, - bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) + bool bypass_limits, const CAmount nAbsurdFee, bool test_accept, CAmount* fee_out) { const CChainParams& chainparams = Params(); - return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept); + return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept, fee_out); } CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock) diff --git a/src/validation.h b/src/validation.h index 53c2dd65e5..0bc80e1cee 100644 --- a/src/validation.h +++ b/src/validation.h @@ -199,10 +199,11 @@ void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune); void PruneBlockFilesManual(int nManualPruneHeight); /** (try to) add transaction to memory pool - * plTxnReplaced will be appended to with all transactions replaced from mempool **/ + * plTxnReplaced will be appended to with all transactions replaced from mempool + * @param[out] fee_out optional argument to return tx fee to the caller **/ bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, std::list<CTransactionRef>* plTxnReplaced, - bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Get the BIP9 state for a given deployment at the current tip. */ ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos); diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 6df8f1c3ec..57a059b7f7 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test mempool acceptance of raw transactions.""" +from decimal import Decimal from io import BytesIO import math @@ -91,20 +92,22 @@ class MempoolAcceptanceTest(BitcoinTestFramework): tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_0))) txid_0 = tx.rehash() self.check_mempool_result( - result_expected=[{'txid': txid_0, 'allowed': True}], + result_expected=[{'txid': txid_0, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal(str(fee))}}], rawtxs=[raw_tx_0], ) self.log.info('A final transaction not in the mempool') coin = coins.pop() # Pick a random coin(base) to spend + output_amount = 0.025 raw_tx_final = node.signrawtransactionwithwallet(node.createrawtransaction( inputs=[{'txid': coin['txid'], 'vout': coin['vout'], "sequence": 0xffffffff}], # SEQUENCE_FINAL - outputs=[{node.getnewaddress(): 0.025}], + outputs=[{node.getnewaddress(): output_amount}], locktime=node.getblockcount() + 2000, # Can be anything ))['hex'] tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_final))) + fee_expected = int(coin['amount']) - output_amount self.check_mempool_result( - result_expected=[{'txid': tx.rehash(), 'allowed': True}], + result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal(str(fee_expected))}}], rawtxs=[tx.serialize().hex()], maxfeerate=0, ) @@ -127,7 +130,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_0))) txid_0 = tx.rehash() self.check_mempool_result( - result_expected=[{'txid': txid_0, 'allowed': True}], + result_expected=[{'txid': txid_0, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal(str(2 * fee))}}], rawtxs=[raw_tx_0], ) @@ -187,7 +190,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference))) # Reference tx should be valid on itself self.check_mempool_result( - result_expected=[{'txid': tx.rehash(), 'allowed': True}], + result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal(str(0.1 - 0.05))}}], rawtxs=[tx.serialize().hex()], maxfeerate=0, ) diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 9503391030..dfbf5d52f4 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -3,6 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test segwit transactions and blocks on P2P network.""" +from decimal import Decimal import math import random import struct @@ -695,13 +696,13 @@ class SegWitTest(BitcoinTestFramework): if not self.segwit_active: # Just check mempool acceptance, but don't add the transaction to the mempool, since witness is disallowed # in blocks and the tx is impossible to mine right now. - assert_equal(self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]), [{'txid': tx3.hash, 'allowed': True}]) + assert_equal(self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]), [{'txid': tx3.hash, 'allowed': True, 'vsize': tx3.get_vsize(), 'fees': { 'base': Decimal('0.00001000')}}]) # Create the same output as tx3, but by replacing tx tx3_out = tx3.vout[0] tx3 = tx tx3.vout = [tx3_out] tx3.rehash() - assert_equal(self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]), [{'txid': tx3.hash, 'allowed': True}]) + assert_equal(self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]), [{'txid': tx3.hash, 'allowed': True, 'vsize': tx3.get_vsize(), 'fees': { 'base': Decimal('0.00011000')}}]) test_transaction_acceptance(self.nodes[0], self.test_node, tx3, with_witness=True, accepted=True) self.nodes[0].generate(1) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index b4e609df3a..1e062ab9a4 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -22,6 +22,7 @@ from codecs import encode import copy import hashlib from io import BytesIO +import math import random import socket import struct @@ -67,6 +68,8 @@ MSG_WITNESS_TX = MSG_TX | MSG_WITNESS_FLAG FILTER_TYPE_BASIC = 0 +WITNESS_SCALE_FACTOR = 4 + # Serialization/deserialization tools def sha256(s): return hashlib.new('sha256', s).digest() @@ -537,6 +540,13 @@ class CTransaction: return False return True + # Calculate the virtual transaction size using witness and non-witness + # serialization size (does NOT use sigops). + def get_vsize(self): + with_witness_size = len(self.serialize_with_witness()) + without_witness_size = len(self.serialize_without_witness()) + return math.ceil(((WITNESS_SCALE_FACTOR - 1) * without_witness_size + with_witness_size) / WITNESS_SCALE_FACTOR) + def __repr__(self): return "CTransaction(nVersion=%i vin=%s vout=%s wit=%s nLockTime=%i)" \ % (self.nVersion, repr(self.vin), repr(self.vout), repr(self.wit), self.nLockTime) |