[Wallet] Do not parse ssValue in CWalletDB::Recover #9101

1. 
   pstratem commented at 1:23 am on November 8, 2016: contributor

   This removes the warning in debug.log when a value fails to parse correctly.

   Removes the last dependency from walletdb.cpp on CWallet.

   I’m not sure this is a good idea, but the circular dependency being removed finally is nice.

2. Do not parse ssValue in CWalletDB::Recover

   This removes the warning in debug.log when a value fails to parse correctly.
   
   Removes the last dependency from walletdb.cpp on CWallet.

   6f8a07a7b8
3. pstratem force-pushed on Nov 8, 2016
4. MarcoFalke added the label Wallet on Nov 8, 2016
5. in src/wallet/walletdb.cpp: in 6f8a07a7b8

   904-                continue;
   905-            }
   906+                ssKey >> strType;
   907+                if (!IsKeyType(strType) && strType != "hdchain")
   908+                    continue;
   909+            }catch(...){continue;}
   

   ---

   ---

   

   laanwj commented at 10:42 am on November 11, 2016:

   Please add a warning when this fails, don’t ignore failures here silently.
6. in src/wallet/walletdb.cpp: in 6f8a07a7b8

   889-            string strType, strErr;
   890-            bool fReadOK;
   891-            {
   892-                // Required in LoadKeyMetadata():
   893-                LOCK(dummyWallet.cs_wallet);
   894-                fReadOK = ReadKeyValue(&dummyWallet, ssKey, ssValue,
   

   ---

   ---

   

   jonasschnelli commented at 12:48 pm on November 11, 2016:

   Not sure about this. Removing the actual deserialization routing may result in not actually checking the correctness of the data. For example, currently each "key" record (pub- & priv-key), is checked in ReadKeyValue() with vchPubKey.IsValid(), also, corrupt keys or a corrupt hdchain dataset will be detected now (log output) , but not after this PR.

   ---

   

   pstratem commented at 3:07 am on November 15, 2016:

   @jonasschnelli indeed that is why I’m not sure it’s a good idea.
7. 
   sipa commented at 3:55 pm on April 9, 2017: member

   Concept ACK. I don’t think that the attempt to parse the data is a particularly strong check anyway.
8. 
   fanquake commented at 10:47 am on April 26, 2017: member

   Needs a rebase/nits addressed.
9. 
   mchrostowski commented at 2:12 am on May 29, 2017: contributor

   NAK Looks like the method this PR modified no longer operates in the same way. ReadKeyValue() is no longer called, instead we call some callback if it’s registered.

   It does not appear that this PR can be applied to the current code in any way. I believe it can be closed.

10. 
    TheBlueMatt commented at 7:15 pm on September 29, 2017: member

    This was superceeded by the split between db.cpp and walletdb.cpp - db.cpp already has no other wallet dependancies.
11. sipa closed this on Sep 29, 2017

    ---

12. DrahtBot locked this on Sep 8, 2021

Contributors
pstratem laanwj jonasschnelli sipa fanquake mchrostowski TheBlueMatt

Labels
Wallet