mirror of
https://github.com/bitcoin/bitcoin.git
synced 2024-11-19 18:09:47 +01:00
Merge #16251: Improve signrawtransaction error reporting
ec4c79326b
signrawtransaction*: improve error for partial signing (Anthony Towns)3c481f8921
signrawtransactionwithkey: better error messages for bad redeemScript/witnessScript (Anthony Towns) Pull request description: Two fixes for `signrawtransactionwith{key,wallet}` (in addition to #16250): one that checks redeemScript/witnessScript matches scriptPubKey (and if both are provided that they match each other sanely), and the other changes the warning when some-but-not-all the signatures for a CHECKMULTISIG are provided to something that suggests more signatures may be all that's required. Fixes: #13218 Fixes: #14823 ACKs for top commit: instagibbs: utACKec4c79326b
achow101: Code Review ACKec4c79326b
meshcollider: utACKec4c79326b
Tree-SHA512: 0c95c91d498e85b834662b9e5c83f336ed5fd306be7701ce1dbfa0836fbeb448a267a796585512f7496e820be668b07c2a0a2f45e52dc23f09ee7d9c87e42b35
This commit is contained in:
commit
2296fe65f5
@ -196,32 +196,73 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keyst
|
||||
}
|
||||
|
||||
// if redeemScript and private keys were given, add redeemScript to the keystore so it can be signed
|
||||
if (keystore && (scriptPubKey.IsPayToScriptHash() || scriptPubKey.IsPayToWitnessScriptHash())) {
|
||||
const bool is_p2sh = scriptPubKey.IsPayToScriptHash();
|
||||
const bool is_p2wsh = scriptPubKey.IsPayToWitnessScriptHash();
|
||||
if (keystore && (is_p2sh || is_p2wsh)) {
|
||||
RPCTypeCheckObj(prevOut,
|
||||
{
|
||||
{"redeemScript", UniValueType(UniValue::VSTR)},
|
||||
{"witnessScript", UniValueType(UniValue::VSTR)},
|
||||
}, true);
|
||||
UniValue rs = find_value(prevOut, "redeemScript");
|
||||
if (!rs.isNull()) {
|
||||
std::vector<unsigned char> rsData(ParseHexV(rs, "redeemScript"));
|
||||
CScript redeemScript(rsData.begin(), rsData.end());
|
||||
keystore->AddCScript(redeemScript);
|
||||
// Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH).
|
||||
// This is only for compatibility, it is encouraged to use the explicit witnessScript field instead.
|
||||
keystore->AddCScript(GetScriptForWitness(redeemScript));
|
||||
}
|
||||
UniValue ws = find_value(prevOut, "witnessScript");
|
||||
if (!ws.isNull()) {
|
||||
std::vector<unsigned char> wsData(ParseHexV(ws, "witnessScript"));
|
||||
CScript witnessScript(wsData.begin(), wsData.end());
|
||||
keystore->AddCScript(witnessScript);
|
||||
// Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH).
|
||||
keystore->AddCScript(GetScriptForWitness(witnessScript));
|
||||
}
|
||||
if (rs.isNull() && ws.isNull()) {
|
||||
throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing redeemScript/witnessScript");
|
||||
}
|
||||
|
||||
// work from witnessScript when possible
|
||||
std::vector<unsigned char> scriptData(!ws.isNull() ? ParseHexV(ws, "witnessScript") : ParseHexV(rs, "redeemScript"));
|
||||
CScript script(scriptData.begin(), scriptData.end());
|
||||
keystore->AddCScript(script);
|
||||
// Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH).
|
||||
// This is done for redeemScript only for compatibility, it is encouraged to use the explicit witnessScript field instead.
|
||||
CScript witness_output_script{GetScriptForWitness(script)};
|
||||
keystore->AddCScript(witness_output_script);
|
||||
|
||||
if (!ws.isNull() && !rs.isNull()) {
|
||||
// if both witnessScript and redeemScript are provided,
|
||||
// they should either be the same (for backwards compat),
|
||||
// or the redeemScript should be the encoded form of
|
||||
// the witnessScript (ie, for p2sh-p2wsh)
|
||||
if (ws.get_str() != rs.get_str()) {
|
||||
std::vector<unsigned char> redeemScriptData(ParseHexV(rs, "redeemScript"));
|
||||
CScript redeemScript(redeemScriptData.begin(), redeemScriptData.end());
|
||||
if (redeemScript != witness_output_script) {
|
||||
throw JSONRPCError(RPC_INVALID_PARAMETER, "redeemScript does not correspond to witnessScript");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (is_p2sh) {
|
||||
const CTxDestination p2sh{ScriptHash(script)};
|
||||
const CTxDestination p2sh_p2wsh{ScriptHash(witness_output_script)};
|
||||
if (scriptPubKey == GetScriptForDestination(p2sh)) {
|
||||
// traditional p2sh; arguably an error if
|
||||
// we got here with rs.IsNull(), because
|
||||
// that means the p2sh script was specified
|
||||
// via witnessScript param, but for now
|
||||
// we'll just quietly accept it
|
||||
} else if (scriptPubKey == GetScriptForDestination(p2sh_p2wsh)) {
|
||||
// p2wsh encoded as p2sh; ideally the witness
|
||||
// script was specified in the witnessScript
|
||||
// param, but also support specifying it via
|
||||
// redeemScript param for backwards compat
|
||||
// (in which case ws.IsNull() == true)
|
||||
} else {
|
||||
// otherwise, can't generate scriptPubKey from
|
||||
// either script, so we got unusable parameters
|
||||
throw JSONRPCError(RPC_INVALID_PARAMETER, "redeemScript/witnessScript does not match scriptPubKey");
|
||||
}
|
||||
} else if (is_p2wsh) {
|
||||
// plain p2wsh; could throw an error if script
|
||||
// was specified by redeemScript rather than
|
||||
// witnessScript (ie, ws.IsNull() == true), but
|
||||
// accept it for backwards compat
|
||||
const CTxDestination p2wsh{WitnessV0ScriptHash(script)};
|
||||
if (scriptPubKey != GetScriptForDestination(p2wsh)) {
|
||||
throw JSONRPCError(RPC_INVALID_PARAMETER, "redeemScript/witnessScript does not match scriptPubKey");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -268,6 +309,9 @@ UniValue SignTransaction(CMutableTransaction& mtx, const SigningProvider* keysto
|
||||
if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) {
|
||||
// Unable to sign input and verification failed (possible attempt to partially sign).
|
||||
TxInErrorToJSON(txin, vErrors, "Unable to sign input, invalid stack size (possibly missing key)");
|
||||
} else if (serror == SCRIPT_ERR_SIG_NULLFAIL) {
|
||||
// Verification failed (possibly due to insufficient signatures).
|
||||
TxInErrorToJSON(txin, vErrors, "CHECK(MULTI)SIG failing with non-zero signature (possibly need more signatures)");
|
||||
} else {
|
||||
TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror));
|
||||
}
|
||||
|
@ -134,6 +134,27 @@ class RpcCreateMultiSigTest(BitcoinTestFramework):
|
||||
|
||||
assert_raises_rpc_error(-8, "Missing redeemScript/witnessScript", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
|
||||
|
||||
# if witnessScript specified, all ok
|
||||
prevtx_err["witnessScript"] = prevtxs[0]["redeemScript"]
|
||||
node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
|
||||
|
||||
# both specified, also ok
|
||||
prevtx_err["redeemScript"] = prevtxs[0]["redeemScript"]
|
||||
node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
|
||||
|
||||
# redeemScript mismatch to witnessScript
|
||||
prevtx_err["redeemScript"] = "6a" # OP_RETURN
|
||||
assert_raises_rpc_error(-8, "redeemScript does not correspond to witnessScript", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
|
||||
|
||||
# redeemScript does not match scriptPubKey
|
||||
del prevtx_err["witnessScript"]
|
||||
assert_raises_rpc_error(-8, "redeemScript/witnessScript does not match scriptPubKey", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
|
||||
|
||||
# witnessScript does not match scriptPubKey
|
||||
prevtx_err["witnessScript"] = prevtx_err["redeemScript"]
|
||||
del prevtx_err["redeemScript"]
|
||||
assert_raises_rpc_error(-8, "redeemScript/witnessScript does not match scriptPubKey", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
|
||||
|
||||
rawtx2 = node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs - 1], prevtxs)
|
||||
rawtx3 = node2.signrawtransactionwithkey(rawtx2["hex"], [self.priv[-1]], prevtxs)
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user