From f9b9371c6027905f73a2558d6bcaca8a355c28a6 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 23:37:15 -0500 Subject: [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. --- qa/rpc-tests/bip68-sequence.py | 4 ++-- qa/rpc-tests/mempool_packages.py | 4 ++-- qa/rpc-tests/prioritise_transaction.py | 6 +++--- qa/rpc-tests/replace-by-fee.py | 4 ++-- src/rpc/client.cpp | 3 +-- src/rpc/mining.cpp | 19 ++++++++----------- src/txmempool.cpp | 28 +++++++++++++--------------- src/txmempool.h | 6 +++--- src/validation.cpp | 10 ++++------ 9 files changed, 38 insertions(+), 46 deletions(-) diff --git a/qa/rpc-tests/bip68-sequence.py b/qa/rpc-tests/bip68-sequence.py index f3a025afa5..fbf31b25f6 100755 --- a/qa/rpc-tests/bip68-sequence.py +++ b/qa/rpc-tests/bip68-sequence.py @@ -256,7 +256,7 @@ class BIP68Test(BitcoinTestFramework): # Now mine some blocks, but make sure tx2 doesn't get mined. # Use prioritisetransaction to lower the effective feerate to 0 - self.nodes[0].prioritisetransaction(tx2.hash, -1e15, int(-self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(tx2.hash, int(-self.relayfee*COIN)) cur_time = int(time.time()) for i in range(10): self.nodes[0].setmocktime(cur_time + 600) @@ -269,7 +269,7 @@ class BIP68Test(BitcoinTestFramework): test_nonzero_locks(tx2, self.nodes[0], self.relayfee, use_height_lock=False) # Mine tx2, and then try again - self.nodes[0].prioritisetransaction(tx2.hash, 1e15, int(self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(tx2.hash, int(self.relayfee*COIN)) # Advance the time on the node so that we can test timelocks self.nodes[0].setmocktime(cur_time+600) diff --git a/qa/rpc-tests/mempool_packages.py b/qa/rpc-tests/mempool_packages.py index 388889d07c..3437c89ec7 100755 --- a/qa/rpc-tests/mempool_packages.py +++ b/qa/rpc-tests/mempool_packages.py @@ -103,7 +103,7 @@ class MempoolPackagesTest(BitcoinTestFramework): # Check that descendant modified fees includes fee deltas from # prioritisetransaction - self.nodes[0].prioritisetransaction(chain[-1], 0, 1000) + self.nodes[0].prioritisetransaction(chain[-1], 1000) mempool = self.nodes[0].getrawmempool(True) descendant_fees = 0 @@ -124,7 +124,7 @@ class MempoolPackagesTest(BitcoinTestFramework): assert_equal(len(self.nodes[0].getrawmempool()), 0) # Prioritise a transaction that has been mined, then add it back to the # mempool by using invalidateblock. - self.nodes[0].prioritisetransaction(chain[-1], 0, 2000) + self.nodes[0].prioritisetransaction(chain[-1], 2000) self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) # Keep node1's tip synced with node0 self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash()) diff --git a/qa/rpc-tests/prioritise_transaction.py b/qa/rpc-tests/prioritise_transaction.py index 91d5d6be7d..13be6eeead 100755 --- a/qa/rpc-tests/prioritise_transaction.py +++ b/qa/rpc-tests/prioritise_transaction.py @@ -51,7 +51,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # add a fee delta to something in the cheapest bucket and make sure it gets mined # also check that a different entry in the cheapest bucket is NOT mined - self.nodes[0].prioritisetransaction(txids[0][0], 0, int(3*base_fee*COIN)) + self.nodes[0].prioritisetransaction(txids[0][0], int(3*base_fee*COIN)) self.nodes[0].generate(1) @@ -70,7 +70,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # Add a prioritisation before a tx is in the mempool (de-prioritising a # high-fee transaction so that it's now low fee). - self.nodes[0].prioritisetransaction(high_fee_tx, -1e15, -int(2*base_fee*COIN)) + self.nodes[0].prioritisetransaction(high_fee_tx, -int(2*base_fee*COIN)) # Add everything back to mempool self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) @@ -118,7 +118,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # This is a less than 1000-byte transaction, so just set the fee # to be the minimum for a 1000 byte transaction and check that it is # accepted. - self.nodes[0].prioritisetransaction(tx_id, 0, int(self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(tx_id, int(self.relayfee*COIN)) print("Assert that prioritised free transaction is accepted to mempool") assert_equal(self.nodes[0].sendrawtransaction(tx_hex), tx_id) diff --git a/qa/rpc-tests/replace-by-fee.py b/qa/rpc-tests/replace-by-fee.py index a3c1deddf6..51cbb4dc4c 100755 --- a/qa/rpc-tests/replace-by-fee.py +++ b/qa/rpc-tests/replace-by-fee.py @@ -543,7 +543,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert(False) # Use prioritisetransaction to set tx1a's fee to 0. - self.nodes[0].prioritisetransaction(tx1a_txid, 0, int(-0.1*COIN)) + self.nodes[0].prioritisetransaction(tx1a_txid, int(-0.1*COIN)) # Now tx1b should be able to replace tx1a tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True) @@ -575,7 +575,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert(False) # Now prioritise tx2b to have a higher modified fee - self.nodes[0].prioritisetransaction(tx2b.hash, 0, int(0.1*COIN)) + self.nodes[0].prioritisetransaction(tx2b.hash, int(0.1*COIN)) # tx2b should now be accepted tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, True) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 2d16868d4d..a8c5c21ef1 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -108,8 +108,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getrawmempool", 0, "verbose" }, { "estimatefee", 0, "nblocks" }, { "estimatesmartfee", 0, "nblocks" }, - { "prioritisetransaction", 1, "priority_delta" }, - { "prioritisetransaction", 2, "fee_delta" }, + { "prioritisetransaction", 1, "fee_delta" }, { "setban", 2, "bantime" }, { "setban", 3, "absolute" }, { "setnetworkactive", 0, "state" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d0d9e05994..4ff8f39edc 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -258,31 +258,28 @@ UniValue getmininginfo(const JSONRPCRequest& request) // NOTE: Unlike wallet RPC (which use BTC values), mining RPCs follow GBT (BIP 22) in using satoshi amounts UniValue prioritisetransaction(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 3) + if (request.fHelp || request.params.size() != 2) throw runtime_error( - "prioritisetransaction \n" + "prioritisetransaction \n" "Accepts the transaction into mined blocks at a higher (or lower) priority\n" "\nArguments:\n" "1. \"txid\" (string, required) The transaction id.\n" - "2. priority_delta (numeric, required) The priority to add or subtract.\n" - " The transaction selection algorithm considers the tx as it would have a higher priority.\n" - " (priority of a transaction is calculated: coinage * value_in_satoshis / txsize) \n" - "3. fee_delta (numeric, required) The fee value (in satoshis) to add (or subtract, if negative).\n" + "2. fee_delta (numeric, required) The fee value (in satoshis) to add (or subtract, if negative).\n" " The fee is not actually paid, only the algorithm for selecting transactions into a block\n" " considers the transaction as it would have paid a higher (or lower) fee.\n" "\nResult:\n" "true (boolean) Returns true\n" "\nExamples:\n" - + HelpExampleCli("prioritisetransaction", "\"txid\" 0.0 10000") - + HelpExampleRpc("prioritisetransaction", "\"txid\", 0.0, 10000") + + HelpExampleCli("prioritisetransaction", "\"txid\" 10000") + + HelpExampleRpc("prioritisetransaction", "\"txid\", 10000") ); LOCK(cs_main); uint256 hash = ParseHashStr(request.params[0].get_str(), "txid"); - CAmount nAmount = request.params[2].get_int64(); + CAmount nAmount = request.params[1].get_int64(); - mempool.PrioritiseTransaction(hash, request.params[1].get_real(), nAmount); + mempool.PrioritiseTransaction(hash, nAmount); return true; } @@ -853,7 +850,7 @@ static const CRPCCommand commands[] = // --------------------- ------------------------ ----------------------- ---------- { "mining", "getnetworkhashps", &getnetworkhashps, true, {"nblocks","height"} }, { "mining", "getmininginfo", &getmininginfo, true, {} }, - { "mining", "prioritisetransaction", &prioritisetransaction, true, {"txid","priority_delta","fee_delta"} }, + { "mining", "prioritisetransaction", &prioritisetransaction, true, {"txid","fee_delta"} }, { "mining", "getblocktemplate", &getblocktemplate, true, {"template_request"} }, { "mining", "submitblock", &submitblock, true, {"hexdata","parameters"} }, diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ef4d4f3c62..58a71ad95e 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -404,11 +404,11 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, // Update transaction for any feeDelta created by PrioritiseTransaction // TODO: refactor so that the fee delta is calculated before inserting // into mapTx. - std::map >::const_iterator pos = mapDeltas.find(hash); + std::map::const_iterator pos = mapDeltas.find(hash); if (pos != mapDeltas.end()) { - const std::pair &deltas = pos->second; - if (deltas.second) { - mapTx.modify(newit, update_fee_delta(deltas.second)); + const CAmount &delta = pos->second; + if (delta) { + mapTx.modify(newit, update_fee_delta(delta)); } } @@ -910,16 +910,15 @@ CTxMemPool::ReadFeeEstimates(CAutoFile& filein) return true; } -void CTxMemPool::PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta) +void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta) { { LOCK(cs); - std::pair &deltas = mapDeltas[hash]; - deltas.first += dPriorityDelta; - deltas.second += nFeeDelta; + CAmount &delta = mapDeltas[hash]; + delta += nFeeDelta; txiter it = mapTx.find(hash); if (it != mapTx.end()) { - mapTx.modify(it, update_fee_delta(deltas.second)); + mapTx.modify(it, update_fee_delta(delta)); // Now update all ancestors' modified fees with descendants setEntries setAncestors; uint64_t nNoLimit = std::numeric_limits::max(); @@ -930,18 +929,17 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, double dPriorityDelt } } } - LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", hash.ToString(), dPriorityDelta, FormatMoney(nFeeDelta)); + LogPrintf("PrioritiseTransaction: %s feerate += %s\n", hash.ToString(), FormatMoney(nFeeDelta)); } -void CTxMemPool::ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const +void CTxMemPool::ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const { LOCK(cs); - std::map >::const_iterator pos = mapDeltas.find(hash); + std::map::const_iterator pos = mapDeltas.find(hash); if (pos == mapDeltas.end()) return; - const std::pair &deltas = pos->second; - dPriorityDelta += deltas.first; - nFeeDelta += deltas.second; + const CAmount &delta = pos->second; + nFeeDelta += delta; } void CTxMemPool::ClearPrioritisation(const uint256 hash) diff --git a/src/txmempool.h b/src/txmempool.h index 9af65bd077..6b2e680305 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -501,7 +501,7 @@ private: public: indirectmap mapNextTx; - std::map > mapDeltas; + std::map mapDeltas; /** Create a new CTxMemPool. */ @@ -543,8 +543,8 @@ public: bool HasNoInputsOf(const CTransaction& tx) const; /** Affect CreateNewBlock prioritisation of transactions */ - void PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta); - void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const; + void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta); + void ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const; void ClearPrioritisation(const uint256 hash); public: diff --git a/src/validation.cpp b/src/validation.cpp index a21dc25a55..1f57468bc6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -720,8 +720,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C CAmount nFees = nValueIn-nValueOut; // nModifiedFees includes any fee deltas from PrioritiseTransaction CAmount nModifiedFees = nFees; - double nPriorityDummy = 0; - pool.ApplyDeltas(hash, nPriorityDummy, nModifiedFees); + pool.ApplyDelta(hash, nModifiedFees); CAmount inChainInputValue; double dPriority = view.GetPriority(tx, chainActive.Height(), inChainInputValue); @@ -4184,7 +4183,6 @@ bool LoadMempool(void) } uint64_t num; file >> num; - double prioritydummy = 0; while (num--) { CTransactionRef tx; int64_t nTime; @@ -4195,7 +4193,7 @@ bool LoadMempool(void) CAmount amountdelta = nFeeDelta; if (amountdelta) { - mempool.PrioritiseTransaction(tx->GetHash(), prioritydummy, amountdelta); + mempool.PrioritiseTransaction(tx->GetHash(), amountdelta); } CValidationState state; if (nTime + nExpiryTimeout > nNow) { @@ -4216,7 +4214,7 @@ bool LoadMempool(void) file >> mapDeltas; for (const auto& i : mapDeltas) { - mempool.PrioritiseTransaction(i.first, prioritydummy, i.second); + mempool.PrioritiseTransaction(i.first, i.second); } } catch (const std::exception& e) { LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); @@ -4237,7 +4235,7 @@ void DumpMempool(void) { LOCK(mempool.cs); for (const auto &i : mempool.mapDeltas) { - mapDeltas[i.first] = i.second.second; + mapDeltas[i.first] = i.second; } vinfo = mempool.infoAll(); } -- cgit v1.2.3