Skip to content

Commit

Permalink
Merge #1666: Base work for the Sapling signatureHash
Browse files Browse the repository at this point in the history 
d1d15c8 Fix missing sigverion in main_test.cpp CreateDummyScriptSigWithKey. (furszy)
a034daf Rename to PrecomputedTransactionData (furszy)
b4b181b Unit test for sighash caching (furszy)
2ef3872 Report non-mandatory script failures correctly. (furszy)
446d340 Precompute sighashes (furszy)
dfd24eb Update wallet_txn_close.py test: (furszy)
a5170f0 BIP143: Signing logic. (furszy)
d2dd547 BIP143: Verification logic. (furszy)
dccc3c6 Refactor script validation to observe amounts (furszy)
daf044a Reduce unnecessary hashing in signrawtransaction (furszy)

Pull request description:

  Base work for the new transaction digest algorithm for signature verification on PIVX Sapling transactions.

  Essentially, an implementation of BIP143 + few more good commits that found down the rabbit hole.

  Back ports:

  * bitcoin#7276
  * bitcoin#7976
  * bitcoin#8118
  * bitcoin#8149 (only amount validation and SignatureHash commits).
  * bitcoin#6088 (only the dummy signature one - will be removed once #1663 get merged -).
  * bitcoin#6379
  * bitcoin#8524

  Next step over this area (need 1553 merged to be able to push it) is the further specialization of BIP143 into our custom implementation of ZIP143 (with a different digest algorithm definition using our tx data and hash personalization).

ACKs for top commit:
  Fuzzbawls:
    utACK d1d15c8
  random-zebra:
    ACK d1d15c8 and merging...

Tree-SHA512: 7665cccf095c5bce0b18ef7ab8fcf7bede9304993b48f1af9c352c568861dec728d1d68671aab857b73d46567678492c4b97c24644a15f3f29fc4d723b183522
  • Loading branch information
@random-zebra
random-zebra committed Aug 5, 2020
2 parents fddf765 + d1d15c8 commit 5a09215
Show file tree
Hide file tree
Showing 24 changed files with 610 additions and 266 deletions.
2 changes: 1 addition & 1 deletion src/kernel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ bool CheckProofOfStake(const CBlock& block, std::string& strError, const CBlockI
const CTxIn& txin = tx.vin[0];
ScriptError serror;
if (!VerifyScript(txin.scriptSig, stakePrevout.scriptPubKey, STANDARD_SCRIPT_VERIFY_FLAGS,
TransactionSignatureChecker(&tx, 0), &serror)) {
TransactionSignatureChecker(&tx, 0, stakePrevout.nValue), &serror)) {
strError = strprintf("signature fails: %s", serror ? ScriptErrorString(serror) : "");
return false;
}
Expand Down
37 changes: 25 additions & 12 deletions src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,9 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
int flags = STANDARD_SCRIPT_VERIFY_FLAGS;
if (fCLTVIsActivated)
flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY;
if (!CheckInputs(tx, state, view, true, flags, true)) {

PrecomputedTransactionData precomTxData(tx);
if (!CheckInputs(tx, state, view, true, flags, true, precomTxData)) {
return false;
}

Expand All @@ -1074,9 +1076,9 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
flags = MANDATORY_SCRIPT_VERIFY_FLAGS;
if (fCLTVIsActivated)
flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY;
if (!CheckInputs(tx, state, view, true, flags, true)) {
if (!CheckInputs(tx, state, view, true, flags, true, precomTxData)) {
return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s",
__func__, hash.ToString(), FormatStateMessage(state));
__func__, hash.ToString(), FormatStateMessage(state));
}

// Store transaction in memory
Expand Down Expand Up @@ -1283,7 +1285,9 @@ bool AcceptableInputs(CTxMemPool& pool, CValidationState& state, const CTransact
int flags = STANDARD_SCRIPT_VERIFY_FLAGS;
if (fCLTVIsActivated)
flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY;
if (!CheckInputs(tx, state, view, false, flags, true)) {

PrecomputedTransactionData precomTxData(tx);
if (!CheckInputs(tx, state, view, false, flags, true, precomTxData)) {
return error("AcceptableInputs: : ConnectInputs failed %s", hash.ToString());
}

Expand All @@ -1297,7 +1301,7 @@ bool AcceptableInputs(CTxMemPool& pool, CValidationState& state, const CTransact
// invalid blocks, however allowing such transactions into the mempool
// can be exploited as a DoS attack.
// for any real tx this will be checked on AcceptToMemoryPool anyway
// if (!CheckInputs(tx, state, view, false, MANDATORY_SCRIPT_VERIFY_FLAGS, true))
// if (!CheckInputs(tx, state, view, false, MANDATORY_SCRIPT_VERIFY_FLAGS, true, precomTxData))
// {
// return error("AcceptableInputs: : BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s", hash.ToString());
// }
Expand Down Expand Up @@ -1735,7 +1739,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache &inputs, int nHeight)
bool CScriptCheck::operator()()
{
const CScript& scriptSig = ptxTo->vin[nIn].scriptSig;
return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, cacheStore), &error);
return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *precomTxData), &error);
}

std::map<COutPoint, COutPoint> mapInvalidOutPoints;
Expand Down Expand Up @@ -1858,7 +1862,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins
}
}// namespace Consensus

bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks)
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, PrecomputedTransactionData& precomTxData, std::vector<CScriptCheck> *pvChecks)
{
if (!tx.IsCoinBase() && !tx.HasZerocoinSpendInputs()) {

Expand All @@ -1882,7 +1886,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
assert(coins);

// Verify signature
CScriptCheck check(*coins, tx, i, flags, cacheStore);
CScriptCheck check(*coins, tx, i, flags, cacheStore, &precomTxData);
if (pvChecks) {
pvChecks->push_back(CScriptCheck());
check.swap(pvChecks->back());
Expand All @@ -1894,9 +1898,9 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
// arguments; if so, don't trigger DoS protection to
// avoid splitting the network between upgraded and
// non-upgraded nodes.
CScriptCheck check(*coins, tx, i,
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore);
if (check())
CScriptCheck check2(*coins, tx, i,
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &precomTxData);
if (check2())
return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));
}
// Failures of other flags indicate a transaction that is
Expand Down Expand Up @@ -2238,6 +2242,9 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
unsigned int nMaxBlockSigOps = MAX_BLOCK_SIGOPS_CURRENT;
std::vector<uint256> vSpendsInBlock;
uint256 hashBlock = block.GetHash();

std::vector<PrecomputedTransactionData> precomTxData;
precomTxData.reserve(block.vtx.size()); // Required so that pointers to individual precomTxData don't get invalidated
for (unsigned int i = 0; i < block.vtx.size(); i++) {
const CTransaction& tx = block.vtx[i];

Expand Down Expand Up @@ -2330,6 +2337,12 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
if (nSigOps > nMaxBlockSigOps)
return state.DoS(100, error("ConnectBlock() : too many sigops"), REJECT_INVALID, "bad-blk-sigops");

}

// Cache the sig ser hashes
precomTxData.emplace_back(tx);

if (!tx.IsCoinBase()) {
if (!tx.IsCoinStake())
nFees += view.GetValueIn(tx) - tx.GetValueOut();
nValueIn += view.GetValueIn(tx);
Expand All @@ -2340,7 +2353,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY;

bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, nScriptCheckThreads ? &vChecks : NULL))
if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, precomTxData[i], nScriptCheckThreads ? &vChecks : NULL))
return error("%s: Check inputs on %s failed with %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
control.Add(vChecks);
}
Expand Down
15 changes: 11 additions & 4 deletions src/main.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class CScriptCheck;
class CValidationInterface;
class CValidationState;

struct PrecomputedTransactionData;
struct CBlockTemplate;
struct CNodeStateStats;

Expand Down Expand Up @@ -281,7 +282,7 @@ CAmount GetMinRelayFee(const CTransaction& tx, const CTxMemPool& pool, unsigned
* This does not modify the UTXO set. If pvChecks is not NULL, script checks are pushed onto it
* instead of being performed inline.
*/
bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector<CScriptCheck>* pvChecks = NULL);
bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, bool fScriptChecks, unsigned int flags, bool cacheStore, PrecomputedTransactionData& precomTxData, std::vector<CScriptCheck>* pvChecks = NULL);

/** Apply the effects of this transaction on the UTXO set represented by view */
void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight);
Expand All @@ -308,27 +309,33 @@ class CScriptCheck
{
private:
CScript scriptPubKey;
CAmount amount;
const CTransaction* ptxTo;
unsigned int nIn;
unsigned int nFlags;
bool cacheStore;
ScriptError error;
PrecomputedTransactionData *precomTxData;

public:
CScriptCheck() : ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey),
ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
CScriptCheck() : amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* cachedHashesIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey),
amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue),
ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR),
precomTxData(cachedHashesIn) {}

bool operator()();

void swap(CScriptCheck& check)
{
scriptPubKey.swap(check.scriptPubKey);
std::swap(ptxTo, check.ptxTo);
std::swap(amount, check.amount);
std::swap(nIn, check.nIn);
std::swap(nFlags, check.nFlags);
std::swap(cacheStore, check.cacheStore);
std::swap(error, check.error);
std::swap(precomTxData, check.precomTxData);
}

ScriptError GetScriptError() const { return error; }
Expand Down
3 changes: 2 additions & 1 deletion src/miner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,8 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn, CWallet* pwallet,
// create only contains transactions that are valid in new blocks.

CValidationState state;
if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true))
PrecomputedTransactionData precomTxData(tx);
if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, precomTxData))
continue;

UpdateCoins(tx, view, nHeight);
Expand Down
38 changes: 33 additions & 5 deletions src/pivx-tx.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,24 @@ uint256 ParseHashUO(std::map<std::string, UniValue>& o, std::string strKey)
return ParseHashUV(o[strKey], strKey);
}

static inline int64_t roundint64(double d)
{
return (int64_t)(d > 0 ? d + 0.5 : d - 0.5);
}

static CAmount AmountFromValue(const UniValue& value)
{
if (!value.isNum() && !value.isStr())
throw std::runtime_error("Amount is not a number or string");
double dAmount = value.get_real();
if (dAmount <= 0.0 || dAmount > 21000000.0)
throw std::runtime_error("Invalid amount");
CAmount nAmount = roundint64(dAmount * COIN);
if (!Params().GetConsensus().MoneyRange(nAmount))
throw std::runtime_error("Amount out of range");
return nAmount;
}

std::vector<unsigned char> ParseHexUO(std::map<std::string, UniValue>& o, std::string strKey)
{
if (!o.count(strKey)) {
Expand Down Expand Up @@ -393,7 +411,10 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
if ((unsigned int)nOut >= coins->vout.size())
coins->vout.resize(nOut + 1);
coins->vout[nOut].scriptPubKey = scriptPubKey;
coins->vout[nOut].nValue = 0; // we don't know the actual output value
coins->vout[nOut].nValue = 0;
if (prevOut.exists("amount")) {
coins->vout[nOut].nValue = AmountFromValue(prevOut["amount"]);
}
}

// if redeemScript given and private keys given,
Expand Down Expand Up @@ -421,17 +442,24 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
continue;
}
const CScript& prevPubKey = coins->vout[txin.prevout.n].scriptPubKey;
const CAmount& amount = coins->vout[txin.prevout.n].nValue;

txin.scriptSig.clear();
SignatureData sigdata;
// Only sign SIGHASH_SINGLE if there's a corresponding output:
if (!fHashSingle || (i < mergedTx.vout.size()))
SignSignature(keystore, prevPubKey, mergedTx, i, nHashType);
ProduceSignature(
MutableTransactionSignatureCreator(&keystore, &mergedTx, i, amount, nHashType),
prevPubKey,
sigdata,
false // no cold stake
);

// ... and merge in other signatures:
for (const CTransaction& txv : txVariants) {
txin.scriptSig = CombineSignatures(prevPubKey, mergedTx, i, txin.scriptSig, txv.vin[i].scriptSig);
sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i));
}
if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i)))
UpdateTransaction(mergedTx, i, sigdata);
if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i, amount)))
fComplete = false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/policy/policy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
// IsStandard() will have already returned false
// and this method isn't called.
std::vector<std::vector<unsigned char> > stack;
if (!EvalScript(stack, tx.vin[i].scriptSig, false, BaseSignatureChecker()))
if (!EvalScript(stack, tx.vin[i].scriptSig, false, BaseSignatureChecker(), SIGVERSION_BASE))
return false;

if (whichType == TX_SCRIPTHASH) {
Expand Down
Loading

0 comments on commit 5a09215

Please sign in to comment.