Skip to content

Commit f1eb073

Browse files
furszyfurszy
committed
Reduce cs_main locks during ConnectTip/SyncWithWallets
Adapted work coming from btc@b3b3c2a5623d5c942d2b3565cc2d833c65105555
1 parent 5e155b4 commit f1eb073

File tree

10 files changed

+87
-60
lines changed

10 files changed

+87
-60
lines changed

‎src/main.cpp‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2433,11 +2433,13 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
24332433
continue;
24342434

24352435
//Search block for matching tx, turn into wtx, set merkle branch, add to wallet
2436-
for (const CTransaction& tx : block.vtx) {
2436+
int posInBlock = 0;
2437+
for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++) {
2438+
auto& tx = block.vtx[posInBlock];
24372439
if (tx.GetHash() == pSpend.second) {
24382440
CWalletTx wtx(pwalletMain, tx);
24392441
wtx.nTimeReceived = pindex->GetBlockTime();
2440-
wtx.SetMerkleBranch(block);
2442+
wtx.SetMerkleBranch(pindex, posInBlock);
24412443
pwalletMain->AddToWallet(wtx);
24422444
setAddedTx.insert(pSpend.second);
24432445
}
@@ -2708,7 +2710,7 @@ bool static DisconnectTip(CValidationState& state)
27082710
// Let wallets know transactions went from 1-confirmed to
27092711
// 0-confirmed or conflicted:
27102712
for (const CTransaction& tx : block.vtx) {
2711-
SyncWithWallets(tx, NULL);
2713+
SyncWithWallets(tx, pindexDelete->pprev);
27122714
}
27132715
return true;
27142716
}
@@ -2723,7 +2725,7 @@ static int64_t nTimePostConnect = 0;
27232725
* Connect a new block to chainActive. pblock is either NULL or a pointer to a CBlock
27242726
* corresponding to pindexNew, to bypass loading it again from disk.
27252727
*/
2726-
bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const CBlock* pblock, bool fAlreadyChecked)
2728+
bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const CBlock* pblock, bool fAlreadyChecked, std::list<CTransaction> &txConflicted, std::vector<std::tuple<CTransaction,CBlockIndex*,int>> &txChanged)
27272729
{
27282730
assert(pindexNew->pprev == chainActive.Tip());
27292731

@@ -2773,18 +2775,12 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const CB
27732775
LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001);
27742776

27752777
// Remove conflicting transactions from the mempool.
2776-
std::list<CTransaction> txConflicted;
27772778
mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, txConflicted, !IsInitialBlockDownload());
27782779
// Update chainActive & related variables.
27792780
UpdateTip(pindexNew);
2780-
// Tell wallet about transactions that went from mempool
2781-
// to conflicted:
2782-
for (const CTransaction& tx : txConflicted) {
2783-
SyncWithWallets(tx, NULL);
2784-
}
2785-
// ... and about transactions that got confirmed:
2786-
for (const CTransaction& tx : pblock->vtx) {
2787-
SyncWithWallets(tx, pblock);
2781+
2782+
for(unsigned int i=0; i < pblock->vtx.size(); i++) {
2783+
txChanged.push_back(std::make_tuple(pblock->vtx[i], pindexNew, i));
27882784
}
27892785

27902786
int64_t nTime6 = GetTimeMicros();
@@ -2915,7 +2911,7 @@ static void PruneBlockIndexCandidates()
29152911
* Try to make some progress towards making pindexMostWork the active block.
29162912
* pblock is either NULL or a pointer to a CBlock corresponding to pindexMostWork.
29172913
*/
2918-
static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMostWork, const CBlock* pblock, bool fAlreadyChecked)
2914+
static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMostWork, const CBlock* pblock, bool fAlreadyChecked, std::list<CTransaction>& txConflicted, std::vector<std::tuple<CTransaction,CBlockIndex*,int>>& txChanged)
29192915
{
29202916
AssertLockHeld(cs_main);
29212917
if (pblock == NULL)
@@ -2951,7 +2947,7 @@ static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMo
29512947

29522948
// Connect new blocks.
29532949
BOOST_REVERSE_FOREACH (CBlockIndex* pindexConnect, vpindexToConnect) {
2954-
if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : NULL, fAlreadyChecked)) {
2950+
if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : NULL, fAlreadyChecked, txConflicted, txChanged)) {
29552951
if (state.IsInvalid()) {
29562952
// The block violates a consensus rule.
29572953
if (!state.CorruptionPossible())
@@ -3009,6 +3005,8 @@ bool ActivateBestChain(CValidationState& state, const CBlock* pblock, bool fAlre
30093005
boost::this_thread::interruption_point();
30103006

30113007
const CBlockIndex *pindexFork;
3008+
std::list<CTransaction> txConflicted;
3009+
std::vector<std::tuple<CTransaction,CBlockIndex*,int>> txChanged;
30123010
bool fInitialDownload;
30133011
while (true) {
30143012
TRY_LOCK(cs_main, lockMain);
@@ -3024,7 +3022,7 @@ bool ActivateBestChain(CValidationState& state, const CBlock* pblock, bool fAlre
30243022
if (pindexMostWork == NULL || pindexMostWork == chainActive.Tip())
30253023
return true;
30263024

3027-
if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : NULL, fAlreadyChecked))
3025+
if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : NULL, fAlreadyChecked, txConflicted, txChanged))
30283026
return false;
30293027

30303028
pindexNewTip = chainActive.Tip();
@@ -3035,6 +3033,17 @@ bool ActivateBestChain(CValidationState& state, const CBlock* pblock, bool fAlre
30353033

30363034
// When we reach this point, we switched to a new tip (stored in pindexNewTip).
30373035
// Notifications/callbacks that can run without cs_main
3036+
3037+
// throw all transactions though the signal-interface
3038+
// while _not_ holding the cs_main lock
3039+
for (const CTransaction &tx : txConflicted) {
3040+
SyncWithWallets(tx, pindexNewTip);
3041+
}
3042+
// ... and about transactions that got confirmed:
3043+
for(unsigned int i = 0; i < txChanged.size(); i++) {
3044+
SyncWithWallets(std::get<0>(txChanged[i]), std::get<1>(txChanged[i]), std::get<2>(txChanged[i]));
3045+
}
3046+
30383047
// Always notify the UI if a new block tip was connected
30393048
if (pindexFork != pindexNewTip) {
30403049

‎src/validationinterface.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ CMainSignals& GetMainSignals()
1616
void RegisterValidationInterface(CValidationInterface* pwalletIn) {
1717
// XX42 g_signals.EraseTransaction.connect(boost::bind(&CValidationInterface::EraseFromWallet, pwalletIn, _1));
1818
g_signals.UpdatedBlockTip.connect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1));
19-
g_signals.SyncTransaction.connect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2));
19+
g_signals.SyncTransaction.connect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2, _3));
2020
g_signals.NotifyTransactionLock.connect(boost::bind(&CValidationInterface::NotifyTransactionLock, pwalletIn, _1));
2121
g_signals.UpdatedTransaction.connect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1));
2222
g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
@@ -36,7 +36,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
3636
g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
3737
g_signals.UpdatedTransaction.disconnect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1));
3838
g_signals.NotifyTransactionLock.disconnect(boost::bind(&CValidationInterface::NotifyTransactionLock, pwalletIn, _1));
39-
g_signals.SyncTransaction.disconnect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2));
39+
g_signals.SyncTransaction.disconnect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2, _3));
4040
g_signals.UpdatedBlockTip.disconnect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1));
4141
// XX42 g_signals.EraseTransaction.disconnect(boost::bind(&CValidationInterface::EraseFromWallet, pwalletIn, _1));
4242
}
@@ -55,6 +55,6 @@ void UnregisterAllValidationInterfaces() {
5555
// XX42 g_signals.EraseTransaction.disconnect_all_slots();
5656
}
5757

58-
void SyncWithWallets(const CTransaction &tx, const CBlock *pblock = NULL) {
59-
g_signals.SyncTransaction(tx, pblock);
58+
void SyncWithWallets(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) {
59+
g_signals.SyncTransaction(tx, pindex, posInBlock);
6060
}

‎src/validationinterface.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn);
2828
/** Unregister all wallets from core */
2929
void UnregisterAllValidationInterfaces();
3030
/** Push an updated transaction to all registered wallets */
31-
void SyncWithWallets(const CTransaction& tx, const CBlock* pblock);
31+
void SyncWithWallets(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock = -1);
3232

3333
class CValidationInterface {
3434
protected:
3535
// XX42 virtual void EraseFromWallet(const uint256& hash){};
3636
virtual void UpdatedBlockTip(const CBlockIndex *pindex) {}
37-
virtual void SyncTransaction(const CTransaction &tx, const CBlock *pblock) {}
37+
virtual void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) {}
3838
virtual void NotifyTransactionLock(const CTransaction &tx) {}
3939
virtual void SetBestChain(const CBlockLocator &locator) {}
4040
virtual bool UpdatedTransaction(const uint256 &hash) { return false;}
@@ -54,7 +54,7 @@ struct CMainSignals {
5454
/** Notifies listeners of updated block chain tip */
5555
boost::signals2::signal<void (const CBlockIndex *)> UpdatedBlockTip;
5656
/** Notifies listeners of updated transaction data (transaction, and optionally the block it is found in. */
57-
boost::signals2::signal<void (const CTransaction &, const CBlock *)> SyncTransaction;
57+
boost::signals2::signal<void (const CTransaction &, const CBlockIndex *pindex, int posInBlock)> SyncTransaction;
5858
/** Notifies listeners of an updated transaction lock without new data. */
5959
boost::signals2::signal<void (const CTransaction &)> NotifyTransactionLock;
6060
/** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */

‎src/wallet/wallet.cpp‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -906,18 +906,18 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn)
906906
* pblock is optional, but should be provided if the transaction is known to be in a block.
907907
* If fUpdate is true, existing transactions will be updated.
908908
*/
909-
bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate)
909+
bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate)
910910
{
911911
{
912912
AssertLockHeld(cs_wallet);
913913

914-
if (pblock && !tx.HasZerocoinSpendInputs() && !tx.IsCoinBase()) {
914+
if (posInBlock != -1 && !tx.HasZerocoinSpendInputs() && !tx.IsCoinBase()) {
915915
for (const CTxIn& txin : tx.vin) {
916916
std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(txin.prevout);
917917
while (range.first != range.second) {
918918
if (range.first->second != tx.GetHash()) {
919-
LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), pblock->GetHash().ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n);
920-
MarkConflicted(pblock->GetHash(), range.first->second);
919+
LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), pIndex->GetBlockHash().ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n);
920+
MarkConflicted(pIndex->GetBlockHash(), range.first->second);
921921
}
922922
range.first++;
923923
}
@@ -941,8 +941,8 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl
941941

942942
CWalletTx wtx(this, tx);
943943
// Get merkle branch if transaction was found in a block
944-
if (pblock)
945-
wtx.SetMerkleBranch(*pblock);
944+
if (posInBlock != -1)
945+
wtx.SetMerkleBranch(pIndex, posInBlock);
946946

947947
return AddToWallet(wtx, false);
948948
}
@@ -1067,10 +1067,10 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
10671067
}
10681068
}
10691069

1070-
void CWallet::SyncTransaction(const CTransaction& tx, const CBlock* pblock)
1070+
void CWallet::SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock)
10711071
{
10721072
LOCK(cs_wallet);
1073-
if (!AddToWalletIfInvolvingMe(tx, pblock, true))
1073+
if (!AddToWalletIfInvolvingMe(tx, pindex, posInBlock, true))
10741074
return; // Not one of ours
10751075

10761076
// If a transaction changes 'conflicted' state, that changes the balance
@@ -1635,8 +1635,9 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b
16351635

16361636
CBlock block;
16371637
ReadBlockFromDisk(block, pindex);
1638-
for (CTransaction& tx : block.vtx) {
1639-
if (AddToWalletIfInvolvingMe(tx, &block, fUpdate))
1638+
int posInBlock;
1639+
for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++) {
1640+
if (AddToWalletIfInvolvingMe(block.vtx[posInBlock], pindex, posInBlock, fUpdate))
16401641
ret++;
16411642
}
16421643

@@ -3831,22 +3832,15 @@ CWalletKey::CWalletKey(int64_t nExpires)
38313832
nTimeExpires = nExpires;
38323833
}
38333834

3834-
void CMerkleTx::SetMerkleBranch(const CBlock& block)
3835+
void CMerkleTx::SetMerkleBranch(const CBlockIndex* pindex, int posInBlock)
38353836
{
38363837
CBlock blockTmp;
38373838

38383839
// Update the tx's hashBlock
3839-
hashBlock = block.GetHash();
3840+
hashBlock = pindex->GetBlockHash();
38403841

3841-
// Locate the transaction
3842-
for (nIndex = 0; nIndex < (int)block.vtx.size(); nIndex++)
3843-
if (block.vtx[nIndex] == *(CTransaction*)this)
3844-
break;
3845-
if (nIndex == (int)block.vtx.size()) {
3846-
nIndex = -1;
3847-
LogPrintf("ERROR: SetMerkleBranch() : couldn't find tx in block\n");
3848-
return;
3849-
}
3842+
// set the position of the transaction in the block
3843+
nIndex = posInBlock;
38503844
}
38513845

38523846
int CMerkleTx::GetDepthInMainChain(bool enableIX) const

‎src/wallet/wallet.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
437437
void MarkDirty();
438438
bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose = true);
439439
bool LoadToWallet(const CWalletTx& wtxIn);
440-
void SyncTransaction(const CTransaction& tx, const CBlock* pblock);
441-
bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate);
440+
void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock);
441+
bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate);
442442
void EraseFromWallet(const uint256& hash);
443443

444444
/**
@@ -762,8 +762,7 @@ class CMerkleTx : public CTransaction
762762
READWRITE(nIndex);
763763
}
764764

765-
void SetMerkleBranch(const CBlock& block);
766-
765+
void SetMerkleBranch(const CBlockIndex* pIndex, int posInBlock);
767766

768767
/**
769768
* Return depth of transaction in blockchain:

‎src/wallet/wallet_zerocoin.cpp‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,20 +102,23 @@ void CWallet::doZPivRescan(const CBlockIndex* pindex, const CBlock& block,
102102
std::list<CZerocoinMint> listMints;
103103
BlockToZerocoinMintList(block, listMints, true);
104104

105+
int posInBlock = 0;
105106
for (auto& m : listMints) {
106107
if (IsMyMint(m.GetValue())) {
107108
LogPrint(BCLog::LEGACYZC, "%s: found mint\n", __func__);
108109
UpdateMint(m.GetValue(), pindex->nHeight, m.GetTxHash(), m.GetDenomination());
109110

110111
// Add the transaction to the wallet
111-
for (auto& tx : block.vtx) {
112+
posInBlock = 0;
113+
for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++) {
114+
auto& tx = block.vtx[posInBlock];
112115
uint256 txid = tx.GetHash();
113116
if (setAddedToWallet.count(txid) || mapWallet.count(txid))
114117
continue;
115118
if (txid == m.GetTxHash()) {
116119
CWalletTx wtx(this, tx);
117120
wtx.nTimeReceived = block.GetBlockTime();
118-
wtx.SetMerkleBranch(block);
121+
wtx.SetMerkleBranch(pindex, posInBlock);
119122
AddToWallet(wtx);
120123
setAddedToWallet.insert(txid);
121124
}
@@ -132,8 +135,14 @@ void CWallet::doZPivRescan(const CBlockIndex* pindex, const CBlock& block,
132135
CWalletTx wtx(this, txSpend);
133136
CBlockIndex* pindexSpend = chainActive[nHeightSpend];
134137
CBlock blockSpend;
135-
if (ReadBlockFromDisk(blockSpend, pindexSpend))
136-
wtx.SetMerkleBranch(blockSpend);
138+
if (ReadBlockFromDisk(blockSpend, pindexSpend)) {
139+
posInBlock = 0;
140+
for (posInBlock = 0; posInBlock < (int)blockSpend.vtx.size(); posInBlock++) {
141+
auto &tx = blockSpend.vtx[posInBlock];
142+
if (tx.GetHash() == txidSpend)
143+
wtx.SetMerkleBranch(pindexSpend, posInBlock);
144+
}
145+
}
137146

138147
wtx.nTimeReceived = pindexSpend->nTime;
139148
AddToWallet(wtx);

‎src/zmq/zmqnotificationinterface.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindex)
143143
}
144144
}
145145

146-
void CZMQNotificationInterface::SyncTransaction(const CTransaction &tx, const CBlock *pblock)
146+
void CZMQNotificationInterface::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int posInBlock)
147147
{
148148
for (std::list<CZMQAbstractNotifier*>::iterator i = notifiers.begin(); i!=notifiers.end(); )
149149
{

‎src/zmq/zmqnotificationinterface.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class CZMQNotificationInterface : public CValidationInterface
2424
void Shutdown();
2525

2626
// CValidationInterface
27-
void SyncTransaction(const CTransaction &tx, const CBlock *pblock);
27+
void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock);
2828
void UpdatedBlockTip(const CBlockIndex *pindex);
2929
void NotifyTransactionLock(const CTransaction &tx);
3030

‎src/zpiv/zpivwallet.cpp‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,15 @@ void CzPIVWallet::SyncWithChain(bool fGenerateMintPool)
261261
if (!setAddedTx.count(txHash)) {
262262
CBlock block;
263263
CWalletTx wtx(wallet, tx);
264-
if (pindex && ReadBlockFromDisk(block, pindex))
265-
wtx.SetMerkleBranch(block);
264+
if (pindex && ReadBlockFromDisk(block, pindex)) {
265+
int posInBlock;
266+
for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++) {
267+
if (wtx.GetHash() == txHash) {
268+
wtx.SetMerkleBranch(pindex, posInBlock);
269+
break;
270+
}
271+
}
272+
}
266273

267274
//Fill out wtx so that a transaction record can be created
268275
wtx.nTimeReceived = pindex->GetBlockTime();
@@ -318,8 +325,15 @@ bool CzPIVWallet::SetMintSeen(const CBigNum& bnValue, const int& nHeight, const
318325
CWalletTx wtx(wallet, txSpend);
319326
CBlockIndex* pindex = chainActive[nHeightTx];
320327
CBlock block;
321-
if (ReadBlockFromDisk(block, pindex))
322-
wtx.SetMerkleBranch(block);
328+
if (ReadBlockFromDisk(block, pindex)) {
329+
int posInBlock;
330+
for (posInBlock = 0; posInBlock < (int) block.vtx.size(); posInBlock++) {
331+
if (wtx.GetHash() == txidSpend) {
332+
wtx.SetMerkleBranch(pindex, posInBlock);
333+
break;
334+
}
335+
}
336+
}
323337

324338
wtx.nTimeReceived = pindex->nTime;
325339
wallet->AddToWallet(wtx);

0 commit comments

Comments
 (0)