Disable prefork SIGHASH_FORKID

h4x3rotab · Aug 20, 2019 · 4dbe037 · 4dbe037
1 parent cabbb7e
commit 4dbe037
Show file tree

Hide file tree

Showing 28 changed files with 195 additions and 138 deletions.
diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp
@@ -53,7  53,7 @@ static CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, co
 // modified to measure performance of other types of scripts.
 static void VerifyScriptBench(benchmark::State& state)
 {
-    const int flags = SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH;
     const int flags = SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH | SCRIPT_FORKID_DISABLED;
     const int witnessversion = 0;
 
     // Keypair.
@@ -76,7  76,7 @@ static void VerifyScriptBench(benchmark::State& state)
     CMutableTransaction txSpend = BuildSpendingTransaction(scriptSig, txCredit);
     CScriptWitness& witness = txSpend.vin[0].scriptWitness;
     witness.stack.emplace_back();
-    key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SigVersion::WITNESS_V0), witness.stack.back());
     key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SigVersion::WITNESS_V0, false), witness.stack.back());
     witness.stack.back().push_back(static_cast<unsigned char>(SIGHASH_ALL));
     witness.stack.push_back(ToByteVector(pubkey));
 

diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
@@ -648,10  648,10 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
         const CScript& prevPubKey = coin.out.scriptPubKey;
         const CAmount& amount = coin.out.nValue;
 
-        SignatureData sigdata = DataFromTransaction(mergedTx, i, coin.out);
         SignatureData sigdata = DataFromTransaction(mergedTx, i, coin.out, false);
         // Only sign SIGHASH_SINGLE if there's a corresponding output:
         if (!fHashSingle || (i < mergedTx.vout.size()))
-            ProduceSignature(keystore, MutableTransactionSignatureCreator(&mergedTx, i, amount, nHashType), prevPubKey, sigdata);
             ProduceSignature(keystore, MutableTransactionSignatureCreator(&mergedTx, i, amount, false, nHashType), prevPubKey, sigdata, false);
 
         UpdateInput(txin, sigdata);
     }

diff --git a/src/core_write.cpp b/src/core_write.cpp
@@ -115,7  115,7 @@ std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDeco
                     // this won't decode correctly formatted public keys in Pubkey or 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_ALLOW_NON_FORKID, nullptr)) {
                     if (CheckSignatureEncoding(vch, SCRIPT_VERIFY_STRICTENC | SCRIPT_FORKID_DISABLED, nullptr)) {
                         const unsigned char chSigHashType = vch.back();
                         if (mapSigHashTypes.count(chSigHashType)) {
                             strSigHashDecode = "["   mapSigHashTypes.find(chSigHashType)->second   "]";

diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
@@ -219,14  219,15 @@ class WalletImpl : public Wallet
     std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients,
         const CCoinControl& coin_control,
         bool sign,
         bool no_forkid,
         int& change_pos,
         CAmount& fee,
         std::string& fail_reason) override
     {
         LOCK2(cs_main, m_wallet.cs_wallet);
         auto pending = MakeUnique<PendingWalletTxImpl>(m_wallet);
         if (!m_wallet.CreateTransaction(recipients, pending->m_tx, pending->m_key, fee, change_pos,
-                fail_reason, coin_control, sign)) {
                 no_forkid, fail_reason, coin_control, sign)) {
             return {};
         }
         return std::move(pending);

diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
@@ -135,6  135,7 @@ class Wallet
     virtual std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients,
         const CCoinControl& coin_control,
         bool sign,
         bool no_forkid,
         int& change_pos,
         CAmount& fee,
         std::string& fail_reason) = 0;

diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
@@ -12,11  12,13 @@
 #include <qt/sendcoinsdialog.h>
 #include <qt/transactiontablemodel.h>
 
 #include <chainparams.h>
 #include <interfaces/handler.h>
 #include <interfaces/node.h>
 #include <key_io.h>
 #include <ui_interface.h>
 #include <util.h> // for GetBoolArg
 #include <validation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/wallet.h>
 
@@ -195,13  197,19 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
         return AmountExceedsBalance;
     }
 
     bool no_forkid;
     {
         LOCK(cs_main);
         no_forkid = !IsBTGHardForkEnabledForCurrentBlock(Params().GetConsensus());
     }
 
     {
         CAmount nFeeRequired = 0;
         int nChangePosRet = -1;
         std::string strFailReason;
 
         auto& newTx = transaction.getWtx();
-        newTx = m_wallet->createTransaction(vecSend, coinControl, true /* sign */, nChangePosRet, nFeeRequired, strFailReason);
         newTx = m_wallet->createTransaction(vecSend, coinControl, true /* sign */, no_forkid, nChangePosRet, nFeeRequired, strFailReason);
         transaction.setTransactionFee(nFeeRequired);
         if (fSubtractFeeFromAmount && newTx)
             transaction.reassignAmounts(nChangePosRet);

diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
@@ -735,6  735,12 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request)
         view.SetBackend(viewDummy); // switch back to avoid locking mempool for too long
     }
 
     bool no_forkid;
     {
         LOCK(cs_main);
         no_forkid = !IsBTGHardForkEnabledForCurrentBlock(Params().GetConsensus());
     }
 
     // Use CTransaction for the constant parts of the
     // transaction to avoid rehashing.
     const CTransaction txConst(mergedTx);
@@ -750,10  756,10 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request)
         // ... and merge in other signatures:
         for (const CMutableTransaction& txv : txVariants) {
             if (txv.vin.size() > i) {
-                sigdata.MergeSignatureData(DataFromTransaction(txv, i, coin.out));
                 sigdata.MergeSignatureData(DataFromTransaction(txv, i, coin.out, no_forkid));
             }
         }
-        ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(&mergedTx, i, coin.out.nValue, 1), coin.out.scriptPubKey, sigdata);
         ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(&mergedTx, i, coin.out.nValue, no_forkid, 1), coin.out.scriptPubKey, sigdata, no_forkid);
 
         UpdateInput(txin, sigdata);
     }
@@ -845,7  851,13 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
         }
     }
 
-    int nHashType = ParseSighashString(hashType);
     bool no_forkid;
     {
         LOCK(cs_main);
         no_forkid = !IsBTGHardForkEnabledForCurrentBlock(Params().GetConsensus());
     }
 
     int nHashType = ParseSighashString(hashType, !no_forkid);
 
     bool fHashSingle = ((nHashType & ~(SIGHASH_ANYONECANPAY | SIGHASH_FORKID)) == SIGHASH_SINGLE);
 
@@ -866,10  878,10 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
         const CScript& prevPubKey = coin.out.scriptPubKey;
         const CAmount& amount = coin.out.nValue;
 
-        SignatureData sigdata = DataFromTransaction(mtx, i, coin.out);
         SignatureData sigdata = DataFromTransaction(mtx, i, coin.out, no_forkid);
         // Only sign SIGHASH_SINGLE if there's a corresponding output:
         if (!fHashSingle || (i < mtx.vout.size())) {
-            ProduceSignature(*keystore, MutableTransactionSignatureCreator(&mtx, i, amount, nHashType), prevPubKey, sigdata);
             ProduceSignature(*keystore, MutableTransactionSignatureCreator(&mtx, i, amount, no_forkid, nHashType), prevPubKey, sigdata, no_forkid);
         }
 
         UpdateInput(txin, sigdata);
@@ -880,7  892,8 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
         }
 
         ScriptError serror = SCRIPT_ERR_OK;
-        if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) {
         int verify_flags = no_forkid ? STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_FORKID_DISABLED : STANDARD_SCRIPT_VERIFY_FLAGS;
         if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, verify_flags, TransactionSignatureChecker(&txConst, i, amount), &serror)) {
             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)");
@@ -1655,14  1668,21 @@ UniValue finalizepsbt(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error));
     }
 
     bool no_forkid;
     {
         LOCK(cs_main);
         no_forkid = !IsBTGHardForkEnabledForCurrentBlock(Params().GetConsensus());
     }
 
     // Finalize input signatures -- in case we have partial signatures that add up to a complete
     //   signature, but have not combined them yet (e.g. because the combiner that created this
     //   PartiallySignedTransaction did not understand them), this will combine them into a final
     //   script.
     bool complete = true;
     for (unsigned int i = 0; i < psbtx.tx->vin.size();   i) {
         SignatureData sigdata;
-        complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, sigdata, i, SIGHASH_ALL | SIGHASH_FORKID);
         complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, sigdata, i, no_forkid,
                                   no_forkid ? SIGHASH_ALL : SIGHASH_ALL | SIGHASH_FORKID);
     }
 
     UniValue result(UniValue::VOBJ);

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
@@ -213,8  213,8 @@ bool static UsesForkId(const valtype &vchSig) {
     return UsesForkId(nHashType);
 }
 
-bool static AllowsNonForkId(unsigned int flags) {
-    return flags & SCRIPT_ALLOW_NON_FORKID;
 bool static ForkIdDisabled(unsigned int flags) {
     return flags & SCRIPT_FORKID_DISABLED;
 }
 
 bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, unsigned int flags, ScriptError* serror) {
@@ -233,7  233,7 @@ bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, unsigned i
             return set_error(serror, SCRIPT_ERR_SIG_HASHTYPE);
         }
 
-        bool requiresForkId = !AllowsNonForkId(flags);
         bool requiresForkId = !ForkIdDisabled(flags);
         bool usesForkId = UsesForkId(vchSig);
         if (requiresForkId && !usesForkId) {
             return set_error(serror, SCRIPT_ERR_SIG_HASHTYPE);
@@ -963,7  963,8 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                         //serror is set
                         return false;
                     }
-                    bool fSuccess = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion);
                     bool no_forkid = ForkIdDisabled(flags);
                     bool fSuccess = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion, no_forkid);
 
                     if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && vchSig.size())
                         return set_error(serror, SCRIPT_ERR_SIG_NULLFAIL);
@@ -1041,7  1042,8 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                         }
 
                         // Check signature
-                        bool fOk = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion);
                         bool no_forkid = ForkIdDisabled(flags);
                         bool fOk = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion, no_forkid);
 
                         if (fOk) {
                             isig  ;
@@ -1261,17  1263,22 @@ template PrecomputedTransactionData::PrecomputedTransactionData(const CTransacti
 template PrecomputedTransactionData::PrecomputedTransactionData(const CMutableTransaction& txTo);
 
 template <class T>
-uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache,  int forkid)
 uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, bool no_forkid, const PrecomputedTransactionData* cache, int forkid)
 {
     assert(nIn < txTo.vin.size());
 
     bool use_forkid = false;
     int nForkHashType = nHashType;
-    if (UsesForkId(nHashType))
-        nForkHashType |= forkid << 8;
     if (!no_forkid) {
         use_forkid = UsesForkId(nHashType);
         if (use_forkid) {
             nForkHashType |= forkid << 8;
         }
     }
 
     // force new tx with FORKID to use bip143 transaction digest algorithm
     // see https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki
-    if (sigversion == SigVersion::WITNESS_V0 || UsesForkId(nHashType)) {
     if (sigversion == SigVersion::WITNESS_V0 || use_forkid) {
         uint256 hashPrevouts;
         uint256 hashSequence;
         uint256 hashOutputs;
@@ -1343,7  1350,7 @@ bool GenericTransactionSignatureChecker<T>::VerifySignature(const std::vector<un
 }
 
 template <class T>
-bool GenericTransactionSignatureChecker<T>::CheckSig(const std::vector<unsigned char>& vchSigIn, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
 bool GenericTransactionSignatureChecker<T>::CheckSig(const std::vector<unsigned char>& vchSigIn, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion, bool no_forkid) const
 {
     CPubKey pubkey(vchPubKey);
     if (!pubkey.IsValid())
@@ -1356,7  1363,7 @@ bool GenericTransactionSignatureChecker<T>::CheckSig(const std::vector<unsigned
     int nHashType = GetHashType(vchSig);
     vchSig.pop_back();
 
-    uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->txdata);
     uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, no_forkid, this->txdata);
 
     if (!VerifySignature(vchSig, pubkey, sighash))
         return false;

diff --git a/src/script/interpreter.h b/src/script/interpreter.h
@@ -128,7  128,7 @@ enum
 
     // Allow NON_FORKID in legacy tests and blocks under BTG hard fork height
     //
-    SCRIPT_ALLOW_NON_FORKID = (1U << 17),
     SCRIPT_FORKID_DISABLED = (1U << 17),
 };
 
 bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, unsigned int flags, ScriptError* serror);
@@ -153,12  153,12 @@ static constexpr size_t WITNESS_V0_SCRIPTHASH_SIZE = 32;
 static constexpr size_t WITNESS_V0_KEYHASH_SIZE = 20;
 
 template <class T>
-uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = nullptr, int forkid=FORKID_IN_USE);
 uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, bool no_forkid, const PrecomputedTransactionData* cache = nullptr, int forkid=FORKID_IN_USE);
 
 class BaseSignatureChecker
 {
 public:
-    virtual bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
     virtual bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion, bool no_forkid) const
     {
         return false;
     }
@@ -191,7  191,7 @@ class GenericTransactionSignatureChecker : public BaseSignatureChecker
 public:
     GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(nullptr) {}
     GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
-    bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override;
     bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion, bool no_forkid) const override;
     bool CheckLockTime(const CScriptNum& nLockTime) const override;
     bool CheckSequence(const CScriptNum& nSequence) const override;
 };