Skip to content

Commit 2bcb3f6

Browse files
committed
Merge bitcoin/bitcoin#34112: rpc: [mempool] Remove erroneous Univalue integral casts
fab1f4b rpc: [mempool] Remove erroneous Univalue integral casts (MarcoFalke) Pull request description: Casting without reason can only be confusing (because it is not needed), or wrong (because it does the wrong thing). For example, the added test that adds a positive chunk prioritization will fail: ``` AssertionError: not(-1.94936096 == 41.000312) ``` Fix all issues by removing the erroneous casts, and by adding a test to check against regressions. ACKs for top commit: rkrux: tACK fab1f4b pablomartin4btc: ACK fab1f4b glozow: ACK fab1f4b Tree-SHA512: b03c888ec07a8bdff25f7ded67f253b2a8edd83adf08980416e2ac8ac1b36ad952cc5828be833d19f64a55abab62d7a1c6f181bc5f1388ed08cc178b4aaec6ee
2 parents 7249bcc + fab1f4b commit 2bcb3f6

File tree

2 files changed

+54
-2
lines changed

2 files changed

+54
-2
lines changed

src/rpc/mempool.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ static std::vector<RPCResult> MempoolEntryDescription()
315315
void AppendChunkInfo(UniValue& all_chunks, FeePerWeight chunk_feerate, std::vector<const CTxMemPoolEntry *> chunk_txs)
316316
{
317317
UniValue chunk(UniValue::VOBJ);
318-
chunk.pushKV("chunkfee", ValueFromAmount((int)chunk_feerate.fee));
318+
chunk.pushKV("chunkfee", ValueFromAmount(chunk_feerate.fee));
319319
chunk.pushKV("chunkweight", chunk_feerate.size);
320320
UniValue chunk_txids(UniValue::VARR);
321321
for (const auto& chunk_tx : chunk_txs) {
@@ -383,7 +383,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
383383
fees.pushKV("modified", ValueFromAmount(e.GetModifiedFee()));
384384
fees.pushKV("ancestor", ValueFromAmount(ancestor_fees));
385385
fees.pushKV("descendant", ValueFromAmount(descendant_fees));
386-
fees.pushKV("chunk", ValueFromAmount((int)feerate.fee));
386+
fees.pushKV("chunk", ValueFromAmount(feerate.fee));
387387
info.pushKV("fees", std::move(fees));
388388

389389
const CTransaction& tx = e.GetTx();

test/functional/mining_prioritisetransaction.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,57 @@ def clear_prioritisation(self, node):
3636
node.prioritisetransaction(txid, 0, -delta)
3737
assert_equal(node.getprioritisedtransactions(), {})
3838

39+
def test_large_fee_bump(self):
40+
self.log.info("Test that a large fee delta is honoured")
41+
tx = self.wallet.create_self_transfer()
42+
txid = tx["txid"]
43+
fee_delta = int(86 * COIN) # large enough to not fit into (u)int32_t
44+
self.nodes[0].prioritisetransaction(txid=txid, fee_delta=fee_delta)
45+
assert_equal(
46+
self.nodes[0].getprioritisedtransactions(),
47+
{
48+
txid: {
49+
"fee_delta": fee_delta,
50+
"in_mempool": False,
51+
},
52+
},
53+
)
54+
self.nodes[0].sendrawtransaction(tx["hex"])
55+
expected_modified_fee = tx["fee"] + Decimal(fee_delta) / COIN
56+
assert_equal(
57+
self.nodes[0].getprioritisedtransactions(),
58+
{
59+
txid: {
60+
"fee_delta": fee_delta,
61+
"in_mempool": True,
62+
"modified_fee": int(expected_modified_fee * COIN),
63+
},
64+
},
65+
)
66+
# This transaction forms its own chunk.
67+
mempool_entry = self.nodes[0].getrawmempool(verbose=True)[txid]
68+
assert_equal(mempool_entry["fees"]["base"], tx["fee"])
69+
assert_equal(mempool_entry["fees"]["modified"], expected_modified_fee)
70+
assert_equal(mempool_entry["fees"]["ancestor"], expected_modified_fee)
71+
assert_equal(mempool_entry["fees"]["descendant"], expected_modified_fee)
72+
assert_equal(mempool_entry["fees"]["chunk"], expected_modified_fee)
73+
assert_equal(mempool_entry["chunkweight"], mempool_entry["weight"])
74+
append_chunk_info = self.nodes[0].getmempoolcluster(txid)
75+
assert_equal(
76+
append_chunk_info,
77+
{
78+
"clusterweight": mempool_entry["weight"],
79+
"txcount": 1,
80+
"chunks": [{
81+
"chunkfee": expected_modified_fee,
82+
"chunkweight": mempool_entry["weight"],
83+
"txs": [txid],
84+
}],
85+
},
86+
)
87+
self.generate(self.nodes[0], 1)
88+
assert_equal(self.nodes[0].getprioritisedtransactions(), {})
89+
3990
def test_replacement(self):
4091
self.log.info("Test tx prioritisation stays after a tx is replaced")
4192
conflicting_input = self.wallet.get_utxo()
@@ -175,6 +226,7 @@ def run_test(self):
175226
# Test `prioritisetransaction` invalid `fee_delta`
176227
assert_raises_rpc_error(-3, "JSON value of type string is not of expected type number", self.nodes[0].prioritisetransaction, txid=txid, fee_delta='foo')
177228

229+
self.test_large_fee_bump()
178230
self.test_replacement()
179231
self.test_diamond()
180232

0 commit comments

Comments
 (0)