Skip to content

Commit f9b9371

Browse files
morcosmorcos
committed
[rpc] Remove priorityDelta from prioritisetransaction
This a breaking API change to the prioritisetransaction RPC call which previously required exactly three arguments and now requires exactly two (hash and feeDelta). The function prioritiseTransaction is also updated.
1 parent 49be7e1 commit f9b9371

File tree

9 files changed

+38
-46
lines changed

9 files changed

+38
-46
lines changed

‎qa/rpc-tests/bip68-sequence.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ def test_nonzero_locks(orig_tx, node, relayfee, use_height_lock):
256256

257257
# Now mine some blocks, but make sure tx2 doesn't get mined.
258258
# Use prioritisetransaction to lower the effective feerate to 0
259-
self.nodes[0].prioritisetransaction(tx2.hash, -1e15, int(-self.relayfee*COIN))
259+
self.nodes[0].prioritisetransaction(tx2.hash, int(-self.relayfee*COIN))
260260
cur_time = int(time.time())
261261
for i in range(10):
262262
self.nodes[0].setmocktime(cur_time + 600)
@@ -269,7 +269,7 @@ def test_nonzero_locks(orig_tx, node, relayfee, use_height_lock):
269269
test_nonzero_locks(tx2, self.nodes[0], self.relayfee, use_height_lock=False)
270270

271271
# Mine tx2, and then try again
272-
self.nodes[0].prioritisetransaction(tx2.hash, 1e15, int(self.relayfee*COIN))
272+
self.nodes[0].prioritisetransaction(tx2.hash, int(self.relayfee*COIN))
273273

274274
# Advance the time on the node so that we can test timelocks
275275
self.nodes[0].setmocktime(cur_time+600)

‎qa/rpc-tests/mempool_packages.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ def run_test(self):
103103

104104
# Check that descendant modified fees includes fee deltas from
105105
# prioritisetransaction
106-
self.nodes[0].prioritisetransaction(chain[-1], 0, 1000)
106+
self.nodes[0].prioritisetransaction(chain[-1], 1000)
107107
mempool = self.nodes[0].getrawmempool(True)
108108

109109
descendant_fees = 0
@@ -124,7 +124,7 @@ def run_test(self):
124124
assert_equal(len(self.nodes[0].getrawmempool()), 0)
125125
# Prioritise a transaction that has been mined, then add it back to the
126126
# mempool by using invalidateblock.
127-
self.nodes[0].prioritisetransaction(chain[-1], 0, 2000)
127+
self.nodes[0].prioritisetransaction(chain[-1], 2000)
128128
self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
129129
# Keep node1's tip synced with node0
130130
self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash())

‎qa/rpc-tests/prioritise_transaction.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ def run_test(self):
5151

5252
# add a fee delta to something in the cheapest bucket and make sure it gets mined
5353
# also check that a different entry in the cheapest bucket is NOT mined
54-
self.nodes[0].prioritisetransaction(txids[0][0], 0, int(3*base_fee*COIN))
54+
self.nodes[0].prioritisetransaction(txids[0][0], int(3*base_fee*COIN))
5555

5656
self.nodes[0].generate(1)
5757

@@ -70,7 +70,7 @@ def run_test(self):
7070

7171
# Add a prioritisation before a tx is in the mempool (de-prioritising a
7272
# high-fee transaction so that it's now low fee).
73-
self.nodes[0].prioritisetransaction(high_fee_tx, -1e15, -int(2*base_fee*COIN))
73+
self.nodes[0].prioritisetransaction(high_fee_tx, -int(2*base_fee*COIN))
7474

7575
# Add everything back to mempool
7676
self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
@@ -118,7 +118,7 @@ def run_test(self):
118118
# This is a less than 1000-byte transaction, so just set the fee
119119
# to be the minimum for a 1000 byte transaction and check that it is
120120
# accepted.
121-
self.nodes[0].prioritisetransaction(tx_id, 0, int(self.relayfee*COIN))
121+
self.nodes[0].prioritisetransaction(tx_id, int(self.relayfee*COIN))
122122

123123
print("Assert that prioritised free transaction is accepted to mempool")
124124
assert_equal(self.nodes[0].sendrawtransaction(tx_hex), tx_id)

‎qa/rpc-tests/replace-by-fee.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ def test_prioritised_transactions(self):
543543
assert(False)
544544

545545
# Use prioritisetransaction to set tx1a's fee to 0.
546-
self.nodes[0].prioritisetransaction(tx1a_txid, 0, int(-0.1*COIN))
546+
self.nodes[0].prioritisetransaction(tx1a_txid, int(-0.1*COIN))
547547

548548
# Now tx1b should be able to replace tx1a
549549
tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True)
@@ -575,7 +575,7 @@ def test_prioritised_transactions(self):
575575
assert(False)
576576

577577
# Now prioritise tx2b to have a higher modified fee
578-
self.nodes[0].prioritisetransaction(tx2b.hash, 0, int(0.1*COIN))
578+
self.nodes[0].prioritisetransaction(tx2b.hash, int(0.1*COIN))
579579

580580
# tx2b should now be accepted
581581
tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, True)

‎src/rpc/client.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
108108
{ "getrawmempool", 0, "verbose" },
109109
{ "estimatefee", 0, "nblocks" },
110110
{ "estimatesmartfee", 0, "nblocks" },
111-
{ "prioritisetransaction", 1, "priority_delta" },
112-
{ "prioritisetransaction", 2, "fee_delta" },
111+
{ "prioritisetransaction", 1, "fee_delta" },
113112
{ "setban", 2, "bantime" },
114113
{ "setban", 3, "absolute" },
115114
{ "setnetworkactive", 0, "state" },

‎src/rpc/mining.cpp‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -258,31 +258,28 @@ UniValue getmininginfo(const JSONRPCRequest& request)
258258
// NOTE: Unlike wallet RPC (which use BTC values), mining RPCs follow GBT (BIP 22) in using satoshi amounts
259259
UniValue prioritisetransaction(const JSONRPCRequest& request)
260260
{
261-
if (request.fHelp || request.params.size() != 3)
261+
if (request.fHelp || request.params.size() != 2)
262262
throw runtime_error(
263-
"prioritisetransaction <txid> <priority delta> <fee delta>\n"
263+
"prioritisetransaction <txid> <fee delta>\n"
264264
"Accepts the transaction into mined blocks at a higher (or lower) priority\n"
265265
"\nArguments:\n"
266266
"1. \"txid\" (string, required) The transaction id.\n"
267-
"2. priority_delta (numeric, required) The priority to add or subtract.\n"
268-
" The transaction selection algorithm considers the tx as it would have a higher priority.\n"
269-
" (priority of a transaction is calculated: coinage * value_in_satoshis / txsize) \n"
270-
"3. fee_delta (numeric, required) The fee value (in satoshis) to add (or subtract, if negative).\n"
267+
"2. fee_delta (numeric, required) The fee value (in satoshis) to add (or subtract, if negative).\n"
271268
" The fee is not actually paid, only the algorithm for selecting transactions into a block\n"
272269
" considers the transaction as it would have paid a higher (or lower) fee.\n"
273270
"\nResult:\n"
274271
"true (boolean) Returns true\n"
275272
"\nExamples:\n"
276-
+ HelpExampleCli("prioritisetransaction", "\"txid\" 0.0 10000")
277-
+ HelpExampleRpc("prioritisetransaction", "\"txid\", 0.0, 10000")
273+
+ HelpExampleCli("prioritisetransaction", "\"txid\" 10000")
274+
+ HelpExampleRpc("prioritisetransaction", "\"txid\", 10000")
278275
);
279276

280277
LOCK(cs_main);
281278

282279
uint256 hash = ParseHashStr(request.params[0].get_str(), "txid");
283-
CAmount nAmount = request.params[2].get_int64();
280+
CAmount nAmount = request.params[1].get_int64();
284281

285-
mempool.PrioritiseTransaction(hash, request.params[1].get_real(), nAmount);
282+
mempool.PrioritiseTransaction(hash, nAmount);
286283
return true;
287284
}
288285

@@ -853,7 +850,7 @@ static const CRPCCommand commands[] =
853850
// --------------------- ------------------------ ----------------------- ----------
854851
{ "mining", "getnetworkhashps", &getnetworkhashps, true, {"nblocks","height"} },
855852
{ "mining", "getmininginfo", &getmininginfo, true, {} },
856-
{ "mining", "prioritisetransaction", &prioritisetransaction, true, {"txid","priority_delta","fee_delta"} },
853+
{ "mining", "prioritisetransaction", &prioritisetransaction, true, {"txid","fee_delta"} },
857854
{ "mining", "getblocktemplate", &getblocktemplate, true, {"template_request"} },
858855
{ "mining", "submitblock", &submitblock, true, {"hexdata","parameters"} },
859856

‎src/txmempool.cpp‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -404,11 +404,11 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
404404
// Update transaction for any feeDelta created by PrioritiseTransaction
405405
// TODO: refactor so that the fee delta is calculated before inserting
406406
// into mapTx.
407-
std::map<uint256, std::pair<double, CAmount> >::const_iterator pos = mapDeltas.find(hash);
407+
std::map<uint256, CAmount>::const_iterator pos = mapDeltas.find(hash);
408408
if (pos != mapDeltas.end()) {
409-
const std::pair<double, CAmount> &deltas = pos->second;
410-
if (deltas.second) {
411-
mapTx.modify(newit, update_fee_delta(deltas.second));
409+
const CAmount &delta = pos->second;
410+
if (delta) {
411+
mapTx.modify(newit, update_fee_delta(delta));
412412
}
413413
}
414414

@@ -910,16 +910,15 @@ CTxMemPool::ReadFeeEstimates(CAutoFile& filein)
910910
return true;
911911
}
912912

913-
void CTxMemPool::PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta)
913+
void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta)
914914
{
915915
{
916916
LOCK(cs);
917-
std::pair<double, CAmount> &deltas = mapDeltas[hash];
918-
deltas.first += dPriorityDelta;
919-
deltas.second += nFeeDelta;
917+
CAmount &delta = mapDeltas[hash];
918+
delta += nFeeDelta;
920919
txiter it = mapTx.find(hash);
921920
if (it != mapTx.end()) {
922-
mapTx.modify(it, update_fee_delta(deltas.second));
921+
mapTx.modify(it, update_fee_delta(delta));
923922
// Now update all ancestors' modified fees with descendants
924923
setEntries setAncestors;
925924
uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();
@@ -930,18 +929,17 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, double dPriorityDelt
930929
}
931930
}
932931
}
933-
LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", hash.ToString(), dPriorityDelta, FormatMoney(nFeeDelta));
932+
LogPrintf("PrioritiseTransaction: %s feerate += %s\n", hash.ToString(), FormatMoney(nFeeDelta));
934933
}
935934

936-
void CTxMemPool::ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const
935+
void CTxMemPool::ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const
937936
{
938937
LOCK(cs);
939-
std::map<uint256, std::pair<double, CAmount> >::const_iterator pos = mapDeltas.find(hash);
938+
std::map<uint256, CAmount>::const_iterator pos = mapDeltas.find(hash);
940939
if (pos == mapDeltas.end())
941940
return;
942-
const std::pair<double, CAmount> &deltas = pos->second;
943-
dPriorityDelta += deltas.first;
944-
nFeeDelta += deltas.second;
941+
const CAmount &delta = pos->second;
942+
nFeeDelta += delta;
945943
}
946944

947945
void CTxMemPool::ClearPrioritisation(const uint256 hash)

‎src/txmempool.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ class CTxMemPool
501501

502502
public:
503503
indirectmap<COutPoint, const CTransaction*> mapNextTx;
504-
std::map<uint256, std::pair<double, CAmount> > mapDeltas;
504+
std::map<uint256, CAmount> mapDeltas;
505505

506506
/** Create a new CTxMemPool.
507507
*/
@@ -543,8 +543,8 @@ class CTxMemPool
543543
bool HasNoInputsOf(const CTransaction& tx) const;
544544

545545
/** Affect CreateNewBlock prioritisation of transactions */
546-
void PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta);
547-
void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const;
546+
void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta);
547+
void ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const;
548548
void ClearPrioritisation(const uint256 hash);
549549

550550
public:

‎src/validation.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -720,8 +720,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
720720
CAmount nFees = nValueIn-nValueOut;
721721
// nModifiedFees includes any fee deltas from PrioritiseTransaction
722722
CAmount nModifiedFees = nFees;
723-
double nPriorityDummy = 0;
724-
pool.ApplyDeltas(hash, nPriorityDummy, nModifiedFees);
723+
pool.ApplyDelta(hash, nModifiedFees);
725724

726725
CAmount inChainInputValue;
727726
double dPriority = view.GetPriority(tx, chainActive.Height(), inChainInputValue);
@@ -4184,7 +4183,6 @@ bool LoadMempool(void)
41844183
}
41854184
uint64_t num;
41864185
file >> num;
4187-
double prioritydummy = 0;
41884186
while (num--) {
41894187
CTransactionRef tx;
41904188
int64_t nTime;
@@ -4195,7 +4193,7 @@ bool LoadMempool(void)
41954193

41964194
CAmount amountdelta = nFeeDelta;
41974195
if (amountdelta) {
4198-
mempool.PrioritiseTransaction(tx->GetHash(), prioritydummy, amountdelta);
4196+
mempool.PrioritiseTransaction(tx->GetHash(), amountdelta);
41994197
}
42004198
CValidationState state;
42014199
if (nTime + nExpiryTimeout > nNow) {
@@ -4216,7 +4214,7 @@ bool LoadMempool(void)
42164214
file >> mapDeltas;
42174215

42184216
for (const auto& i : mapDeltas) {
4219-
mempool.PrioritiseTransaction(i.first, prioritydummy, i.second);
4217+
mempool.PrioritiseTransaction(i.first, i.second);
42204218
}
42214219
} catch (const std::exception& e) {
42224220
LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what());
@@ -4237,7 +4235,7 @@ void DumpMempool(void)
42374235
{
42384236
LOCK(mempool.cs);
42394237
for (const auto &i : mempool.mapDeltas) {
4240-
mapDeltas[i.first] = i.second.second;
4238+
mapDeltas[i.first] = i.second;
42414239
}
42424240
vinfo = mempool.infoAll();
42434241
}

0 commit comments

Comments
 (0)