diff --git a/qa/rpc-tests/abc-p2p-activation.py b/qa/rpc-tests/abc-p2p-activation.py --- a/qa/rpc-tests/abc-p2p-activation.py +++ b/qa/rpc-tests/abc-p2p-activation.py @@ -47,6 +47,7 @@ self.forkid_key.set_secretbytes(b"forkid") self.forkid_pubkey = self.forkid_key.get_pubkey() self.tip = None + self.uahfEnabled = False self.blocks = {} def setup_network(self): @@ -138,8 +139,14 @@ scriptSig = CScript([OP_TRUE]) else: # We have to actually sign it - (sighash, err) = SignatureHash(spend.tx.vout[spend.n].scriptPubKey, tx, 0, SIGHASH_ALL) - scriptSig = CScript([self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL]))]) + nHashType = SIGHASH_ALL + sighash = None + if self.uahfEnabled == False: + (sighash, err) = SignatureHash(spend.tx.vout[spend.n].scriptPubKey, tx, 0, SIGHASH_ALL) + else: + nHashType |= SIGHASH_FORKID + sighash = SignatureHashForkId(spend.tx.vout[spend.n].scriptPubKey, tx, 0, nHashType, spend.tx.vout[spend.n].nValue) + scriptSig = CScript([self.coinbase_key.sign(sighash) + bytes(bytearray([nHashType]))]) tx.vin[0].scriptSig = scriptSig # Now add the transaction to the block self.add_transactions_to_block(block, [tx]) @@ -314,6 +321,9 @@ tx_spend_id = node.sendrawtransaction(ToHex(tx_spend)) assert_equal(set(node.getrawmempool()), {tx_spend_id}) + # Mark the HF + self.uahfEnabled = True + # HF is active now, we MUST create a big block. block(11, spend=out[7], block_size=LEGACY_MAX_BLOCK_SIZE); yield rejected(RejectResult(16, b'bad-blk-too-small')) diff --git a/qa/rpc-tests/abc-p2p-fullblocktest.py b/qa/rpc-tests/abc-p2p-fullblocktest.py --- a/qa/rpc-tests/abc-p2p-fullblocktest.py +++ b/qa/rpc-tests/abc-p2p-fullblocktest.py @@ -121,8 +121,8 @@ if (scriptPubKey[0] == OP_TRUE): # an anyone-can-spend tx.vin[0].scriptSig = CScript() return - (sighash, err) = SignatureHash(spend_tx.vout[n].scriptPubKey, tx, 0, SIGHASH_ALL) - tx.vin[0].scriptSig = CScript([self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL]))]) + sighash = SignatureHashForkId(spend_tx.vout[n].scriptPubKey, tx, 0, SIGHASH_ALL|SIGHASH_FORKID, spend_tx.vout[n].nValue) + tx.vin[0].scriptSig = CScript([self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL|SIGHASH_FORKID]))]) def create_and_sign_transaction(self, spend_tx, n, value, script=CScript([OP_TRUE])): tx = self.create_tx(spend_tx, n, value, script) @@ -169,8 +169,8 @@ scriptSig = CScript([OP_TRUE]) else: # We have to actually sign it - (sighash, err) = SignatureHash(spend.tx.vout[spend.n].scriptPubKey, tx, 0, SIGHASH_ALL) - scriptSig = CScript([self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL]))]) + sighash = SignatureHashForkId(spend.tx.vout[spend.n].scriptPubKey, tx, 0, SIGHASH_ALL|SIGHASH_FORKID, spend.tx.vout[spend.n].nValue) + scriptSig = CScript([self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL|SIGHASH_FORKID]))]) tx.vin[0].scriptSig = scriptSig # Now add the transaction to the block self.add_transactions_to_block(block, [tx]) @@ -384,14 +384,14 @@ yield accepted() # Creates a new transaction using the p2sh transaction included in the last block - def spend_p2sh_tx (output_script=CScript([OP_TRUE])): + def spend_p2sh_tx(output_script=CScript([OP_TRUE])): # Create the transaction spent_p2sh_tx = CTransaction() spent_p2sh_tx.vin.append(CTxIn(COutPoint(p2sh_tx.sha256, 0), b'')) spent_p2sh_tx.vout.append(CTxOut(1, output_script)) # Sign the transaction using the redeem script - (sighash, err) = SignatureHash(redeem_script, spent_p2sh_tx, 0, SIGHASH_ALL) - sig = self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL])) + sighash = SignatureHashForkId(redeem_script, spent_p2sh_tx, 0, SIGHASH_ALL|SIGHASH_FORKID, p2sh_tx.vout[0].nValue) + sig = self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL|SIGHASH_FORKID])) spent_p2sh_tx.vin[0].scriptSig = CScript([sig, redeem_script]) spent_p2sh_tx.rehash() return spent_p2sh_tx diff --git a/src/core_write.cpp b/src/core_write.cpp --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -128,10 +128,14 @@ // Multisig scripts due to the restrictions on the pubkey // formats (see IsCompressedOrUncompressedPubKey) being // incongruous with the checks in CheckSignatureEncoding. - if (CheckSignatureEncoding(vch, - SCRIPT_VERIFY_STRICTENC | - SCRIPT_ENABLE_SIGHASH_FORKID, - nullptr)) { + uint32_t flags = SCRIPT_VERIFY_STRICTENC; + if (vch.back() & SIGHASH_FORKID) { + // If the transaction is using SIGHASH_FORKID, we need + // to set the apropriate flag. + // TODO: Remove after the Hard Fork. + flags |= SCRIPT_ENABLE_SIGHASH_FORKID; + } + if (CheckSignatureEncoding(vch, flags, nullptr)) { const unsigned char chSigHashType = vch.back(); if (mapSigHashTypes.count(chSigHashType)) { strSigHashDecode = diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -865,7 +865,7 @@ if (request.params.size() > 2 && !request.params[2].isNull()) { fGivenKeys = true; UniValue keys = request.params[2].get_array(); - for (unsigned int idx = 0; idx < keys.size(); idx++) { + for (size_t idx = 0; idx < keys.size(); idx++) { UniValue k = keys[idx]; CBitcoinSecret vchSecret; bool fGood = vchSecret.SetString(k.get_str()); @@ -891,7 +891,7 @@ // Add previous txouts given in the RPC call: if (request.params.size() > 1 && !request.params[1].isNull()) { UniValue prevTxs = request.params[1].get_array(); - for (unsigned int idx = 0; idx < prevTxs.size(); idx++) { + for (size_t idx = 0; idx < prevTxs.size(); idx++) { const UniValue &p = prevTxs[idx]; if (!p.isObject()) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, @@ -1043,12 +1043,22 @@ UpdateTransaction(mergedTx, i, sigdata); - ScriptError serror = SCRIPT_ERR_OK; - if (!VerifyScript( - txin.scriptSig, prevPubKey, - STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_ENABLE_SIGHASH_FORKID, - TransactionSignatureChecker(&txConst, i, amount), &serror)) { - TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror)); + // Because we do not know if forkid is used or not, we just try both. + // TODO: Remove after the Hard Fork. + ScriptError serror0 = SCRIPT_ERR_OK; + ScriptError serror1 = SCRIPT_ERR_OK; + TransactionSignatureChecker checker(&txConst, i, amount); + if (!VerifyScript(txin.scriptSig, prevPubKey, + STANDARD_SCRIPT_VERIFY_FLAGS | + SCRIPT_ENABLE_SIGHASH_FORKID, + checker, &serror0) && + !VerifyScript(txin.scriptSig, prevPubKey, + STANDARD_SCRIPT_VERIFY_FLAGS, checker, &serror1)) { + std::string error; + error += ScriptErrorString(serror0); + error += " "; + error += ScriptErrorString(serror1); + TxInErrorToJSON(txin, vErrors, error); } } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -238,10 +238,14 @@ if (!IsDefinedHashtypeSignature(vchSig)) { return set_error(serror, SCRIPT_ERR_SIG_HASHTYPE); } - if (!(flags & SCRIPT_ENABLE_SIGHASH_FORKID) && - (GetHashType(vchSig) & SIGHASH_FORKID)) { + bool usesForkId = GetHashType(vchSig) & SIGHASH_FORKID; + bool forkIdEnabled = flags & SCRIPT_ENABLE_SIGHASH_FORKID; + if (!forkIdEnabled && usesForkId) { return set_error(serror, SCRIPT_ERR_ILLEGAL_FORKID); } + if (forkIdEnabled && !usesForkId) { + return set_error(serror, SCRIPT_ERR_MUST_USE_FORKID); + } } return true; } diff --git a/src/script/script_error.h b/src/script/script_error.h --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -59,6 +59,7 @@ /* anti replay */ SCRIPT_ERR_ILLEGAL_FORKID, + SCRIPT_ERR_MUST_USE_FORKID, SCRIPT_ERR_ERROR_COUNT } ScriptError; diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp --- a/src/script/script_error.cpp +++ b/src/script/script_error.cpp @@ -79,6 +79,8 @@ return "Using non-compressed public key"; case SCRIPT_ERR_ILLEGAL_FORKID: return "Illegal use of SIGHASH_FORKID"; + case SCRIPT_ERR_MUST_USE_FORKID: + return "Signature must use SIGHASH_FORKID"; case SCRIPT_ERR_UNKNOWN_ERROR: case SCRIPT_ERR_ERROR_COUNT: default: diff --git a/src/script/sign.cpp b/src/script/sign.cpp --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -33,7 +33,7 @@ return false; } - vchSig.push_back((unsigned char)nHashType); + vchSig.push_back(uint8_t(nHashType)); return true; } @@ -161,10 +161,16 @@ sigdata.scriptSig = PushAll(result); // Test solution - return solved && VerifyScript(sigdata.scriptSig, fromPubKey, - STANDARD_SCRIPT_VERIFY_FLAGS | - SCRIPT_ENABLE_SIGHASH_FORKID, - creator.Checker()); + // Because we have no good way to get nHashType here, we just try with and + // without enabling it. One of the two must pass. + // TODO: Remove after the fork. + return solved && + (VerifyScript(sigdata.scriptSig, fromPubKey, + STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker()) || + VerifyScript(sigdata.scriptSig, fromPubKey, + STANDARD_SCRIPT_VERIFY_FLAGS | + SCRIPT_ENABLE_SIGHASH_FORKID, + creator.Checker())); } SignatureData DataFromTransaction(const CMutableTransaction &tx, @@ -215,11 +221,15 @@ // Combine all the signatures we've got: std::set allsigs; for (const valtype &v : sigs1) { - if (!v.empty()) allsigs.insert(v); + if (!v.empty()) { + allsigs.insert(v); + } } for (const valtype &v : sigs2) { - if (!v.empty()) allsigs.insert(v); + if (!v.empty()) { + allsigs.insert(v); + } } // Build a map of pubkey -> signature by matching sigs to pubkeys: @@ -235,8 +245,14 @@ continue; } - if (checker.CheckSig(sig, pubkey, scriptPubKey, - SCRIPT_ENABLE_SIGHASH_FORKID)) { + // If the transaction is using SIGHASH_FORKID, we ned to set the + // apropriate flags. + // TODO: Remove after the Hard Fork. + uint32_t flags = STANDARD_SCRIPT_VERIFY_FLAGS; + if (sig.back() & SIGHASH_FORKID) { + flags |= SCRIPT_ENABLE_SIGHASH_FORKID; + } + if (checker.CheckSig(sig, pubkey, scriptPubKey, flags)) { sigs[pubkey] = sig; break; } diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -87,6 +87,7 @@ "DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM"}, {SCRIPT_ERR_NONCOMPRESSED_PUBKEY, "NONCOMPRESSED_PUBKEY"}, {SCRIPT_ERR_ILLEGAL_FORKID, "ILLEGAL_FORKID"}, + {SCRIPT_ERR_MUST_USE_FORKID, "MISSING_FORKID"}, }; const char *FormatScriptError(ScriptError_t err) { diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -50,7 +50,7 @@ json_tests::tx_valid + sizeof(json_tests::tx_valid))); ScriptError err; - for (unsigned int idx = 0; idx < tests.size(); idx++) { + for (size_t idx = 0; idx < tests.size(); idx++) { UniValue test = tests[idx]; std::string strTest = test.write(); if (test[0].isArray()) { @@ -63,7 +63,7 @@ std::map mapprevOutValues; UniValue inputs = test[0].get_array(); bool fValid = true; - for (unsigned int inpIdx = 0; inpIdx < inputs.size(); inpIdx++) { + for (size_t inpIdx = 0; inpIdx < inputs.size(); inpIdx++) { const UniValue &input = inputs[inpIdx]; if (!input.isArray()) { fValid = false; @@ -100,7 +100,7 @@ BOOST_CHECK(state.IsValid()); PrecomputedTransactionData txdata(tx); - for (unsigned int i = 0; i < tx.vin.size(); i++) { + for (size_t i = 0; i < tx.vin.size(); i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) { BOOST_ERROR("Bad test: " << strTest); break; @@ -111,7 +111,7 @@ amount = mapprevOutValues[tx.vin[i].prevout]; } - unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); + uint32_t verify_flags = ParseScriptFlags(test[2].get_str()); BOOST_CHECK_MESSAGE( VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], @@ -141,7 +141,7 @@ json_tests::tx_invalid + sizeof(json_tests::tx_invalid))); ScriptError err; - for (unsigned int idx = 0; idx < tests.size(); idx++) { + for (size_t idx = 0; idx < tests.size(); idx++) { UniValue test = tests[idx]; std::string strTest = test.write(); if (test[0].isArray()) { @@ -154,7 +154,7 @@ std::map mapprevOutValues; UniValue inputs = test[0].get_array(); bool fValid = true; - for (unsigned int inpIdx = 0; inpIdx < inputs.size(); inpIdx++) { + for (size_t inpIdx = 0; inpIdx < inputs.size(); inpIdx++) { const UniValue &input = inputs[inpIdx]; if (!input.isArray()) { fValid = false; @@ -187,7 +187,7 @@ fValid = CheckRegularTransaction(tx, state) && state.IsValid(); PrecomputedTransactionData txdata(tx); - for (unsigned int i = 0; i < tx.vin.size() && fValid; i++) { + for (size_t i = 0; i < tx.vin.size() && fValid; i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) { BOOST_ERROR("Bad test: " << strTest); break; @@ -198,7 +198,7 @@ amount = mapprevOutValues[tx.vin[i].prevout]; } - unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); + uint32_t verify_flags = ParseScriptFlags(test[2].get_str()); fValid = VerifyScript( tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], verify_flags, diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -48,11 +48,11 @@ spends[i].vout[0].scriptPubKey = scriptPubKey; // Sign: - std::vector vchSig; + std::vector vchSig; uint256 hash = SignatureHash(scriptPubKey, spends[i], 0, SIGHASH_ALL, 0); BOOST_CHECK(coinbaseKey.Sign(hash, vchSig)); - vchSig.push_back((unsigned char)SIGHASH_ALL); + vchSig.push_back(uint8_t(SIGHASH_ALL)); spends[i].vin[0].scriptSig << vchSig; } diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -857,13 +857,14 @@ "too-long-mempool-chain", false, errString); } - unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; + uint32_t scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; if (!Params().RequireStandard()) { scriptVerifyFlags = GetArg("-promiscuousmempoolflags", scriptVerifyFlags); } - if (IsUAHFenabledForCurrentBlock(config)) { + const bool hasUAHF = IsUAHFenabledForCurrentBlock(config); + if (hasUAHF) { scriptVerifyFlags |= SCRIPT_ENABLE_SIGHASH_FORKID; } @@ -888,13 +889,22 @@ // // SCRIPT_ENABLE_SIGHASH_FORKID is also added as to ensure we do not // filter out transactions using the antireplay feature. - if (!CheckInputs(tx, state, view, true, - MANDATORY_SCRIPT_VERIFY_FLAGS | - SCRIPT_ENABLE_SIGHASH_FORKID, - true, txdata)) { - return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed " - "against MANDATORY but not STANDARD flags %s, %s", - __func__, txid.ToString(), FormatStateMessage(state)); + { + // Depending on the UAHF activation, we require SIGHASH_FORKID or + // not. + // TODO: Cleanup after the Hard Fork. + uint32_t mandatoryFlags = MANDATORY_SCRIPT_VERIFY_FLAGS; + if (hasUAHF) { + mandatoryFlags |= SCRIPT_ENABLE_SIGHASH_FORKID; + } + + if (!CheckInputs(tx, state, view, true, mandatoryFlags, true, + txdata)) { + return error( + "%s: BUG! PLEASE REPORT THIS! ConnectInputs failed " + "against MANDATORY but not STANDARD flags %s, %s", + __func__, txid.ToString(), FormatStateMessage(state)); + } } // This transaction should only count for fee estimation if @@ -1865,7 +1875,7 @@ int64_t nBIP16SwitchTime = 1333238400; bool fStrictPayToScriptHash = (pindex->GetBlockTime() >= nBIP16SwitchTime); - unsigned int flags = + uint32_t flags = fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE; // Start enforcing the DERSIG (BIP66) rule @@ -1889,7 +1899,9 @@ } // If the UAHF is enabled, we start accepting replay protected txns - if (IsUAHFenabled(config, pindex->pprev)) { + const bool hasUAHF = IsUAHFenabled(config, pindex->pprev); + if (hasUAHF) { + flags |= SCRIPT_VERIFY_STRICTENC; flags |= SCRIPT_ENABLE_SIGHASH_FORKID; } @@ -2071,6 +2083,12 @@ LogPrint("bench", " - Callbacks: %.2fms [%.2fs]\n", 0.001 * (nTime6 - nTime5), nTimeCallbacks * 0.000001); + // If this block activates UAHF, we clear the mempool. This ensure that + // we'll only get replay protected transaction in the mempool going forward. + if (!hasUAHF && IsUAHFenabled(config, pindex)) { + mempool.clear(); + } + return true; }