From 6ce373dd023248f2eb6d53ef4fbe7fbfe4bea6c9 Mon Sep 17 00:00:00 2001 From: Griffith <8345264+Greg-Griffith@users.noreply.github.com> Date: Sat, 17 Aug 2019 14:16:55 +0900 Subject: [PATCH 1/8] refactor tx consensus code (#198) --- src/.formatted-files | 4 +- src/Makefile.am | 4 +- src/blockgeneration/miner.cpp | 1 + src/blockgeneration/minter.cpp | 1 + src/consensus/tx_verify.cpp | 308 +++++++++++++++++++++++++++++++++ src/consensus/tx_verify.h | 64 +++++++ src/main.cpp | 227 +----------------------- src/main.h | 36 ---- src/net/messages.cpp | 2 +- src/processblock.cpp | 2 +- src/processtx.cpp | 70 -------- src/processtx.h | 17 -- src/test/sighash_tests.cpp | 2 +- src/test/transaction_tests.cpp | 2 +- src/wallet/walletdb.cpp | 2 +- 15 files changed, 385 insertions(+), 357 deletions(-) create mode 100644 src/consensus/tx_verify.cpp create mode 100644 src/consensus/tx_verify.h delete mode 100644 src/processtx.cpp delete mode 100644 src/processtx.h diff --git a/src/.formatted-files b/src/.formatted-files index 9be35e9c..584670a1 100644 --- a/src/.formatted-files +++ b/src/.formatted-files @@ -51,6 +51,8 @@ consensus/consensus.h consensus/merkle.cpp consensus/merkle.h consensus/params.h +consensus/tx_verify.cpp +consensus/tx_verify.h consensus/validation.h core_io.h core_memusage.h @@ -112,8 +114,6 @@ processblock.cpp processblock.h processheader.cpp processheader.h -processtx.cpp -processtx.h pubkey.cpp pubkey.h random.cpp diff --git a/src/Makefile.am b/src/Makefile.am index 07bbe07f..28498c41 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -76,6 +76,7 @@ BITCOIN_CORE_H = \ consensus/consensus.h \ consensus/merkle.h \ consensus/params.h \ + consensus/tx_verify.h \ consensus/validation.h \ core_io.h \ core_memusage.h \ @@ -112,7 +113,6 @@ BITCOIN_CORE_H = \ crypto/pbkdf2.h \ processblock.h \ processheader.h \ - processtx.h \ pubkey.h \ random.h \ reverselock.h \ @@ -218,6 +218,7 @@ libbitcoin_server_a_SOURCES = \ coins.cpp \ compressor.cpp \ consensus/merkle.cpp \ + consensus/tx_verify.cpp \ core_read.cpp \ core_write.cpp \ crypto/hash.cpp \ @@ -228,7 +229,6 @@ libbitcoin_server_a_SOURCES = \ chain/blockindex.cpp \ processblock.cpp \ processheader.cpp \ - processtx.cpp \ chain/tx.cpp \ net/nodestate.cpp \ net/protocol.cpp \ diff --git a/src/blockgeneration/miner.cpp b/src/blockgeneration/miner.cpp index 64630331..51ee594d 100644 --- a/src/blockgeneration/miner.cpp +++ b/src/blockgeneration/miner.cpp @@ -10,6 +10,7 @@ #include "blockgeneration.h" #include "compare.h" #include "consensus/consensus.h" +#include "consensus/tx_verify.h" #include "consensus/validation.h" #include "crypto/scrypt.h" #include "init.h" diff --git a/src/blockgeneration/minter.cpp b/src/blockgeneration/minter.cpp index e70044f2..81dd2ba4 100644 --- a/src/blockgeneration/minter.cpp +++ b/src/blockgeneration/minter.cpp @@ -9,6 +9,7 @@ #include "args.h" #include "blockgeneration.h" +#include "consensus/tx_verify.h" #include "init.h" #include "processblock.h" #include "txmempool.h" diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp new file mode 100644 index 00000000..8a16a42f --- /dev/null +++ b/src/consensus/tx_verify.cpp @@ -0,0 +1,308 @@ +// This file is part of the Eccoin project +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2014-2018 The Eccoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "consensus/tx_verify.h" +#include "base58.h" +#include "consensus/consensus.h" +#include "main.h" +#include "net/messages.h" +#include "script/standard.h" +#include "util/utilmoneystr.h" +#include "wallet/wallet.h" + +bool CheckTransaction(const CTransaction &tx, CValidationState &state) +{ + // Basic checks that don't depend on any context + if (tx.vin.empty()) + return state.DoS(10, false, REJECT_INVALID, "bad-txns-vin-empty"); + if (tx.vout.empty()) + return state.DoS(10, false, REJECT_INVALID, "bad-txns-vout-empty"); + // Size limits + if (::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) > MAX_BLOCK_SIZE) + return state.DoS(100, false, REJECT_INVALID, "bad-txns-oversize"); + + // Check for negative or overflow output values + CAmount nValueOut = 0; + int32_t out = 0; + for (auto const &txout : tx.vout) + { + if (txout.nValue < 0) + { + return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-negative"); + } + if (txout.nValue > MAX_MONEY) + return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-toolarge"); + nValueOut += txout.nValue; + if (!MoneyRange(nValueOut)) + return state.DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge"); + out++; + } + + // Check for duplicate inputs + std::set vInOutPoints; + for (auto const &txin : tx.vin) + { + if (vInOutPoints.count(txin.prevout)) + return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate"); + vInOutPoints.insert(txin.prevout); + } + + if (tx.IsCoinBase()) + { + if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100) + return state.DoS(100, false, REJECT_INVALID, "bad-cb-length"); + } + else + { + for (auto const &txin : tx.vin) + if (txin.prevout.IsNull()) + return state.DoS(10, false, REJECT_INVALID, "bad-txns-prevout-null"); + } + if (tx.nVersion == 2) + { + if (tx.serviceReferenceHash.IsNull()) + { + state.DoS(100, false, REJECT_INVALID, "bad-stx-ref-hash"); + } + } + return true; +} + +/** + * Return the spend height, which is one more than the inputs.GetBestBlock(). + * While checking, GetBestBlock() refers to the parent block. (protected by cs_main) + * This is also true for mempool checks. + */ +int GetSpendHeight(const CCoinsViewCache &inputs) +{ + RECURSIVEREADLOCK(pnetMan->getChainActive()->cs_mapBlockIndex); + BlockMap::iterator i = pnetMan->getChainActive()->mapBlockIndex.find(inputs.GetBestBlock()); + if (i != pnetMan->getChainActive()->mapBlockIndex.end()) + { + CBlockIndex *pindexPrev = i->second; + if (pindexPrev) + return pindexPrev->nHeight + 1; + else + { + throw std::runtime_error("GetSpendHeight(): mapBlockIndex contains null block"); + } + } + throw std::runtime_error("GetSpendHeight(): best block does not exist"); +} + +bool Consensus::CheckTxInputs(const CTransaction &tx, CValidationState &state, const CCoinsViewCache &inputs) +{ + // This doesn't trigger the DoS code on purpose; if it did, it would make it easier + // for an attacker to attempt to split the network. + if (!inputs.HaveInputs(tx)) + return state.Invalid(false, 0, "", "Inputs unavailable"); + + CAmount nValueIn = 0; + CAmount nFees = 0; + int nSpendHeight = -1; + for (uint64_t i = 0; i < tx.vin.size(); i++) + { + const COutPoint &prevout = tx.vin[i].prevout; + Coin coin; + inputs.GetCoin(prevout, coin); // Make a copy so I don't hold the utxo lock + assert(!coin.IsSpent()); + + // If prev is coinbase or coinstake, check that it's matured + if (coin.IsCoinBase() || tx.IsCoinStake()) + { + if (nSpendHeight == -1) + nSpendHeight = GetSpendHeight(inputs); + if (nSpendHeight - coin.nHeight < COINBASE_MATURITY && + pnetMan->getChainActive()->chainActive.Tip()->nHeight > 1600000) + return state.Invalid(false, REJECT_INVALID, "bad-txns-premature-spend-of-coinbase", + strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); + } + + // Check for negative or overflow input values + nValueIn += coin.out.nValue; + if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) + return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange"); + } + + if (!tx.IsCoinStake()) + { + if (nValueIn < tx.GetValueOut()) + return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-belowout", false, + strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(tx.GetValueOut()))); + // Tally transaction fees + CAmount nTxFee = nValueIn - tx.GetValueOut(); + if (nTxFee < 0) + return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-negative"); + nFees += nTxFee; + if (!MoneyRange(nFees)) + return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange"); + } + else + { + // ppcoin: coin stake tx earns reward instead of paying fee + uint64_t nCoinAge; + if (!tx.GetCoinAge(nCoinAge)) + { + return state.DoS(100, false, REJECT_INVALID, "bad-txns-cant-get-coin-age", false, + strprintf("ConnectInputs() : %s unable to get coin age for coinstake", + tx.GetHash().ToString().substr(0, 10).c_str())); + } + + int64_t nStakeReward = tx.GetValueOut() - nValueIn; + if (nStakeReward > + GetProofOfStakeReward(tx.GetCoinAge(nCoinAge, true), nSpendHeight) + DEFAULT_TRANSACTION_MINFEE) + { + LogPrint("kernel", "nStakeReward = %d , CoinAge = %d \n", nStakeReward, nCoinAge); + return state.DoS(100, false, REJECT_INVALID, "bad-txns-stake-reward-too-high", false, + strprintf("ConnectInputs() : %s stake reward exceeded", tx.GetHash().ToString().substr(0, 10).c_str())); + } + } + return true; +} + +unsigned int GetLegacySigOpCount(const CTransaction &tx) +{ + unsigned int nSigOps = 0; + for (auto const &txin : tx.vin) + { + nSigOps += txin.scriptSig.GetSigOpCount(false); + } + for (auto const &txout : tx.vout) + { + nSigOps += txout.scriptPubKey.GetSigOpCount(false); + } + return nSigOps; +} + +unsigned int GetP2SHSigOpCount(const CTransaction &tx, const CCoinsViewCache &inputs) +{ + if (tx.IsCoinBase()) + return 0; + + unsigned int nSigOps = 0; + for (unsigned int i = 0; i < tx.vin.size(); i++) + { + CoinAccessor coin(inputs, tx.vin[i].prevout); + assert(!coin->IsSpent()); + if (coin && coin->out.scriptPubKey.IsPayToScriptHash()) + nSigOps += coin->out.scriptPubKey.GetSigOpCount(tx.vin[i].scriptSig); + } + return nSigOps; +} + +bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) +{ + if (tx.nLockTime == 0) + { + return true; + } + if ((int64_t)tx.nLockTime < ((int64_t)tx.nLockTime < LOCKTIME_THRESHOLD ? (int64_t)nBlockHeight : nBlockTime)) + { + return true; + } + for (auto const &txin : tx.vin) + { + if (!(txin.nSequence == CTxIn::SEQUENCE_FINAL)) + { + return false; + } + } + return true; +} + +/** + * Calculates the block height and previous block's median time past at + * which the transaction will be considered final in the context of BIP 68. + * Also removes from the vector of input heights any entries which did not + * correspond to sequence locked inputs as they do not affect the calculation. + */ +std::pair CalculateSequenceLocks(const CTransaction &tx, + int flags, + std::vector *prevHeights, + const CBlockIndex &block) +{ + assert(prevHeights->size() == tx.vin.size()); + + // Will be set to the equivalent height- and time-based nLockTime + // values that would be necessary to satisfy all relative lock- + // time constraints given our view of block chain history. + // The semantics of nLockTime are the last invalid height/time, so + // use -1 to have the effect of any height or time being valid. + int nMinHeight = -1; + int64_t nMinTime = -1; + + // tx.nVersion is signed integer so requires cast to unsigned otherwise + // we would be doing a signed comparison and half the range of nVersion + // wouldn't support BIP 68. + bool fEnforceBIP68 = static_cast(tx.nVersion) >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE; + + // Do not enforce sequence numbers as a relative lock time + // unless we have been instructed to + if (!fEnforceBIP68) + { + return std::make_pair(nMinHeight, nMinTime); + } + + for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) + { + const CTxIn &txin = tx.vin[txinIndex]; + + // Sequence numbers with the most significant bit set are not + // treated as relative lock-times, nor are they given any + // consensus-enforced meaning at this point. + if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_DISABLE_FLAG) + { + // The height of this input is not relevant for sequence locks + (*prevHeights)[txinIndex] = 0; + continue; + } + + int nCoinHeight = (*prevHeights)[txinIndex]; + + if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) + { + int64_t nCoinTime = block.GetAncestor(std::max(nCoinHeight - 1, 0))->GetMedianTimePast(); + // NOTE: Subtract 1 to maintain nLockTime semantics + // BIP 68 relative lock times have the semantics of calculating + // the first block or time at which the transaction would be + // valid. When calculating the effective block time or height + // for the entire transaction, we switch to using the + // semantics of nLockTime which is the last invalid block + // time or height. Thus we subtract 1 from the calculated + // time or height. + + // Time-based relative lock-times are measured from the + // smallest allowed timestamp of the block containing the + // txout being spent, which is the median time past of the + // block prior. + nMinTime = std::max(nMinTime, nCoinTime + (int64_t)((txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_MASK) + << CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) - + 1); + } + else + { + nMinHeight = std::max(nMinHeight, nCoinHeight + (int)(txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_MASK) - 1); + } + } + + return std::make_pair(nMinHeight, nMinTime); +} + +bool EvaluateSequenceLocks(const CBlockIndex &block, std::pair lockPair) +{ + assert(block.pprev); + int64_t nBlockTime = block.pprev->GetMedianTimePast(); + if (lockPair.first >= block.nHeight || lockPair.second >= nBlockTime) + return false; + + return true; +} + +bool SequenceLocks(const CTransaction &tx, int flags, std::vector *prevHeights, const CBlockIndex &block) +{ + return EvaluateSequenceLocks(block, CalculateSequenceLocks(tx, flags, prevHeights, block)); +} diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h new file mode 100644 index 00000000..affe898d --- /dev/null +++ b/src/consensus/tx_verify.h @@ -0,0 +1,64 @@ +// This file is part of the Eccoin project +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2014-2018 The Eccoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "chain/tx.h" +#include "coins.h" +#include "validationinterface.h" + +/** Context-independent validity checks */ +bool CheckTransaction(const CTransaction &tx, CValidationState &state); + +namespace Consensus +{ +/** + * Check whether all inputs of this transaction are valid (no double spends and amounts) + * This does not modify the UTXO set. This does not check scripts and sigs. + * Preconditions: tx.IsCoinBase() is false. + */ +bool CheckTxInputs(const CTransaction &tx, CValidationState &state, const CCoinsViewCache &inputs); +} // namespace Consensus + +/** + * Count ECDSA signature operations the old-fashioned (pre-0.6) way + * @return number of sigops this transaction's outputs will produce when spent + * @see CTransaction::FetchInputs + */ +unsigned int GetLegacySigOpCount(const CTransaction &tx); + +/** + * Count ECDSA signature operations in pay-to-script-hash inputs. + * + * @param[in] mapInputs Map of previous transactions that have outputs we're spending + * @return maximum number of sigops required to validate this transaction's inputs + * @see CTransaction::FetchInputs + */ +unsigned int GetP2SHSigOpCount(const CTransaction &tx, const CCoinsViewCache &mapInputs); + +/** + * Check if transaction is final and can be included in a block with the + * specified height and time. Consensus critical. + */ +bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime); + +/** + * Calculates the block height and previous block's median time past at + * which the transaction will be considered final in the context of BIP 68. + * Also removes from the vector of input heights any entries which did not + * correspond to sequence locked inputs as they do not affect the calculation. + */ +std::pair CalculateSequenceLocks(const CTransaction &tx, + int flags, + std::vector *prevHeights, + const CBlockIndex &block); + +bool EvaluateSequenceLocks(const CBlockIndex &block, std::pair lockPair); + +/** + * Check if transaction is final per BIP 68 sequence numbers and can be included in a block. + * Consensus critical. Takes as input a list of heights at which tx's inputs (in order) confirmed. + */ +bool SequenceLocks(const CTransaction &tx, int flags, std::vector *prevHeights, const CBlockIndex &block); diff --git a/src/main.cpp b/src/main.cpp index e1fb8f37..0b538ae3 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -15,6 +15,7 @@ #include "checkqueue.h" #include "consensus/consensus.h" #include "consensus/merkle.h" +#include "consensus/tx_verify.h" #include "consensus/validation.h" #include "crypto/hash.h" #include "init.h" @@ -29,7 +30,6 @@ #include "pow.h" #include "processblock.h" #include "processheader.h" -#include "processtx.h" #include "random.h" #include "script/script.h" #include "script/sigcache.h" @@ -138,26 +138,6 @@ int GetHeight() { return pnetMan->getChainActive()->chainActive.Height(); } // mapOrphanTransactions // -bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) -{ - if (tx.nLockTime == 0) - { - return true; - } - if ((int64_t)tx.nLockTime < ((int64_t)tx.nLockTime < LOCKTIME_THRESHOLD ? (int64_t)nBlockHeight : nBlockTime)) - { - return true; - } - for (auto const &txin : tx.vin) - { - if (!(txin.nSequence == CTxIn::SEQUENCE_FINAL)) - { - return false; - } - } - return true; -} - bool CheckFinalTx(const CTransaction &tx, int flags) { // By convention a negative value for flags indicates that the @@ -188,99 +168,6 @@ bool CheckFinalTx(const CTransaction &tx, int flags) return IsFinalTx(tx, nBlockHeight, nBlockTime); } -/** - * Calculates the block height and previous block's median time past at - * which the transaction will be considered final in the context of BIP 68. - * Also removes from the vector of input heights any entries which did not - * correspond to sequence locked inputs as they do not affect the calculation. - */ -static std::pair CalculateSequenceLocks(const CTransaction &tx, - int flags, - std::vector *prevHeights, - const CBlockIndex &block) -{ - assert(prevHeights->size() == tx.vin.size()); - - // Will be set to the equivalent height- and time-based nLockTime - // values that would be necessary to satisfy all relative lock- - // time constraints given our view of block chain history. - // The semantics of nLockTime are the last invalid height/time, so - // use -1 to have the effect of any height or time being valid. - int nMinHeight = -1; - int64_t nMinTime = -1; - - // tx.nVersion is signed integer so requires cast to unsigned otherwise - // we would be doing a signed comparison and half the range of nVersion - // wouldn't support BIP 68. - bool fEnforceBIP68 = static_cast(tx.nVersion) >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE; - - // Do not enforce sequence numbers as a relative lock time - // unless we have been instructed to - if (!fEnforceBIP68) - { - return std::make_pair(nMinHeight, nMinTime); - } - - for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) - { - const CTxIn &txin = tx.vin[txinIndex]; - - // Sequence numbers with the most significant bit set are not - // treated as relative lock-times, nor are they given any - // consensus-enforced meaning at this point. - if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_DISABLE_FLAG) - { - // The height of this input is not relevant for sequence locks - (*prevHeights)[txinIndex] = 0; - continue; - } - - int nCoinHeight = (*prevHeights)[txinIndex]; - - if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) - { - int64_t nCoinTime = block.GetAncestor(std::max(nCoinHeight - 1, 0))->GetMedianTimePast(); - // NOTE: Subtract 1 to maintain nLockTime semantics - // BIP 68 relative lock times have the semantics of calculating - // the first block or time at which the transaction would be - // valid. When calculating the effective block time or height - // for the entire transaction, we switch to using the - // semantics of nLockTime which is the last invalid block - // time or height. Thus we subtract 1 from the calculated - // time or height. - - // Time-based relative lock-times are measured from the - // smallest allowed timestamp of the block containing the - // txout being spent, which is the median time past of the - // block prior. - nMinTime = std::max(nMinTime, nCoinTime + (int64_t)((txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_MASK) - << CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) - - 1); - } - else - { - nMinHeight = std::max(nMinHeight, nCoinHeight + (int)(txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_MASK) - 1); - } - } - - return std::make_pair(nMinHeight, nMinTime); -} - -static bool EvaluateSequenceLocks(const CBlockIndex &block, std::pair lockPair) -{ - assert(block.pprev); - int64_t nBlockTime = block.pprev->GetMedianTimePast(); - if (lockPair.first >= block.nHeight || lockPair.second >= nBlockTime) - return false; - - return true; -} - -bool SequenceLocks(const CTransaction &tx, int flags, std::vector *prevHeights, const CBlockIndex &block) -{ - return EvaluateSequenceLocks(block, CalculateSequenceLocks(tx, flags, prevHeights, block)); -} - bool TestLockPointValidity(const LockPoints *lp) { assert(lp); @@ -381,37 +268,6 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints *lp, bool return EvaluateSequenceLocks(index, lockPair); } - -unsigned int GetLegacySigOpCount(const CTransaction &tx) -{ - unsigned int nSigOps = 0; - for (auto const &txin : tx.vin) - { - nSigOps += txin.scriptSig.GetSigOpCount(false); - } - for (auto const &txout : tx.vout) - { - nSigOps += txout.scriptPubKey.GetSigOpCount(false); - } - return nSigOps; -} - -unsigned int GetP2SHSigOpCount(const CTransaction &tx, const CCoinsViewCache &inputs) -{ - if (tx.IsCoinBase()) - return 0; - - unsigned int nSigOps = 0; - for (unsigned int i = 0; i < tx.vin.size(); i++) - { - CoinAccessor coin(inputs, tx.vin[i].prevout); - assert(!coin->IsSpent()); - if (coin && coin->out.scriptPubKey.IsPayToScriptHash()) - nSigOps += coin->out.scriptPubKey.GetSigOpCount(tx.vin[i].scriptSig); - } - return nSigOps; -} - void LimitMempoolSize(CTxMemPool &pool, size_t limit, unsigned long age) { std::vector vCoinsToUncache; @@ -811,85 +667,6 @@ bool CScriptCheck::operator()() return true; } -int GetSpendHeight(const CCoinsViewCache &inputs) -{ - CBlockIndex *pindexPrev = pnetMan->getChainActive()->LookupBlockIndex(inputs.GetBestBlock()); - return pindexPrev->nHeight + 1; -} - -namespace Consensus -{ -bool CheckTxInputs(const CTransaction &tx, CValidationState &state, const CCoinsViewCache &inputs, int64_t nSpendHeight) -{ - // This doesn't trigger the DoS code on purpose; if it did, it would make it easier - // for an attacker to attempt to split the network. - if (!inputs.HaveInputs(tx)) - return state.Invalid(false, 0, "", "Inputs unavailable"); - - CAmount nValueIn = 0; - CAmount nFees = 0; - nSpendHeight = -1; - for (uint64_t i = 0; i < tx.vin.size(); i++) - { - const COutPoint &prevout = tx.vin[i].prevout; - Coin coin; - inputs.GetCoin(prevout, coin); // Make a copy so I don't hold the utxo lock - assert(!coin.IsSpent()); - - // If prev is coinbase or coinstake, check that it's matured - if (coin.IsCoinBase() || tx.IsCoinStake()) - { - if (nSpendHeight == -1) - nSpendHeight = GetSpendHeight(inputs); - if (nSpendHeight - coin.nHeight < COINBASE_MATURITY && - pnetMan->getChainActive()->chainActive.Tip()->nHeight > 1600000) - return state.Invalid(false, REJECT_INVALID, "bad-txns-premature-spend-of-coinbase", - strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); - } - - // Check for negative or overflow input values - nValueIn += coin.out.nValue; - if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange"); - } - - if (!tx.IsCoinStake()) - { - if (nValueIn < tx.GetValueOut()) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-belowout", false, - strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(tx.GetValueOut()))); - // Tally transaction fees - CAmount nTxFee = nValueIn - tx.GetValueOut(); - if (nTxFee < 0) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-negative"); - nFees += nTxFee; - if (!MoneyRange(nFees)) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange"); - } - else - { - // ppcoin: coin stake tx earns reward instead of paying fee - uint64_t nCoinAge; - if (!tx.GetCoinAge(nCoinAge)) - { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-cant-get-coin-age", false, - strprintf("ConnectInputs() : %s unable to get coin age for coinstake", - tx.GetHash().ToString().substr(0, 10).c_str())); - } - - int64_t nStakeReward = tx.GetValueOut() - nValueIn; - if (nStakeReward > - GetProofOfStakeReward(tx.GetCoinAge(nCoinAge, true), nSpendHeight) + DEFAULT_TRANSACTION_MINFEE) - { - LogPrint("kernel", "nStakeReward = %d , CoinAge = %d \n", nStakeReward, nCoinAge); - return state.DoS(100, false, REJECT_INVALID, "bad-txns-stake-reward-too-high", false, - strprintf("ConnectInputs() : %s stake reward exceeded", tx.GetHash().ToString().substr(0, 10).c_str())); - } - } - return true; -} -} // namespace Consensus - bool CheckInputs(const CTransaction &tx, CValidationState &state, const CCoinsViewCache &inputs, @@ -900,7 +677,7 @@ bool CheckInputs(const CTransaction &tx, { if (!tx.IsCoinBase()) { - if (!Consensus::CheckTxInputs(tx, state, inputs, GetSpendHeight(inputs))) + if (!Consensus::CheckTxInputs(tx, state, inputs)) return false; if (pvChecks) diff --git a/src/main.h b/src/main.h index 317ef4e3..467822eb 100644 --- a/src/main.h +++ b/src/main.h @@ -298,23 +298,6 @@ struct CDiskTxPos : public CDiskBlockPos }; -/** - * Count ECDSA signature operations the old-fashioned (pre-0.6) way - * @return number of sigops this transaction's outputs will produce when spent - * @see CTransaction::FetchInputs - */ -unsigned int GetLegacySigOpCount(const CTransaction &tx); - -/** - * Count ECDSA signature operations in pay-to-script-hash inputs. - * - * @param[in] mapInputs Map of previous transactions that have outputs we're spending - * @return maximum number of sigops required to validate this transaction's inputs - * @see CTransaction::FetchInputs - */ -unsigned int GetP2SHSigOpCount(const CTransaction &tx, const CCoinsViewCache &mapInputs); - - /** * Check whether all inputs of this transaction are valid (no double spends, scripts & sigs, amounts) * This does not modify the UTXO set. If pvChecks is not NULL, script checks are pushed onto it @@ -328,12 +311,6 @@ bool CheckInputs(const CTransaction &tx, bool cacheStore, std::vector *pvChecks = nullptr); -/** - * Check if transaction is final and can be included in a block with the - * specified height and time. Consensus critical. - */ -bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime); - /** * Check if transaction will be final in the next block to be created. * @@ -348,12 +325,6 @@ bool CheckFinalTx(const CTransaction &tx, int flags = -1); */ bool TestLockPointValidity(const LockPoints *lp); -/** - * Check if transaction is final per BIP 68 sequence numbers and can be included in a block. - * Consensus critical. Takes as input a list of heights at which tx's inputs (in order) confirmed. - */ -bool SequenceLocks(const CTransaction &tx, int flags, std::vector *prevHeights, const CBlockIndex &block); - /** * Check if transaction will be BIP 68 final in the next block to be created. * @@ -484,13 +455,6 @@ bool InvalidateBlock(CValidationState &state, const Consensus::Params &consensus /** Remove invalidity status from a block and its descendants. */ bool ReconsiderBlock(CValidationState &state, CBlockIndex *pindex); -/** - * Return the spend height, which is one more than the inputs.GetBestBlock(). - * While checking, GetBestBlock() refers to the parent block. (protected by cs_main) - * This is also true for mempool checks. - */ -int GetSpendHeight(const CCoinsViewCache &inputs); - /** * Determine what nVersion a new block should use. */ diff --git a/src/net/messages.cpp b/src/net/messages.cpp index 05eb38cc..9bec63e5 100644 --- a/src/net/messages.cpp +++ b/src/net/messages.cpp @@ -12,6 +12,7 @@ #include "blockstorage/blockstorage.h" #include "chain/chain.h" #include "chain/tx.h" +#include "consensus/tx_verify.h" #include "consensus/validation.h" #include "init.h" #include "main.h" @@ -25,7 +26,6 @@ #include "policy/policy.h" #include "processblock.h" #include "processheader.h" -#include "processtx.h" #include "serialize.h" #include "sync.h" #include "txmempool.h" diff --git a/src/processblock.cpp b/src/processblock.cpp index a9e13a01..e6f7ec7b 100644 --- a/src/processblock.cpp +++ b/src/processblock.cpp @@ -19,6 +19,7 @@ #include "chain/checkpoints.h" #include "checkqueue.h" #include "consensus/merkle.h" +#include "consensus/tx_verify.h" #include "crypto/hash.h" #include "init.h" #include "kernel.h" @@ -30,7 +31,6 @@ #include "policy/policy.h" #include "processblock.h" #include "processheader.h" -#include "processtx.h" #include "txmempool.h" #include "undo.h" diff --git a/src/processtx.cpp b/src/processtx.cpp deleted file mode 100644 index af4c3425..00000000 --- a/src/processtx.cpp +++ /dev/null @@ -1,70 +0,0 @@ -// This file is part of the Eccoin project -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2016 The Bitcoin Core developers -// Copyright (c) 2014-2018 The Eccoin developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include "base58.h" -#include "consensus/consensus.h" -#include "main.h" -#include "net/messages.h" -#include "script/standard.h" - -bool CheckTransaction(const CTransaction &tx, CValidationState &state) -{ - // Basic checks that don't depend on any context - if (tx.vin.empty()) - return state.DoS(10, false, REJECT_INVALID, "bad-txns-vin-empty"); - if (tx.vout.empty()) - return state.DoS(10, false, REJECT_INVALID, "bad-txns-vout-empty"); - // Size limits - if (::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) > MAX_BLOCK_SIZE) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-oversize"); - - // Check for negative or overflow output values - CAmount nValueOut = 0; - int32_t out = 0; - for (auto const &txout : tx.vout) - { - if (txout.nValue < 0) - { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-negative"); - } - if (txout.nValue > MAX_MONEY) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-toolarge"); - nValueOut += txout.nValue; - if (!MoneyRange(nValueOut)) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge"); - out++; - } - - // Check for duplicate inputs - std::set vInOutPoints; - for (auto const &txin : tx.vin) - { - if (vInOutPoints.count(txin.prevout)) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate"); - vInOutPoints.insert(txin.prevout); - } - - if (tx.IsCoinBase()) - { - if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100) - return state.DoS(100, false, REJECT_INVALID, "bad-cb-length"); - } - else - { - for (auto const &txin : tx.vin) - if (txin.prevout.IsNull()) - return state.DoS(10, false, REJECT_INVALID, "bad-txns-prevout-null"); - } - if (tx.nVersion == 2) - { - if (tx.serviceReferenceHash.IsNull()) - { - state.DoS(100, false, REJECT_INVALID, "bad-stx-ref-hash"); - } - } - return true; -} diff --git a/src/processtx.h b/src/processtx.h deleted file mode 100644 index aa63dd7f..00000000 --- a/src/processtx.h +++ /dev/null @@ -1,17 +0,0 @@ -// This file is part of the Eccoin project -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2016 The Bitcoin Core developers -// Copyright (c) 2014-2018 The Eccoin developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef TXVALIDATION_H -#define TXVALIDATION_H - -#include "chain/tx.h" -#include "validationinterface.h" - -/** Context-independent validity checks */ -bool CheckTransaction(const CTransaction &tx, CValidationState &state); - -#endif // TXVALIDATION_H diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index 5d6b3bc6..b82c92d5 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -3,11 +3,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include "consensus/tx_verify.h" #include "consensus/validation.h" #include "crypto/hash.h" #include "data/sighash.json.h" #include "main.h" // For CheckTransaction -#include "processtx.h" #include "random.h" #include "script/interpreter.h" #include "script/script.h" diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index a4462276..3391985a 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -8,13 +8,13 @@ #include "test/test_bitcoin.h" #include "clientversion.h" +#include "consensus/tx_verify.h" #include "consensus/validation.h" #include "core_io.h" #include "key.h" #include "keystore.h" #include "main.h" // For CheckTransaction #include "policy/policy.h" -#include "processtx.h" #include "script/interpreter.h" #include "script/script.h" #include "script/script_error.h" diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 5530ed5e..b9699e0d 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -9,9 +9,9 @@ #include "args.h" #include "base58.h" +#include "consensus/tx_verify.h" // For CheckTransaction #include "consensus/validation.h" #include "net/protocol.h" -#include "processtx.h" // For CheckTransaction #include "serialize.h" #include "sync.h" #include "util/util.h" From 25f17409043aa6ea9930b26cf871aeed61f9d4f7 Mon Sep 17 00:00:00 2001 From: Griffith <8345264+Greg-Griffith@users.noreply.github.com> Date: Mon, 26 Aug 2019 15:52:18 -0700 Subject: [PATCH 2/8] Deadlock detector improvements (#238) * change isExclusive bool to an enum * fix incorrect logic, a thread with only readlocks can deadlock * change tests to rely on counters instead of sleeping/timing * remove functions from emptysuite so it is now empty * debug assert if we attempt to push_lock for an unsupported mutex type * some minor code cleanup * fix issue with try locks not being shown in heldlocks maps * store the locktype in the locklocation struct, add getter for it * do not erase locks from held map that we have recursively locked them and have more than 0 remaining locked * skip deadlock detection when recursively locking * add held locks even if waiting locks do not exist for some reason * fix issue where a try lock would be considered waiting, try never waits * add some debug asserts for missing waiting locks * dont call SetWaitingToHeld for try locks, they should never be waiting * use locktype and ownership names consistently --- src/coins.cpp | 9 +- src/sync.cpp | 6 +- src/sync.h | 76 +++-- src/test/deadlock_tests/suite.h | 13 +- src/test/deadlock_tests/test1-4.cpp | 15 + src/test/deadlock_tests/test5.cpp | 46 +-- src/test/deadlock_tests/test6.cpp | 42 ++- src/test/deadlock_tests/test7.cpp | 65 ++++- src/test/deadlock_tests/test8.cpp | 66 ++++- src/threaddeadlock.cpp | 419 +++++++++++----------------- src/threaddeadlock.h | 36 ++- src/util/util.h | 22 ++ 12 files changed, 434 insertions(+), 381 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 519248cc..841565d9 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -496,7 +496,8 @@ static const size_t nMaxOutputsPerBlock = CoinAccessor::CoinAccessor(const CCoinsViewCache &view, const uint256 &txid) : cache(&view), lock(cache->csCacheInsert) { - EnterCritical("CCoinsViewCache.cs_utxo", __FILE__, __LINE__, (void *)(&cache->cs_utxo), LockType::SHARED, false); + EnterCritical("CCoinsViewCache.cs_utxo", __FILE__, __LINE__, (void *)(&cache->cs_utxo), LockType::SHARED_MUTEX, + OwnershipType::SHARED); cache->cs_utxo.lock_shared(); COutPoint iter(txid, 0); coin = &emptyCoin; @@ -515,7 +516,8 @@ CoinAccessor::CoinAccessor(const CCoinsViewCache &view, const uint256 &txid) : c CoinAccessor::CoinAccessor(const CCoinsViewCache &cacheObj, const COutPoint &output) : cache(&cacheObj), lock(cache->csCacheInsert) { - EnterCritical("CCoinsViewCache.cs_utxo", __FILE__, __LINE__, (void *)(&cache->cs_utxo), LockType::SHARED, false); + EnterCritical("CCoinsViewCache.cs_utxo", __FILE__, __LINE__, (void *)(&cache->cs_utxo), LockType::SHARED_MUTEX, + OwnershipType::SHARED); cache->cs_utxo.lock_shared(); it = cache->FetchCoin(output, &lock); if (it != cache->cacheCoins.end()) @@ -534,7 +536,8 @@ CoinAccessor::~CoinAccessor() CoinModifier::CoinModifier(const CCoinsViewCache &cacheObj, const COutPoint &output) : cache(&cacheObj) { - EnterCritical("CCoinsViewCache.cs_utxo", __FILE__, __LINE__, (void *)(&cache->cs_utxo), LockType::SHARED, true); + EnterCritical("CCoinsViewCache.cs_utxo", __FILE__, __LINE__, (void *)(&cache->cs_utxo), LockType::SHARED_MUTEX, + OwnershipType::EXCLUSIVE); cache->cs_utxo.lock(); it = cache->FetchCoin(output, nullptr); if (it != cache->cacheCoins.end()) diff --git a/src/sync.cpp b/src/sync.cpp index a4c4d0a6..242d1c97 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -22,11 +22,11 @@ void EnterCritical(const char *pszName, const char *pszFile, unsigned int nLine, void *cs, - LockType type, - bool isExclusive, + LockType locktype, + OwnershipType ownership, bool fTry) { - push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry, isExclusive), type, isExclusive, fTry); + push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry, ownership, locktype), locktype, ownership, fTry); } void LeaveCritical(void *cs) { remove_lock_critical_exit(cs); } diff --git a/src/sync.h b/src/sync.h index 8986a9a0..794e53ae 100644 --- a/src/sync.h +++ b/src/sync.h @@ -182,8 +182,8 @@ void EnterCritical(const char *pszName, const char *pszFile, unsigned int nLine, void *cs, - LockType type, - bool isExclusive, + LockType locktype, + OwnershipType ownership, bool fTry = false); void LeaveCritical(void *cs); std::string LocksHeld(); @@ -204,8 +204,8 @@ void static inline EnterCritical(const char *pszName, const char *pszFile, unsigned int nLine, void *cs, - LockType type, - bool isExclusive, + LockType locktype, + OwnershipType ownership, bool fTry = false) { } @@ -256,9 +256,9 @@ class SCOPED_LOCKABLE CMutexLock name = pszName; file = pszFile; line = nLine; - EnterCritical(pszName, pszFile, nLine, (void *)(lock.mutex()), type, true, false); + EnterCritical(pszName, pszFile, nLine, (void *)(lock.mutex()), type, OwnershipType::EXCLUSIVE, false); lock.lock(); - SetWaitingToHeld((void *)(lock.mutex()), true); + SetWaitingToHeld((void *)(lock.mutex()), OwnershipType::EXCLUSIVE); #ifdef DEBUG_LOCKTIME lockedTime = GetStopwatch(); if (lockedTime - startWait > LOCK_WARN_TIME) @@ -273,7 +273,7 @@ class SCOPED_LOCKABLE CMutexLock name = pszName; file = pszFile; line = nLine; - EnterCritical(pszName, pszFile, nLine, (void *)(lock.mutex()), type, true, true); + EnterCritical(pszName, pszFile, nLine, (void *)(lock.mutex()), type, OwnershipType::EXCLUSIVE, true); lock.try_lock(); if (!lock.owns_lock()) { @@ -286,12 +286,7 @@ class SCOPED_LOCKABLE CMutexLock else lockedTime = GetStopwatch(); #endif - bool owned = lock.owns_lock(); - if (owned) - { - SetWaitingToHeld((void *)(lock.mutex()), false); - } - return owned; + return lock.owns_lock(); } public: @@ -366,9 +361,9 @@ class SCOPED_LOCKABLE CMutexReadLock name = pszName; file = pszFile; line = nLine; - EnterCritical(pszName, pszFile, nLine, (void *)(lock.mutex()), type, false, false); + EnterCritical(pszName, pszFile, nLine, (void *)(lock.mutex()), type, OwnershipType::SHARED, false); lock.lock(); - SetWaitingToHeld((void *)(lock.mutex()), false); + SetWaitingToHeld((void *)(lock.mutex()), OwnershipType::SHARED); #ifdef DEBUG_LOCKTIME lockedTime = GetStopwatch(); if (lockedTime - startWait > LOCK_WARN_TIME) @@ -383,7 +378,7 @@ class SCOPED_LOCKABLE CMutexReadLock name = pszName; file = pszFile; line = nLine; - EnterCritical(pszName, pszFile, nLine, (void *)(lock.mutex()), type, false, true); + EnterCritical(pszName, pszFile, nLine, (void *)(lock.mutex()), type, OwnershipType::SHARED, true); if (!lock.try_lock()) { #ifdef DEBUG_LOCKTIME @@ -395,12 +390,7 @@ class SCOPED_LOCKABLE CMutexReadLock else lockedTime = GetStopwatch(); #endif - bool owned = lock.owns_lock(); - if (owned) - { - SetWaitingToHeld((void *)(lock.mutex()), false); - } - return owned; + return lock.owns_lock(); } public: @@ -459,37 +449,37 @@ typedef CMutexReadLock CRecursiveReadBlock; typedef CMutexLock CRecursiveWriteBlock; #define RECURSIVEREADLOCK(cs) \ - CRecursiveReadBlock UNIQUIFY(readblock)(cs, #cs, __FILE__, __LINE__, LockType::RECRUSIVESHARED) + CRecursiveReadBlock UNIQUIFY(readblock)(cs, #cs, __FILE__, __LINE__, LockType::RECURSIVE_SHARED_MUTEX) #define RECURSIVEWRITELOCK(cs) \ - CRecursiveWriteBlock UNIQUIFY(writeblock)(cs, #cs, __FILE__, __LINE__, LockType::RECRUSIVESHARED) -#define RECURSIVEREADLOCK2(cs1, cs2) \ - CRecursiveReadBlock UNIQUIFY(readblock1)(cs1, #cs1, __FILE__, __LINE__, LockType::RECRUSIVESHARED), \ - UNIQUIFY(readblock2)(cs2, #cs2, __FILE__, __LINE__, LockType::RECRUSIVESHARED) + CRecursiveWriteBlock UNIQUIFY(writeblock)(cs, #cs, __FILE__, __LINE__, LockType::RECURSIVE_SHARED_MUTEX) +#define RECURSIVEREADLOCK2(cs1, cs2) \ + CRecursiveReadBlock UNIQUIFY(readblock1)(cs1, #cs1, __FILE__, __LINE__, LockType::RECURSIVE_SHARED_MUTEX), \ + UNIQUIFY(readblock2)(cs2, #cs2, __FILE__, __LINE__, LockType::RECURSIVE_SHARED_MUTEX) #define TRY_READ_LOCK_RECURSIVE(cs, name) \ - CRecursiveReadBlock name(cs, #cs, __FILE__, __LINE__, LockType::RECRUSIVESHARED, true) + CRecursiveReadBlock name(cs, #cs, __FILE__, __LINE__, LockType::RECURSIVE_SHARED_MUTEX, true) typedef CMutexReadLock CReadBlock; typedef CMutexLock CWriteBlock; -#define READLOCK(cs) CReadBlock UNIQUIFY(readblock)(cs, #cs, __FILE__, __LINE__, LockType::SHARED) -#define WRITELOCK(cs) CWriteBlock UNIQUIFY(writeblock)(cs, #cs, __FILE__, __LINE__, LockType::SHARED) -#define READLOCK2(cs1, cs2) \ - CReadBlock UNIQUIFY(readblock1)(cs1, #cs1, __FILE__, __LINE__, LockType::SHARED), \ - UNIQUIFY(readblock2)(cs2, #cs2, __FILE__, __LINE__, LockType::SHARED) -#define TRY_READ_LOCK(cs, name) CReadBlock name(cs, #cs, __FILE__, __LINE__, LockType::SHARED, true) +#define READLOCK(cs) CReadBlock UNIQUIFY(readblock)(cs, #cs, __FILE__, __LINE__, LockType::SHARED_MUTEX) +#define WRITELOCK(cs) CWriteBlock UNIQUIFY(writeblock)(cs, #cs, __FILE__, __LINE__, LockType::SHARED_MUTEX) +#define READLOCK2(cs1, cs2) \ + CReadBlock UNIQUIFY(readblock1)(cs1, #cs1, __FILE__, __LINE__, LockType::SHARED_MUTEX), \ + UNIQUIFY(readblock2)(cs2, #cs2, __FILE__, __LINE__, LockType::SHARED_MUTEX) +#define TRY_READ_LOCK(cs, name) CReadBlock name(cs, #cs, __FILE__, __LINE__, LockType::SHARED_MUTEX, true) typedef CMutexLock CCriticalBlock; -#define LOCK(cs) CCriticalBlock UNIQUIFY(criticalblock)(cs, #cs, __FILE__, __LINE__, LockType::RECURSIVE) -#define LOCK2(cs1, cs2) \ - CCriticalBlock UNIQUIFY(criticalblock1)(cs1, #cs1, __FILE__, __LINE__, LockType::RECURSIVE), \ - UNIQUIFY(criticalblock2)(cs2, #cs2, __FILE__, __LINE__, LockType::RECURSIVE) -#define TRY_LOCK(cs, name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, LockType::RECURSIVE, true) +#define LOCK(cs) CCriticalBlock UNIQUIFY(criticalblock)(cs, #cs, __FILE__, __LINE__, LockType::RECURSIVE_MUTEX) +#define LOCK2(cs1, cs2) \ + CCriticalBlock UNIQUIFY(criticalblock1)(cs1, #cs1, __FILE__, __LINE__, LockType::RECURSIVE_MUTEX), \ + UNIQUIFY(criticalblock2)(cs2, #cs2, __FILE__, __LINE__, LockType::RECURSIVE_MUTEX) +#define TRY_LOCK(cs, name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, LockType::RECURSIVE_MUTEX, true) -#define ENTER_CRITICAL_SECTION(cs) \ - { \ - EnterCritical(#cs, __FILE__, __LINE__, (void *)(&cs), LockType::RECURSIVE, true); \ - (cs).lock(); \ +#define ENTER_CRITICAL_SECTION(cs) \ + { \ + EnterCritical(#cs, __FILE__, __LINE__, (void *)(&cs), LockType::RECURSIVE_MUTEX, OwnershipType::EXCLUSIVE); \ + (cs).lock(); \ } #define LEAVE_CRITICAL_SECTION(cs) \ diff --git a/src/test/deadlock_tests/suite.h b/src/test/deadlock_tests/suite.h index 7a87ba49..2f8ce762 100644 --- a/src/test/deadlock_tests/suite.h +++ b/src/test/deadlock_tests/suite.h @@ -4,17 +4,8 @@ #include "sync.h" -#include "test/test_bitcoin.h" - struct EmptySuite { - EmptySuite() - { - ECC_Start(); - SetupEnvironment(); - SetupNetworking(); - g_logger->fPrintToDebugLog = false; // don't want to write to debug.log file - } - - ~EmptySuite() { ECC_Stop(); } + EmptySuite() {} + ~EmptySuite() {} }; diff --git a/src/test/deadlock_tests/test1-4.cpp b/src/test/deadlock_tests/test1-4.cpp index 76936905..355b14f8 100644 --- a/src/test/deadlock_tests/test1-4.cpp +++ b/src/test/deadlock_tests/test1-4.cpp @@ -19,6 +19,9 @@ BOOST_FIXTURE_TEST_SUITE(self_deadlock_tests, EmptySuite) #ifdef DEBUG_LOCKORDER // this ifdef covers the rest of the file +// shared lock a shared mutex +// then try to exclusive lock the same shared mutex while holding shared lock, +// should self deadlock BOOST_AUTO_TEST_CASE(TEST_1_SM) { CSharedCriticalSection shared_mutex; @@ -26,6 +29,9 @@ BOOST_AUTO_TEST_CASE(TEST_1_SM) BOOST_CHECK_THROW(WRITELOCK(shared_mutex), std::logic_error); } +// shared lock a RSM +// then try to exclusive lock the RSM while holding shared lock, no promotion, +// should self deadlock BOOST_AUTO_TEST_CASE(TEST_1_RSM) { CRecursiveSharedCriticalSection rsm; @@ -33,6 +39,9 @@ BOOST_AUTO_TEST_CASE(TEST_1_RSM) BOOST_CHECK_THROW(RECURSIVEWRITELOCK(rsm), std::logic_error); } +// exclusive lock a shared mutex +// then try to shared lock the same shared mutex while holding the exclusive +// lock, should self deadlock BOOST_AUTO_TEST_CASE(TEST_2) { CSharedCriticalSection shared_mutex; @@ -40,6 +49,9 @@ BOOST_AUTO_TEST_CASE(TEST_2) BOOST_CHECK_THROW(READLOCK(shared_mutex), std::logic_error); } +// shared lock a shared mutex +// then try to shared lock the same shared mutex while holding the original +// shared lock, should self deadlock, no recursion allowed in a shared mutex BOOST_AUTO_TEST_CASE(TEST_3) { CSharedCriticalSection shared_mutex; @@ -47,6 +59,9 @@ BOOST_AUTO_TEST_CASE(TEST_3) BOOST_CHECK_THROW(READLOCK(shared_mutex), std::logic_error); } +// exclusive lock a shared mutex +// then try to exclusive likc the same shared mutex while holding the original +// exclusive lock, should self deadlock, no recursion allowed in a shared mutex BOOST_AUTO_TEST_CASE(TEST_4) { CSharedCriticalSection shared_mutex; diff --git a/src/test/deadlock_tests/test5.cpp b/src/test/deadlock_tests/test5.cpp index f0c28797..27d0860c 100644 --- a/src/test/deadlock_tests/test5.cpp +++ b/src/test/deadlock_tests/test5.cpp @@ -16,35 +16,45 @@ #include #include -BOOST_FIXTURE_TEST_SUITE(test5, EmptySuite) +BOOST_FIXTURE_TEST_SUITE(deadlock_test5, EmptySuite) #ifdef DEBUG_LOCKORDER // this ifdef covers the rest of the file -CSharedCriticalSection mutexA; -CSharedCriticalSection mutexB; +std::atomic done{false}; +std::atomic lock_exceptions{0}; +std::atomic writelocks{0}; -void Thread1() +void TestThread(CSharedCriticalSection *mutexA, CSharedCriticalSection *mutexB) { - WRITELOCK(mutexA); - MilliSleep(100); - READLOCK(mutexB); + WRITELOCK(*mutexA); + writelocks++; + while (writelocks != 2) + ; + try + { + READLOCK(*mutexB); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } -void Thread2() -{ - MilliSleep(50); - WRITELOCK(mutexB); - MilliSleep(100); - BOOST_CHECK_THROW(READLOCK(mutexA), std::logic_error); -} - - BOOST_AUTO_TEST_CASE(TEST_5) { - std::thread thread1(Thread1); - std::thread thread2(Thread2); + CSharedCriticalSection mutexA; + CSharedCriticalSection mutexB; + + std::thread thread1(TestThread, &mutexA, &mutexB); + std::thread thread2(TestThread, &mutexB, &mutexA); + while (!lock_exceptions) + ; + done = true; thread1.join(); thread2.join(); + BOOST_CHECK(lock_exceptions == 1); } #else diff --git a/src/test/deadlock_tests/test6.cpp b/src/test/deadlock_tests/test6.cpp index d849e9b8..a0c26086 100644 --- a/src/test/deadlock_tests/test6.cpp +++ b/src/test/deadlock_tests/test6.cpp @@ -16,35 +16,63 @@ #include #include -BOOST_FIXTURE_TEST_SUITE(test6, EmptySuite) +BOOST_FIXTURE_TEST_SUITE(deadlock_test6, EmptySuite) #ifdef DEBUG_LOCKORDER // this ifdef covers the rest of the file CSharedCriticalSection mutexA; CSharedCriticalSection mutexB; +std::atomic done{false}; +std::atomic lock_exceptions{0}; +std::atomic writelocks{0}; +std::atomic readlocks{0}; void Thread1() { READLOCK(mutexA); - MilliSleep(100); - READLOCK(mutexB); + readlocks++; + while (writelocks != 1) + ; + try + { + READLOCK(mutexB); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } void Thread2() { - MilliSleep(50); + while (readlocks != 1) + ; WRITELOCK(mutexB); - MilliSleep(100); - BOOST_CHECK_THROW(WRITELOCK(mutexA), std::logic_error); + writelocks++; + try + { + WRITELOCK(mutexA); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } - BOOST_AUTO_TEST_CASE(TEST_6) { std::thread thread1(Thread1); std::thread thread2(Thread2); + while (!lock_exceptions) + ; + done = true; thread1.join(); thread2.join(); + BOOST_CHECK(lock_exceptions == 1); } #else diff --git a/src/test/deadlock_tests/test7.cpp b/src/test/deadlock_tests/test7.cpp index 571e3f23..0bd4880e 100644 --- a/src/test/deadlock_tests/test7.cpp +++ b/src/test/deadlock_tests/test7.cpp @@ -16,7 +16,7 @@ #include #include -BOOST_FIXTURE_TEST_SUITE(test7, EmptySuite) +BOOST_FIXTURE_TEST_SUITE(deadlock_test7, EmptySuite) #ifdef DEBUG_LOCKORDER // this ifdef covers the rest of the file @@ -24,40 +24,81 @@ CSharedCriticalSection mutexA; CSharedCriticalSection mutexB; CSharedCriticalSection mutexC; +std::atomic done{false}; +std::atomic lock_exceptions{0}; +std::atomic readlocks{0}; + + void Thread1() { READLOCK(mutexA); // 1 - MilliSleep(150); - WRITELOCK(mutexB); // 4 - MilliSleep(1000); + readlocks++; + while (readlocks != 3) + ; + try + { + WRITELOCK(mutexB); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } void Thread2() { - MilliSleep(50); + while (readlocks != 1) + ; READLOCK(mutexB); // 2 - MilliSleep(150); - WRITELOCK(mutexC); // 5 - MilliSleep(1000); + readlocks++; + while (readlocks != 3) + ; + try + { + WRITELOCK(mutexC); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } void Thread3() { - MilliSleep(100); + while (readlocks != 2) + ; READLOCK(mutexC); // 3 - MilliSleep(150); - BOOST_CHECK_THROW(WRITELOCK(mutexA), std::logic_error); // 6 + readlocks++; + while (readlocks != 3) + ; + try + { + WRITELOCK(mutexA); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } - BOOST_AUTO_TEST_CASE(TEST_7) { std::thread thread1(Thread1); std::thread thread2(Thread2); std::thread thread3(Thread3); + while (!lock_exceptions) + ; + done = true; thread1.join(); thread2.join(); thread3.join(); + BOOST_CHECK(lock_exceptions == 1); } #else diff --git a/src/test/deadlock_tests/test8.cpp b/src/test/deadlock_tests/test8.cpp index 92593c00..821a7569 100644 --- a/src/test/deadlock_tests/test8.cpp +++ b/src/test/deadlock_tests/test8.cpp @@ -16,7 +16,7 @@ #include #include -BOOST_FIXTURE_TEST_SUITE(test8, EmptySuite) +BOOST_FIXTURE_TEST_SUITE(deadlock_test8, EmptySuite) #ifdef DEBUG_LOCKORDER // this ifdef covers the rest of the file @@ -24,40 +24,80 @@ CSharedCriticalSection mutexA; CSharedCriticalSection mutexB; CSharedCriticalSection mutexC; +std::atomic done{false}; +std::atomic lock_exceptions{0}; +std::atomic writelocks{0}; + void Thread1() { WRITELOCK(mutexA); // 1 - MilliSleep(150); - READLOCK(mutexB); // 4 - MilliSleep(1000); + writelocks++; + while (writelocks != 3) + ; + try + { + READLOCK(mutexB); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } void Thread2() { - MilliSleep(50); + while (writelocks != 1) + ; WRITELOCK(mutexB); // 2 - MilliSleep(150); - READLOCK(mutexC); // 5 - MilliSleep(1000); + writelocks++; + while (writelocks != 3) + ; + try + { + READLOCK(mutexC); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } void Thread3() { - MilliSleep(100); - WRITELOCK(mutexC); // 3 - MilliSleep(150); - BOOST_CHECK_THROW(READLOCK(mutexA), std::logic_error); // 6 + while (writelocks != 2) + ; + WRITELOCK(mutexC); + writelocks++; + while (writelocks != 3) + ; + try + { + READLOCK(mutexA); + } + catch (const std::logic_error &) + { + lock_exceptions++; + } + while (!done) + ; } - BOOST_AUTO_TEST_CASE(TEST_8) { std::thread thread1(Thread1); std::thread thread2(Thread2); std::thread thread3(Thread3); + while (!lock_exceptions) + ; + done = true; thread1.join(); thread2.join(); thread3.join(); + BOOST_CHECK(lock_exceptions == 1); } #else diff --git a/src/threaddeadlock.cpp b/src/threaddeadlock.cpp index a88474ba..4255c14f 100644 --- a/src/threaddeadlock.cpp +++ b/src/threaddeadlock.cpp @@ -87,7 +87,7 @@ void potential_deadlock_detected(LockStackEntry now, LockStack &deadlocks, std:: throw std::logic_error("potential deadlock detected"); } -static bool ReadRecursiveCheck(const uint64_t &tid, +static bool RecursiveCheck(const uint64_t &tid, const void *c, uint64_t lastTid, void *lastLock, @@ -100,115 +100,15 @@ static bool ReadRecursiveCheck(const uint64_t &tid, // we are back where we started, infinite loop means there is a deadlock return true; } - // first check if we currently have any exclusive ownerships - bool haveExclusives = false; - size_t selfOtherLockCount = 0; + // first check if we currently have any other ownerships auto self_iter = lockdata.locksheldbythread.find(lastTid); if (self_iter != lockdata.locksheldbythread.end()) { - selfOtherLockCount = self_iter->second.size(); - for (auto &lockStackLock : self_iter->second) + if (self_iter->second.size() == 0) { - if (lockStackLock.second.GetExclusive() == true) - { - haveExclusives = true; - break; - } - } - } - // we cant deadlock if we dont own any other mutexs - if (selfOtherLockCount == 0) - { - return false; - } - // at this point we have at least 1 lock for a mutex somewhere - - // if we do not have any exclusive locks and we arent requesting an exclusive lock... - if (!haveExclusives) - { - // then we cant block - return false; - } - - // check if a thread has an ownership of c - auto writeiter = lockdata.writelocksheld.find(lastLock); - - // NOTE: be careful when adjusting these booleans, the order of the checks is important - bool writeIsEnd = ((writeiter == lockdata.writelocksheld.end()) || writeiter->second.empty()); - if (writeIsEnd) - { - // no exclusive owners, no deadlock possible - return false; - } - - // we have other locks, so check if we have any in common with the holder(s) of the write lock - for (auto &threadId : writeiter->second) - { - if (threadId == lastTid) - { - continue; - } - auto other_iter = lockdata.locksheldbythread.find(threadId); - // we dont need to check empty here, other thread has at least 1 lock otherwise we wouldnt be checking it - if (other_iter->second.size() == 1) - { - // it does not have any locks aside from known exclusive, no deadlock possible - // we can just wait until that exclusive lock is released + // we cant deadlock if we dont own any other mutexs return false; } - // if the other thread has 1+ other locks aside from the known exclusive, check them for matches with our own - // locks - for (auto &lock : other_iter->second) - { - // if they have a lock that is on a lock that we have exclusive ownership for - if (HasAnyOwners(lock.first)) - { - // and their lock is waiting... - if (lock.second.GetWaiting() == true) - { - deadlocks.push_back(lock); - threads.emplace(other_iter->first); - if (other_iter->first == tid && lock.first == c) - { - // we are back where we started and there is a deadlock - return true; - } - if (ReadRecursiveCheck(tid, c, other_iter->first, lock.first, false, deadlocks, threads)) - { - return true; - } - } - // no deadlock, other lock is not waiting, we simply have to wait until they release that lock - } - } - } - return false; -} - -static bool WriteRecursiveCheck(const uint64_t &tid, - const void *c, - uint64_t lastTid, - void *lastLock, - bool firstRun, - LockStack &deadlocks, - std::set &threads) -{ - if (!firstRun && c == lastLock && tid == lastTid) - { - // we are back where we started, infinite loop means there is a deadlock - return true; - } - // first check if we currently have any exclusive ownerships - size_t selfOtherLockCount = 0; - auto self_iter = lockdata.locksheldbythread.find(lastTid); - if (self_iter != lockdata.locksheldbythread.end() && self_iter->second.empty() == false) - { - selfOtherLockCount = self_iter->second.size(); - } - // we cant deadlock if we dont own any other mutexs - if (selfOtherLockCount == 0) - { - return false; } // at this point we have at least 1 lock for a mutex somewhere @@ -238,6 +138,7 @@ static bool WriteRecursiveCheck(const uint64_t &tid, { if (threadId == lastTid) { + // this continue fixes an infinite looping problem continue; } auto other_iter = lockdata.locksheldbythread.find(threadId); @@ -265,7 +166,7 @@ static bool WriteRecursiveCheck(const uint64_t &tid, // we are back where we started and there is a deadlock return true; } - if (WriteRecursiveCheck(tid, c, other_iter->first, lock.first, false, deadlocks, threads)) + if (RecursiveCheck(tid, c, other_iter->first, lock.first, false, deadlocks, threads)) { return true; } @@ -324,16 +225,16 @@ void AddNewLock(LockStackEntry newEntry, const uint64_t &tid) } } -void AddNewWaitingLock(void *c, const uint64_t &tid, bool &isExclusive) +void AddNewHeldLock(void *c, const uint64_t &tid, OwnershipType ownership) { - if (isExclusive) + if (ownership == OwnershipType::EXCLUSIVE) { - auto it = lockdata.writelockswaiting.find(c); - if (it == lockdata.writelockswaiting.end()) + auto it = lockdata.writelocksheld.find(c); + if (it == lockdata.writelocksheld.end()) { std::set holders; holders.emplace(tid); - lockdata.writelockswaiting.emplace(c, holders); + lockdata.writelocksheld.emplace(c, holders); } else { @@ -342,12 +243,12 @@ void AddNewWaitingLock(void *c, const uint64_t &tid, bool &isExclusive) } else // !isExclusive { - auto it = lockdata.readlockswaiting.find(c); - if (it == lockdata.readlockswaiting.end()) + auto it = lockdata.readlocksheld.find(c); + if (it == lockdata.readlocksheld.end()) { std::set holders; holders.emplace(tid); - lockdata.readlockswaiting.emplace(c, holders); + lockdata.readlocksheld.emplace(c, holders); } else { @@ -356,32 +257,20 @@ void AddNewWaitingLock(void *c, const uint64_t &tid, bool &isExclusive) } } -void SetWaitingToHeld(void *c, bool isExclusive) +void AddNewWaitingLock(void *c, const uint64_t &tid, OwnershipType &isExclusive) { - std::lock_guard lock(lockdata.dd_mutex); - - const uint64_t tid = getTid(); - if (isExclusive) + if (isExclusive == OwnershipType::EXCLUSIVE) { auto it = lockdata.writelockswaiting.find(c); if (it == lockdata.writelockswaiting.end()) { - return; + std::set holders; + holders.emplace(tid); + lockdata.writelockswaiting.emplace(c, holders); } else { - it->second.erase(tid); - auto iter = lockdata.writelocksheld.find(c); - if (iter == lockdata.writelocksheld.end()) - { - std::set holders; - holders.emplace(tid); - lockdata.writelocksheld.emplace(c, holders); - } - else - { - iter->second.emplace(tid); - } + it->second.emplace(tid); } } else // !isExclusive @@ -389,25 +278,24 @@ void SetWaitingToHeld(void *c, bool isExclusive) auto it = lockdata.readlockswaiting.find(c); if (it == lockdata.readlockswaiting.end()) { - return; + std::set holders; + holders.emplace(tid); + lockdata.readlockswaiting.emplace(c, holders); } else { - it->second.erase(tid); - auto iter = lockdata.readlocksheld.find(c); - if (iter == lockdata.readlocksheld.end()) - { - std::set holders; - holders.emplace(tid); - lockdata.readlocksheld.emplace(c, holders); - } - else - { - iter->second.emplace(tid); - } + it->second.emplace(tid); } } +} + +void SetWaitingToHeld(void *c, OwnershipType isExclusive) +{ + std::lock_guard lock(lockdata.dd_mutex); + const uint64_t tid = getTid(); + // we do this first so changes are still made for trylocks which dont have + // a waiting lock to be edited auto itheld = lockdata.locksheldbythread.find(tid); if (itheld != lockdata.locksheldbythread.end()) { @@ -420,42 +308,85 @@ void SetWaitingToHeld(void *c, bool isExclusive) } } } + + if (isExclusive == OwnershipType::EXCLUSIVE) + { + auto it = lockdata.writelockswaiting.find(c); + if (it != lockdata.writelockswaiting.end()) + { + it->second.erase(tid); + } + else + { + DbgAssert(!"Missing write lock waiting", ); + } + auto iter = lockdata.writelocksheld.find(c); + if (iter == lockdata.writelocksheld.end()) + { + std::set holders; + holders.emplace(tid); + lockdata.writelocksheld.emplace(c, holders); + } + else + { + iter->second.emplace(tid); + } + } + else // !isExclusive + { + auto it = lockdata.readlockswaiting.find(c); + if (it != lockdata.readlockswaiting.end()) + { + it->second.erase(tid); + } + else + { + DbgAssert(!"Missing read lock waiting", ); + } + auto iter = lockdata.readlocksheld.find(c); + if (iter == lockdata.readlocksheld.end()) + { + std::set holders; + holders.emplace(tid); + lockdata.readlocksheld.emplace(c, holders); + } + else + { + iter->second.emplace(tid); + } + } } // c = the cs // isExclusive = is the current lock exclusive, for a recursive mutex (CCriticalSection) this value should always be // true -void push_lock(void *c, const CLockLocation &locklocation, LockType type, bool isExclusive, bool fTry) +void push_lock(void *c, const CLockLocation &locklocation, LockType locktype, OwnershipType ownership, bool fTry) { std::lock_guard lock(lockdata.dd_mutex); LockStackEntry now = std::make_pair(c, locklocation); + if (fTry) + { + // try locks can not be waiting + now.second.ChangeWaitingToHeld(); + } // tid of the originating request const uint64_t tid = getTid(); // If this is a blocking lock operation, we want to make sure that the locking order between 2 mutexes is consistent // across the program if (fTry) { - // a try lock will either get it, or it wont. so just add it. - // if we dont get the lock this will be undone in destructor AddNewLock(now, tid); - // AddNewWaitingLock(c, tid, isExclusive); + // we need to add a held lock + AddNewHeldLock(c, tid, ownership); return; } // first check lock specific issues - if (type == LockType::SHARED) + if (locktype == LockType::SHARED_MUTEX) { - TEST_1_SM: - TEST_2: - TEST_3: - TEST_4: // shared mutexs cant recursively lock at all, check if we already have a lock on the mutex auto it = lockdata.locksheldbythread.find(tid); - if (it == lockdata.locksheldbythread.end() || it->second.empty()) - { - // intentionally left blank - } - else + if (it != lockdata.locksheldbythread.end() && !it->second.empty()) { for (auto &lockStackLock : it->second) { @@ -467,73 +398,70 @@ void push_lock(void *c, const CLockLocation &locklocation, LockType type, bool i } } } - else if (type == LockType::RECRUSIVESHARED) + else if (locktype == LockType::RECURSIVE_SHARED_MUTEX) { - TEST_1_RSM: // we cannot lock exclusive if we already hold shared, check for this scenario - if (isExclusive) + if (ownership == OwnershipType::EXCLUSIVE) { auto it = lockdata.locksheldbythread.find(tid); - if (it == lockdata.locksheldbythread.end() || it->second.empty()) - { - // intentionally left blank - } - else + if (it != lockdata.locksheldbythread.end() && !it->second.empty()) { for (auto &lockStackLock : it->second) { // if we have a lock and it isnt exclusive and we are attempting to get exclusive // then we will deadlock ourself - if (lockStackLock.first == c && lockStackLock.second.GetExclusive() == false) + if (lockStackLock.first == c && lockStackLock.second.GetOwnershipType() == OwnershipType::SHARED) { self_deadlock_detected(now, lockStackLock); } } } } - else - { - // intentionally left blank - } } - else if (type == LockType::RECURSIVE) + else if (locktype == LockType::RECURSIVE_MUTEX) { // this lock can not deadlock itself // intentionally left blank } + else + { + DbgAssert(!"unsupported lock type", return ); + } // Begin general deadlock checks for all lock types - AddNewLock(now, tid); - AddNewWaitingLock(c, tid, isExclusive); - - // if we have exclusive lock(s) and we arent requesting an exclusive lock... - if (!isExclusive) + bool lockingRecursively = false; + // if lock not shared mutex, check if we are doing a recursive lock + if (locktype != LockType::SHARED_MUTEX) { - TEST_5: - TEST_8: - std::vector deadlocks; - std::set threads; - // then we can only deadlock if we are locking a thread that is currently held in exclusive state by someone - // else - if (ReadRecursiveCheck(tid, c, tid, c, true, deadlocks, threads)) + auto it = lockdata.locksheldbythread.find(tid); + if (it != lockdata.locksheldbythread.end()) { - // we have a deadlock where we are requesting shared ownership on a mutex that is exclusively owned by - // another thread which has either a shared or exlcusive request on a mutex we have exclusive ownership over - potential_deadlock_detected(now, deadlocks, threads); + // check if we have locked this lock before + for (auto &entry : it->second) + { + // if we have locked this lock before... + if (entry.first == c) + { + // then we are locking recursively + lockingRecursively = true; + break; + } + } } } - // if we have exclusive lock(s) and we are requesting another exclusive lock - if (isExclusive) + AddNewLock(now, tid); + if (lockingRecursively == true) { - TEST_6: - TEST_7: - TEST_9: - std::vector deadlocks; - std::set threads; - if (WriteRecursiveCheck(tid, c, tid, c, true, deadlocks, threads)) - { - potential_deadlock_detected(now, deadlocks, threads); - } + // we can skip the deadlock detection checks because it is a recursive lock + // and self deadlocks were checked earlier + return; + } + AddNewWaitingLock(c, tid, ownership); + std::vector deadlocks; + std::set threads; + if (RecursiveCheck(tid, c, tid, c, true, deadlocks, threads)) + { + potential_deadlock_detected(now, deadlocks, threads); } } @@ -548,14 +476,10 @@ void DeleteLock(void *cs) // lockdata was already deleted return; } - if (lockdata.readlockswaiting.count(cs)) - lockdata.readlockswaiting.erase(cs); - if (lockdata.writelockswaiting.count(cs)) - lockdata.writelockswaiting.erase(cs); - if (lockdata.readlocksheld.count(cs)) - lockdata.readlocksheld.erase(cs); - if (lockdata.writelocksheld.count(cs)) - lockdata.writelocksheld.erase(cs); + lockdata.readlockswaiting.erase(cs); + lockdata.writelockswaiting.erase(cs); + lockdata.readlocksheld.erase(cs); + lockdata.writelocksheld.erase(cs); for (auto &iter : lockdata.locksheldbythread) { LockStack newStack; @@ -578,80 +502,56 @@ void _remove_lock_critical_exit(void *cs) return; } uint64_t tid = getTid(); - bool isExclusive = false; - bool fTry = false; auto it = lockdata.locksheldbythread.find(tid); if (it == lockdata.locksheldbythread.end()) { throw std::logic_error("unlocking non-existant lock"); } - else + if (it->second.back().first != cs) { - if (it->second.back().first != cs) - { - LogPrintf("got %s but was not expecting it\n", it->second.back().second.ToString().c_str()); - throw std::logic_error("unlock order inconsistant with lock order"); - } - isExclusive = it->second.back().second.GetExclusive(); - fTry = it->second.back().second.GetTry(); - it->second.pop_back(); + LogPrintf("got %s but was not expecting it\n", it->second.back().second.ToString().c_str()); + throw std::logic_error("unlock order inconsistant with lock order"); } - // remove from the other maps - if (isExclusive) + LockType type = it->second.back().second.GetLockType(); + OwnershipType ownership = it->second.back().second.GetOwnershipType(); + // assuming we unlock in the reverse order of locks, we can simply pop back + it->second.pop_back(); + // if we have no more locks on this critical section... + if (type != LockType::SHARED_MUTEX) { - if (fTry) + for (auto entry : it->second) { - auto iter = lockdata.writelockswaiting.find(cs); - if (iter != lockdata.writelockswaiting.end()) + if (entry.first == cs) { - if (iter->second.empty()) - return; - if (iter->second.count(tid) != 0) - { - iter->second.erase(tid); - } + // we have another lock on this critical section + return; } } - else + } + // remove from the other maps + if (ownership == OwnershipType::EXCLUSIVE) + { + auto iter = lockdata.writelocksheld.find(cs); + if (iter != lockdata.writelocksheld.end()) { - auto iter = lockdata.writelocksheld.find(cs); - if (iter != lockdata.writelocksheld.end()) + if (iter->second.empty()) + return; + if (iter->second.count(tid) != 0) { - if (iter->second.empty()) - return; - if (iter->second.count(tid) != 0) - { - iter->second.erase(tid); - } + iter->second.erase(tid); } } } else // !isExclusive { - if (fTry) - { - auto iter = lockdata.readlockswaiting.find(cs); - if (iter != lockdata.readlockswaiting.end()) - { - if (iter->second.empty()) - return; - if (iter->second.count(tid) != 0) - { - iter->second.erase(tid); - } - } - } - else + auto iter = lockdata.readlocksheld.find(cs); + if (iter != lockdata.readlocksheld.end()) { - auto iter = lockdata.readlocksheld.find(cs); - if (iter != lockdata.readlocksheld.end()) + if (iter->second.empty()) + return; + if (iter->second.count(tid) != 0) { - if (iter->second.empty()) - return; - if (iter->second.count(tid) != 0) - { - iter->second.erase(tid); - } + iter->second.erase(tid); } } } @@ -659,7 +559,6 @@ void _remove_lock_critical_exit(void *cs) void remove_lock_critical_exit(void *cs) { - // assuming we unlock in the reverse order of locks, we can simply pop back std::lock_guard lock(lockdata.dd_mutex); _remove_lock_critical_exit(cs); } diff --git a/src/threaddeadlock.h b/src/threaddeadlock.h index 87c3ab9d..a505bba8 100644 --- a/src/threaddeadlock.h +++ b/src/threaddeadlock.h @@ -18,9 +18,15 @@ enum LockType { - RECURSIVE, // CCriticalSection - SHARED, // CSharedCriticalSection - RECRUSIVESHARED, // CRecursiveSharedCriticalSection + RECURSIVE_MUTEX, // CCriticalSection + SHARED_MUTEX, // CSharedCriticalSection + RECURSIVE_SHARED_MUTEX, // CRecursiveSharedCriticalSection +}; + +enum OwnershipType +{ + SHARED, + EXCLUSIVE }; #ifdef DEBUG_LOCKORDER @@ -43,32 +49,40 @@ inline uint64_t getTid(void) struct CLockLocation { - CLockLocation(const char *pszName, const char *pszFile, int nLine, bool fTryIn, bool fExclusiveIn) + CLockLocation(const char *pszName, + const char *pszFile, + int nLine, + bool fTryIn, + OwnershipType eOwnershipIn, + LockType eLockTypeIn) { mutexName = pszName; sourceFile = pszFile; sourceLine = nLine; fTry = fTryIn; - fExclusive = fExclusiveIn; + eOwnership = eOwnershipIn; + eLockType = eLockTypeIn; fWaiting = true; } std::string ToString() const { return mutexName + " " + sourceFile + ":" + itostr(sourceLine) + (fTry ? " (TRY)" : "") + - (fExclusive ? " (EXCLUSIVE)" : "") + (fWaiting ? " (WAITING)" : ""); + (eOwnership == OwnershipType::EXCLUSIVE ? " (EXCLUSIVE)" : "") + (fWaiting ? " (WAITING)" : ""); } bool GetTry() const { return fTry; } - bool GetExclusive() const { return fExclusive; } + OwnershipType GetOwnershipType() const { return eOwnership; } bool GetWaiting() const { return fWaiting; } void ChangeWaitingToHeld() { fWaiting = false; } + LockType GetLockType() const { return eLockType; } private: bool fTry; std::string mutexName; std::string sourceFile; int sourceLine; - bool fExclusive; // signifies Exclusive Ownership, this is always true for a CCriticalSection + OwnershipType eOwnership; // signifies Exclusive Ownership, this is always true for a CCriticalSection + LockType eLockType; bool fWaiting; // determines if lock is held or is waiting to be held }; @@ -110,18 +124,18 @@ struct LockData extern LockData lockdata; -void push_lock(void *c, const CLockLocation &locklocation, LockType type, bool isExclusive, bool fTry); +void push_lock(void *c, const CLockLocation &locklocation, LockType type, OwnershipType isExclusive, bool fTry); void DeleteLock(void *cs); void _remove_lock_critical_exit(void *cs); void remove_lock_critical_exit(void *cs); std::string LocksHeld(); -void SetWaitingToHeld(void *c, bool isExclusive); +void SetWaitingToHeld(void *c, OwnershipType isExclusive); bool HasAnyOwners(void *c); std::string _LocksHeld(); #else // NOT DEBUG_LOCKORDER -static inline void SetWaitingToHeld(void *c, bool isExclusive) {} +static inline void SetWaitingToHeld(void *c, OwnershipType isExclusive) {} #endif // END DEBUG_LOCKORDER diff --git a/src/util/util.h b/src/util/util.h index 1685cff6..cef771b2 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -15,6 +15,7 @@ #include "compat.h" #include "fs.h" #include "tinyformat.h" +#include "util/logger.h" #include "util/utiltime.h" #include @@ -34,6 +35,27 @@ // that is unique in this file. #define UNIQUIFY(pfx) UNIQUE1(pfx, __LINE__) +#ifdef DEBUG_ASSERTION +/// If DEBUG_ASSERTION is enabled this asserts when the predicate is false. +// If DEBUG_ASSERTION is disabled and the predicate is false, it executes the execInRelease statements. +// Typically, the programmer will error out -- return false, raise an exception, etc in the execInRelease code. +// DO NOT USE break or continue inside the DbgAssert! +#define DbgAssert(pred, execInRelease) assert(pred) +#else +#define DbgStringify(x) #x +#define DbgStringifyIntLiteral(x) DbgStringify(x) +#define DbgAssert(pred, execInRelease) \ + do \ + { \ + if (!(pred)) \ + { \ + g_logger->LogPrintStr(std::string( \ + __FILE__ "(" DbgStringifyIntLiteral(__LINE__) "): Debug Assertion failed: \"" #pred "\"\n")); \ + execInRelease; \ + } \ + } while (0) +#endif + extern bool fDaemon; extern bool fServer; extern std::string strMiscWarning; From 758cc32d12a2abcb9a2598ffdfbfa95b314b3fb2 Mon Sep 17 00:00:00 2001 From: Griffith <8345264+Greg-Griffith@users.noreply.github.com> Date: Wed, 28 Aug 2019 01:18:56 -0700 Subject: [PATCH 3/8] nodestate accessor cs now uses enter critical section function to lock (#239) * nodestate accessor cs now uses enter critical section function to lock * shorten the verifydb.py test so it doesnt timeout so often * temporarily disabled getchaintips.py test --- qa/pull-tester/rpc-tests.py | 2 +- qa/rpc-tests/getchaintips.py | 1 + qa/rpc-tests/verifydb.py | 47 +++++++++++++++--------------------- src/net/nodestate.h | 11 ++++++--- 4 files changed, 29 insertions(+), 32 deletions(-) diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 11dfc782..21e5200d 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -182,7 +182,7 @@ def option_passed(option_without_dashes): 'multi_rpc', # 'fundrawtransaction', ??? 'reindex', - 'getchaintips', + Disabled('getchaintips', "issue with syncpoints getting stuck"), 'httpbasics', 'keypool', 'listtransactions', diff --git a/qa/rpc-tests/getchaintips.py b/qa/rpc-tests/getchaintips.py index 8f4d0bf7..4977f1d8 100755 --- a/qa/rpc-tests/getchaintips.py +++ b/qa/rpc-tests/getchaintips.py @@ -26,6 +26,7 @@ def run_test (self): # Split the network and build two chains of different lengths. self.split_network () self.nodes[0].generate(10) + self.sync_all () self.nodes[2].generate(20) self.sync_all () diff --git a/qa/rpc-tests/verifydb.py b/qa/rpc-tests/verifydb.py index c68a6e29..d9a0e3a6 100755 --- a/qa/rpc-tests/verifydb.py +++ b/qa/rpc-tests/verifydb.py @@ -51,56 +51,50 @@ def run_test (self): # get to 100 blocks for i in range (68): self.nodes[1].generate(1) - j = randint(1, 5) - for k in range(j): - self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) - if i % 10 == 0: + if i % 15 == 0: + j = randint(1, 5) + for k in range(j): + self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) self.sync_all() self.sync_all() #get to 500 blocks for i in range (100): self.nodes[0].generate(1) - j = randint(1, 5) - for k in range(j): - self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1) - if i % 50 == 0: + if i % 15 == 0: + j = randint(1, 5) + for k in range(j): + self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1) self.sync_all() self.sync_all() for i in range (100): self.nodes[1].generate(1) - j = randint(1, 5) - for k in range(j): - self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) - if i % 50 == 0: + if i % 25 == 0: self.sync_all() self.sync_all() for i in range (100): self.nodes[0].generate(1) - j = randint(1, 5) - for k in range(j): - self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1) - if i % 50 == 0: + if i % 25 == 0: self.sync_all() self.sync_all() for i in range (100): self.nodes[1].generate(1) - j = randint(1, 5) - for k in range(j): - self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) - if i % 50 == 0: + if i % 25 == 0: + j = randint(1, 5) + for k in range(j): + self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) self.sync_all() self.sync_all() #pos blocks for i in range (100): self.nodes[1].generatepos(1) - j = randint(1, 5) - for k in range(j): - self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) - if i % 10 == 0: + if i % 25 == 0: + j = randint(1, 5) + for k in range(j): + self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) self.sync_all() self.sync_all() @@ -111,10 +105,7 @@ def run_test (self): # start the nodes again with high db checks self.node_args = [['-checklevel=4', '-checkblocks=0'], ['-checklevel=4', '-checkblocks=0']] self.nodes = start_nodes(2, self.options.tmpdir, self.node_args) - - #generate more blocks - self.nodes[0].generate(500) - + #stop the nodes stop_nodes(self.nodes) wait_bitcoinds() diff --git a/src/net/nodestate.h b/src/net/nodestate.h index cd7c491c..a093f48a 100644 --- a/src/net/nodestate.h +++ b/src/net/nodestate.h @@ -125,11 +125,16 @@ class CNodeStateAccessor CNodeState *obj; public: - CNodeStateAccessor(CCriticalSection *_cs, CNodeState *_obj) : cs(_cs), obj(_obj) { cs->lock(); } + CNodeStateAccessor(CCriticalSection *_cs, CNodeState *_obj) : cs(_cs), obj(_obj) + { + EnterCritical("CNodeStateAccessor.cs", __FILE__, __LINE__, (void *)(&cs), LockType::RECURSIVE_MUTEX, + OwnershipType::EXCLUSIVE); + } CNodeStateAccessor(CNodesStateManager &ns, const NodeId id) { cs = &ns.cs; - cs->lock(); + EnterCritical("CNodeStateAccessor.cs", __FILE__, __LINE__, (void *)(&cs), LockType::RECURSIVE_MUTEX, + OwnershipType::EXCLUSIVE); obj = ns._GetNodeState(id); } @@ -142,7 +147,7 @@ class CNodeStateAccessor ~CNodeStateAccessor() { obj = nullptr; - cs->unlock(); + LeaveCritical(&cs); } }; From 646e31dfc74ff798d4e46bb270894c567676d9c1 Mon Sep 17 00:00:00 2001 From: Griffith <8345264+Greg-Griffith@users.noreply.github.com> Date: Fri, 30 Aug 2019 01:19:27 -0700 Subject: [PATCH 4/8] Cleanup build warnings (#240) * remove unused time variables * fix ifdef for removing checksum checking to include grabbing the hash * remove unused fupdated variable * make encode/decode functions non static functions * remove unused height variable * remove unused deltapri variable * cast chain height to unsigned int for comparison * remove unused variable blocksize * fix shadow warning * update prevector [port from BU] --- src/net/messages.cpp | 26 ++++++------ src/prevector.h | 67 +++++++++++------------------- src/processblock.cpp | 10 ----- src/rpc/rpcdump.cpp | 6 +-- src/rpc/rpcrawtransaction.cpp | 2 +- src/test/checkblock_tests.cpp | 3 -- src/test/net_tests.cpp | 1 - src/test/policyestimator_tests.cpp | 1 - src/test/prevector_tests.cpp | 4 +- src/wallet/wallet.cpp | 3 -- 10 files changed, 43 insertions(+), 80 deletions(-) diff --git a/src/net/messages.cpp b/src/net/messages.cpp index 9bec63e5..1e0aad6e 100644 --- a/src/net/messages.cpp +++ b/src/net/messages.cpp @@ -2107,20 +2107,20 @@ bool ProcessMessages(CNode *pfrom, CConnman &connman) // Checksum CDataStream &vRecv = msg.vRecv; - const uint256 &hash = msg.GetMessageHash(); - -#if 0 // Do not waste my CPU calculating a checksum provided by an untrusted node - // TCP already has one that is sufficient for network errors. The checksum does not increase security since - // an attacker can always provide a bad message with a good checksum. - // This code is removed by comment so it is clear that it is a deliberate omission. - if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) - { - LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__, SanitizeString(strCommand), - nMessageSize, HexStr(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE), - HexStr(hdr.pchChecksum, hdr.pchChecksum + CMessageHeader::CHECKSUM_SIZE)); - return fMoreWork; - } +#if 0 + const uint256 &hash = msg.GetMessageHash(); + // Do not waste my CPU calculating a checksum provided by an untrusted node + // TCP already has one that is sufficient for network errors. The checksum does not increase security since + // an attacker can always provide a bad message with a good checksum. + // This code is removed by comment so it is clear that it is a deliberate omission. + if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) + { + LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__, SanitizeString(strCommand), + nMessageSize, HexStr(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE), + HexStr(hdr.pchChecksum, hdr.pchChecksum + CMessageHeader::CHECKSUM_SIZE)); + return fMoreWork; + } #endif // Process message diff --git a/src/prevector.h b/src/prevector.h index 65f65431..321ae8a7 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -1,8 +1,8 @@ // This file is part of the Eccoin project -// Copyright (c) 2015-2016 The Bitcoin Core developers +// Copyright (c) 2015-2018 The Bitcoin Core developers +// Copyright (c) 2015-2019 The Bitcoin Unlimited developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. - #ifndef _BITCOIN_PREVECTOR_H_ #define _BITCOIN_PREVECTOR_H_ @@ -10,32 +10,26 @@ #include #include -#include -#include -#include -#include - #include #pragma pack(push, 1) -/** - * Implements a drop-in replacement for std::vector which stores up to N - * elements directly (without heap allocation). The types Size and Diff are used - * to store element counts, and can be any unsigned + signed type. +/** Implements a drop-in replacement for std::vector which stores up to N + * elements directly (without heap allocation). The types Size and Diff are + * used to store element counts, and can be any unsigned + signed type. * - * Storage layout is either: - * - Direct allocation: - * - Size _size: the number of used elements (between 0 and N) - * - T direct[N]: an array of N elements of type T - * (only the first _size are initialized). - * - Indirect allocation: - * - Size _size: the number of used elements plus N + 1 - * - Size capacity: the number of allocated elements - * - T* indirect: a pointer to an array of capacity elements of type T - * (only the first _size are initialized). + * Storage layout is either: + * - Direct allocation: + * - Size _size: the number of used elements (between 0 and N) + * - T direct[N]: an array of N elements of type T + * (only the first _size are initialized). + * - Indirect allocation: + * - Size _size: the number of used elements plus N + 1 + * - Size capacity: the number of allocated elements + * - T* indirect: a pointer to an array of capacity elements of type T + * (only the first _size are initialized). * - * The data type T must be movable by memmove/realloc(). Once we switch to C++, - * move constructors can be used instead. + * The data type T must be movable by memmove/realloc(). Once we switch to C++, + * move constructors can be used instead. */ template class prevector @@ -59,7 +53,6 @@ class prevector typedef T *pointer; typedef T &reference; typedef std::random_access_iterator_tag iterator_category; - iterator() : ptr(nullptr) {} iterator(T *ptr_) : ptr(ptr_) {} T &operator*() const { return *ptr; } T *operator->() const { return ptr; } @@ -118,7 +111,6 @@ class prevector typedef T *pointer; typedef T &reference; typedef std::bidirectional_iterator_tag iterator_category; - reverse_iterator() : ptr(nullptr) {} reverse_iterator(T *ptr_) : ptr(ptr_) {} T &operator*() { return *ptr; } const T &operator*() const { return *ptr; } @@ -160,7 +152,6 @@ class prevector typedef const T *pointer; typedef const T &reference; typedef std::random_access_iterator_tag iterator_category; - const_iterator() : ptr(nullptr) {} const_iterator(const T *ptr_) : ptr(ptr_) {} const_iterator(iterator x) : ptr(&(*x)) {} const T &operator*() const { return *ptr; } @@ -219,8 +210,7 @@ class prevector typedef const T *pointer; typedef const T &reference; typedef std::bidirectional_iterator_tag iterator_category; - const_reverse_iterator() : ptr(nullptr) {} - const_reverse_iterator(T *ptr_) : ptr(ptr_) {} + const_reverse_iterator(const T *ptr_) : ptr(ptr_) {} const_reverse_iterator(reverse_iterator x) : ptr(&(*x)) {} const T &operator*() const { return *ptr; } const T *operator->() const { return ptr; } @@ -284,19 +274,12 @@ class prevector { if (!is_direct()) { - // FIXME: Because malloc/realloc here won't call new_handler if - // allocation fails, assert success. These should instead use an - // allocator or new/delete so that handlers are called as - // necessary, but performance would be slightly degraded by - // doing so. _union.indirect = static_cast(realloc(_union.indirect, ((size_t)sizeof(T)) * new_capacity)); - assert(_union.indirect); _union.capacity = new_capacity; } else { char *new_indirect = static_cast(malloc(((size_t)sizeof(T)) * new_capacity)); - assert(new_indirect); T *src = direct_ptr(0); T *dst = reinterpret_cast(new_indirect); memcpy(dst, src, size() * sizeof(T)); @@ -378,7 +361,6 @@ class prevector } } - prevector(prevector &&other) : _size(0) { swap(other); } prevector &operator=(const prevector &other) { if (&other == this) @@ -397,12 +379,6 @@ class prevector return *this; } - prevector &operator=(prevector &&other) - { - swap(other); - return *this; - } - size_type size() const { return is_direct() ? _size : _size - N - 1; } bool empty() const { return size() == 0; } iterator begin() { return iterator(item_ptr(0)); } @@ -507,6 +483,12 @@ class prevector iterator erase(iterator pos) { return erase(pos, pos + 1); } iterator erase(iterator first, iterator last) { + // Erase is not allowed to the change the object's capacity. That means + // that when starting with an indirectly allocated prevector with + // size and capacity > N, the result may be a still indirectly allocated + // prevector with size <= N and capacity > N. A shrink_to_fit() call is + // necessary to switch to the (more efficient) directly allocated + // representation (with capacity N and size <= N). iterator p = first; char *endp = (char *)&(*end()); while (p != last) @@ -619,5 +601,4 @@ class prevector }; #pragma pack(pop) - #endif diff --git a/src/processblock.cpp b/src/processblock.cpp index e6f7ec7b..adf533ff 100644 --- a/src/processblock.cpp +++ b/src/processblock.cpp @@ -79,7 +79,6 @@ bool DisconnectTip(CValidationState &state, const Consensus::Params &consensusPa return AbortNode(state, "Failed to read block"); } // Apply the block atomically to the chain state. - int64_t nStart = GetTimeMicros(); { CCoinsViewCache view(pcoinsTip.get()); if (DisconnectBlock(block, pindexDelete, view) != DISCONNECT_OK) @@ -861,13 +860,6 @@ void ThreadScriptCheck() } void InterruptScriptCheck() { scriptcheckqueue.Stop(); } -static int64_t nTimeCheck = 0; -static int64_t nTimeForks = 0; -static int64_t nTimeVerify = 0; -static int64_t nTimeConnect = 0; -static int64_t nTimeIndex = 0; -static int64_t nTimeCallbacks = 0; - bool ConnectBlock(const CBlock &block, CValidationState &state, CBlockIndex *pindex, @@ -877,8 +869,6 @@ bool ConnectBlock(const CBlock &block, const CNetworkTemplate &chainparams = pnetMan->getActivePaymentNetwork(); AssertLockHeld(cs_main); - int64_t nTimeStart = GetTimeMicros(); - if (pindex->GetBlockHash() != chainparams.GetConsensus().hashGenesisBlock) { // need cs_mapBlockIndex to update the index diff --git a/src/rpc/rpcdump.cpp b/src/rpc/rpcdump.cpp index 6387e32e..7adcb9c0 100644 --- a/src/rpc/rpcdump.cpp +++ b/src/rpc/rpcdump.cpp @@ -28,8 +28,8 @@ void EnsureWalletIsUnlocked(); bool EnsureWalletIsAvailable(bool avoidException); -std::string static EncodeDumpTime(int64_t nTime) { return DateTimeStrFormat("%Y-%m-%dT%H:%M:%SZ", nTime); } -int64_t static DecodeDumpTime(const std::string &str) +std::string EncodeDumpTime(int64_t nTime) { return DateTimeStrFormat("%Y-%m-%dT%H:%M:%SZ", nTime); } +int64_t DecodeDumpTime(const std::string &str) { static const boost::posix_time::ptime epoch = boost::posix_time::from_time_t(0); static const std::locale loc(std::locale::classic(), new boost::posix_time::time_input_facet("%Y-%m-%dT%H:%M:%SZ")); @@ -42,7 +42,7 @@ int64_t static DecodeDumpTime(const std::string &str) return (ptime - epoch).total_seconds(); } -std::string static EncodeDumpString(const std::string &str) +std::string EncodeDumpString(const std::string &str) { std::stringstream ret; for (unsigned char c : str) diff --git a/src/rpc/rpcrawtransaction.cpp b/src/rpc/rpcrawtransaction.cpp index 346379e5..e865c5bc 100644 --- a/src/rpc/rpcrawtransaction.cpp +++ b/src/rpc/rpcrawtransaction.cpp @@ -263,7 +263,7 @@ UniValue gettxoutproof(const UniValue ¶ms, bool fHelp) LOCK(cs_main); CoinAccessor coin(*pcoinsTip, oneTxid); if (coin && !coin->IsSpent() && coin->nHeight > 0 && - coin->nHeight <= pnetMan->getChainActive()->chainActive.Height()) + coin->nHeight <= (unsigned int)pnetMan->getChainActive()->chainActive.Height()) { pblockindex = pnetMan->getChainActive()->chainActive[coin->nHeight]; } diff --git a/src/test/checkblock_tests.cpp b/src/test/checkblock_tests.cpp index a0ab7517..44d350e5 100644 --- a/src/test/checkblock_tests.cpp +++ b/src/test/checkblock_tests.cpp @@ -47,9 +47,6 @@ BOOST_AUTO_TEST_CASE(TestBlock) if (read_block("testblock.dat", testblock)) { CValidationState state; - - uint64_t blockSize = ::GetSerializeSize(testblock, SER_NETWORK, PROTOCOL_VERSION); // 53298 B for test.dat - BOOST_CHECK_MESSAGE(CheckBlock(testblock, state, false, false), "Basic CheckBlock failed"); BOOST_CHECK_MESSAGE(CheckBlock(testblock, state, false, false), "Basic CheckBlock failed"); } diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 0959698d..ea77eed9 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -159,7 +159,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) { SOCKET hSocket = INVALID_SOCKET; NodeId id = 0; - int height = 0; in_addr ipv4Addr; ipv4Addr.s_addr = 0xa0b0c001; diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index dc280c9d..6993e878 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -21,7 +21,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) CAmount basefee(2000); double basepri = 1e6; CAmount deltaFee(100); - double deltaPri = 5e5; std::vector feeV[2]; std::vector priV[2]; diff --git a/src/test/prevector_tests.cpp b/src/test/prevector_tests.cpp index 951bf85a..c4a84f5f 100644 --- a/src/test/prevector_tests.cpp +++ b/src/test/prevector_tests.cpp @@ -219,9 +219,9 @@ BOOST_AUTO_TEST_CASE(PrevectorTestInt) { int values[4]; int num = 1 + (insecure_rand() % 4); - for (int i = 0; i < num; i++) + for (int k = 0; k < num; k++) { - values[i] = insecure_rand(); + values[k] = insecure_rand(); } test.insert_range(insecure_rand() % (test.size() + 1), values, values + num); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ed37faf2..d7db2e2b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2538,11 +2538,8 @@ DBErrors CWallet::ZapWalletTx(std::vector &vWtx) bool CWallet::SetAddressBook(const CTxDestination &address, const std::string &strType) { - bool fUpdated = false; { LOCK(cs_wallet); // mapAddressBook - std::map::iterator mi = mapAddressBook.find(address); - fUpdated = mi != mapAddressBook.end(); if (!strType.empty()) /* update purpose only if requested */ mapAddressBook[address].type = strType; } From 541edd2d6d29e601c0573aad97f567417c0cfd68 Mon Sep 17 00:00:00 2001 From: Griffith Date: Sun, 19 May 2019 03:03:43 +0900 Subject: [PATCH 5/8] [data-routing-mvp] add global identifier to node config. pass global identifier to peers (#178) * adjust network service versioning for easier use via macros * remove alert message definition, add RREQ, RREP, and RERR * add random key generation for aodv routing to connman * add protocol version for network services, increment procotol version * add nsversion and nsverack message definitions * pass network service information as part of startup --- src/net/messages.cpp | 23 +++++++++++++++++++++++ src/net/net.cpp | 11 +++++++++++ src/net/net.h | 7 +++++++ src/net/protocol.cpp | 12 +++++++++--- src/net/protocol.h | 41 ++++++++++++++++++++++++++++++++++------- src/version.h | 20 ++++++++++++++------ 6 files changed, 98 insertions(+), 16 deletions(-) diff --git a/src/net/messages.cpp b/src/net/messages.cpp index 1e0aad6e..ce498d58 100644 --- a/src/net/messages.cpp +++ b/src/net/messages.cpp @@ -1158,11 +1158,18 @@ bool static ProcessMessage(CNode *pfrom, CNodeStateAccessor state(nodestateman, pfrom->GetId()); state->fCurrentlyConnected = true; } + // Tell our peer we prefer to receive headers rather than inv's // We send this to non-NODE NETWORK peers as well, because even // non-NODE NETWORK peers can announce blocks (such as pruning // nodes) connman.PushMessage(pfrom, NetMsgType::SENDHEADERS); + + if (pfrom->nVersion >= NETWORK_SERVICE_PROTOCOL_VERSION) + { + connman.PushMessage(pfrom, NetMsgType::NSVERSION, NETWORK_SERVICE_VERSION); + } + pfrom->fSuccessfullyConnected = true; } @@ -1181,6 +1188,22 @@ bool static ProcessMessage(CNode *pfrom, } return false; } + else if (strCommand == NetMsgType::NSVERSION) + { + uint64_t netservice = 0; + vRecv >> netservice; + pfrom->nNetworkServiceVersion = netservice; + if (netservice >= MIN_AODV_VERSION) + { + connman.PushMessage(pfrom, NetMsgType::NSVERACK, g_connman->GetRoutingKey()); + } + } + + else if (strCommand == NetMsgType::NSVERACK) + { + CPubKey peerPubKey; + vRecv >> peerPubKey; + } else if (strCommand == NetMsgType::ADDR) { diff --git a/src/net/net.cpp b/src/net/net.cpp index 556d2954..1944f698 100644 --- a/src/net/net.cpp +++ b/src/net/net.cpp @@ -2450,6 +2450,14 @@ bool CConnman::Start(std::string &strNodeError) } nMaxOutboundTimeframe = MAX_UPLOAD_TIMEFRAME; + SetBestHeight(pnetMan->getChainActive()->chainActive.Height()); + + LogPrintf("Generating random routing id..."); + + pub_routing_key.MakeNewKey(true); + pub_routing_id = pub_routing_key.GetPubKey(); + assert(pub_routing_key.VerifyPubKey(pub_routing_id)); + LogPrintf("Loading addresses..."); // Load addresses from peers.dat int64_t nStart = GetTimeMillis(); @@ -2920,6 +2928,7 @@ CNode::CNode(NodeId idIn, fPauseRecv = false; fPauseSend = false; nProcessQueueSize = 0; + nNetworkServiceVersion = 0; for (const std::string &msg : getAllNetMessageTypes()) { @@ -3030,3 +3039,5 @@ uint64_t CConnman::CalculateKeyedNetGroup(const CAddress &ad) const return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(&vchNetGroup[0], vchNetGroup.size()).Finalize(); } + +CPubKey CConnman::GetRoutingKey() const { return pub_routing_id; } diff --git a/src/net/net.h b/src/net/net.h index 3f4bc4bd..9e039ff0 100644 --- a/src/net/net.h +++ b/src/net/net.h @@ -302,6 +302,8 @@ class CNode // Whether a ping is requested. std::atomic fPingQueued; + std::atomic nNetworkServiceVersion; + CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, @@ -620,6 +622,8 @@ class CConnman unsigned int GetReceiveFloodSize() const; + CPubKey GetRoutingKey() const; + private: struct ListenSocket { @@ -725,6 +729,9 @@ class CConnman std::atomic interruptNet; thread_group netThreads; + + CKey pub_routing_key; + CPubKey pub_routing_id; }; extern std::unique_ptr g_connman; diff --git a/src/net/protocol.cpp b/src/net/protocol.cpp index f966e216..f53dafa1 100644 --- a/src/net/protocol.cpp +++ b/src/net/protocol.cpp @@ -31,13 +31,17 @@ const char *BLOCK = "block"; const char *GETADDR = "getaddr"; const char *PING = "ping"; const char *PONG = "pong"; -const char *ALERT = "alert"; const char *NOTFOUND = "notfound"; const char *FILTERLOAD = "filterload"; const char *FILTERADD = "filteradd"; const char *FILTERCLEAR = "filterclear"; const char *REJECT = "reject"; const char *SENDHEADERS = "sendheaders"; +const char *RREQ = "rreq"; +const char *RREP = "rrep"; +const char *RERR = "rerr"; +const char *NSVERSION = "nsversion"; +const char *NSVERACK = "nsverack"; }; static const char *ppszTypeName[] = { @@ -51,9 +55,11 @@ static const char *ppszTypeName[] = { */ const static std::string allNetMessageTypes[] = {NetMsgType::VERSION, NetMsgType::VERACK, NetMsgType::ADDR, NetMsgType::INV, NetMsgType::GETDATA, NetMsgType::MERKLEBLOCK, NetMsgType::GETHEADERS, NetMsgType::TX, - NetMsgType::HEADERS, NetMsgType::BLOCK, NetMsgType::GETADDR, NetMsgType::PING, NetMsgType::PONG, NetMsgType::ALERT, + NetMsgType::HEADERS, NetMsgType::BLOCK, NetMsgType::GETADDR, NetMsgType::PING, NetMsgType::PONG, NetMsgType::NOTFOUND, NetMsgType::FILTERLOAD, NetMsgType::FILTERADD, NetMsgType::FILTERCLEAR, NetMsgType::REJECT, - NetMsgType::SENDHEADERS}; + NetMsgType::SENDHEADERS, NetMsgType::RREQ, NetMsgType::RREP, NetMsgType::RERR, NetMsgType::NSVERSION, + NetMsgType::NSVERACK}; + const static std::vector allNetMessageTypesVec(allNetMessageTypes, allNetMessageTypes + ARRAYLEN(allNetMessageTypes)); diff --git a/src/net/protocol.h b/src/net/protocol.h index b7f26121..ec073284 100644 --- a/src/net/protocol.h +++ b/src/net/protocol.h @@ -154,13 +154,6 @@ extern const char *PING; * @see https://bitcoin.org/en/developer-reference#pong */ extern const char *PONG; -/** - * The alert message warns nodes of problems that may affect them or the rest - * of the network. - * @since protocol version 311. - * @see https://bitcoin.org/en/developer-reference#alert - */ -extern const char *ALERT; /** * The notfound message is a reply to a getdata message which requested an * object the receiving node does not have available for relay. @@ -209,6 +202,40 @@ extern const char *REJECT; * @see https://bitcoin.org/en/developer-reference#sendheaders */ extern const char *SENDHEADERS; +/** + * A message broadcast to all peers when searching for a route to a peer + * with a specific public routing id which should be specified in this + * message. + * @Heavily based on RREQ network message in AODV routing + * @see https://www.ietf.org/rfc/rfc3561.txt + */ +extern const char *RREQ; // ROUTE_REQUEST +/** + * A message sent back to the originator of the RREQ notifying them that + * we either are the node they are looking for or know of a route + * to the node with the public routing id they are looking for. + * @Heavily based on RREP network message in AODV routing + * @see https://www.ietf.org/rfc/rfc3561.txt + */ +extern const char *RREP; // ROUTE_REPLY +/** + * Broadcast to peers in the event that: a route it was known to have is + * now broken, it gets data for a route it does not have, or it recieved + * a RERR from a neighbor and needs for forward the linkage break down + * to its peers to notify them as well. + * @Heavily based on RERR network message in AODV routing + * @see https://www.ietf.org/rfc/rfc3561.txt + */ +extern const char *RERR; // ROUTE_ERROR +/** + * The nsversion message provides information the network services supported + * by the transmitting node to the receiving node. + */ +extern const char *NSVERSION; +/** + * The nsverack message acknowledges a previously-received nsversion message, + */ +extern const char *NSVERACK; }; /* Get a vector of all valid message types (see above) */ diff --git a/src/version.h b/src/version.h index 33c3bb78..945b6c6f 100644 --- a/src/version.h +++ b/src/version.h @@ -21,16 +21,24 @@ static const int MIN_PROTO_VERSION = 60037; //! "filter*" commands are disabled without NODE_BLOOM after and including this version static const int NO_BLOOM_VERSION = 60034; +static const int NETWORK_SERVICE_PROTOCOL_VERSION = 60040; + /** * Versioning for network services */ -#define NETWORK_SERVICE_VERSION_MAJOR 0 -#define NETWORK_SERVICE_VERSION_MINOR 1 -#define NETWORK_SERVICE_VERSION_REVISION 0 +#define MAJOR(major) 1000000 * major +#define MINOR(minor) 1000 * minor +#define REVISION(revision) 1 * revision + +// version of the network service code +static const uint64_t NETWORK_SERVICE_VERSION = MAJOR(0) + MINOR(1) + REVISION(0); + +// AODV routing for public routing ids was introduced in this network service version +static const uint64_t MIN_AODV_VERSION = MAJOR(0) + MINOR(1) + REVISION(0); + +// This nodes AODV protocol version, this is unrelated to the network service version +static const uint64_t AODV_PROTOCOL_VERSION = MAJOR(0) + MINOR(1) + REVISION(1); -// version of the service transaction resolution code -static const int NETWORK_SERVICE_VERSION = - 10000 * NETWORK_SERVICE_VERSION_MAJOR + 100 * NETWORK_SERVICE_VERSION_MINOR + 1 * NETWORK_SERVICE_VERSION_REVISION; #endif // BITCOIN_VERSION_H From 5ad644053fbd571ca12377c09533dbdcec6e6c59 Mon Sep 17 00:00:00 2001 From: Griffith <8345264+Greg-Griffith@users.noreply.github.com> Date: Wed, 12 Jun 2019 23:30:57 +0900 Subject: [PATCH 6/8] Implement aodv routing (#187) * implement aodv * add aodv python test * add missing LIBRSM include in bench compilation * remove internal lock check, rsm is guarenteed to not deadlock itself --- qa/rpc-tests/aodv.py | 116 ++++++++++++++++++++++++ src/Makefile.am | 3 + src/net/aodv.cpp | 203 ++++++++++++++++++++++++++++++++++++++++++ src/net/aodv.h | 88 ++++++++++++++++++ src/net/messages.cpp | 81 +++++++++++++++++ src/net/net.cpp | 2 - src/net/net.h | 30 ++++++- src/pubkey.h | 11 +++ src/rpc/rpcaodv.cpp | 171 +++++++++++++++++++++++++++++++++++ src/rpc/rpcserver.cpp | 5 +- src/rpc/rpcserver.h | 10 ++- 11 files changed, 710 insertions(+), 10 deletions(-) create mode 100755 qa/rpc-tests/aodv.py create mode 100644 src/net/aodv.cpp create mode 100644 src/net/aodv.h create mode 100644 src/rpc/rpcaodv.cpp diff --git a/qa/rpc-tests/aodv.py b/qa/rpc-tests/aodv.py new file mode 100755 index 00000000..7ff4c73c --- /dev/null +++ b/qa/rpc-tests/aodv.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +# Copyright (c) 2015-2018 The Bitcoin Unlimited developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +import test_framework.loginit +# This is a template to make creating new QA tests easy. +# You can also use this template to quickly start and connect a few regtest nodes. + +import time +import sys +if sys.version_info[0] < 3: + raise "Use Python 3" +import logging + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import * + +class AodvTest (BitcoinTestFramework): + + def setup_chain(self,bitcoinConfDict=None, wallets=None): + print("Initializing test directory "+self.options.tmpdir) + # pick this one to start from the cached 4 node 100 blocks mined configuration + # initialize_chain(self.options.tmpdir) + # pick this one to start at 0 mined blocks + initialize_chain_clean(self.options.tmpdir, 6, bitcoinConfDict, wallets) + # Number of nodes to initialize ----------> ^ + + def setup_network(self, split=False): + self.nodes = start_nodes(6, self.options.tmpdir) + connect_nodes_bi(self.nodes,0,1) + connect_nodes_bi(self.nodes,1,2) + connect_nodes_bi(self.nodes,2,3) + connect_nodes_bi(self.nodes,3,4) + connect_nodes_bi(self.nodes,4,5) + self.is_network_split=False + self.sync_all() + + def run_test (self): + self.sync_blocks() + + for x in range(0, 1): + self.nodes[0].generate(1); + self.sync_blocks() + + assert_not_equal(self.nodes[0].getconnectioncount(), 3) + print(self.nodes[0].getconnectioncount()) + print(self.nodes[0].getaodvtable()) + assert_not_equal(self.nodes[1].getconnectioncount(), 3) + print(self.nodes[1].getconnectioncount()) + assert_not_equal(self.nodes[2].getconnectioncount(), 3) + print(self.nodes[2].getconnectioncount()) + assert_not_equal(self.nodes[3].getconnectioncount(), 3) + print(self.nodes[3].getconnectioncount()) + + key0 = self.nodes[0].getroutingpubkey() + key1 = self.nodes[1].getroutingpubkey() + key2 = self.nodes[2].getroutingpubkey() + key3 = self.nodes[3].getroutingpubkey() + key4 = self.nodes[4].getroutingpubkey() + key5 = self.nodes[5].getroutingpubkey() + + self.nodes[0].findroute(key0) + time.sleep(1) + assert_equal(self.nodes[0].haveroute(key0), True) + self.nodes[0].findroute(key1) + time.sleep(1) + assert_equal(self.nodes[0].haveroute(key1), True) + self.nodes[0].findroute(key2) + time.sleep(1) + assert_equal(self.nodes[0].haveroute(key2), True) + self.nodes[0].findroute(key3) + time.sleep(1) + assert_equal(self.nodes[0].haveroute(key3), True) + self.nodes[0].findroute(key4) + time.sleep(1) + assert_equal(self.nodes[0].haveroute(key4), True) + self.nodes[0].findroute(key5) + time.sleep(1) + assert_equal(self.nodes[0].haveroute(key5), True) + + +if __name__ == '__main__': + AodvTest ().main () + +# Create a convenient function for an interactive python debugging session +def Test(): + t = AodvTest() + bitcoinConf = { + "debug": ["net", "blk", "thin", "mempool", "req", "bench", "evict"], + "blockprioritysize": 2000000 # we don't want any transactions rejected due to insufficient fees... + } + + + flags = [] + # you may want these additional flags: + # flags.append("--nocleanup") + # flags.append("--noshutdown") + + # Execution is much faster if a ramdisk is used, so use it if one exists in a typical location + if os.path.isdir("/ramdisk/test"): + flags.append("--tmppfx=/ramdisk/test") + + # Out-of-source builds are awkward to start because they need an additional flag + # automatically add this flag during testing for common out-of-source locations + here = os.path.dirname(os.path.abspath(__file__)) + if not os.path.exists(os.path.abspath(here + "/../../src/eccoind")): + dbg = os.path.abspath(here + "/../../debug/src/eccoind") + rel = os.path.abspath(here + "/../../release/src/eccoind") + if os.path.exists(dbg): + print("Running from the debug directory (%s)" % dbg) + flags.append("--srcdir=%s" % os.path.dirname(dbg)) + elif os.path.exists(rel): + print("Running from the release directory (%s)" % rel) + flags.append("--srcdir=%s" % os.path.dirname(rel)) + + t.main(flags, bitcoinConf, None) diff --git a/src/Makefile.am b/src/Makefile.am index 28498c41..d07c5094 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -95,6 +95,7 @@ BITCOIN_CORE_H = \ merkleblock.h \ net/addrdb.h \ net/addrman.h \ + net/aodv.h \ net/messages.h \ net/net.h \ net/netaddress.h \ @@ -196,6 +197,7 @@ libbitcoin_server_a_SOURCES = \ merkleblock.cpp \ net/net.cpp \ net/netaddress.cpp \ + net/aodv.cpp \ policy/fees.cpp \ policy/policy.cpp \ pow.cpp \ @@ -279,6 +281,7 @@ libbitcoin_server_a_SOURCES = \ crypto/scrypt.cpp \ wallet/crypter.cpp \ wallet/db.cpp \ + rpc/rpcaodv.cpp \ rpc/rpcdump.cpp \ rpc/rpcwallet.cpp \ wallet/wallet.cpp \ diff --git a/src/net/aodv.cpp b/src/net/aodv.cpp new file mode 100644 index 00000000..87962ae3 --- /dev/null +++ b/src/net/aodv.cpp @@ -0,0 +1,203 @@ +// This file is part of the Eccoin project +// Copyright (c) 2019 The Eccoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "aodv.h" +#include "net.h" + + +CAodvRouteTable g_aodvtable; + + +void CAodvRouteTable::GetRoutingTables(std::map &IdKey, std::map &KeyId) +{ + RECURSIVEREADLOCK(cs_aodv); + IdKey = mapIdKey; + KeyId = mapKeyId; + return; +} + +bool CAodvRouteTable::HaveKeyEntry(const CPubKey &key) +{ + RECURSIVEREADLOCK(cs_aodv); + return (mapKeyId.find(key) != mapKeyId.end()); +} + +bool CAodvRouteTable::HaveIdEntry(const NodeId &node) +{ + RECURSIVEREADLOCK(cs_aodv); + return (mapIdKey.find(node) != mapIdKey.end()); +} + +void CAodvRouteTable::AddPeerKeyId(const CPubKey &key, const NodeId &node, bool update) +{ + RECURSIVEWRITELOCK(cs_aodv); + bool haveId = HaveIdEntry(node); + bool haveKey = HaveKeyEntry(key); + if (!update) + { + if (haveKey || haveId) + { + return; + } + } + if (haveId) + { + mapIdKey[node] = key; + } + else + { + mapIdKey.emplace(node, key); + } + + if (haveKey) + { + mapKeyId[key] = node; + } + else + { + mapKeyId.emplace(key, node); + } +} + +bool CAodvRouteTable::HaveKeyRoute(const CPubKey &key) +{ + RECURSIVEREADLOCK(cs_aodv); + return (mapRoutesByPubKey.find(key) != mapRoutesByPubKey.end()); +} + +bool CAodvRouteTable::HaveIdRoute(const NodeId &node) +{ + RECURSIVEREADLOCK(cs_aodv); + return (mapRoutesByPeerId.find(node) != mapRoutesByPeerId.end()); +} + +void CAodvRouteTable::AddRouteKeyId(const CPubKey &key, const NodeId &node) +{ + RECURSIVEWRITELOCK(cs_aodv); + bool haveId = HaveIdRoute(node); + bool haveKey = HaveKeyRoute(key); + if (haveId) + { + std::set keys = mapRoutesByPeerId[node]; + keys.emplace(key); + mapRoutesByPeerId[node] = keys; + } + else + { + std::set keys; + keys.emplace(key); + mapRoutesByPeerId.emplace(node, keys); + } + + if (haveKey) + { + mapRoutesByPubKey[key] = node; + } + else + { + mapRoutesByPubKey.emplace(key, node); + } +} + +bool CAodvRouteTable::GetKeyNode(const CPubKey &key, NodeId &result) +{ + RECURSIVEREADLOCK(cs_aodv); + if (!HaveKeyEntry(key)) + { + return false; + } + result = mapKeyId[key]; + return true; +} + +bool CAodvRouteTable::GetNodeKey(const NodeId &node, CPubKey &result) +{ + RECURSIVEREADLOCK(cs_aodv); + if (!HaveIdEntry(node)) + { + return false; + } + result = mapIdKey[node]; + return true; +} + +void CAodvRouteTable::AddNewRequestTimeData(const uint64_t &nonce) +{ + RECURSIVEWRITELOCK(cs_aodv); + mapRequestTime.emplace(nonce, GetTimeMillis()); +} + +bool CAodvRouteTable::IsOldRequest(const uint64_t &nonce) +{ + RECURSIVEREADLOCK(cs_aodv); + return (mapRequestSource.find(nonce) != mapRequestSource.end()); +} + +void CAodvRouteTable::AddNewRequestSource(const uint64_t &nonce, const CPubKey &source) +{ + RECURSIVEWRITELOCK(cs_aodv); + if (source.IsValid() == false) + { + return; + } + if (mapRequestSource.find(nonce) != mapRequestSource.end()) + { + return; + } + mapRequestSource.emplace(nonce, source); +} + +bool CAodvRouteTable::GetRequestSource(const uint64_t &nonce, CPubKey &source) +{ + RECURSIVEREADLOCK(cs_aodv); + auto iter = mapRequestSource.find(nonce); + if (iter == mapRequestSource.end()) + { + return false; + } + source = iter->second; + return true; +} + +void RecordRequestOrigin(const uint64_t &nonce, const CPubKey &source) +{ + g_aodvtable.AddNewRequestSource(nonce, source); +} + +bool GetRequestOrigin(const uint64_t &nonce, CPubKey &source) +{ + return g_aodvtable.GetRequestSource(nonce, source); +} + +void _RequestRouteToPeer(CConnman &connman, const CPubKey &source, const uint64_t &nonce, const CPubKey &searchKey) +{ + if (g_aodvtable.IsOldRequest(nonce)) + { + return; + } + g_aodvtable.AddNewRequestTimeData(nonce); + connman.PushMessageAll(source, NetMsgType::RREQ, nonce, searchKey); +} + +void RequestRouteToPeer(CConnman &connman, const CPubKey &source, const uint64_t &nonce, const CPubKey &searchKey) +{ + _RequestRouteToPeer(connman, source, nonce, searchKey); +} + +void RequestRouteToPeer(CConnman *connman, const CPubKey &source, const uint64_t &nonce, const CPubKey &searchKey) +{ + _RequestRouteToPeer(*connman, source, nonce, searchKey); +} + +void RecordRouteToPeer(const CPubKey &searchKey, const NodeId &node) +{ + g_aodvtable.AddRouteKeyId(searchKey, node); +} + +void AddResponseToQueue(CPubKey source, uint64_t nonce, CPubKey pubkey) +{ + RECURSIVEWRITELOCK(g_aodvtable.cs_aodv); + g_aodvtable.responseQueue.emplace(RREQRESPONSE(nonce, source, pubkey, true)); +} diff --git a/src/net/aodv.h b/src/net/aodv.h new file mode 100644 index 00000000..dc675de7 --- /dev/null +++ b/src/net/aodv.h @@ -0,0 +1,88 @@ +// This file is part of the Eccoin project +// Copyright (c) 2019 The Eccoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include + +#include "net.h" +#include "pubkey.h" +#include "sync.h" + +// read https://www.ietf.org/rfc/rfc3561.txt for details on aodv +// the difference with this system is instead of trying to find +// and ip address, we are trying to find a peer with a specific +// pubkey + +struct RREQRESPONSE +{ + uint64_t nonce; + CPubKey source; + CPubKey pubkey; + bool found; + + RREQRESPONSE(uint64_t _nonce, CPubKey _source, CPubKey _pubkey, bool _found) + { + nonce = _nonce; + source = _source; + pubkey = _pubkey; + found = _found; + } + + friend inline bool operator < (const RREQRESPONSE &a, const RREQRESPONSE &b) + { + return (a.nonce < b.nonce); + } + +}; + +class CAodvRouteTable +{ +private: + std::map mapIdKey; + std::map mapKeyId; + + std::map >mapRoutesByPeerId; + std::mapmapRoutesByPubKey; + + // nonce, GetTimeMillis + std::mapmapRequestTime; + // nonce, nodeId + std::mapmapRequestSource; +public: + mutable CRecursiveSharedCriticalSection cs_aodv; + std::setresponseQueue; + + +public: + CAodvRouteTable() + { + mapIdKey.clear(); + mapKeyId.clear(); + } + void GetRoutingTables(std::map &IdKey, std::map &KeyId); + bool HaveKeyEntry(const CPubKey &key); + bool HaveIdEntry(const NodeId &node); + void AddPeerKeyId(const CPubKey &key, const NodeId &node, bool update = false); + bool HaveKeyRoute(const CPubKey &key); + bool HaveIdRoute(const NodeId &node); + void AddRouteKeyId(const CPubKey &key, const NodeId &node); + bool GetKeyNode(const CPubKey &key, NodeId &result); + bool GetNodeKey(const NodeId &node, CPubKey &result); + void AddNewRequestTimeData(const uint64_t &nonce); + bool IsOldRequest(const uint64_t &nonce); + void AddNewRequestSource(const uint64_t &nonce, const CPubKey &nodeId); + bool GetRequestSource(const uint64_t &nonce, CPubKey &source); +}; + +void RecordRequestOrigin(const uint64_t &nonce, const CPubKey &source); +bool GetRequestOrigin(const uint64_t &nonce, CPubKey &source); +void RequestRouteToPeer(CConnman &connman, const CPubKey &source, const uint64_t &nonce, const CPubKey &searchKey); +void RequestRouteToPeer(CConnman *connman, const CPubKey &source, const uint64_t &nonce, const CPubKey &searchKey); +void RecordRouteToPeer(const CPubKey &searchKey, const NodeId &node); + +void AddResponseToQueue(CPubKey source, uint64_t nonce, CPubKey pubkey); + +extern CAodvRouteTable g_aodvtable; diff --git a/src/net/messages.cpp b/src/net/messages.cpp index ce498d58..8e92ecb8 100644 --- a/src/net/messages.cpp +++ b/src/net/messages.cpp @@ -8,6 +8,7 @@ #include "net/messages.h" +#include "aodv.h" #include "args.h" #include "blockstorage/blockstorage.h" #include "chain/chain.h" @@ -1203,6 +1204,9 @@ bool static ProcessMessage(CNode *pfrom, { CPubKey peerPubKey; vRecv >> peerPubKey; + pfrom->routing_id = peerPubKey; + g_aodvtable.AddPeerKeyId(peerPubKey, pfrom->GetId(), true); + g_aodvtable.AddPeerKeyId(peerPubKey, pfrom->GetId(), true); } else if (strCommand == NetMsgType::ADDR) @@ -2049,6 +2053,62 @@ bool static ProcessMessage(CNode *pfrom, } } + else if (strCommand == NetMsgType::RREQ) + { + /** + * A peer has requested a route to a specific id. we need to: + * keep track of who sent the request, make sure it was unique, + * check our routing table to see if we know of it, if we do respond with a RREP, + * otherwise forward this request to our peers except the one that sent it. + * + * if we have the node we will respond with our preffered privacy settings. + */ + uint64_t nonce = 0; + CPubKey searchKey; + vRecv >> nonce; + vRecv >> searchKey; + bool peerKnown = g_aodvtable.HaveKeyEntry(searchKey) || connman.GetRoutingKey() == searchKey; + if (peerKnown) + { + connman.PushMessage(pfrom, NetMsgType::RREP, nonce, searchKey, peerKnown); + } + else + { + RequestRouteToPeer(connman, pfrom->routing_id, nonce, searchKey); + RecordRequestOrigin(nonce, pfrom->routing_id); + } + } + + else if (strCommand == NetMsgType::RREP) + { + uint64_t nonce = 0; + CPubKey searchKey; + bool found; + vRecv >> nonce; + vRecv >> searchKey; + vRecv >> found; + /** + * we got a response from someone who knows of or has what we are looking for. + * if they allow direct connections try to form one with them to reduce network load. + * if they dont, try to establish a more or less direct route to the general area of peers + * to proceed with an application specific conversation + */ + if (found) + { + RecordRouteToPeer(searchKey, pfrom->GetId()); + CPubKey source; + if (GetRequestOrigin(nonce, source)) + { + AddResponseToQueue(source, nonce, searchKey); + } + } + } + + else if (strCommand == NetMsgType::RERR) + { + // intentionally left blank + } + else { // Ignore unknown commands for extensibility @@ -2646,5 +2706,26 @@ bool SendMessages(CNode *pto, CConnman &connman) { connman.PushMessage(pto, NetMsgType::GETDATA, vGetData); } + + std::set responseQueue_copy; + { + RECURSIVEREADLOCK(g_aodvtable.cs_aodv); + for (const auto &response : g_aodvtable.responseQueue) + { + if (response.source == pto->routing_id) + { + connman.PushMessage(pto, NetMsgType::RREP, response.nonce, response.pubkey, response.found); + } + else + { + responseQueue_copy.emplace(response); + } + } + } + { + RECURSIVEWRITELOCK(g_aodvtable.cs_aodv); + g_aodvtable.responseQueue = responseQueue_copy; + } + return true; } diff --git a/src/net/net.cpp b/src/net/net.cpp index 1944f698..07c05763 100644 --- a/src/net/net.cpp +++ b/src/net/net.cpp @@ -2450,8 +2450,6 @@ bool CConnman::Start(std::string &strNodeError) } nMaxOutboundTimeframe = MAX_UPLOAD_TIMEFRAME; - SetBestHeight(pnetMan->getChainActive()->chainActive.Height()); - LogPrintf("Generating random routing id..."); pub_routing_key.MakeNewKey(true); diff --git a/src/net/net.h b/src/net/net.h index 9e039ff0..37718a35 100644 --- a/src/net/net.h +++ b/src/net/net.h @@ -248,6 +248,7 @@ class CNode const uint64_t nKeyedNetGroup; std::atomic_bool fPauseRecv; std::atomic_bool fPauseSend; + CPubKey routing_id; protected: mapMsgCmdSize mapSendBytesPerMsgCmd; @@ -497,6 +498,32 @@ class CConnman } } + template + void PushMessageAll(const std::string sCommand, Args &&... args) + { + LOCK(cs_vNodes); + for (auto &&node : vNodes) + { + if (NodeFullyConnected(node)) + { + PushMessage(node, sCommand, std::forward(args)...); + } + } + } + + template + void PushMessageAll(const CPubKey &source, const std::string sCommand, Args &&... args) + { + LOCK(cs_vNodes); + for (auto &&node : vNodes) + { + if (NodeFullyConnected(node) && node->routing_id != source) + { + PushMessage(node, sCommand, std::forward(args)...); + } + } + } + template void ForEachNode(Callable &&func) { @@ -614,9 +641,6 @@ class CConnman uint64_t GetTotalBytesRecv(); uint64_t GetTotalBytesSent(); - void SetBestHeight(int height); - int GetBestHeight() const; - /** Get a unique deterministic randomizer. */ CSipHasher GetDeterministicRandomizer(uint64_t id) const; diff --git a/src/pubkey.h b/src/pubkey.h index 66de56c2..5a8497d2 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -12,6 +12,7 @@ #include "crypto/hash.h" #include "serialize.h" #include "uint256.h" +#include "util/utilstrencodings.h" #include #include @@ -61,6 +62,16 @@ class CPubKey return ch; } + std::string Raw64Encoded() const + { + std::vector ch; + for (unsigned int i = 0; i < size(); i++) + { + ch.emplace_back(vch[i]); + } + return EncodeBase64(&ch[0], size()); + } + //! Initialize a public key using begin/end iterators to byte data. template void Set(const T pbegin, const T pend) diff --git a/src/rpc/rpcaodv.cpp b/src/rpc/rpcaodv.cpp new file mode 100644 index 00000000..9fe39487 --- /dev/null +++ b/src/rpc/rpcaodv.cpp @@ -0,0 +1,171 @@ +#include "net/aodv.h" +#include "rpcserver.h" +#include "util/logger.h" +#include "util/utilstrencodings.h" +#include + +extern CCriticalSection cs_main; + +UniValue getaodvtable(const UniValue ¶ms, bool fHelp) +{ + if (fHelp || params.size() != 0) + throw std::runtime_error( + "getaodvtable\n" + "\nreturns the aodv routing table.\n" + "\nResult:\n" + "{\n" + " \"mapidkey\" : {\n" + " \"NodeId : Key,\"\n" + " ...\n" + " },\n" + " \"mapkeyid\" : {\n" + " \"Key : NodeId,\"\n" + " ...\n" + " }\n" + "}\n" + "\nExamples:\n" + + HelpExampleCli("getaodvtable", "") + + HelpExampleRpc("getaodvtable", "") + ); + std::map IdKey; + std::map KeyId; + g_aodvtable.GetRoutingTables(IdKey, KeyId); + UniValue obj(UniValue::VOBJ); + UniValue IdKeyObj(UniValue::VOBJ); + for (auto &entry : IdKey) + { + IdKeyObj.push_back(Pair(std::to_string(entry.first), entry.second.Raw64Encoded())); + } + UniValue KeyIdObj(UniValue::VOBJ); + for (auto &entry : KeyId) + { + KeyIdObj.push_back(Pair(entry.first.Raw64Encoded(), entry.second)); + } + obj.push_back(Pair("mapidkey", IdKeyObj)); + obj.push_back(Pair("mapkeyid", KeyIdObj)); + return obj; +} + + +UniValue getaodvkeyentry(const UniValue ¶ms, bool fHelp) +{ + if (fHelp || params.size() != 1) + throw std::runtime_error( + "getaodvkeyentry \"keyhash\" \n" + "\nChecks AODV routing table for a key with a hash that matches keyhash, returns its NodeId.\n" + "\nArguments:\n" + "1. \"keyhash\" (string, required) The hash of the key of the desired entry\n" + "\nNote: This call is fairly expensive due to number of hashes being done.\n" + "\nExamples:\n"+ + HelpExampleCli("getaodvkeyentry", "\"keyhash\"") + + HelpExampleRpc("getaodvkeyentry", "\"keyhash\"")); + + UniValue obj(UniValue::VOBJ); + obj.push_back(Pair("Error", "this rpc call is currently disabled")); + return obj; +} + +UniValue getaodvidentry(const UniValue ¶ms, bool fHelp) +{ + if (fHelp || params.size() != 1) + throw std::runtime_error( + "getaodvidentry \"nodeid\" \n" + "\nChecks AODV routing table for the desired nodeid, returns its keys hash.\n" + "\nArguments:\n" + "1. \"nodeid\" (number, required) The nodeid of the desired entry\n" + "\nExamples:\n"+ + HelpExampleCli("getaodvidentry", "12") + + HelpExampleRpc("getaodvidentry", "32")); + + UniValue obj(UniValue::VOBJ); + obj.push_back(Pair("Error", "this rpc call is currently disabled")); + return obj; +} + +UniValue getroutingpubkey(const UniValue ¶ms, bool fHelp) +{ + if (fHelp || params.size() != 0) + throw std::runtime_error( + "getroutingpubkey\n" + "\nreturns the routing public key used by this node.\n" + "\nResult:\n" + "\"Key\" (string)\n" + "\nExamples:\n" + + HelpExampleCli("getroutingpubkey", "") + + HelpExampleRpc("getroutingpubkey", "") + ); + + LOCK(cs_main); + if (!g_connman) + { + throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + } + + return g_connman->GetRoutingKey().Raw64Encoded(); +} + + +UniValue findroute(const UniValue ¶ms, bool fHelp) +{ + if (fHelp || params.size() != 1) + throw std::runtime_error( + "findroute\n" + "\nattempts to find a route to the node with the given pub key.\n" + "\nResult:\n" + "\"None\n" + "\nExamples:\n" + + HelpExampleCli("findroute", "1139d39a984a0ff431c467f738d534c36824401a4735850561f7ac64e4d49f5b") + + HelpExampleRpc("findroute", "1139d39a984a0ff431c467f738d534c36824401a4735850561f7ac64e4d49f5b") + ); + + LOCK(cs_main); + if (!g_connman) + { + throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + } + + bool fInvalid = false; + std::vector vPubKey = DecodeBase64(params[0].get_str().c_str(), &fInvalid); + if (fInvalid) + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Malformed pubkey base64 encoding"); + if (g_aodvtable.HaveKeyEntry(CPubKey(vPubKey.begin(), vPubKey.end()))) + { + return NullUniValue; + } + uint64_t nonce = 0; + GetRandBytes((uint8_t *)&nonce, sizeof(nonce)); + CPubKey key; + RequestRouteToPeer(g_connman.get(), key, nonce, CPubKey(vPubKey.begin(), vPubKey.end())); + LogPrintf("done sending route requests \n"); + return NullUniValue; +} + +UniValue haveroute(const UniValue ¶ms, bool fHelp) +{ + if (fHelp || params.size() != 1) + throw std::runtime_error( + "haveroute\n" + "\nchecks if a route to the node with the given pub key is known.\n" + "\nResult:\n" + "\"true/false\n" + "\nExamples:\n" + + HelpExampleCli("haveroute", "1139d39a984a0ff431c467f738d534c36824401a4735850561f7ac64e4d49f5b") + + HelpExampleRpc("haveroute", "1139d39a984a0ff431c467f738d534c36824401a4735850561f7ac64e4d49f5b") + ); + + LOCK(cs_main); + if (!g_connman) + { + throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + } + LogPrintf("have route was given pubkey %s \n", params[0].get_str()); + bool fInvalid = false; + std::vector vPubKey = DecodeBase64(params[0].get_str().c_str(), &fInvalid); + if (fInvalid) + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Malformed pubkey base64 encoding"); + if (g_aodvtable.HaveKeyEntry(CPubKey(vPubKey.begin(), vPubKey.end()))) + { + return true; + } + return g_aodvtable.HaveKeyRoute(CPubKey(vPubKey.begin(), vPubKey.end())); +} diff --git a/src/rpc/rpcserver.cpp b/src/rpc/rpcserver.cpp index 23583b13..94e3f668 100644 --- a/src/rpc/rpcserver.cpp +++ b/src/rpc/rpcserver.cpp @@ -294,7 +294,10 @@ static const CRPCCommand vRPCCommands[] = { {"network", "getconnectioncount", &getconnectioncount, true}, {"network", "getnettotals", &getnettotals, true}, {"network", "getpeerinfo", &getpeerinfo, true}, {"network", "ping", &ping, true}, {"network", "setban", &setban, true}, {"network", "listbanned", &listbanned, true}, - {"network", "clearbanned", &clearbanned, true}, + {"network", "clearbanned", &clearbanned, true}, {"network", "getaodvtable", &getaodvtable, true}, + {"network", "getaodvkeyentry", &getaodvkeyentry, true}, {"network", "getaodvidentry", &getaodvidentry, true}, + {"network", "getroutingpubkey", &getroutingpubkey, true}, {"network", "findroute", &findroute, true}, + {"network", "haveroute", &haveroute, true}, /* Block chain and UTXO */ {"blockchain", "getblockchaininfo", &getblockchaininfo, true}, diff --git a/src/rpc/rpcserver.h b/src/rpc/rpcserver.h index 15d80acc..3c126bc7 100644 --- a/src/rpc/rpcserver.h +++ b/src/rpc/rpcserver.h @@ -263,10 +263,12 @@ extern UniValue getchaintips(const UniValue ¶ms, bool fHelp); extern UniValue invalidateblock(const UniValue ¶ms, bool fHelp); extern UniValue reconsiderblock(const UniValue ¶ms, bool fHelp); -extern UniValue getansrecord(const UniValue ¶ms, bool fHelp); -extern UniValue registerans(const UniValue ¶ms, bool fHelp); -extern UniValue renewans(const UniValue ¶ms, bool fHelp); -extern UniValue sendtoans(const UniValue ¶ms, bool fHelp); +extern UniValue getaodvtable(const UniValue ¶ms, bool fHelp); // in rpcaodv.cpp +extern UniValue getaodvkeyentry(const UniValue ¶ms, bool fHelp); +extern UniValue getaodvidentry(const UniValue ¶ms, bool fHelp); +extern UniValue getroutingpubkey(const UniValue ¶ms, bool fHelp); +extern UniValue findroute(const UniValue ¶ms, bool fHelp); +extern UniValue haveroute(const UniValue ¶ms, bool fHelp); bool StartRPC(); From e4fbac90f51ecdf18f5cb12a3af8ee014fc56666 Mon Sep 17 00:00:00 2001 From: Griffith <8345264+Greg-Griffith@users.noreply.github.com> Date: Sat, 7 Sep 2019 09:50:02 -0700 Subject: [PATCH 7/8] guard beta features with fBeta, add -beta param to toggle fBeta (#241) * guard beta features with fBeta, add -beta param to toggle fBeta * add beta param to aodv qa test --- qa/rpc-tests/aodv.py | 5 ++-- src/Makefile.am | 2 ++ src/beta.cpp | 13 +++++++++ src/beta.h | 17 +++++++++++ src/init.cpp | 3 ++ src/net/aodv.h | 5 ++++ src/net/messages.cpp | 68 ++++++++++++++++++++++++++++---------------- src/net/net.cpp | 10 +++++-- src/rpc/rpcaodv.cpp | 36 +++++++++++++++++++++++ 9 files changed, 129 insertions(+), 30 deletions(-) create mode 100644 src/beta.cpp create mode 100644 src/beta.h diff --git a/qa/rpc-tests/aodv.py b/qa/rpc-tests/aodv.py index 7ff4c73c..0ca4059b 100755 --- a/qa/rpc-tests/aodv.py +++ b/qa/rpc-tests/aodv.py @@ -80,14 +80,15 @@ def run_test (self): if __name__ == '__main__': - AodvTest ().main () + AodvTest().main(bitcoinConfDict={"beta": 1}) # Create a convenient function for an interactive python debugging session def Test(): t = AodvTest() bitcoinConf = { "debug": ["net", "blk", "thin", "mempool", "req", "bench", "evict"], - "blockprioritysize": 2000000 # we don't want any transactions rejected due to insufficient fees... + "blockprioritysize": 2000000, # we don't want any transactions rejected due to insufficient fees... + "beta": 1 } diff --git a/src/Makefile.am b/src/Makefile.am index d07c5094..2e63d636 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -52,6 +52,7 @@ BITCOIN_CORE_H = \ args.h \ arith_uint256.h \ base58.h \ + beta.h \ blockgeneration/blockgeneration.h \ blockgeneration/compare.h \ blockgeneration/miner.h \ @@ -178,6 +179,7 @@ libbitcoin_server_a_SOURCES = \ globals.cpp \ threaddeadlock.cpp \ sync.cpp \ + beta.cpp \ net/addrdb.cpp \ net/addrman.cpp \ blockgeneration/blockgeneration.cpp \ diff --git a/src/beta.cpp b/src/beta.cpp new file mode 100644 index 00000000..9a3eaea6 --- /dev/null +++ b/src/beta.cpp @@ -0,0 +1,13 @@ +// This file is part of the Eccoin project +// Copyright (c) 2019 The Eccoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "beta.h" + +std::atomic fBeta{DEFAULT_BETA_ENABLED}; + +bool IsBetaEnabled() +{ + return fBeta.load(); +} diff --git a/src/beta.h b/src/beta.h new file mode 100644 index 00000000..e1ff1a65 --- /dev/null +++ b/src/beta.h @@ -0,0 +1,17 @@ +// This file is part of the Eccoin project +// Copyright (c) 2019 The Eccoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef ECCOIN_BETA_H +#define ECCOIN_BETA_H + +#include + +static const bool DEFAULT_BETA_ENABLED = false; + +extern std::atomic fBeta; + +bool IsBetaEnabled(); + +#endif diff --git a/src/init.cpp b/src/init.cpp index f3d32df8..52f642c3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -9,6 +9,7 @@ #include "amount.h" #include "args.h" +#include "beta.h" #include "blockgeneration/blockgeneration.h" #include "blockstorage/blockstorage.h" #include "chain/chain.h" @@ -974,6 +975,8 @@ bool AppInit2(thread_group &threadGroup) // ********************************************************* Step 3: parameter-to-internal-flags + fBeta.store(gArgs.IsArgSet("-beta")); + g_logger->fDebug = gArgs.IsArgSet("-debug"); // Special-case: if -debug=0/-nodebug is set, turn off debugging messages const std::vector &categories = gArgs.GetArgs("-debug"); diff --git a/src/net/aodv.h b/src/net/aodv.h index dc675de7..b33eca90 100644 --- a/src/net/aodv.h +++ b/src/net/aodv.h @@ -3,6 +3,9 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#ifndef ECCOIN_AODV_H +#define ECCOIN_AODV_H + #include #include #include @@ -86,3 +89,5 @@ void RecordRouteToPeer(const CPubKey &searchKey, const NodeId &node); void AddResponseToQueue(CPubKey source, uint64_t nonce, CPubKey pubkey); extern CAodvRouteTable g_aodvtable; + +#endif diff --git a/src/net/messages.cpp b/src/net/messages.cpp index 8e92ecb8..6446ad47 100644 --- a/src/net/messages.cpp +++ b/src/net/messages.cpp @@ -10,6 +10,7 @@ #include "aodv.h" #include "args.h" +#include "beta.h" #include "blockstorage/blockstorage.h" #include "chain/chain.h" #include "chain/tx.h" @@ -1166,7 +1167,7 @@ bool static ProcessMessage(CNode *pfrom, // nodes) connman.PushMessage(pfrom, NetMsgType::SENDHEADERS); - if (pfrom->nVersion >= NETWORK_SERVICE_PROTOCOL_VERSION) + if (pfrom->nVersion >= NETWORK_SERVICE_PROTOCOL_VERSION && IsBetaEnabled()) { connman.PushMessage(pfrom, NetMsgType::NSVERSION, NETWORK_SERVICE_VERSION); } @@ -1191,22 +1192,28 @@ bool static ProcessMessage(CNode *pfrom, } else if (strCommand == NetMsgType::NSVERSION) { - uint64_t netservice = 0; - vRecv >> netservice; - pfrom->nNetworkServiceVersion = netservice; - if (netservice >= MIN_AODV_VERSION) + if (IsBetaEnabled()) { - connman.PushMessage(pfrom, NetMsgType::NSVERACK, g_connman->GetRoutingKey()); + uint64_t netservice = 0; + vRecv >> netservice; + pfrom->nNetworkServiceVersion = netservice; + if (netservice >= MIN_AODV_VERSION) + { + connman.PushMessage(pfrom, NetMsgType::NSVERACK, g_connman->GetRoutingKey()); + } } } else if (strCommand == NetMsgType::NSVERACK) { - CPubKey peerPubKey; - vRecv >> peerPubKey; - pfrom->routing_id = peerPubKey; - g_aodvtable.AddPeerKeyId(peerPubKey, pfrom->GetId(), true); - g_aodvtable.AddPeerKeyId(peerPubKey, pfrom->GetId(), true); + if (IsBetaEnabled()) + { + CPubKey peerPubKey; + vRecv >> peerPubKey; + pfrom->routing_id = peerPubKey; + g_aodvtable.AddPeerKeyId(peerPubKey, pfrom->GetId(), true); + g_aodvtable.AddPeerKeyId(peerPubKey, pfrom->GetId(), true); + } } else if (strCommand == NetMsgType::ADDR) @@ -2055,6 +2062,10 @@ bool static ProcessMessage(CNode *pfrom, else if (strCommand == NetMsgType::RREQ) { + if (!IsBetaEnabled()) + { + return true; + } /** * A peer has requested a route to a specific id. we need to: * keep track of who sent the request, make sure it was unique, @@ -2081,6 +2092,10 @@ bool static ProcessMessage(CNode *pfrom, else if (strCommand == NetMsgType::RREP) { + if (!IsBetaEnabled()) + { + return true; + } uint64_t nonce = 0; CPubKey searchKey; bool found; @@ -2191,7 +2206,7 @@ bool ProcessMessages(CNode *pfrom, CConnman &connman) // Checksum CDataStream &vRecv = msg.vRecv; -#if 0 +#if 0 const uint256 &hash = msg.GetMessageHash(); // Do not waste my CPU calculating a checksum provided by an untrusted node // TCP already has one that is sufficient for network errors. The checksum does not increase security since @@ -2707,24 +2722,27 @@ bool SendMessages(CNode *pto, CConnman &connman) connman.PushMessage(pto, NetMsgType::GETDATA, vGetData); } - std::set responseQueue_copy; + if (IsBetaEnabled()) { - RECURSIVEREADLOCK(g_aodvtable.cs_aodv); - for (const auto &response : g_aodvtable.responseQueue) + std::set responseQueue_copy; { - if (response.source == pto->routing_id) - { - connman.PushMessage(pto, NetMsgType::RREP, response.nonce, response.pubkey, response.found); - } - else + RECURSIVEREADLOCK(g_aodvtable.cs_aodv); + for (const auto &response : g_aodvtable.responseQueue) { - responseQueue_copy.emplace(response); + if (response.source == pto->routing_id) + { + connman.PushMessage(pto, NetMsgType::RREP, response.nonce, response.pubkey, response.found); + } + else + { + responseQueue_copy.emplace(response); + } } } - } - { - RECURSIVEWRITELOCK(g_aodvtable.cs_aodv); - g_aodvtable.responseQueue = responseQueue_copy; + { + RECURSIVEWRITELOCK(g_aodvtable.cs_aodv); + g_aodvtable.responseQueue = responseQueue_copy; + } } return true; diff --git a/src/net/net.cpp b/src/net/net.cpp index 07c05763..6bb8486a 100644 --- a/src/net/net.cpp +++ b/src/net/net.cpp @@ -9,6 +9,7 @@ #include "net/net.h" #include "args.h" +#include "beta.h" #include "chain/tx.h" #include "clientversion.h" #include "consensus/consensus.h" @@ -2452,9 +2453,12 @@ bool CConnman::Start(std::string &strNodeError) LogPrintf("Generating random routing id..."); - pub_routing_key.MakeNewKey(true); - pub_routing_id = pub_routing_key.GetPubKey(); - assert(pub_routing_key.VerifyPubKey(pub_routing_id)); + if (IsBetaEnabled()) + { + pub_routing_key.MakeNewKey(true); + pub_routing_id = pub_routing_key.GetPubKey(); + assert(pub_routing_key.VerifyPubKey(pub_routing_id)); + } LogPrintf("Loading addresses..."); // Load addresses from peers.dat diff --git a/src/rpc/rpcaodv.cpp b/src/rpc/rpcaodv.cpp index 9fe39487..b6d277b0 100644 --- a/src/rpc/rpcaodv.cpp +++ b/src/rpc/rpcaodv.cpp @@ -1,3 +1,9 @@ +// This file is part of the Eccoin project +// Copyright (c) 2019 The Eccoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "beta.h" #include "net/aodv.h" #include "rpcserver.h" #include "util/logger.h" @@ -8,6 +14,11 @@ extern CCriticalSection cs_main; UniValue getaodvtable(const UniValue ¶ms, bool fHelp) { + if (!IsBetaEnabled()) + { + return "This rpc call requires beta features to be enabled (-beta or beta=1) \n"; + } + if (fHelp || params.size() != 0) throw std::runtime_error( "getaodvtable\n" @@ -49,6 +60,11 @@ UniValue getaodvtable(const UniValue ¶ms, bool fHelp) UniValue getaodvkeyentry(const UniValue ¶ms, bool fHelp) { + if (!IsBetaEnabled()) + { + return "This rpc call requires beta features to be enabled (-beta or beta=1) \n"; + } + if (fHelp || params.size() != 1) throw std::runtime_error( "getaodvkeyentry \"keyhash\" \n" @@ -67,6 +83,11 @@ UniValue getaodvkeyentry(const UniValue ¶ms, bool fHelp) UniValue getaodvidentry(const UniValue ¶ms, bool fHelp) { + if (!IsBetaEnabled()) + { + return "This rpc call requires beta features to be enabled (-beta or beta=1) \n"; + } + if (fHelp || params.size() != 1) throw std::runtime_error( "getaodvidentry \"nodeid\" \n" @@ -84,6 +105,11 @@ UniValue getaodvidentry(const UniValue ¶ms, bool fHelp) UniValue getroutingpubkey(const UniValue ¶ms, bool fHelp) { + if (!IsBetaEnabled()) + { + return "This rpc call requires beta features to be enabled (-beta or beta=1) \n"; + } + if (fHelp || params.size() != 0) throw std::runtime_error( "getroutingpubkey\n" @@ -107,6 +133,11 @@ UniValue getroutingpubkey(const UniValue ¶ms, bool fHelp) UniValue findroute(const UniValue ¶ms, bool fHelp) { + if (!IsBetaEnabled()) + { + return "This rpc call requires beta features to be enabled (-beta or beta=1) \n"; + } + if (fHelp || params.size() != 1) throw std::runtime_error( "findroute\n" @@ -142,6 +173,11 @@ UniValue findroute(const UniValue ¶ms, bool fHelp) UniValue haveroute(const UniValue ¶ms, bool fHelp) { + if (!IsBetaEnabled()) + { + return "This rpc call requires beta features to be enabled (-beta or beta=1) \n"; + } + if (fHelp || params.size() != 1) throw std::runtime_error( "haveroute\n" From 3efe5389ccb32a1fb2406f941004aa5dc04a22fb Mon Sep 17 00:00:00 2001 From: Griffith <8345264+Greg-Griffith@users.noreply.github.com> Date: Sat, 7 Sep 2019 10:17:55 -0700 Subject: [PATCH 8/8] bump version to 0.2.5.18 (#243) --- configure.ac | 2 +- contrib/gitian-descriptors/gitian-arm.yml | 2 +- contrib/gitian-descriptors/gitian-linux.yml | 2 +- contrib/gitian-descriptors/gitian-osx.yml | 2 +- contrib/gitian-descriptors/gitian-win.yml | 2 +- src/clientversion.h | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index 69538460..15d7218f 100644 --- a/configure.ac +++ b/configure.ac @@ -12,7 +12,7 @@ AC_PREREQ([2.60]) define(_CLIENT_VERSION_MAJOR, 0) define(_CLIENT_VERSION_MINOR, 2) define(_CLIENT_VERSION_REVISION, 5) -define(_CLIENT_VERSION_BUILD, 17) # version 99 here indicates an unreleased version +define(_CLIENT_VERSION_BUILD, 18) # version 99 here indicates an unreleased version define(_CLIENT_VERSION_IS_RELEASE, true) define(_COPYRIGHT_YEAR, 2019) define(_COPYRIGHT_HOLDERS,[The %s developers]) diff --git a/contrib/gitian-descriptors/gitian-arm.yml b/contrib/gitian-descriptors/gitian-arm.yml index 49f98a97..3727fc66 100644 --- a/contrib/gitian-descriptors/gitian-arm.yml +++ b/contrib/gitian-descriptors/gitian-arm.yml @@ -1,5 +1,5 @@ --- -name: "eccoin-linux-0.2.5.17" +name: "eccoin-linux-0.2.5.18" enable_cache: true suites: - "bionic" diff --git a/contrib/gitian-descriptors/gitian-linux.yml b/contrib/gitian-descriptors/gitian-linux.yml index 0ecee992..29f01407 100644 --- a/contrib/gitian-descriptors/gitian-linux.yml +++ b/contrib/gitian-descriptors/gitian-linux.yml @@ -1,5 +1,5 @@ --- -name: "eccoin-linux-0.2.5.17" +name: "eccoin-linux-0.2.5.18" enable_cache: true suites: - "bionic" diff --git a/contrib/gitian-descriptors/gitian-osx.yml b/contrib/gitian-descriptors/gitian-osx.yml index d81af90d..decc43fb 100644 --- a/contrib/gitian-descriptors/gitian-osx.yml +++ b/contrib/gitian-descriptors/gitian-osx.yml @@ -1,5 +1,5 @@ --- -name: "eccoin-osx-0.2.5.17" +name: "eccoin-osx-0.2.5.18" enable_cache: true suites: - "bionic" diff --git a/contrib/gitian-descriptors/gitian-win.yml b/contrib/gitian-descriptors/gitian-win.yml index 1575c0a5..bcd84a12 100644 --- a/contrib/gitian-descriptors/gitian-win.yml +++ b/contrib/gitian-descriptors/gitian-win.yml @@ -1,5 +1,5 @@ --- -name: "eccoin-win-0.2.5.17" +name: "eccoin-win-0.2.5.18" enable_cache: true suites: - "bionic" diff --git a/src/clientversion.h b/src/clientversion.h index 8f7aa136..10207488 100644 --- a/src/clientversion.h +++ b/src/clientversion.h @@ -21,7 +21,7 @@ #define CLIENT_VERSION_MAJOR 0 #define CLIENT_VERSION_MINOR 2 #define CLIENT_VERSION_REVISION 5 -#define CLIENT_VERSION_BUILD 17 +#define CLIENT_VERSION_BUILD 18 //! Set to true for release, false for prerelease or test build #define CLIENT_VERSION_IS_RELEASE true