mirror of
https://github.com/dashpay/dash.git
synced 2024-12-25 12:02:48 +01:00
Use callbacks to cache whether wallet transactions are in mempool
This avoid calling out to mempool state during coin selection, balance calculation, etc. In the next commit we ensure all wallet callbacks from CValidationInterface happen in the same queue, serialized with each other. This helps to avoid re-introducing one of the issues described in #9584 [1] by further disconnecting wallet from current chain/mempool state. Thanks to @morcos for the suggestion to do this. Note that there are several race conditions introduced here: * If a user calls sendrawtransaction from RPC, adding a transaction which is "trusted" (ie from them) and pays them change, it may not be immediately used by coin selection until the notification callbacks finish running. No such race is introduced in normal transaction-sending RPCs as this case is explicitly handled. * Until Block{Connected,Disconnected} and TransactionAddedToMempool calls also run in the CSceduler background thread, there is a race where TransactionAddedToMempool might be called after a Block{Connected,Disconnected} call happens. * Wallet will write a new best chain from the SetBestChain callback prior to having processed the transaction from that block. [1] "you could go to select coins, need to use 0-conf change, but such 0-conf change may have been included in a block who's callbacks have not yet been processed - resulting in thinking they are not in mempool and, thus, not selectable."
This commit is contained in:
parent
5d67a7868d
commit
17220d6325
@ -1215,6 +1215,19 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pin
|
||||
void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) {
|
||||
LOCK2(cs_main, cs_wallet);
|
||||
SyncTransaction(ptx);
|
||||
|
||||
auto it = mapWallet.find(ptx->GetHash());
|
||||
if (it != mapWallet.end()) {
|
||||
it->second.fInMempool = true;
|
||||
}
|
||||
}
|
||||
|
||||
void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) {
|
||||
LOCK(cs_wallet);
|
||||
auto it = mapWallet.find(ptx->GetHash());
|
||||
if (it != mapWallet.end()) {
|
||||
it->second.fInMempool = false;
|
||||
}
|
||||
}
|
||||
|
||||
void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) {
|
||||
@ -1229,9 +1242,11 @@ void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const
|
||||
|
||||
for (const CTransactionRef& ptx : vtxConflicted) {
|
||||
SyncTransaction(ptx);
|
||||
TransactionRemovedFromMempool(ptx);
|
||||
}
|
||||
for (size_t i = 0; i < pblock->vtx.size(); i++) {
|
||||
SyncTransaction(pblock->vtx[i], pindex, i);
|
||||
TransactionRemovedFromMempool(pblock->vtx[i]);
|
||||
}
|
||||
|
||||
m_last_block_processed = pindex;
|
||||
@ -1901,8 +1916,7 @@ CAmount CWalletTx::GetChange() const
|
||||
|
||||
bool CWalletTx::InMempool() const
|
||||
{
|
||||
LOCK(mempool.cs);
|
||||
return mempool.exists(GetHash());
|
||||
return fInMempool;
|
||||
}
|
||||
|
||||
bool CWalletTx::IsTrusted() const
|
||||
@ -3010,14 +3024,18 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon
|
||||
// Track how many getdata requests our transaction gets
|
||||
mapRequestCount[wtxNew.GetHash()] = 0;
|
||||
|
||||
// Get the inserted-CWalletTx from mapWallet so that the
|
||||
// fInMempool flag is cached properly
|
||||
CWalletTx& wtx = mapWallet[wtxNew.GetHash()];
|
||||
|
||||
if (fBroadcastTransactions)
|
||||
{
|
||||
// Broadcast
|
||||
if (!wtxNew.AcceptToMemoryPool(maxTxFee, state)) {
|
||||
if (!wtx.AcceptToMemoryPool(maxTxFee, state)) {
|
||||
LogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", state.GetRejectReason());
|
||||
// TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
|
||||
} else {
|
||||
wtxNew.RelayWalletTransaction(connman);
|
||||
wtx.RelayWalletTransaction(connman);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -4091,8 +4109,15 @@ int CMerkleTx::GetBlocksToMaturity() const
|
||||
}
|
||||
|
||||
|
||||
bool CMerkleTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state)
|
||||
bool CWalletTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state)
|
||||
{
|
||||
return ::AcceptToMemoryPool(mempool, state, tx, nullptr /* pfMissingInputs */,
|
||||
// We must set fInMempool here - while it will be re-set to true by the
|
||||
// entered-mempool callback, if we did not there would be a race where a
|
||||
// user could call sendmoney in a loop and hit spurious out of funds errors
|
||||
// because we think that the transaction they just generated's change is
|
||||
// unavailable as we're not yet aware its in mempool.
|
||||
bool ret = ::AcceptToMemoryPool(mempool, state, tx, nullptr /* pfMissingInputs */,
|
||||
nullptr /* plTxnReplaced */, false /* bypass_limits */, nAbsurdFee);
|
||||
fInMempool = ret;
|
||||
return ret;
|
||||
}
|
||||
|
@ -252,8 +252,6 @@ public:
|
||||
int GetDepthInMainChain() const { const CBlockIndex *pindexRet; return GetDepthInMainChain(pindexRet); }
|
||||
bool IsInMainChain() const { const CBlockIndex *pindexRet; return GetDepthInMainChain(pindexRet) > 0; }
|
||||
int GetBlocksToMaturity() const;
|
||||
/** Pass this transaction to the mempool. Fails if absolute fee exceeds absurd fee. */
|
||||
bool AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state);
|
||||
bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); }
|
||||
bool isAbandoned() const { return (hashBlock == ABANDON_HASH); }
|
||||
void setAbandoned() { hashBlock = ABANDON_HASH; }
|
||||
@ -330,6 +328,7 @@ public:
|
||||
mutable bool fImmatureWatchCreditCached;
|
||||
mutable bool fAvailableWatchCreditCached;
|
||||
mutable bool fChangeCached;
|
||||
mutable bool fInMempool;
|
||||
mutable CAmount nDebitCached;
|
||||
mutable CAmount nCreditCached;
|
||||
mutable CAmount nImmatureCreditCached;
|
||||
@ -369,6 +368,7 @@ public:
|
||||
fImmatureWatchCreditCached = false;
|
||||
fAvailableWatchCreditCached = false;
|
||||
fChangeCached = false;
|
||||
fInMempool = false;
|
||||
nDebitCached = 0;
|
||||
nCreditCached = 0;
|
||||
nImmatureCreditCached = 0;
|
||||
@ -473,6 +473,9 @@ public:
|
||||
// RelayWalletTransaction may only be called if fBroadcastTransactions!
|
||||
bool RelayWalletTransaction(CConnman* connman);
|
||||
|
||||
/** Pass this transaction to the mempool. Fails if absolute fee exceeds absurd fee. */
|
||||
bool AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state);
|
||||
|
||||
std::set<uint256> GetConflicts() const;
|
||||
};
|
||||
|
||||
@ -932,6 +935,7 @@ public:
|
||||
bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate);
|
||||
int64_t RescanFromTime(int64_t startTime, bool update);
|
||||
CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, bool fUpdate = false);
|
||||
void TransactionRemovedFromMempool(const CTransactionRef &ptx) override;
|
||||
void ReacceptWalletTransactions();
|
||||
void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override;
|
||||
// ResendWalletTransactionsBefore may only be called if fBroadcastTransactions!
|
||||
|
Loading…
Reference in New Issue
Block a user