Skip to content

UTXO hasher abstraction #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

UTXO hasher abstraction #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
da3da1b
Move internal functions into anonymous namespace.
domob1812 Mar 16, 2021
4c491ec
Clean up some mn config code.
domob1812 Apr 7, 2021
aba067e
Add TransactionUtxoHasher abstraction.
domob1812 Jan 19, 2021
7130c3d
Unit test for UpdateCoins UTXO creation.
domob1812 Jan 20, 2021
b69df5a
Unit test for mempool with UTXO hasher.
domob1812 Jan 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions divi/src/ActiveChainManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,21 @@ bool ActiveChainManager::DisconnectBlock(
return error("DisconnectBlock() : block and undo data inconsistent");
}

const BlockUtxoHasher utxoHasher;

bool fClean = true;
IndexDatabaseUpdates indexDBUpdates;
// undo transactions in reverse order
for (int transactionIndex = block.vtx.size() - 1; transactionIndex >= 0; transactionIndex--) {
const CTransaction& tx = block.vtx[transactionIndex];
const TxReversalStatus status = UpdateCoinsReversingTransaction(tx,view,blockUndo.vtxundo[transactionIndex-1],pindex->nHeight);
const TransactionLocationReference txLocationReference(utxoHasher, tx, pindex->nHeight, transactionIndex);
const TxReversalStatus status = UpdateCoinsReversingTransaction(tx, txLocationReference, view, blockUndo.vtxundo[transactionIndex-1]);
if(!CheckTxReversalStatus(status,fClean))
{
return false;
}
if(!pfClean)
{
TransactionLocationReference txLocationReference(tx.GetHash(),pindex->nHeight,transactionIndex);
IndexDatabaseUpdateCollector::ReverseTransaction(tx,txLocationReference,view,indexDBUpdates);
}
}
Expand Down Expand Up @@ -136,4 +138,4 @@ void ActiveChainManager::DisconnectBlock(
int64_t nStart = GetTimeMicros();
status = DisconnectBlock(block,state,pindex,coins);
LogPrint("bench", "- Disconnect block: %.2fms\n", (GetTimeMicros() - nStart) * 0.001);
}
}
6 changes: 3 additions & 3 deletions divi/src/BlockMemoryPoolTransactionCollector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ std::vector<TxPriority> BlockMemoryPoolTransactionCollector::PrioritizeMempoolTr
// Read prev transaction
if (!view.HaveCoins(txin.prevout.hash)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The view, as its looking for coins by hash should perhaps also depend on the utxo hasher in some fashion? Not saying this is the best idea, just pointing out that it doesn't seem like this behaviour is covered at all by the current changes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hasher actually comes into play not for referencing the UTXO set, but for creating the UTXOs from transactions. So in my opinion, that is something that logically happens before CCoinsViewCache is even involved; the CCoinsView then just stores the UTXO-hash-keyed entries, and allows looking them up. Hence I believe the UTXO hasher should not be connected to the coins view.

CTransaction prevTx;
if(!mempool_.lookup(txin.prevout.hash, prevTx)) {
if(!mempool_.lookupOutpoint(txin.prevout.hash, prevTx)) {
// This should never happen; all transactions in the memory
// pool should connect to either transactions in the chain
// or other transactions in the memory pool.
Expand Down Expand Up @@ -338,15 +338,15 @@ std::vector<PrioritizedTransactionData> BlockMemoryPoolTransactionCollector::Pri
nBlockSigOps += nTxSigOps;

CTxUndo txundo;
UpdateCoinsWithTransaction(tx, view, txundo, nHeight);
UpdateCoinsWithTransaction(tx, view, txundo, mempool_.GetUtxoHasher(), nHeight);

if (fPrintPriority) {
LogPrintf("priority %.1f fee %s txid %s\n",
dPriority, feeRate.ToString(), tx.GetHash().ToString());
}

// Add transactions that depend on this one to the priority queue
AddDependingTransactionsToPriorityQueue(dependentTransactions, hash, vecPriority, comparer);
AddDependingTransactionsToPriorityQueue(dependentTransactions, mempool_.GetUtxoHasher().GetUtxoHash(tx), vecPriority, comparer);
}

LogPrintf("CreateNewBlock(): total size %u\n", nBlockSize);
Expand Down
8 changes: 5 additions & 3 deletions divi/src/BlockTransactionChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ void TransactionLocationRecorder::RecordTxLocationData(

BlockTransactionChecker::BlockTransactionChecker(
const CBlock& block,
const TransactionUtxoHasher& utxoHasher,
CValidationState& state,
CBlockIndex* pindex,
CCoinsViewCache& view,
const int blocksToSkipChecksFor
): blockundo_(block.vtx.size() - 1)
, block_(block)
, utxoHasher_(utxoHasher)
, state_(state)
, pindex_(pindex)
, view_(view)
Expand All @@ -59,7 +61,7 @@ bool BlockTransactionChecker::Check(const CBlockRewards& nExpectedMint,bool fJus
pindex_->nMint = 0;
for (unsigned int i = 0; i < block_.vtx.size(); i++) {
const CTransaction& tx = block_.vtx[i];
const TransactionLocationReference txLocationRef(tx.GetHash(),pindex_->nHeight,i);
const TransactionLocationReference txLocationRef(utxoHasher_, tx, pindex_->nHeight, i);

if(!txInputChecker_.TotalSigOpsAreBelowMaximum(tx))
{
Expand All @@ -79,7 +81,7 @@ bool BlockTransactionChecker::Check(const CBlockRewards& nExpectedMint,bool fJus
}

IndexDatabaseUpdateCollector::RecordTransaction(tx,txLocationRef,view_, indexDatabaseUpdates);
UpdateCoinsWithTransaction(tx, view_, blockundo_.vtxundo[i>0u? i-1: 0u], pindex_->nHeight);
UpdateCoinsWithTransaction(tx, view_, blockundo_.vtxundo[i>0u? i-1: 0u], utxoHasher_, pindex_->nHeight);
txLocationRecorder_.RecordTxLocationData(tx,indexDatabaseUpdates.txLocationData);
}
return true;
Expand All @@ -93,4 +95,4 @@ bool BlockTransactionChecker::WaitForScriptsToBeChecked()
CBlockUndo& BlockTransactionChecker::getBlockUndoData()
{
return blockundo_;
}
}
5 changes: 4 additions & 1 deletion divi/src/BlockTransactionChecker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class CBlock;
class CValidationState;
class CCoinsViewCache;
class CBlockRewards;
class TransactionUtxoHasher;

class TransactionLocationRecorder
{
Expand All @@ -35,6 +36,7 @@ class BlockTransactionChecker
private:
CBlockUndo blockundo_;
const CBlock& block_;
const TransactionUtxoHasher& utxoHasher_;
CValidationState& state_;
CBlockIndex* pindex_;
CCoinsViewCache& view_;
Expand All @@ -43,6 +45,7 @@ class BlockTransactionChecker
public:
BlockTransactionChecker(
const CBlock& block,
const TransactionUtxoHasher& utxoHasher,
CValidationState& state,
CBlockIndex* pindex,
CCoinsViewCache& view,
Expand All @@ -56,4 +59,4 @@ class BlockTransactionChecker
CBlockUndo& getBlockUndoData();
};

#endif// BLOCK_TRANSACTION_CHECKER_H
#endif// BLOCK_TRANSACTION_CHECKER_H
6 changes: 3 additions & 3 deletions divi/src/IndexDatabaseUpdateCollector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void CollectUpdatesFromOutputs(
std::make_pair(CAddressIndexKey(addressType, uint160(hashBytes), txLocationRef.blockHeight, txLocationRef.transactionIndex, txLocationRef.hash, k, false), out.nValue));
// record unspent output
indexDatabaseUpdates.addressUnspentIndex.push_back(
std::make_pair(CAddressUnspentKey(addressType, uint160(hashBytes), txLocationRef.hash, k), CAddressUnspentValue(out.nValue, out.scriptPubKey, txLocationRef.blockHeight)));
std::make_pair(CAddressUnspentKey(addressType, uint160(hashBytes), txLocationRef.utxoHash, k), CAddressUnspentValue(out.nValue, out.scriptPubKey, txLocationRef.blockHeight)));
} else {
continue;
}
Expand Down Expand Up @@ -156,7 +156,7 @@ static void CollectUpdatesFromOutputs(
// undo unspent index
indexDBUpdates.addressUnspentIndex.push_back(
std::make_pair(
CAddressUnspentKey(addressType, uint160(hashBytes), txLocationReference.hash, k),
CAddressUnspentKey(addressType, uint160(hashBytes), txLocationReference.utxoHash, k),
CAddressUnspentValue()));

}
Expand All @@ -182,4 +182,4 @@ void IndexDatabaseUpdateCollector::ReverseTransaction(
{
ReverseSpending::CollectUpdatesFromOutputs(tx,txLocationRef,indexDatabaseUpdates);
ReverseSpending::CollectUpdatesFromInputs(tx,txLocationRef,view, indexDatabaseUpdates);
}
}
11 changes: 8 additions & 3 deletions divi/src/IndexDatabaseUpdates.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#include <IndexDatabaseUpdates.h>

#include <primitives/transaction.h>
#include <UtxoCheckingAndUpdating.h>

IndexDatabaseUpdates::IndexDatabaseUpdates(
): addressIndex()
, addressUnspentIndex()
Expand All @@ -9,11 +12,13 @@ IndexDatabaseUpdates::IndexDatabaseUpdates(
}

TransactionLocationReference::TransactionLocationReference(
uint256 hashValue,
const TransactionUtxoHasher& utxoHasher,
const CTransaction& tx,
unsigned blockheightValue,
int transactionIndexValue
): hash(hashValue)
): hash(tx.GetHash())
, utxoHash(utxoHasher.GetUtxoHash(tx))
, blockHeight(blockheightValue)
, transactionIndex(transactionIndexValue)
{
}
}
9 changes: 7 additions & 2 deletions divi/src/IndexDatabaseUpdates.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
#include <spentindex.h>
#include <uint256.h>

class CTransaction;
class TransactionUtxoHasher;

struct IndexDatabaseUpdates
{
std::vector<std::pair<CAddressIndexKey, CAmount> > addressIndex;
Expand All @@ -19,12 +22,14 @@ struct IndexDatabaseUpdates
struct TransactionLocationReference
{
uint256 hash;
uint256 utxoHash;
unsigned blockHeight;
int transactionIndex;

TransactionLocationReference(
uint256 hashValue,
const TransactionUtxoHasher& utxoHasher,
const CTransaction& tx,
unsigned blockheightValue,
int transactionIndexValue);
};
#endif// INDEX_DATABASE_UPDATES_H
#endif// INDEX_DATABASE_UPDATES_H
2 changes: 2 additions & 0 deletions divi/src/Makefile.test.include
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ BITCOIN_TESTS =\
test/MockBlockIncentivesPopulator.h \
test/MockBlockSubsidyProvider.h \
test/MockPoSStakeModifierService.h \
test/MockUtxoHasher.cpp \
test/MockVaultManagerDatabase.h \
test/monthlywalletbackupcreator_tests.cpp \
test/ActiveMasternode_tests.cpp \
Expand Down Expand Up @@ -101,6 +102,7 @@ BITCOIN_TESTS =\
test/PoSStakeModifierService_tests.cpp \
test/LegacyPoSStakeModifierService_tests.cpp \
test/LotteryWinnersCalculatorTests.cpp \
test/UtxoCheckingAndUpdating_tests.cpp \
test/VaultManager_tests.cpp \
test/multi_wallet_tests.cpp

Expand Down
35 changes: 20 additions & 15 deletions divi/src/MasternodeBroadcastFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ extern CChain chainActive;
extern bool fReindex;
extern bool fImporting;

bool CMasternodeBroadcastFactory::checkBlockchainSync(std::string& strErrorRet, bool fOffline)
namespace
{

bool checkBlockchainSync(std::string& strErrorRet, bool fOffline)
{
if (!fOffline && !IsBlockchainSynced()) {
strErrorRet = "Sync in progress. Must wait until sync is complete to start Masternode";
Expand All @@ -26,7 +29,7 @@ bool CMasternodeBroadcastFactory::checkBlockchainSync(std::string& strErrorRet,
}
return true;
}
bool CMasternodeBroadcastFactory::setMasternodeKeys(
bool setMasternodeKeys(
const std::string& strKeyMasternode,
std::pair<CKey,CPubKey>& masternodeKeyPair,
std::string& strErrorRet)
Expand All @@ -38,7 +41,7 @@ bool CMasternodeBroadcastFactory::setMasternodeKeys(
}
return true;
}
bool CMasternodeBroadcastFactory::setMasternodeCollateralKeys(
bool setMasternodeCollateralKeys(
const std::string& txHash,
const std::string& outputIndex,
const std::string& service,
Expand All @@ -63,7 +66,7 @@ bool CMasternodeBroadcastFactory::setMasternodeCollateralKeys(
return true;
}

bool CMasternodeBroadcastFactory::checkMasternodeCollateral(
bool checkMasternodeCollateral(
const CTxIn& txin,
const std::string& txHash,
const std::string& outputIndex,
Expand Down Expand Up @@ -95,7 +98,7 @@ bool CMasternodeBroadcastFactory::checkMasternodeCollateral(
return true;
}

bool CMasternodeBroadcastFactory::createArgumentsFromConfig(
bool createArgumentsFromConfig(
const CMasternodeConfig::CMasternodeEntry configEntry,
std::string& strErrorRet,
bool fOffline,
Expand All @@ -121,6 +124,8 @@ bool CMasternodeBroadcastFactory::createArgumentsFromConfig(
return true;
}

} // anonymous namespace

bool CMasternodeBroadcastFactory::Create(
const CMasternodeConfig::CMasternodeEntry configEntry,
CPubKey pubkeyCollateralAddress,
Expand Down Expand Up @@ -278,10 +283,10 @@ CMasternodePing createDelayedMasternodePing(const CMasternodeBroadcast& mnb)
} // anonymous namespace

void CMasternodeBroadcastFactory::createWithoutSignatures(
CTxIn txin,
CService service,
CPubKey pubKeyCollateralAddressNew,
CPubKey pubKeyMasternodeNew,
const CTxIn& txin,
const CService& service,
const CPubKey& pubKeyCollateralAddressNew,
const CPubKey& pubKeyMasternodeNew,
const MasternodeTier nMasternodeTier,
bool deferRelay,
CMasternodeBroadcast& mnbRet)
Expand All @@ -299,12 +304,12 @@ void CMasternodeBroadcastFactory::createWithoutSignatures(
}

bool CMasternodeBroadcastFactory::Create(
CTxIn txin,
CService service,
CKey keyCollateralAddressNew,
CPubKey pubKeyCollateralAddressNew,
CKey keyMasternodeNew,
CPubKey pubKeyMasternodeNew,
const CTxIn& txin,
const CService& service,
const CKey& keyCollateralAddressNew,
const CPubKey& pubKeyCollateralAddressNew,
const CKey& keyMasternodeNew,
const CPubKey& pubKeyMasternodeNew,
const MasternodeTier nMasternodeTier,
std::string& strErrorRet,
CMasternodeBroadcast& mnbRet,
Expand Down
60 changes: 15 additions & 45 deletions divi/src/MasternodeBroadcastFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ class CMasternodeBroadcastFactory
std::string& strErrorRet);
private:
static void createWithoutSignatures(
CTxIn txin,
CService service,
CPubKey pubKeyCollateralAddressNew,
CPubKey pubKeyMasternodeNew,
const CTxIn& txin,
const CService& service,
const CPubKey& pubKeyCollateralAddressNew,
const CPubKey& pubKeyMasternodeNew,
MasternodeTier nMasternodeTier,
bool deferRelay,
CMasternodeBroadcast& mnbRet);
Expand All @@ -57,45 +57,15 @@ class CMasternodeBroadcastFactory
CMasternodeBroadcast& mnb,
std::string& strErrorRet);

static bool Create(CTxIn vin,
CService service,
CKey keyCollateralAddressNew,
CPubKey pubKeyCollateralAddressNew,
CKey keyMasternodeNew,
CPubKey pubKeyMasternodeNew,
MasternodeTier nMasternodeTier,
std::string& strErrorRet,
CMasternodeBroadcast& mnbRet,
bool deferRelay);
static bool checkBlockchainSync(
std::string& strErrorRet, bool fOffline);
static bool setMasternodeKeys(
const std::string& strKeyMasternode,
std::pair<CKey,CPubKey>& masternodeKeyPair,
std::string& strErrorRet);
static bool setMasternodeCollateralKeys(
const std::string& txHash,
const std::string& outputIndex,
const std::string& service,
bool collateralPrivKeyIsRemote,
CTxIn& txin,
std::pair<CKey,CPubKey>& masternodeCollateralKeyPair,
std::string& error);
static bool checkMasternodeCollateral(
const CTxIn& txin,
const std::string& txHash,
const std::string& outputIndex,
const std::string& service,
MasternodeTier& nMasternodeTier,
std::string& strErrorRet);
static bool createArgumentsFromConfig(
const CMasternodeConfig::CMasternodeEntry configEntry,
std::string& strErrorRet,
bool fOffline,
bool collateralPrivKeyIsRemote,
CTxIn& txin,
std::pair<CKey,CPubKey>& masternodeKeyPair,
std::pair<CKey,CPubKey>& masternodeCollateralKeyPair,
MasternodeTier& nMasternodeTier);
static bool Create(const CTxIn& vin,
const CService& service,
const CKey& keyCollateralAddressNew,
const CPubKey& pubKeyCollateralAddressNew,
const CKey& keyMasternodeNew,
const CPubKey& pubKeyMasternodeNew,
MasternodeTier nMasternodeTier,
std::string& strErrorRet,
CMasternodeBroadcast& mnbRet,
bool deferRelay);
};
#endif //MASTERNODE_BROADCAST_FACTORY_H
#endif //MASTERNODE_BROADCAST_FACTORY_H
Loading