Skip to content

Commit 6d07c62

Browse files
author
John Newbery
committed
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.
1 parent 47510ad commit 6d07c62

File tree

2 files changed

+20
-22
lines changed

2 files changed

+20
-22
lines changed

‎qa/rpc-tests/bumpfee.py‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ def test_segwit_bumpfee_succeeds(rbf_node, dest_address):
128128
def test_nonrbf_bumpfee_fails(peer_node, dest_address):
129129
# cannot replace a non RBF transaction (from node which did not enable RBF)
130130
not_rbfid = create_fund_sign_send(peer_node, {dest_address: 0.00090000})
131-
assert_raises_message(JSONRPCException, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid)
131+
assert_raises_jsonrpc(-4, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid)
132132

133133

134134
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):
148148
signedtx = rbf_node.signrawtransaction(rawtx)
149149
signedtx = peer_node.signrawtransaction(signedtx["hex"])
150150
rbfid = rbf_node.sendrawtransaction(signedtx["hex"])
151-
assert_raises_message(JSONRPCException, "Transaction contains inputs that don't belong to this wallet",
151+
assert_raises_jsonrpc(-4, "Transaction contains inputs that don't belong to this wallet",
152152
rbf_node.bumpfee, rbfid)
153153

154154

@@ -159,7 +159,7 @@ def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address)
159159
tx = rbf_node.createrawtransaction([{"txid": parent_id, "vout": 0}], {dest_address: 0.00020000})
160160
tx = rbf_node.signrawtransaction(tx)
161161
txid = rbf_node.sendrawtransaction(tx["hex"])
162-
assert_raises_message(JSONRPCException, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id)
162+
assert_raises_jsonrpc(-8, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id)
163163

164164

165165
def test_small_output_fails(rbf_node, dest_address):
@@ -174,7 +174,7 @@ def test_small_output_fails(rbf_node, dest_address):
174174
Decimal("0.00100000"),
175175
{dest_address: 0.00080000,
176176
get_change_address(rbf_node): Decimal("0.00010000")})
177-
assert_raises_message(JSONRPCException, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 20001})
177+
assert_raises_jsonrpc(-4, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 20001})
178178

179179

180180
def test_dust_to_fee(rbf_node, dest_address):
@@ -209,15 +209,15 @@ def test_rebumping(rbf_node, dest_address):
209209
rbf_node.settxfee(Decimal("0.00001000"))
210210
rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000})
211211
bumped = rbf_node.bumpfee(rbfid, {"totalFee": 1000})
212-
assert_raises_message(JSONRPCException, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 2000})
212+
assert_raises_jsonrpc(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 2000})
213213
rbf_node.bumpfee(bumped["txid"], {"totalFee": 2000})
214214

215215

216216
def test_rebumping_not_replaceable(rbf_node, dest_address):
217217
# check that re-bumping a non-replaceable bump tx fails
218218
rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000})
219219
bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000, "replaceable": False})
220-
assert_raises_message(JSONRPCException, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"],
220+
assert_raises_jsonrpc(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"],
221221
{"totalFee": 20000})
222222

223223

@@ -268,7 +268,7 @@ def test_bumpfee_metadata(rbf_node, dest_address):
268268
def test_locked_wallet_fails(rbf_node, dest_address):
269269
rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000})
270270
rbf_node.walletlock()
271-
assert_raises_message(JSONRPCException, "Please enter the wallet passphrase with walletpassphrase first.",
271+
assert_raises_jsonrpc(-13, "Please enter the wallet passphrase with walletpassphrase first.",
272272
rbf_node.bumpfee, rbfid)
273273

274274

@@ -315,9 +315,7 @@ def submit_block_with_tx(node, tx):
315315
block.rehash()
316316
block.hashMerkleRoot = block.calc_merkle_root()
317317
block.solve()
318-
error = node.submitblock(bytes_to_hex_str(block.serialize(True)))
319-
if error is not None:
320-
raise Exception(error)
318+
node.submitblock(bytes_to_hex_str(block.serialize(True)))
321319
return block
322320

323321

‎src/wallet/rpcwallet.cpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2707,7 +2707,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
27072707
CBitcoinAddress address(options["changeAddress"].get_str());
27082708

27092709
if (!address.IsValid())
2710-
throw JSONRPCError(RPC_INVALID_PARAMETER, "changeAddress must be a valid bitcoin address");
2710+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "changeAddress must be a valid bitcoin address");
27112711

27122712
changeAddress = address.Get();
27132713
}
@@ -2862,33 +2862,33 @@ UniValue bumpfee(const JSONRPCRequest& request)
28622862
CWalletTx& wtx = pwallet->mapWallet[hash];
28632863

28642864
if (pwallet->HasWalletSpend(hash)) {
2865-
throw JSONRPCError(RPC_MISC_ERROR, "Transaction has descendants in the wallet");
2865+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction has descendants in the wallet");
28662866
}
28672867

28682868
{
28692869
LOCK(mempool.cs);
28702870
auto it = mempool.mapTx.find(hash);
28712871
if (it != mempool.mapTx.end() && it->GetCountWithDescendants() > 1) {
2872-
throw JSONRPCError(RPC_MISC_ERROR, "Transaction has descendants in the mempool");
2872+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction has descendants in the mempool");
28732873
}
28742874
}
28752875

28762876
if (wtx.GetDepthInMainChain() != 0) {
2877-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction has been mined, or is conflicted with a mined transaction");
2877+
throw JSONRPCError(RPC_WALLET_ERROR, "Transaction has been mined, or is conflicted with a mined transaction");
28782878
}
28792879

28802880
if (!SignalsOptInRBF(wtx)) {
2881-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction is not BIP 125 replaceable");
2881+
throw JSONRPCError(RPC_WALLET_ERROR, "Transaction is not BIP 125 replaceable");
28822882
}
28832883

28842884
if (wtx.mapValue.count("replaced_by_txid")) {
2885-
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")));
2885+
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")));
28862886
}
28872887

28882888
// check that original tx consists entirely of our inputs
28892889
// 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)
28902890
if (!pwallet->IsAllFromMe(wtx, ISMINE_SPENDABLE)) {
2891-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction contains inputs that don't belong to this wallet");
2891+
throw JSONRPCError(RPC_WALLET_ERROR, "Transaction contains inputs that don't belong to this wallet");
28922892
}
28932893

28942894
// figure out which output was change
@@ -2897,13 +2897,13 @@ UniValue bumpfee(const JSONRPCRequest& request)
28972897
for (size_t i = 0; i < wtx.tx->vout.size(); ++i) {
28982898
if (pwallet->IsChange(wtx.tx->vout[i])) {
28992899
if (nOutput != -1) {
2900-
throw JSONRPCError(RPC_MISC_ERROR, "Transaction has multiple change outputs");
2900+
throw JSONRPCError(RPC_WALLET_ERROR, "Transaction has multiple change outputs");
29012901
}
29022902
nOutput = i;
29032903
}
29042904
}
29052905
if (nOutput == -1) {
2906-
throw JSONRPCError(RPC_MISC_ERROR, "Transaction does not have a change output");
2906+
throw JSONRPCError(RPC_WALLET_ERROR, "Transaction does not have a change output");
29072907
}
29082908

29092909
// Calculate the expected size of the new transaction.
@@ -2994,7 +2994,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
29942994

29952995
// Check that in all cases the new fee doesn't violate maxTxFee
29962996
if (nNewFee > maxTxFee) {
2997-
throw JSONRPCError(RPC_MISC_ERROR,
2997+
throw JSONRPCError(RPC_WALLET_ERROR,
29982998
strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)",
29992999
FormatMoney(nNewFee), FormatMoney(maxTxFee)));
30003000
}
@@ -3006,7 +3006,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
30063006
// moment earlier. In this case, we report an error to the user, who may use totalFee to make an adjustment.
30073007
CFeeRate minMempoolFeeRate = mempool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
30083008
if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
3009-
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())));
3009+
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())));
30103010
}
30113011

30123012
// Now modify the output to increase the fee.
@@ -3016,7 +3016,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
30163016
CMutableTransaction tx(*(wtx.tx));
30173017
CTxOut* poutput = &(tx.vout[nOutput]);
30183018
if (poutput->nValue < nDelta) {
3019-
throw JSONRPCError(RPC_MISC_ERROR, "Change output is too small to bump the fee");
3019+
throw JSONRPCError(RPC_WALLET_ERROR, "Change output is too small to bump the fee");
30203020
}
30213021

30223022
// If the output would become dust, discard it (converting the dust to fee)

0 commit comments

Comments
 (0)