From 870824e919474c5b39da73fe73199f8453fd540f Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Fri, 21 Apr 2017 16:41:01 +0000 Subject: [PATCH] RPC/Mining: Restore API compatibility for prioritisetransaction Breaking API serves no purpose other than to be incompatible with older versions and other implementations that do support priority --- src/rpc/client.cpp | 3 ++- src/rpc/mining.cpp | 19 ++++++++++++------- test/functional/bip68-sequence.py | 4 ++-- test/functional/mempool_packages.py | 8 ++++---- test/functional/prioritise_transaction.py | 8 ++++---- test/functional/replace-by-fee.py | 4 ++-- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index a3a692c14..ed83d1da1 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -112,7 +112,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "estimaterawfee", 0, "nblocks" }, { "estimaterawfee", 1, "threshold" }, { "estimaterawfee", 2, "horizon" }, - { "prioritisetransaction", 1, "fee_delta" }, + { "prioritisetransaction", 1, "priority_delta" }, + { "prioritisetransaction", 2, "fee_delta" }, { "setban", 2, "bantime" }, { "setban", 3, "absolute" }, { "setnetworkactive", 0, "state" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d744269df..1c6f4082e 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -257,26 +257,31 @@ 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() != 2) + if (request.fHelp || request.params.size() != 3) throw std::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. fee_delta (numeric, required) The fee value (in satoshis) to add (or subtract, if negative).\n" + "2. priority_delta (numeric, optional) Fee-independent priority adjustment. Not supported, so must be zero or null.\n" + "3. 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\" 10000") - + HelpExampleRpc("prioritisetransaction", "\"txid\", 10000") + + HelpExampleCli("prioritisetransaction", "\"txid\" 0.0 10000") + + HelpExampleRpc("prioritisetransaction", "\"txid\", 0.0, 10000") ); LOCK(cs_main); uint256 hash = ParseHashStr(request.params[0].get_str(), "txid"); - CAmount nAmount = request.params[1].get_int64(); + CAmount nAmount = request.params[2].get_int64(); + + if (!(request.params[1].isNull() || request.params[1].get_real() == 0)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is not supported, and adjustment thereof must be zero."); + } mempool.PrioritiseTransaction(hash, nAmount); return true; @@ -959,7 +964,7 @@ static const CRPCCommand commands[] = // --------------------- ------------------------ ----------------------- ---------- { "mining", "getnetworkhashps", &getnetworkhashps, true, {"nblocks","height"} }, { "mining", "getmininginfo", &getmininginfo, true, {} }, - { "mining", "prioritisetransaction", &prioritisetransaction, true, {"txid","fee_delta"} }, + { "mining", "prioritisetransaction", &prioritisetransaction, true, {"txid","priority_delta","fee_delta"} }, { "mining", "getblocktemplate", &getblocktemplate, true, {"template_request"} }, { "mining", "submitblock", &submitblock, true, {"hexdata","parameters"} }, diff --git a/test/functional/bip68-sequence.py b/test/functional/bip68-sequence.py index db66b7719..87a50692f 100755 --- a/test/functional/bip68-sequence.py +++ b/test/functional/bip68-sequence.py @@ -241,7 +241,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, int(-self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(txid=tx2.hash, fee_delta=int(-self.relayfee*COIN)) cur_time = int(time.time()) for i in range(10): self.nodes[0].setmocktime(cur_time + 600) @@ -254,7 +254,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, int(self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(txid=tx2.hash, fee_delta=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/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index 72f04095f..e22549381 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -96,7 +96,7 @@ class MempoolPackagesTest(BitcoinTestFramework): # Check that ancestor modified fees includes fee deltas from # prioritisetransaction - self.nodes[0].prioritisetransaction(chain[0], 1000) + self.nodes[0].prioritisetransaction(txid=chain[0], fee_delta=1000) mempool = self.nodes[0].getrawmempool(True) ancestor_fees = 0 for x in chain: @@ -104,11 +104,11 @@ class MempoolPackagesTest(BitcoinTestFramework): assert_equal(mempool[x]['ancestorfees'], ancestor_fees * COIN + 1000) # Undo the prioritisetransaction for later tests - self.nodes[0].prioritisetransaction(chain[0], -1000) + self.nodes[0].prioritisetransaction(txid=chain[0], fee_delta=-1000) # Check that descendant modified fees includes fee deltas from # prioritisetransaction - self.nodes[0].prioritisetransaction(chain[-1], 1000) + self.nodes[0].prioritisetransaction(txid=chain[-1], fee_delta=1000) mempool = self.nodes[0].getrawmempool(True) descendant_fees = 0 @@ -126,7 +126,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], 2000) + self.nodes[0].prioritisetransaction(txid=chain[-1], fee_delta=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/test/functional/prioritise_transaction.py b/test/functional/prioritise_transaction.py index 9c3b3fd5d..4fc03d254 100755 --- a/test/functional/prioritise_transaction.py +++ b/test/functional/prioritise_transaction.py @@ -46,7 +46,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], int(3*base_fee*COIN)) + self.nodes[0].prioritisetransaction(txid=txids[0][0], fee_delta=int(3*base_fee*COIN)) self.nodes[0].generate(1) @@ -65,7 +65,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, -int(2*base_fee*COIN)) + self.nodes[0].prioritisetransaction(txid=high_fee_tx, fee_delta=-int(2*base_fee*COIN)) # Add everything back to mempool self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) @@ -109,7 +109,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, int(self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(txid=tx_id, fee_delta=int(self.relayfee*COIN)) self.log.info("Assert that prioritised free transaction is accepted to mempool") assert_equal(self.nodes[0].sendrawtransaction(tx_hex), tx_id) @@ -120,7 +120,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): mock_time = int(time.time()) self.nodes[0].setmocktime(mock_time) template = self.nodes[0].getblocktemplate() - self.nodes[0].prioritisetransaction(tx_id, -int(self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(txid=tx_id, fee_delta=-int(self.relayfee*COIN)) self.nodes[0].setmocktime(mock_time+10) new_template = self.nodes[0].getblocktemplate() diff --git a/test/functional/replace-by-fee.py b/test/functional/replace-by-fee.py index e940ce535..077348e3b 100755 --- a/test/functional/replace-by-fee.py +++ b/test/functional/replace-by-fee.py @@ -482,7 +482,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert_raises_jsonrpc(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx1b_hex, True) # Use prioritisetransaction to set tx1a's fee to 0. - self.nodes[0].prioritisetransaction(tx1a_txid, int(-0.1*COIN)) + self.nodes[0].prioritisetransaction(txid=tx1a_txid, fee_delta=int(-0.1*COIN)) # Now tx1b should be able to replace tx1a tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True) @@ -509,7 +509,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert_raises_jsonrpc(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx2b_hex, True) # Now prioritise tx2b to have a higher modified fee - self.nodes[0].prioritisetransaction(tx2b.hash, int(0.1*COIN)) + self.nodes[0].prioritisetransaction(txid=tx2b.hash, fee_delta=int(0.1*COIN)) # tx2b should now be accepted tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, True)