Skip to content

Commit 77de5c6

Browse files
committed
wallet: guard and alert about a wallet invalid state during chain sync
-Context: If `AddToWallet` db write fails, the method returns a wtx nullptr without removing the recently added transaction from the wallet's map. -Problem: When a db write error occurs, `AddToWalletIfInvolvingMe` return false even when the tx is on the wallet's map already --> which makes `SyncTransaction` skip the `MarkInputsDirty` call --> which leads to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map. Plus, as we only store arriving transaction inside `AddToWalletIfInvolvingMe` when we synchronize/scan blocks from the chain and nowhere else, it makes sense to treat the tx db write error as a runtime error to notify the user about the problem. Otherwise, the user will lose all the not stored transactions after a wallet shutdown (without be able to recover them automatically on the next startup because the chain sync would be above the block where the txs arrived).
1 parent c395c8d commit 77de5c6

File tree

2 files changed

+11
-1
lines changed

2 files changed

+11
-1
lines changed

src/wallet/wallet.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1130,7 +1130,13 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
11301130
// Block disconnection override an abandoned tx as unconfirmed
11311131
// which means user may have to call abandontransaction again
11321132
TxState tx_state = std::visit([](auto&& s) -> TxState { return s; }, state);
1133-
return AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, rescanning_old_block);
1133+
CWalletTx* wtx = AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, rescanning_old_block);
1134+
if (!wtx) {
1135+
// Can only be nullptr if there was a db write error (missing db, read-only db or a db engine internal writing error).
1136+
// As we only store arriving transaction in this process, and we don't want an inconsistent state, let's throw an error.
1137+
throw std::runtime_error("DB error adding transaction to wallet, write failed");
1138+
}
1139+
return true;
11341140
}
11351141
}
11361142
return false;

src/wallet/wallet.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
505505
//! @return true if wtx is changed and needs to be saved to disk, otherwise false
506506
using UpdateWalletTxFn = std::function<bool(CWalletTx& wtx, bool new_tx)>;
507507

508+
/**
509+
* Add the transaction to the wallet, wrapping it up inside a CWalletTx
510+
* @return the recently added wtx pointer or nullptr if there was a db write error.
511+
*/
508512
CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false);
509513
bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
510514
void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override;

0 commit comments

Comments
 (0)