Make explicitly requested salvage operations keep going when there is an error. #2410

1. 
   gmaxwell commented at 8:01 PM on March 24, 2013: contributor

   In my tests corrupted wallets would often result in BDB dropping an error just due to duplicate records being found, which appears harmless.

2. Make explicitly requested salvage operations keep going when there is an error.

   In my tests corrupted wallets would often result in BDB dropping an error
   just due to duplicate records being found, which appears harmless.

   14c9d116be
3. 
   BitcoinPullTester commented at 2:42 AM on March 25, 2013: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/14c9d116be476d08dd18f2e9f4a8ed251a6bbf79 for binaries and test log.

4. in src/db.cpp:None in 14c9d116be

    178 | +            return false;
    179 | +        }
    180 | +    }
    181 | +    if (result != 0 && result != DB_VERIFY_BAD)
    182 | +    {
    183 | +        printf("ERROR: db salvage failed: %d\n",result);
   

   ---

   ---

   

   Diapolo commented at 6:42 AM on March 25, 2013:

   You managed to use Error above 2 times, can you update this also ;)?

   ---

   

   sipa commented at 9:05 PM on April 12, 2013:

   @Diapolo What do you mean?

   ---

   

   Diapolo commented at 11:36 PM on April 12, 2013:

   Was just another nit about how it is written ;). Error vs. ERROR

   ---

   

   gmaxwell commented at 11:36 PM on April 12, 2013:

   Oh!

5. 
   Diapolo commented at 6:43 AM on March 25, 2013: none

   How can I start aggressive mode? Or is the client doing this for me, if "normal" mode fails?

6. 
   sipa commented at 6:35 PM on April 7, 2013: member

   This is one change for which I'd like to see a test plan, and what behaviour changes compared to before...

7. 
   sipa commented at 10:33 PM on April 20, 2013: member

   It seems there are several issues with salvaging wallets, and some can perhaps be fixed. This may seem pointless, as we want to move away from BDB for wallets anyway, but we'll still need conversion tools (either built-in, or as separate applications) that can reliably import wallets.

   First of all, we're not dealing at all with environment errors, which actually seem to be the most common. As wallets are detached from the env at shutdown anyway, they really serve no purpose except crash recovery. And even when the detaching failed, almost always the actual data is written to the wallet file anyway. If opening an environment fails, I wonder whether we can just play safe, and delete database/* before trying anything else. In addition, I think the database dir could just be deleted after a clean detach of the wallet too. That would reduce problems when files get moved without their environment.

   Then regarding the wallet database file itself, I've frequently seen people who were unable to open their wallet file, while a simple db4.8_dump old.dat | db4.8_load new.dat fixed the problem completely. That suggests to me that we should try the salvaging code, without removing all transactions, and only when importing fails or corrupted keys are found, do an implicit rescan. Even then, I'm not sure there's a need for completely resetting the other entries.

8. 
   sipa commented at 2:06 PM on May 4, 2013: member

   Ok, I've only tested this superficially, but the worst case side effect seems to be continuing a salvage operation that would fail anyway.

   ACK.

9. gavinandresen referenced this in commit e5ddaf5ec8 on May 7, 2013
10. gavinandresen merged this on May 7, 2013
11. gavinandresen closed this on May 7, 2013

    ---

12. laudney referenced this in commit 48089c9b98 on Mar 19, 2014
13. DrahtBot locked this on Sep 8, 2021

Contributors

gmaxwellBitcoinPullTesterDiapolosipa

Milestone
v0.8.2