More user-friendly error message when partially signing #11288

pull meshcollider wants to merge 1 commits into bitcoin:master from meshcollider:201709_partial_sign_error changing 1 files +6 −1

meshcollider commented at 11:43 AM on September 8, 2017: contributor

When partially signing a transaction using signrawtransaction, if the wallet doesn't have access to a key, it will output a scary error message "error": "Operation not valid with the current stack size", yet it will partially sign the transaction anyway. This puts a lot of users off, because they don't realise the signing actually succeeded for some inputs. This catches that specific error when signing, and outputs a friendlier message which says Unable to sign input, invalid stack size (possibly missing key).

This is the best way I could think of to fix the issue, but please let me know if you come up with a better way to do it :)

Fixes https://github.com/bitcoin/bitcoin/issues/9988
fanquake added the label RPC/REST/ZMQ on Sep 8, 2017

in src/rpc/rawtransaction.cpp:877 in b0a1413ea1 outdated

 872 | @@ -873,7 +873,13 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
 873 |  
 874 |          ScriptError serror = SCRIPT_ERR_OK;
 875 |          if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) {
 876 | -            TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror));
 877 | +            if(serror == SCRIPT_ERR_INVALID_STACK_OPERATION) {
 878 | +                // we haven't been able to sign this input and verification failed, could be attempting to partially sign

promag commented at 9:37 AM on September 11, 2017:

Nit:

// Unable to sign input and verification failed (possible attempt to partially sign).

in src/rpc/rawtransaction.cpp:880 in b0a1413ea1 outdated

 872 | @@ -873,7 +873,13 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
 873 |  
 874 |          ScriptError serror = SCRIPT_ERR_OK;
 875 |          if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) {
 876 | -            TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror));
 877 | +            if(serror == SCRIPT_ERR_INVALID_STACK_OPERATION) {
 878 | +                // we haven't been able to sign this input and verification failed, could be attempting to partially sign
 879 | +                TxInErrorToJSON(txin, vErrors, "Unable to sign input, invalid stack size (possibly missing key)");
 880 | +            } else {
 881 | +               // some other issue

promag commented at 9:38 AM on September 11, 2017:

Nit, remove comment.

promag commented at 9:38 AM on September 11, 2017: member

Care to add a test for expected output?
More user-friendly error message when partially signing df10edfd03

in src/rpc/rawtransaction.cpp:876 in b0a1413ea1 outdated

 872 | @@ -873,7 +873,13 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
 873 |  
 874 |          ScriptError serror = SCRIPT_ERR_OK;
 875 |          if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) {
 876 | -            TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror));
 877 | +            if(serror == SCRIPT_ERR_INVALID_STACK_OPERATION) {

promag commented at 9:39 AM on September 11, 2017:

Missing space after if.

laanwj commented at 9:33 AM on September 16, 2017: member

utACK, to me honest this caught me off-guard too the first time I used multi-signed transactions.
achow101 commented at 4:06 PM on September 16, 2017: member

utACK df10edfd03fe4c9d1eca43a17ac4c54a14ac4832
instagibbs commented at 6:55 PM on September 18, 2017: member

utACK https://github.com/bitcoin/bitcoin/pull/11288/commits/df10edfd03fe4c9d1eca43a17ac4c54a14ac4832

I've come across this as well too. thanks.
laanwj merged this on Sep 26, 2017
laanwj closed this on Sep 26, 2017
laanwj referenced this in commit 0f8e09599d on Sep 26, 2017
meshcollider deleted the branch on Oct 13, 2017
AmirAbrams referenced this in commit 45d069d5d6 on Nov 8, 2019
AmirAbrams referenced this in commit 41c999ca2f on Nov 22, 2019
PastaPastaPasta referenced this in commit c53d9a4075 on Dec 22, 2019
PastaPastaPasta referenced this in commit 50f8e063f0 on Jan 2, 2020
PastaPastaPasta referenced this in commit 7de2fb5927 on Jan 4, 2020
PastaPastaPasta referenced this in commit 6cd5df75d4 on Jan 12, 2020
PastaPastaPasta referenced this in commit 268f3a23a8 on Jan 12, 2020
PastaPastaPasta referenced this in commit 40389c5fa5 on Jan 12, 2020
PastaPastaPasta referenced this in commit c9e865fc16 on Jan 12, 2020
PastaPastaPasta referenced this in commit 8af0f8e2d8 on Jan 12, 2020
ckti referenced this in commit 9df4fced2f on Mar 28, 2021
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#9988 signrawtransaction shows errors after partially signing a multisig transaction