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

Merge bitcoin/bitcoin#23142: Return false on corrupt tx rather than asserting

Browse Source 
0ab4c3b27265401c59e40adc494041927dc9dbe3 Return false on corrupt tx rather than asserting (Samuel Dobson)

Pull request description:

  Takes up #19793

  Rather than asserting, we log an error and return CORRUPT so that the user is informed. This type of error isn't critical so it isn't worth `assert`ing.

ACKs for top commit:
  achow101:
    ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3
  laanwj:
    Code review ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3
  ryanofsky:
    Code review ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3. There may be room for more improvements later like better error messages or easier recovery options, but changing from an assert to an error seems like a clear improvement, and this seems to avoid all the pitfalls of the last PR that tried this.

Tree-SHA512: 4a1a412e7c473d176c4e09123b85f390a6b0ea195e78d28ebd50b13814b7852f8225a172511a2efb6affb555b11bd4e667c19eb8c78b060c5444b62f0fae5f7a
This commit is contained in:
W. J. van der Laan 2021-10-01 10:30:26 +02:00 committed by Vijay
parent 1464e69058
commit 49c87e93a6
No known key found for this signature in database
GPG Key ID: 47820EC166FDF549
1 changed files with 13 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/wallet/walletdb.cpp
View File

@ -324,6 +324,7 @@ public:
std::map<uint256, DescriptorCache> m_descriptor_caches;
std::map<std::pair<uint256, CKeyID>, CKey> m_descriptor_keys;
std::map<std::pair<uint256, CKeyID>, std::pair<CPubKey, std::vector<unsigned char>>> m_descriptor_crypt_keys;
bool tx_corrupt{false};
CWalletScanState() {
}
@ -358,7 +359,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
// LoadToWallet call below creates a new CWalletTx that fill_wtx
// callback fills with transaction metadata.
auto fill_wtx = [&](CWalletTx& wtx, bool new_tx) {
assert(new_tx);
if(!new_tx) {
// There's some corruption here since the tx we just tried to load was already in the wallet.
// We don't consider this type of corruption critical, and can fix it by removing tx data and
// rescanning.
wss.tx_corrupt = true;
return false;
}
ssValue >> wtx;
if (wtx.GetHash() != hash)
return false;
@ -797,6 +804,11 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
} else if (strType == DBKeys::FLAGS) {
// reading the wallet flags can only fail if unknown flags are present
result = DBErrors::TOO_NEW;
} else if (wss.tx_corrupt) {
pwallet->WalletLogPrintf("Error: Corrupt transaction found. This can be fixed by removing transactions from wallet and rescanning.\n");
// Set tx_corrupt back to false so that the error is only printed once (per corrupt tx)
wss.tx_corrupt = false;
result = DBErrors::CORRUPT;
} else {
// Leave other errors alone, if we try to fix them we might make things worse.
fNoncriticalErrors = true; // ... but do warn the user there is something wrong.