kvazar
/
kevacoin
mirror of https://github.com/kvazar-network/kevacoin.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Return correct error codes in bumpfee(). 

The bumpfee() RPC was returning misleading or incorrect error codes
(for example RPC_INVALID_ADDRESS_OR_KEY when the transaction was not
BIP125 replacable). This commit fixes those error codes:

- RPC_INVALID_ADDRESS_OR_KEY if an invalid address was provided:
    - Invalid change address given
- RPC_INVALID_PARAMETER if a single (non-address/key) parameter is incorrect
    - confTarget and totalFee options should not both be set.
    - Invalid confTarget
    - Insufficient totalFee (cannot be less than required fee)
- RPC_WALLET_ERROR for any other error
    - Transaction has descendants in the wallet
    - Transaction has descendants in the mempool
    - Transaction has been mined, or is conflicted with a mined transaction
    - Transaction is not BIP 125 replaceable
    - Transaction has already been bumped
    - Transaction contains inputs that don't belong to the wallet
    - Transaction has multiple change outputs
    - Transaction does not have a change output
    - Fee is higher than maxTxFee
    - New fee rate is less than the minimum fee rate
    - Change output is too small.

This commit also updates the test cases to explicitly test the error code.
0.15
John Newbery 8 years ago
parent
47510ad3dd
commit
6d07c62322
2 changed files with 20 additions and 22 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 18
      qa/rpc-tests/bumpfee.py
  2. 24
      src/wallet/rpcwallet.cpp

18
qa/rpc-tests/bumpfee.py
Unescape View File

@ -128,7 +128,7 @@ def test_segwit_bumpfee_succeeds(rbf_node, dest_address):
def test_nonrbf_bumpfee_fails(peer_node, dest_address): def test_nonrbf_bumpfee_fails(peer_node, dest_address):
# cannot replace a non RBF transaction (from node which did not enable RBF) # cannot replace a non RBF transaction (from node which did not enable RBF)
not_rbfid = create_fund_sign_send(peer_node, {dest_address: 0.00090000}) not_rbfid = create_fund_sign_send(peer_node, {dest_address: 0.00090000})
assert_raises_message(JSONRPCException, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid) assert_raises_jsonrpc(-4, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid)
def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address): def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address):
@ -148,7 +148,7 @@ def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address):
signedtx = rbf_node.signrawtransaction(rawtx) signedtx = rbf_node.signrawtransaction(rawtx)
signedtx = peer_node.signrawtransaction(signedtx["hex"]) signedtx = peer_node.signrawtransaction(signedtx["hex"])
rbfid = rbf_node.sendrawtransaction(signedtx["hex"]) rbfid = rbf_node.sendrawtransaction(signedtx["hex"])
assert_raises_message(JSONRPCException, "Transaction contains inputs that don't belong to this wallet", assert_raises_jsonrpc(-4, "Transaction contains inputs that don't belong to this wallet",
rbf_node.bumpfee, rbfid) rbf_node.bumpfee, rbfid)
@ -159,7 +159,7 @@ def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address)
tx = rbf_node.createrawtransaction([{"txid": parent_id, "vout": 0}], {dest_address: 0.00020000}) tx = rbf_node.createrawtransaction([{"txid": parent_id, "vout": 0}], {dest_address: 0.00020000})
tx = rbf_node.signrawtransaction(tx) tx = rbf_node.signrawtransaction(tx)
txid = rbf_node.sendrawtransaction(tx["hex"]) txid = rbf_node.sendrawtransaction(tx["hex"])
assert_raises_message(JSONRPCException, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id) assert_raises_jsonrpc(-8, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id)
def test_small_output_fails(rbf_node, dest_address): def test_small_output_fails(rbf_node, dest_address):
@ -174,7 +174,7 @@ def test_small_output_fails(rbf_node, dest_address):
Decimal("0.00100000"), Decimal("0.00100000"),
{dest_address: 0.00080000, {dest_address: 0.00080000,
get_change_address(rbf_node): Decimal("0.00010000")}) get_change_address(rbf_node): Decimal("0.00010000")})
assert_raises_message(JSONRPCException, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 20001}) assert_raises_jsonrpc(-4, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 20001})
def test_dust_to_fee(rbf_node, dest_address): def test_dust_to_fee(rbf_node, dest_address):
@ -209,7 +209,7 @@ def test_rebumping(rbf_node, dest_address):
rbf_node.settxfee(Decimal("0.00001000")) rbf_node.settxfee(Decimal("0.00001000"))
rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000}) rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000})
bumped = rbf_node.bumpfee(rbfid, {"totalFee": 1000}) bumped = rbf_node.bumpfee(rbfid, {"totalFee": 1000})
assert_raises_message(JSONRPCException, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 2000}) assert_raises_jsonrpc(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 2000})
rbf_node.bumpfee(bumped["txid"], {"totalFee": 2000}) rbf_node.bumpfee(bumped["txid"], {"totalFee": 2000})
@ -217,7 +217,7 @@ def test_rebumping_not_replaceable(rbf_node, dest_address):
# check that re-bumping a non-replaceable bump tx fails # check that re-bumping a non-replaceable bump tx fails
rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000}) rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000})
bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000, "replaceable": False}) bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000, "replaceable": False})
assert_raises_message(JSONRPCException, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"], assert_raises_jsonrpc(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"],
{"totalFee": 20000}) {"totalFee": 20000})
@ -268,7 +268,7 @@ def test_bumpfee_metadata(rbf_node, dest_address):
def test_locked_wallet_fails(rbf_node, dest_address): def test_locked_wallet_fails(rbf_node, dest_address):
rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000}) rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000})
rbf_node.walletlock() rbf_node.walletlock()
assert_raises_message(JSONRPCException, "Please enter the wallet passphrase with walletpassphrase first.", assert_raises_jsonrpc(-13, "Please enter the wallet passphrase with walletpassphrase first.",
rbf_node.bumpfee, rbfid) rbf_node.bumpfee, rbfid)
@ -315,9 +315,7 @@ def submit_block_with_tx(node, tx):
block.rehash() block.rehash()
block.hashMerkleRoot = block.calc_merkle_root() block.hashMerkleRoot = block.calc_merkle_root()
block.solve() block.solve()
error = node.submitblock(bytes_to_hex_str(block.serialize(True))) node.submitblock(bytes_to_hex_str(block.serialize(True)))
if error is not None:
raise Exception(error)
return block return block

24
src/wallet/rpcwallet.cpp
Unescape View File

@ -2707,7 +2707,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
CBitcoinAddress address(options["changeAddress"].get_str()); CBitcoinAddress address(options["changeAddress"].get_str());
if (!address.IsValid()) if (!address.IsValid())
throw JSONRPCError(RPC_INVALID_PARAMETER, "changeAddress must be a valid bitcoin address"); throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "changeAddress must be a valid bitcoin address");
changeAddress = address.Get(); changeAddress = address.Get();
} }
@ -2862,33 +2862,33 @@ UniValue bumpfee(const JSONRPCRequest& request)
CWalletTx& wtx = pwallet->mapWallet[hash]; CWalletTx& wtx = pwallet->mapWallet[hash];
if (pwallet->HasWalletSpend(hash)) { if (pwallet->HasWalletSpend(hash)) {
throw JSONRPCError(RPC_MISC_ERROR, "Transaction has descendants in the wallet"); throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction has descendants in the wallet");
} }
{ {
LOCK(mempool.cs); LOCK(mempool.cs);
auto it = mempool.mapTx.find(hash); auto it = mempool.mapTx.find(hash);
if (it != mempool.mapTx.end() && it->GetCountWithDescendants() > 1) { if (it != mempool.mapTx.end() && it->GetCountWithDescendants() > 1) {
throw JSONRPCError(RPC_MISC_ERROR, "Transaction has descendants in the mempool"); throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction has descendants in the mempool");
} }
} }
if (wtx.GetDepthInMainChain() != 0) { if (wtx.GetDepthInMainChain() != 0) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction has been mined, or is conflicted with a mined transaction"); throw JSONRPCError(RPC_WALLET_ERROR, "Transaction has been mined, or is conflicted with a mined transaction");
} }
if (!SignalsOptInRBF(wtx)) { if (!SignalsOptInRBF(wtx)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction is not BIP 125 replaceable"); throw JSONRPCError(RPC_WALLET_ERROR, "Transaction is not BIP 125 replaceable");
} }
if (wtx.mapValue.count("replaced_by_txid")) { if (wtx.mapValue.count("replaced_by_txid")) {
throw JSONRPCError(RPC_INVALID_REQUEST, strprintf("Cannot bump transaction %s which was already bumped by transaction %s", hash.ToString(), wtx.mapValue.at("replaced_by_txid"))); throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Cannot bump transaction %s which was already bumped by transaction %s", hash.ToString(), wtx.mapValue.at("replaced_by_txid")));
} }
// check that original tx consists entirely of our inputs // check that original tx consists entirely of our inputs
// if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee) // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
if (!pwallet->IsAllFromMe(wtx, ISMINE_SPENDABLE)) { if (!pwallet->IsAllFromMe(wtx, ISMINE_SPENDABLE)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction contains inputs that don't belong to this wallet"); throw JSONRPCError(RPC_WALLET_ERROR, "Transaction contains inputs that don't belong to this wallet");
} }
// figure out which output was change // figure out which output was change
@ -2897,13 +2897,13 @@ UniValue bumpfee(const JSONRPCRequest& request)
for (size_t i = 0; i < wtx.tx->vout.size(); ++i) { for (size_t i = 0; i < wtx.tx->vout.size(); ++i) {
if (pwallet->IsChange(wtx.tx->vout[i])) { if (pwallet->IsChange(wtx.tx->vout[i])) {
if (nOutput != -1) { if (nOutput != -1) {
throw JSONRPCError(RPC_MISC_ERROR, "Transaction has multiple change outputs"); throw JSONRPCError(RPC_WALLET_ERROR, "Transaction has multiple change outputs");
} }
nOutput = i; nOutput = i;
} }
} }
if (nOutput == -1) { if (nOutput == -1) {
throw JSONRPCError(RPC_MISC_ERROR, "Transaction does not have a change output"); throw JSONRPCError(RPC_WALLET_ERROR, "Transaction does not have a change output");
} }
// Calculate the expected size of the new transaction. // Calculate the expected size of the new transaction.
@ -2994,7 +2994,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
// Check that in all cases the new fee doesn't violate maxTxFee // Check that in all cases the new fee doesn't violate maxTxFee
if (nNewFee > maxTxFee) { if (nNewFee > maxTxFee) {
throw JSONRPCError(RPC_MISC_ERROR, throw JSONRPCError(RPC_WALLET_ERROR,
strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)", strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)",
FormatMoney(nNewFee), FormatMoney(maxTxFee))); FormatMoney(nNewFee), FormatMoney(maxTxFee)));
} }
@ -3006,7 +3006,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
// moment earlier. In this case, we report an error to the user, who may use totalFee to make an adjustment. // moment earlier. In this case, we report an error to the user, who may use totalFee to make an adjustment.
CFeeRate minMempoolFeeRate = mempool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); CFeeRate minMempoolFeeRate = mempool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) { if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
throw JSONRPCError(RPC_MISC_ERROR, strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK()))); throw JSONRPCError(RPC_WALLET_ERROR, strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK())));
} }
// Now modify the output to increase the fee. // Now modify the output to increase the fee.
@ -3016,7 +3016,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
CMutableTransaction tx(*(wtx.tx)); CMutableTransaction tx(*(wtx.tx));
CTxOut* poutput = &(tx.vout[nOutput]); CTxOut* poutput = &(tx.vout[nOutput]);
if (poutput->nValue < nDelta) { if (poutput->nValue < nDelta) {
throw JSONRPCError(RPC_MISC_ERROR, "Change output is too small to bump the fee"); throw JSONRPCError(RPC_WALLET_ERROR, "Change output is too small to bump the fee");
} }
// If the output would become dust, discard it (converting the dust to fee) // If the output would become dust, discard it (converting the dust to fee)

Write Preview
Loading…
Cancel
Save