Skip to content

Commit c30f79d

Browse files
committed
Merge #19940: rpc: Return fee and vsize from testmempoolaccept
23c35bf [test] add get_vsize util for more programmatic testing (gzhao408) 2233a93 [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 23c35bf fjahr: utACK 23c35bf instagibbs: reACK bitcoin/bitcoin@23c35bf Tree-SHA512: dcb81b7b817a4684e9076bc5d427a6f2d549d2edc66544e718260c4b5f8f1d5ae1d47b754175e9f0c8a3bd8371ce116c2dca0583588d513a7d733d5d614f2b04
2 parents 967be53 + 23c35bf commit c30f79d

File tree

7 files changed

+53
-15
lines changed

7 files changed

+53
-15
lines changed

doc/release-notes.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ will trigger BIP 125 (replace-by-fee) opt-in. (#11413)
102102
option `-deprecatedrpc=banscore` is used. The `banscore` field will be fully
103103
removed in the next major release. (#19469)
104104

105+
- The `testmempoolaccept` RPC returns `vsize` and a `fee` object with the `base` fee
106+
if the transaction passes validation. (#19940)
107+
105108
- The `walletcreatefundedpsbt` RPC call will now fail with
106109
`Insufficient funds` when inputs are manually selected but are not enough to cover
107110
the outputs and fee. Additional inputs can automatically be added through the

src/rpc/rawtransaction.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,11 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
878878
{
879879
{RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"},
880880
{RPCResult::Type::BOOL, "allowed", "If the mempool allows this tx to be inserted"},
881+
{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)"},
882+
{RPCResult::Type::OBJ, "fees", "Transaction fees (only present if 'allowed' is true)",
883+
{
884+
{RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT},
885+
}},
881886
{RPCResult::Type::STR, "reject-reason", "Rejection string (only present when 'allowed' is false)"},
882887
}},
883888
}
@@ -924,13 +929,22 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
924929

925930
TxValidationState state;
926931
bool test_accept_res;
932+
CAmount fee;
927933
{
928934
LOCK(cs_main);
929935
test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx),
930-
nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true);
936+
nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true, &fee);
931937
}
932938
result_0.pushKV("allowed", test_accept_res);
933-
if (!test_accept_res) {
939+
940+
// Only return the fee and vsize if the transaction would pass ATMP.
941+
// These can be used to calculate the feerate.
942+
if (test_accept_res) {
943+
result_0.pushKV("vsize", virtual_size);
944+
UniValue fees(UniValue::VOBJ);
945+
fees.pushKV("base", ValueFromAmount(fee));
946+
result_0.pushKV("fees", fees);
947+
} else {
934948
if (state.IsInvalid()) {
935949
if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
936950
result_0.pushKV("reject-reason", "missing-inputs");

src/validation.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ class MemPoolAccept
475475
*/
476476
std::vector<COutPoint>& m_coins_to_uncache;
477477
const bool m_test_accept;
478+
CAmount* m_fee_out;
478479
};
479480

480481
// Single transaction acceptance
@@ -687,6 +688,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
687688
return false; // state filled in by CheckTxInputs
688689
}
689690

691+
// If fee_out is passed, return the fee to the caller
692+
if (args.m_fee_out) {
693+
*args.m_fee_out = nFees;
694+
}
695+
690696
// Check for non-standard pay-to-script-hash in inputs
691697
if (fRequireStandard && !AreInputsStandard(tx, m_view)) {
692698
return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs");
@@ -1061,10 +1067,10 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs
10611067
/** (try to) add transaction to memory pool with a specified acceptance time **/
10621068
static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
10631069
int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
1064-
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
1070+
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
10651071
{
10661072
std::vector<COutPoint> coins_to_uncache;
1067-
MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept };
1073+
MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept, fee_out };
10681074
bool res = MemPoolAccept(pool).AcceptSingleTransaction(tx, args);
10691075
if (!res) {
10701076
// Remove coins that were not present in the coins cache before calling ATMPW;
@@ -1083,10 +1089,10 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo
10831089

10841090
bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
10851091
std::list<CTransactionRef>* plTxnReplaced,
1086-
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept)
1092+
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept, CAmount* fee_out)
10871093
{
10881094
const CChainParams& chainparams = Params();
1089-
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept);
1095+
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept, fee_out);
10901096
}
10911097

10921098
CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock)

src/validation.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,11 @@ void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune);
199199
void PruneBlockFilesManual(int nManualPruneHeight);
200200

201201
/** (try to) add transaction to memory pool
202-
* plTxnReplaced will be appended to with all transactions replaced from mempool **/
202+
* plTxnReplaced will be appended to with all transactions replaced from mempool
203+
* @param[out] fee_out optional argument to return tx fee to the caller **/
203204
bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
204205
std::list<CTransactionRef>* plTxnReplaced,
205-
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
206+
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
206207

207208
/** Get the BIP9 state for a given deployment at the current tip. */
208209
ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos);

test/functional/mempool_accept.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Test mempool acceptance of raw transactions."""
66

7+
from decimal import Decimal
78
from io import BytesIO
89
import math
910

@@ -91,20 +92,22 @@ def run_test(self):
9192
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_0)))
9293
txid_0 = tx.rehash()
9394
self.check_mempool_result(
94-
result_expected=[{'txid': txid_0, 'allowed': True}],
95+
result_expected=[{'txid': txid_0, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal(str(fee))}}],
9596
rawtxs=[raw_tx_0],
9697
)
9798

9899
self.log.info('A final transaction not in the mempool')
99100
coin = coins.pop() # Pick a random coin(base) to spend
101+
output_amount = 0.025
100102
raw_tx_final = node.signrawtransactionwithwallet(node.createrawtransaction(
101103
inputs=[{'txid': coin['txid'], 'vout': coin['vout'], "sequence": 0xffffffff}], # SEQUENCE_FINAL
102-
outputs=[{node.getnewaddress(): 0.025}],
104+
outputs=[{node.getnewaddress(): output_amount}],
103105
locktime=node.getblockcount() + 2000, # Can be anything
104106
))['hex']
105107
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_final)))
108+
fee_expected = int(coin['amount']) - output_amount
106109
self.check_mempool_result(
107-
result_expected=[{'txid': tx.rehash(), 'allowed': True}],
110+
result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal(str(fee_expected))}}],
108111
rawtxs=[tx.serialize().hex()],
109112
maxfeerate=0,
110113
)
@@ -127,7 +130,7 @@ def run_test(self):
127130
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_0)))
128131
txid_0 = tx.rehash()
129132
self.check_mempool_result(
130-
result_expected=[{'txid': txid_0, 'allowed': True}],
133+
result_expected=[{'txid': txid_0, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal(str(2 * fee))}}],
131134
rawtxs=[raw_tx_0],
132135
)
133136

@@ -187,7 +190,7 @@ def run_test(self):
187190
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
188191
# Reference tx should be valid on itself
189192
self.check_mempool_result(
190-
result_expected=[{'txid': tx.rehash(), 'allowed': True}],
193+
result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal(str(0.1 - 0.05))}}],
191194
rawtxs=[tx.serialize().hex()],
192195
maxfeerate=0,
193196
)

test/functional/p2p_segwit.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# Distributed under the MIT software license, see the accompanying
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Test segwit transactions and blocks on P2P network."""
6+
from decimal import Decimal
67
import math
78
import random
89
import struct
@@ -695,13 +696,13 @@ def test_standardness_v0(self):
695696
if not self.segwit_active:
696697
# Just check mempool acceptance, but don't add the transaction to the mempool, since witness is disallowed
697698
# in blocks and the tx is impossible to mine right now.
698-
assert_equal(self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]), [{'txid': tx3.hash, 'allowed': True}])
699+
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')}}])
699700
# Create the same output as tx3, but by replacing tx
700701
tx3_out = tx3.vout[0]
701702
tx3 = tx
702703
tx3.vout = [tx3_out]
703704
tx3.rehash()
704-
assert_equal(self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]), [{'txid': tx3.hash, 'allowed': True}])
705+
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')}}])
705706
test_transaction_acceptance(self.nodes[0], self.test_node, tx3, with_witness=True, accepted=True)
706707

707708
self.nodes[0].generate(1)

test/functional/test_framework/messages.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import copy
2323
import hashlib
2424
from io import BytesIO
25+
import math
2526
import random
2627
import socket
2728
import struct
@@ -67,6 +68,8 @@
6768

6869
FILTER_TYPE_BASIC = 0
6970

71+
WITNESS_SCALE_FACTOR = 4
72+
7073
# Serialization/deserialization tools
7174
def sha256(s):
7275
return hashlib.new('sha256', s).digest()
@@ -537,6 +540,13 @@ def is_valid(self):
537540
return False
538541
return True
539542

543+
# Calculate the virtual transaction size using witness and non-witness
544+
# serialization size (does NOT use sigops).
545+
def get_vsize(self):
546+
with_witness_size = len(self.serialize_with_witness())
547+
without_witness_size = len(self.serialize_without_witness())
548+
return math.ceil(((WITNESS_SCALE_FACTOR - 1) * without_witness_size + with_witness_size) / WITNESS_SCALE_FACTOR)
549+
540550
def __repr__(self):
541551
return "CTransaction(nVersion=%i vin=%s vout=%s wit=%s nLockTime=%i)" \
542552
% (self.nVersion, repr(self.vin), repr(self.vout), repr(self.wit), self.nLockTime)

0 commit comments

Comments
 (0)