diff options
| author | fanquake <[email protected]> | 2020-08-14 15:32:04 +0800 |
|---|---|---|
| committer | fanquake <[email protected]> | 2020-08-14 16:04:31 +0800 |
| commit | c0b17069643271fb8cc36b1eb6c1dec7180977cb (patch) | |
| tree | 54b6bbb0a26b23ff17746bf1eda948df90fdcb24 /src/script/script_error.cpp | |
| parent | Merge #19528: rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc) (diff) | |
| parent | wallet: Don't override signing errors (diff) | |
| download | discoin-c0b17069643271fb8cc36b1eb6c1dec7180977cb.tar.xz discoin-c0b17069643271fb8cc36b1eb6c1dec7180977cb.zip | |
Merge #19568: Wallet should not override signing errors
e7448d66803f42984018ef8dfa6699027cb9536d wallet: Don't override signing errors (Fabian Jahr)
Pull request description:
While reviewing #17204 I noticed that the errors in `input_errors` from `::SignTransaction` where being overridden by `CWallet::SignTransaction`. For example, a Script related error led to incomplete signature data which led to `CWallet::SignTransaction` reporting that keys were missing, which was a less precise error than the original one.
Additionally, the error `"Input not found or already spent"` is [duplicated in `sign.cpp`](https://github.com/bitcoin/bitcoin/blob/c7b4968552c704f1e2e9a046911e1207f5af5fe0/src/script/sign.cpp#L481), so the error here is redundant at the moment. So technically the whole error block could be removed, I think. However, this code is affected by the ongoing work on the wallet so there might be a reason why these errors are here. But even if there is a reason to keep them, I don't think existing, potentially more precise errors should be overridden here unless we want to hide them from the users. I am looking for feedback if this is a work in progress state where these errors could be more useful in the future or if they can be removed.
On testing: even though [the errors in `CWallet` are covered](https://marcofalke.github.io/btc_cov/total.coverage/src/wallet/wallet.cpp.gcov.html), all tests still pass after removing them. I am not sure if there is a desire to cover these specific error messages, tests in `test/functional/rpc_signrawtransaction.py` seem to aim for a more generic approach.
ACKs for top commit:
achow101:
ACK e7448d66803f42984018ef8dfa6699027cb9536d
meshcollider:
Code review ACK e7448d66803f42984018ef8dfa6699027cb9536d
Tree-SHA512: 3e2bc11d05379d2aef87b093a383d1b044787efc70e35955b2f8ecd028b6acef02f386180566af6a1a63193635f5d685466e2f6141c96326c49ffc5c81ca3e23
Diffstat (limited to 'src/script/script_error.cpp')
0 files changed, 0 insertions, 0 deletions