sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-25 12:02:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #18982: wallet: Minimal fix to restore conflicted transaction notifications

Browse Source 
7eaf86d3bfc83f2beb3ef449707d5156853126fb trivial: Suggested cleanups to surrounding code (Russell Yanofsky)
b604c5c8b5892842f13dee89ae31812a28ab25d1 wallet: Minimal fix to restore conflicted transaction notifications (Russell Yanofsky)

Pull request description:

  This fix is a based on the fix by Antoine Riard (ariard) in https://github.com/bitcoin/bitcoin/pull/18600.

  Unlike that PR, which implements some new behavior, this just restores previous wallet notification and status behavior for transactions removed from the mempool because they conflict with transactions in a block. The behavior was accidentally changed in two `CWallet::BlockConnected` updates: a31be09bfd77eed497a8e251d31358e16e2f2eb1 and 7e89994133725125dddbfa8d45484e3b9ed51c6e from https://github.com/bitcoin/bitcoin/pull/16624, causing issue https://github.com/bitcoin/bitcoin/issues/18325.

  The change here could be improved and replaced with a more comprehensive cleanup, so it includes a detailed comment explaining future considerations.

  Fixes #18325

  Co-authored-by: Antoine Riard (ariard)

ACKs for top commit:
  jonatack:
    Re-ACK 7eaf86d3bfc83f
  ariard:
    ACK 7eaf86d, reviewed, built and ran tests.
  MarcoFalke:
    ACK 7eaf86d3bfc83f2beb3ef449707d5156853126fb 🍡

Tree-SHA512: 9a1efe975969bb522a9dd73c41064a9348887cb67883cd92c6571fd2df4321b9f4568363891abdaae14a3b9b168ef8142e95c373fc04677e46289b251fb84689
This commit is contained in:
MarcoFalke 2020-06-02 18:11:33 -04:00 committed by pasta
parent 118794dfb4
commit 3b7140efe7
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
6 changed files with 63 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
doc/release-notes-18982.md Normal file
View File

@ -0,0 +1,7 @@
Notification changes
--------------------
`-walletnotify` notifications are now sent for wallet transactions that are
removed from the mempool because they conflict with a new block. These
notifications were sent previously before the v18.1 release, but had been
broken since that release.

3
src/interfaces/chain.h
View File

@ -27,6 +27,7 @@ class CFeeRate;
class CBlockIndex;
class Coin;
class uint256;
enum class MemPoolRemovalReason;
struct bilingual_str;
struct CBlockLocator;
struct FeeCalculation;
@ -258,7 +259,7 @@ public:
public:
virtual ~Notifications() {}
virtual void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) {}
virtual void transactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason) {}
virtual void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {}
virtual void blockConnected(const CBlock& block, int height) {}
virtual void blockDisconnected(const CBlock& block, int height) {}
virtual void updatedBlockTip() {}

16
src/validationinterface.cpp
View File

@ -208,20 +208,20 @@ void CMainSignals::SynchronousUpdatedBlockTip(const CBlockIndex *pindexNew, cons
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.SynchronousUpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); });
}
void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx, int64_t nAcceptTime) {
auto event = [ptx, nAcceptTime, this] {
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionAddedToMempool(ptx, nAcceptTime); });
void CMainSignals::TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) {
auto event = [tx, nAcceptTime, this] {
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionAddedToMempool(tx, nAcceptTime); });
};
ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s", __func__,
ptx->GetHash().ToString());
tx->GetHash().ToString());
}
void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
auto event = [ptx, reason, this] {
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(ptx, reason); });
void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {
auto event = [tx, reason, this] {
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(tx, reason); });
};
ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s", __func__,
ptx->GetHash().ToString());
tx->GetHash().ToString());
}
void CMainSignals::BlockConnected(const std::shared_ptr<const CBlock> &pblock, const CBlockIndex *pindex) {

4
src/validationinterface.h
View File

@ -117,7 +117,7 @@ protected:
*
* Called on a background thread.
*/
virtual void TransactionAddedToMempool(const CTransactionRef &ptxn, int64_t nAcceptTime) {}
virtual void TransactionAddedToMempool(const CTransactionRef &xn, int64_t nAcceptTime) {}
/**
* Notifies listeners of a transaction leaving mempool.
*
@ -149,7 +149,7 @@ protected:
*
* Called on a background thread.
*/
virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {}
virtual void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {}
/**
* Notifies listeners of a block being connected.
* Provides a vector of transactions evicted from the mempool as a result.

53
src/wallet/wallet.cpp
View File

@ -1210,26 +1210,58 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio
fAnonymizableTallyCachedNonDenom = false;
}
void CWallet::transactionAddedToMempool(const CTransactionRef& ptx, int64_t nAcceptTime) {
void CWallet::transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) {
LOCK(cs_wallet);
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
WalletBatch batch(GetDatabase());
SyncTransaction(ptx, confirm, batch);
SyncTransaction(tx, confirm, batch);
auto it = mapWallet.find(ptx->GetHash());
auto it = mapWallet.find(tx->GetHash());
if (it != mapWallet.end()) {
it->second.fInMempool = true;
}
}
void CWallet::transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {
if (reason != MemPoolRemovalReason::CONFLICT) {
LOCK(cs_wallet);
auto it = mapWallet.find(ptx->GetHash());
auto it = mapWallet.find(tx->GetHash());
if (it != mapWallet.end()) {
it->second.fInMempool = false;
}
}
// Handle transactions that were removed from the mempool because they
// conflict with transactions in a newly connected block.
if (reason == MemPoolRemovalReason::CONFLICT) {
// Call SyncNotifications, so external -walletnotify notifications will
// be triggered for these transactions. Set Status::UNCONFIRMED instead
// of Status::CONFLICTED for a few reasons:
//
// 1. The transactionRemovedFromMempool callback does not currently
// provide the conflicting block's hash and height, and for backwards
// compatibility reasons it may not be not safe to store conflicted
// wallet transactions with a null block hash. See
// https://github.com/bitcoin/bitcoin/pull/18600#discussion_r420195993.
// 2. For most of these transactions, the wallet's internal conflict
// detection in the blockConnected handler will subsequently call
// MarkConflicted and update them with CONFLICTED status anyway. This
// applies to any wallet transaction that has inputs spent in the
// block, or that has ancestors in the wallet with inputs spent by
// the block.
// 3. Longstanding behavior since the sync implementation in
// https://github.com/bitcoin/bitcoin/pull/9371 and the prior sync
// implementation before that was to mark these transactions
// unconfirmed rather than conflicted.
//
// Nothing described above should be seen as an unchangeable requirement
// when improving this code in the future. The wallet's heuristics for
// distinguishing between conflicted and unconfirmed transactions are
// imperfect, and could be improved in general, see
// https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking
LOCK(cs_wallet);
WalletBatch batch(GetDatabase());
SyncTransaction(tx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0}, batch);
}
}
void CWallet::blockConnected(const CBlock& block, int height)
@ -1241,9 +1273,8 @@ void CWallet::blockConnected(const CBlock& block, int height)
m_last_block_processed = block_hash;
WalletBatch batch(GetDatabase());
for (size_t index = 0; index < block.vtx.size(); index++) {
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, index);
SyncTransaction(block.vtx[index], confirm, batch);
transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::MANUAL);
SyncTransaction(block.vtx[index], {CWalletTx::Status::CONFIRMED, height, block_hash, (int)index}, batch);
transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::BLOCK);
}
// reset cache to make sure no longer immature coins are included
@ -1263,8 +1294,7 @@ void CWallet::blockDisconnected(const CBlock& block, int height)
m_last_block_processed = block.hashPrevBlock;
WalletBatch batch(GetDatabase());
for (const CTransactionRef& ptx : block.vtx) {
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
SyncTransaction(ptx, confirm, batch);
SyncTransaction(ptx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0}, batch);
}
// reset cache to make sure no longer mature coins are excluded
@ -1957,8 +1987,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
break;
}
for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block_height, block_hash, posInBlock);
SyncTransaction(block.vtx[posInBlock], confirm, batch, fUpdate);
SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, block_height, block_hash, (int)posInBlock}, batch, fUpdate);
}
// scan succeeded, record block as most recent successfully scanned
result.last_scanned_block = block_hash;

2
src/wallet/wallet.h
View File

@ -1026,7 +1026,7 @@ public:
uint256 last_failed_block;
};
ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate);
void transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override;
void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override;
void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void ResendWalletTransactions();
struct Balance {