sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-25 20:12:57 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #19568: Wallet should not override signing errors

Browse Source 
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`](c7b4968552/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
This commit is contained in:
fanquake 2020-08-14 15:32:04 +08:00 committed by Konstantin Akimov
parent 9600f9ca35
commit 3575a6c4b1
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
1 changed files with 0 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/wallet/wallet.cpp
View File

@ -2960,23 +2960,6 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint,
}
// At this point, one input was not fully signed otherwise we would have exited already
// Find that input and figure out what went wrong.
for (unsigned int i = 0; i < tx.vin.size(); i++) {
// Get the prevout
CTxIn& txin = tx.vin[i];
auto coin = coins.find(txin.prevout);
if (coin == coins.end() || coin->second.IsSpent()) {
input_errors[i] = "Input not found or already spent";
continue;
}
// Check if this input is complete
SignatureData sigdata = DataFromTransaction(tx, i, coin->second.out);
if (!sigdata.complete) {
input_errors[i] = "Unable to sign input, missing keys";
continue;
}
}
return false;
}