Return correct error codes from bumpfee()

[jnewbery]

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 PR changes those error codes to what I think are
more sensible:

[EDITED]

• 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.

[/EDITED]

This PR also updates the test cases to explicitly test the error code.

This PR builds on top of #9707

[EDIT: updated error codes/classes of error above]

[laanwj]

Thanks for trying to improve the error codes, however please don't use these two for application-level errors, they are reserved for errors in low-level protocol handling:

• RPC_INVALID_REQUEST is mapped to HTTP_BAD_REQUEST (400)
• RPC_METHOD_NOT_FOUND is mapped to HTTP_NOT_FOUND (404)

[jnewbery]

Thanks @laanwj - can you confirm that none of the error codes in this block should be used for application-level errors:

    //! Standard JSON-RPC 2.0 errors
    RPC_INVALID_REQUEST  = -32600,
    RPC_METHOD_NOT_FOUND = -32601,
    RPC_INVALID_PARAMS   = -32602,
    RPC_INTERNAL_ERROR   = -32603,
    RPC_PARSE_ERROR      = -32700,

I've grepped through the RPC code and it appears that those are mostly not being used in the application-level code except for:

• RPC_INTERNAL_ERROR - used (correctly I think) in rpc/blockchain.cpp and rawtransaction.cpp when the internal datastructures are corrupt.
• RPC_INTERNAL_ERROR - abused in mining.cpp for application-level errors
• RPC_INVALID_REQUEST, RPC_METHOD_NOT_FOUND, and RPC_INTERNAL_ERROR abused in rpcwallet.cpp for application-level errors.

If you confirm my understanding is correct, I'll fix those up in future commits (and add a comment to protocol.h to warn people not to use the -32xxx codes for application-level errors).

[jnewbery]

I've removed all uses of RPC_INTERNAL_ERROR from bumpfee()

Reviewers: please only review the second commit in this PR. The first commit is #9707

[laanwj]

@jnewbery Not so sure about avoiding all the standard JSON-RPC errors. But propagating the two I mention above to the HTTP layer is a historical accident, which we can't undo without breaking the interface, but it means it's better to avoid them.

[jnewbery]

#9707 is now merged so this PR only contains the single commit which needs to be reviewed.

[morcos]

can you update the PR text at the top to what you think now as to which error codes should be used for which class of errors?

[jnewbery]

@morcos I've updated the error codes in the original PR notes (and updated the commit message).

[jnewbery]

Closing in favour of #9853.