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).
This commit is contained in:
furszy 2022-06-03 10:46:14 -03:00
parent c395c8d6bb
commit 77de5c693f
No known key found for this signature in database
GPG key ID: 5DD23CCC686AA623
2 changed files with 11 additions and 1 deletions

View file

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

View file

@ -505,6 +505,10 @@ public:
//! @return true if wtx is changed and needs to be saved to disk, otherwise false //! @return true if wtx is changed and needs to be saved to disk, otherwise false
using UpdateWalletTxFn = std::function<bool(CWalletTx& wtx, bool new_tx)>; using UpdateWalletTxFn = std::function<bool(CWalletTx& wtx, bool new_tx)>;
/**
* Add the transaction to the wallet, wrapping it up inside a CWalletTx
* @return the recently added wtx pointer or nullptr if there was a db write error.
*/
CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false); CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false);
bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override; void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override;