Refactor keytime/metadata and wallet encryption bugfix

Refactor keytime:
* Key metadata is kept in a CWallet::mapKeyMetadata (std::map<CKeyId,CKeyMetadata>).
* When generating a new key, time is put in that map, and new key is written.
* AddKeyPubKey and AddCryptedKey do not take a creation time argument, but instead
  pull it from that map, if it exists there.

Bugfix:
* AddKeyPubKey and AddCryptedKey in CWallet didn't override the CKeyStore
  definition anymore. This is fixed, as they no longed need the nCreationTime
  argument now.

Also a few related other changes:
* Metadata can be overwritten.
* Only GenerateNewKey calls GetTime(), as it's the only place where we know for
  sure a key was not constructed earlier.
* When the nTimeFirstKey is known to be inaccurate, it is set to the value 1
  (instead of 0, which would mean unknown).
* Use CPubKey instead of std::vector<unsigned char> where possible.

4 files changed, 36 insertions, 28 deletions

diff --git a/src/wallet.cpp b/src/wallet.cpp
index 4cd04bb6e2..1087db632d 100644
--- a/src/wallet.cpp
+++ b/src/wallet.cpp
@@ -40,18 +40,20 @@ CPubKey CWallet::GenerateNewKey()
         SetMinVersion(FEATURE_COMPRPUBKEY);
 
     CPubKey pubkey = secret.GetPubKey();
+
+    // Create new metadata
+    int64 nCreationTime = GetTime();
+    mapKeyMetadata[pubkey.GetID()] = CKeyMetadata(nCreationTime);
+    if (!nTimeFirstKey || nCreationTime < nTimeFirstKey)
+        nTimeFirstKey = nCreationTime;
+
     if (!AddKeyPubKey(secret, pubkey))
         throw std::runtime_error("CWallet::GenerateNewKey() : AddKey failed");
     return pubkey;
 }
 
-bool CWallet::AddKeyPubKey(const CKey& secret, const CPubKey &pubkey,
-                           int64 nCreateTime)
+bool CWallet::AddKeyPubKey(const CKey& secret, const CPubKey &pubkey)
 {
-    if (!nCreateTime)
-        nCreateTime = GetTime();
-    if (!nTimeFirstKey || (nCreateTime < nTimeFirstKey))
-        nTimeFirstKey = nCreateTime;
     if (!CCryptoKeyStore::AddKeyPubKey(secret, pubkey))
         return false;
     if (!fFileBacked)
@@ -59,19 +61,14 @@ bool CWallet::AddKeyPubKey(const CKey& secret, const CPubKey &pubkey,
     if (!IsCrypted()) {
         return CWalletDB(strWalletFile).WriteKey(pubkey,
                                                  secret.GetPrivKey(),
-                                                 nCreateTime);
+                                                 mapKeyMetadata[pubkey.GetID()]);
     }
     return true;
 }
 
 bool CWallet::AddCryptedKey(const CPubKey &vchPubKey,
-                            const vector<unsigned char> &vchCryptedSecret,
-                            int64 nCreateTime)
+                            const vector<unsigned char> &vchCryptedSecret)
 {
-    if (!nCreateTime)
-        nCreateTime = GetTime();
-    if (!nTimeFirstKey || (nCreateTime < nTimeFirstKey))
-        nTimeFirstKey = nCreateTime;
     if (!CCryptoKeyStore::AddCryptedKey(vchPubKey, vchCryptedSecret))
         return false;
     if (!fFileBacked)
@@ -81,15 +78,24 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey,
         if (pwalletdbEncryption)
             return pwalletdbEncryption->WriteCryptedKey(vchPubKey,
                                                         vchCryptedSecret,
-                                                        nCreateTime);
+                                                        mapKeyMetadata[vchPubKey.GetID()]);
         else
             return CWalletDB(strWalletFile).WriteCryptedKey(vchPubKey,
                                                             vchCryptedSecret,
-                                                            nCreateTime);
+                                                            mapKeyMetadata[vchPubKey.GetID()]);
     }
     return false;
 }
 
+bool CWallet::LoadKeyMetadata(const CPubKey &pubkey, const CKeyMetadata &meta)
+{
+    if (meta.nCreateTime && (!nTimeFirstKey || meta.nCreateTime < nTimeFirstKey))
+        nTimeFirstKey = meta.nCreateTime;
+
+    mapKeyMetadata[pubkey.GetID()] = meta;
+    return true;
+}
+
 bool CWallet::LoadCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret)
 {
     return CCryptoKeyStore::AddCryptedKey(vchPubKey, vchCryptedSecret);
diff --git a/src/wallet.h b/src/wallet.h
index 2b3131261c..48bd511971 100644
--- a/src/wallet.h
+++ b/src/wallet.h
@@ -87,7 +87,7 @@ public:
     std::string strWalletFile;
 
     std::set<int64> setKeyPool;
-
+    std::map<CKeyID, CKeyMetadata> mapKeyMetadata;
 
     typedef std::map<unsigned int, CMasterKey> MasterKeyMap;
     MasterKeyMap mapMasterKeys;
@@ -140,14 +140,16 @@ public:
     // Generate a new key
     CPubKey GenerateNewKey();
     // Adds a key to the store, and saves it to disk.
-    bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey, int64 nCreateTime = 0);
+    bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey);
     // Adds a key to the store, without saving it to disk (used by LoadWallet)
     bool LoadKey(const CKey& key, const CPubKey &pubkey) { return CCryptoKeyStore::AddKeyPubKey(key, pubkey); }
+    // Load metadata (used by LoadWallet)
+    bool LoadKeyMetadata(const CPubKey &pubkey, const CKeyMetadata &metadata);
 
     bool LoadMinVersion(int nVersion) { nWalletVersion = nVersion; nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); return true; }
 
     // Adds an encrypted key to the store, and saves it to disk.
-    bool AddCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret, int64 nCreateTime = 0);
+    bool AddCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret);
     // Adds an encrypted key to the store, without saving it to disk (used by LoadWallet)
     bool LoadCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret);
     bool AddCScript(const CScript& redeemScript);
diff --git a/src/walletdb.cpp b/src/walletdb.cpp
index 96fd4a5fc1..bf23357f79 100644
--- a/src/walletdb.cpp
+++ b/src/walletdb.cpp
@@ -344,12 +344,14 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
         }
         else if (strType == "keymeta")
         {
-            vector<unsigned char> vchPubKey;
+            CPubKey vchPubKey;
             ssKey >> vchPubKey;
             CKeyMetadata keyMeta;
             ssValue >> keyMeta;
             wss.nKeyMeta++;
 
+            pwallet->LoadKeyMetadata(vchPubKey, keyMeta);
+
             // find earliest key creation time, as wallet birthday
             if (!pwallet->nTimeFirstKey ||
                 (keyMeta.nCreateTime < pwallet->nTimeFirstKey))
@@ -483,7 +485,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
 
     // nTimeFirstKey is only reliable if all keys have metadata
     if ((wss.nKeys + wss.nCKeys) != wss.nKeyMeta)
-        pwallet->nTimeFirstKey = 0;
+        pwallet->nTimeFirstKey = 1; // 0 would be considered 'no value'
 
     BOOST_FOREACH(uint256 hash, wss.vWalletUpgrade)
         WriteTx(hash, pwallet->mapWallet[hash]);
diff --git a/src/walletdb.h b/src/walletdb.h
index 287361b33d..4dfa35d82a 100644
--- a/src/walletdb.h
+++ b/src/walletdb.h
@@ -30,7 +30,7 @@ class CKeyMetadata
 public:
     static const int CURRENT_VERSION=1;
     int nVersion;
-    int64 nCreateTime;
+    int64 nCreateTime; // 0 means unknown
 
     CKeyMetadata()
     {
@@ -52,7 +52,7 @@ public:
     void SetNull()
     {
         nVersion = CKeyMetadata::CURRENT_VERSION;
-        nCreateTime = GetTime();
+        nCreateTime = 0;
     }
 };
 
@@ -84,13 +84,12 @@ public:
     }
 
     bool WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey,
-                  int64 nCreateTime)
+                  const CKeyMetadata &keyMeta)
     {
         nWalletDBUpdated++;
 
-        CKeyMetadata keyMeta(nCreateTime);
         if (!Write(std::make_pair(std::string("keymeta"), vchPubKey),
-                   keyMeta, false))
+                   keyMeta))
             return false;
 
         return Write(std::make_pair(std::string("key"), vchPubKey), vchPrivKey, false);
@@ -98,14 +97,13 @@ public:
 
     bool WriteCryptedKey(const CPubKey& vchPubKey,
                          const std::vector<unsigned char>& vchCryptedSecret,
-                         int64 nCreateTime)
+                         const CKeyMetadata &keyMeta)
     {
         const bool fEraseUnencryptedKey = true;
         nWalletDBUpdated++;
 
-        CKeyMetadata keyMeta(nCreateTime);
         if (!Write(std::make_pair(std::string("keymeta"), vchPubKey),
-                   keyMeta, false))
+                   keyMeta))
             return false;
 
         if (!Write(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false))